fsverity: skip PKCS#7 parser when keyring is empty
Eric Biggers
maintainerPATCH 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.
--- fs/verity/signature.c | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-)
fs/verity/signature.c +20 -4
@@ -70,10 +70,26 @@ int fsverity_verify_signature(const struct fsverity_info *vi, d->digest_size =cpu_to_le16 (hash_alg->digest_size);memcpy (d->digest, vi->file_digest, hash_alg->digest_size); - err =verify_pkcs7_signature (d,sizeof (*d) + hash_alg->digest_size, - signature, sig_size, fsverity_keyring, -VERIFYING_UNSPECIFIED_SIGNATURE , -NULL ,NULL ); +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. +*/ + err = -ENOKEY ; + }else { + err =verify_pkcs7_signature (d, +sizeof (*d) + hash_alg->digest_size, + signature, sig_size, + fsverity_keyring, +VERIFYING_UNSPECIFIED_SIGNATURE , +NULL ,NULL ); + }kfree (d);if (err) {
base-commit: 456ae5fe9b448f44ebe98b391a3bae9c75df465e
Signature · 1 line
2.41.0
Should the whole use of "d" be moved into the else block?
Quoted 58 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")Signed-off-by Eric Biggers <[email protected]>--- fs/verity/signature.c | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-)fs/verity/signature.c +20
-4@@ -70,10 +70,26 @@ int fsverity_verify_signature(const struct fsverity_info *vi, d->digest_size =cpu_to_le16 (hash_alg->digest_size);memcpy (d->digest, vi->file_digest, hash_alg->digest_size); - err =verify_pkcs7_signature (d,sizeof (*d) + hash_alg->digest_size, - signature, sig_size, fsverity_keyring, -VERIFYING_UNSPECIFIED_SIGNATURE , -NULL ,NULL ); +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. +*/ + err = -ENOKEY ; + }else { + err =verify_pkcs7_signature (d, +sizeof (*d) + hash_alg->digest_size, + signature, sig_size, + fsverity_keyring, +VERIFYING_UNSPECIFIED_SIGNATURE , +NULL ,NULL ); + }kfree (d);if (err) {base-commit:
456ae5fe9b448f44ebe98b391a3bae9c75df465e--
2.41.0
Eric Biggers
maintainerQuoted 1 line · Victor Hsieh
Should the whole use of "d" be moved into the else block?
In v2, changed to use an early return instead.
- Eric
Quoted 53 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")Signed-off-by Eric Biggers <[email protected]>--- fs/verity/signature.c | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-)fs/verity/signature.c +20
-4@@ -70,10 +70,26 @@ int fsverity_verify_signature(const struct fsverity_info *vi, d->digest_size =cpu_to_le16 (hash_alg->digest_size);memcpy (d->digest, vi->file_digest, hash_alg->digest_size); - err =verify_pkcs7_signature (d,sizeof (*d) + hash_alg->digest_size, - signature, sig_size, fsverity_keyring, -VERIFYING_UNSPECIFIED_SIGNATURE , -NULL ,NULL ); +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. +*/ + err = -ENOKEY ; + }else { + err =verify_pkcs7_signature (d, +sizeof (*d) + hash_alg->digest_size, + signature, sig_size, + fsverity_keyring, +VERIFYING_UNSPECIFIED_SIGNATURE , +NULL ,NULL ); + }kfree (d);if (err) {
Quoted 3 lines
base-commit:
456ae5fe9b448f44ebe98b391a3bae9c75df465eSignature · 1 line
2.41.0
BR, Jarkko