On Wed, Aug 02, 2023 at 07:27:15AM +0300, Jarkko Sakkinen wrote:
On Wed Aug 2, 2023 at 7:15 AM EEST, Eric Biggers wrote:
From: Eric Biggers ebiggers@google.com
If an fsverity builtin signature is given for a file but the ".fs-verity" keyring is empty, there's no real reason to run the PKCS#7 parser. Skip this to avoid the PKCS#7 attack surface when builtin signature support is configured into the kernel but is not being used.
This is a hardening improvement, not a fix per se, but I've added Fixes and Cc stable to get it out to more users.
Fixes: 432434c9f8e1 ("fs-verity: support builtin file signatures") Cc: stable@vger.kernel.org Acked-by: Jarkko Sakkinen jarkko@kernel.org Signed-off-by: Eric Biggers ebiggers@google.com
v2: check keyring and return early before allocating formatted digest
fs/verity/signature.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
diff --git a/fs/verity/signature.c b/fs/verity/signature.c index b95acae64eac6..8f474702aa249 100644 --- a/fs/verity/signature.c +++ b/fs/verity/signature.c @@ -62,6 +62,21 @@ int fsverity_verify_signature(const struct fsverity_info *vi, return 0; }
- if (fsverity_keyring->keys.nr_leaves_on_tree == 0) {
/** The ".fs-verity" keyring is empty, due to builtin signatures* being supported by the kernel but not actually being used.* In this case, verify_pkcs7_signature() would always return an* error, usually ENOKEY. It could also be EBADMSG if the* PKCS#7 is malformed, but that isn't very important to* distinguish. So, just skip to ENOKEY to avoid the attack* surface of the PKCS#7 parser, which would otherwise be* reachable by any task able to execute FS_IOC_ENABLE_VERITY.*/fsverity_err(inode, "fs-verity keyring is empty");return -ENOKEY;- }
- d = kzalloc(sizeof(*d) + hash_alg->digest_size, GFP_KERNEL); if (!d) return -ENOMEM;
base-commit: 456ae5fe9b448f44ebe98b391a3bae9c75df465e
2.41.0
Reviewed-by: Jarkko Sakkinen jarkko@kernel.org
applied
BR, Jarkko
Hi Jarkko, thanks for the review!
I actually intended to take this through the fsverity tree. Is that okay?
BTW, we could actually make this change to verify_pkcs7_signature() itself. I wasn't sure it would be appropriate for all callers, though. Any thoughts?
- Eric