Skip to content

Commit

Permalink
KEYS: asymmetric: enforce that sig algo matches key algo
Browse files Browse the repository at this point in the history
Most callers of public_key_verify_signature(), including most indirect
callers via verify_signature() as well as pkcs7_verify_sig_chain(),
don't check that public_key_signature::pkey_algo matches
public_key::pkey_algo.  These should always match.  However, a malicious
signature could intentionally declare an unintended algorithm.  It is
essential that such signatures be rejected outright, or that the
algorithm of the *key* be used -- not the algorithm of the signature as
that would allow attackers to choose the algorithm used.

Currently, public_key_verify_signature() correctly uses the key's
algorithm when deciding which akcipher to allocate.  That's good.
However, it uses the signature's algorithm when deciding whether to do
the first step of SM2, which is incorrect.  Also, v4.19 and older
kernels used the signature's algorithm for the entire process.

Prevent such errors by making public_key_verify_signature() enforce that
the signature's algorithm (if given) matches the key's algorithm.

Also remove two checks of this done by callers, which are now redundant.

Cc: [email protected]
Tested-by: Stefan Berger <[email protected]>
Tested-by: Tianjia Zhang <[email protected]>
Signed-off-by: Eric Biggers <[email protected]>
Reviewed-by: Vitaly Chikunov <[email protected]>
Reviewed-by: Jarkko Sakkinen <[email protected]>
Signed-off-by: Jarkko Sakkinen <[email protected]>
  • Loading branch information
ebiggers authored and jarkkojs committed Mar 9, 2022
1 parent d3cff4a commit 2abc9c2
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 12 deletions.
6 changes: 0 additions & 6 deletions crypto/asymmetric_keys/pkcs7_verify.c
Original file line number Diff line number Diff line change
Expand Up @@ -174,12 +174,6 @@ static int pkcs7_find_key(struct pkcs7_message *pkcs7,
pr_devel("Sig %u: Found cert serial match X.509[%u]\n",
sinfo->index, certix);

if (strcmp(x509->pub->pkey_algo, sinfo->sig->pkey_algo) != 0) {
pr_warn("Sig %u: X.509 algo and PKCS#7 sig algo don't match\n",
sinfo->index);
continue;
}

sinfo->signer = x509;
return 0;
}
Expand Down
15 changes: 15 additions & 0 deletions crypto/asymmetric_keys/public_key.c
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,21 @@ int public_key_verify_signature(const struct public_key *pkey,
BUG_ON(!sig);
BUG_ON(!sig->s);

/*
* If the signature specifies a public key algorithm, it *must* match
* the key's actual public key algorithm.
*
* Small exception: ECDSA signatures don't specify the curve, but ECDSA
* keys do. So the strings can mismatch slightly in that case:
* "ecdsa-nist-*" for the key, but "ecdsa" for the signature.
*/
if (sig->pkey_algo) {
if (strcmp(pkey->pkey_algo, sig->pkey_algo) != 0 &&
(strncmp(pkey->pkey_algo, "ecdsa-", 6) != 0 ||
strcmp(sig->pkey_algo, "ecdsa") != 0))
return -EKEYREJECTED;
}

ret = software_key_determine_akcipher(sig->encoding,
sig->hash_algo,
pkey, alg_name);
Expand Down
6 changes: 0 additions & 6 deletions crypto/asymmetric_keys/x509_public_key.c
Original file line number Diff line number Diff line change
Expand Up @@ -116,12 +116,6 @@ int x509_check_for_self_signed(struct x509_certificate *cert)
goto out;
}

ret = -EKEYREJECTED;
if (strcmp(cert->pub->pkey_algo, cert->sig->pkey_algo) != 0 &&
(strncmp(cert->pub->pkey_algo, "ecdsa-", 6) != 0 ||
strcmp(cert->sig->pkey_algo, "ecdsa") != 0))
goto out;

ret = public_key_verify_signature(cert->pub, cert->sig);
if (ret < 0) {
if (ret == -ENOPKG) {
Expand Down

0 comments on commit 2abc9c2

Please sign in to comment.