fsverity: skip PKCS#7 parser when keyring is empty
Eric Biggers
maintainerPATCH v2 1/1 fsverity: skip PKCS#7 parser when keyring is empty
From: Eric Biggers <[email protected]>
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.
---
v2: check keyring and return early before allocating formatted digest
fs/verity/signature.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
fs/verity/signature.c +15 -0
@@ -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
Signature · 1 line
2.41.0
Quoted 48 lines · Eric Biggers
From: Eric Biggers <[email protected]>
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")Acked-by Jarkko Sakkinen <[email protected]>Signed-off-by Eric Biggers <[email protected]>---
v2: check keyring and return early before allocating formatted digest
fs/verity/signature.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)fs/verity/signature.c +15
-0@@ -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 ;
Quoted 3 lines
base-commit:
456ae5fe9b448f44ebe98b391a3bae9c75df465eSignature · 1 line
2.41.0
applied
BR, Jarkko
Eric Biggers
maintainerQuoted 55 lines · Jarkko Sakkinen
Quoted 48 lines · Eric Biggers
From: Eric Biggers <[email protected]>
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")Acked-by Jarkko Sakkinen <[email protected]>Signed-off-by Eric Biggers <[email protected]>---
v2: check keyring and return early before allocating formatted digest
fs/verity/signature.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)fs/verity/signature.c +15
-0@@ -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 ;Quoted 3 lines
base-commit:
456ae5fe9b448f44ebe98b391a3bae9c75df465eSignature · 1 line
2.41.0Reviewed-by Jarkko Sakkinen <[email protected]>
Quoted 3 lines
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
Jarkko Sakkinen
maintainerQuoted 58 lines · Eric Biggers
Quoted 57 lines · Jarkko Sakkinen
Quoted 48 lines · Eric Biggers
From: Eric Biggers <[email protected]>
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")Acked-by Jarkko Sakkinen <[email protected]>Signed-off-by Eric Biggers <[email protected]>---
v2: check keyring and return early before allocating formatted digest
fs/verity/signature.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)fs/verity/signature.c +15
-0@@ -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 ;Quoted 3 lines
base-commit:
456ae5fe9b448f44ebe98b391a3bae9c75df465eSignature · 1 line
2.41.0Reviewed-by Jarkko Sakkinen <[email protected]>applied
Quoted 8 lines
Quoted 1 line
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?
It is OK for me. I just wanted make sure that I don't get yelled let's say month from now, why I haven't picked it already. That's why the "more eager" approach :-)
I'll drop it from my master branch today.
BR, Jarkko