Skip to contentSkip to conversation

crypto: pkcs7_verify: use constant-time comparison for digest and signature verification

PATCH In review David C.C.M. Gall 2 participants 3 messages Jul 10 – Jul 13

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 crypto: pkcs7_verify: use constant-time comparison for digest and signature verification

Replace memcmp() with crypto_memneq() for cryptographic digest and signature comparisons to prevent timing side-channel attacks.

crypto/asymmetric_keys/pkcs7_verify.c: PKCS#7 message digest comparison during signature verification passes argument pkcs7 and attached signatures to pkcs7_digest via pkcs7_verify_one. pkcs7_digest utilized memcmp which could leak valid prefix length for attached signatures via timing side-channel.

Assisted-by gregkh_clanker_t1000
---
 crypto/asymmetric_keys/pkcs7_verify.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
crypto/asymmetric_keys/pkcs7_verify.c +3 -2
index 474e2c1ae21b..28953e53177b 100644
@@ -13,6 +13,7 @@
 #include <linux/asn1.h>
 #include <crypto/hash.h>
 #include <crypto/hash_info.h>
+#include <crypto/utils.h>
 #include <crypto/public_key.h>
 #include "pkcs7_parser.h"
 
@@ -93,8 +94,8 @@ static int pkcs7_digest(struct pkcs7_message *pkcs7,
 			goto error;
 		}
 
-		if (memcmp(sig->m, sinfo->msgdigest,
-			   sinfo->msgdigest_len) != 0) {
+		if (crypto_memneq(sig->m, sinfo->msgdigest,
+				  sinfo->msgdigest_len)) {
 			pr_warn("Sig %u: Message digest doesn't match\n",
 				sinfo->index);
 			ret = -EKEYREJECTED;
Signature · 1 line
2.43.0
#1 lore raw
Quoted 8 lines · David C.C.M. Gall

Replace memcmp() with crypto_memneq() for cryptographic digest and signature comparisons to prevent timing side-channel attacks.

crypto/asymmetric_keys/pkcs7_verify.c: PKCS#7 message digest comparison during signature verification passes argument pkcs7 and attached signatures to pkcs7_digest via pkcs7_verify_one. pkcs7_digest utilized memcmp which could leak valid prefix length for attached signatures via timing side-channel.

Please explain how this information is usable by an attacker? The assumption is the attacker sees the module (or whatever is signed) so the pkcs7 digest is inside the signature in plain text and the digest of the entity being compared should be computable by any attacker.

Regards,

James

author
#2 lore raw
Quoted 10 lines · James Bottomley
Quoted 8 lines · David C.C.M. Gall

Replace memcmp() with crypto_memneq() for cryptographic digest and signature comparisons to prevent timing side-channel attacks.

crypto/asymmetric_keys/pkcs7_verify.c: PKCS#7 message digest comparison during signature verification passes argument pkcs7 and attached signatures to pkcs7_digest via pkcs7_verify_one. pkcs7_digest utilized memcmp which could leak valid prefix length for attached signatures via timing side-channel.

Quoted 9 lines

Please explain how this information is usable by an attacker? The assumption is the attacker sees the module (or whatever is signed) so the pkcs7 digest is inside the signature in plain text and the digest of the entity being compared should be computable by any attacker.

Regards,

James

Looking into the usage of these methods a bit deeper, I agree with you that an attacker does not gain any useful information. I double checked and the method in question is also used as part of IMA modsig collection during the measurement collection process, but there too the method is not used to verify a signature, just to generate a hash, so the comparison itself is never reached in that path.

In that case, I'd actually drop this patch. The only affected paths don't disclose anything useful to an attacker that can't already be computed by just examining the content.

David