On Tue, Feb 01, 2022 at 07:10:33PM -0800, Eric Biggers wrote:
On Wed, Feb 02, 2022 at 05:52:30AM +0300, Vitaly Chikunov wrote:
Eric,
On Mon, Jan 31, 2022 at 04:34:13PM -0800, Eric Biggers wrote:
From: Eric Biggers ebiggers@google.com
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.
Why should they match?
For the reasons I explain in the rest of the commit message. To summarize: to have a valid signature verification scheme the algorithm must be fixed by the key, and not attacker-controlled.
public_key_signature is the data prepared to verify the cert's signature. The cert's signature algorithm could be different from the public key algorithm defined in the cert itself. They should match only for self-signed certs. For example, you should be able to sign RSA public key with ECDSA signature and vice versa. Or 256-bit EC-RDSA with 512-bit EC-RDSA. This check will prevent this.
That has nothing to do with this patch, as this patch is only dealing with the signature. A cert's public key algorithm can be different, and that is fine.
You are right and I was mistaken about that (obscured by keyctl pkey_verify error and self-signed keys verification). Then it's all good!
I also tested these patches to work well with rsa-ecdsa and ecrdsa certificates using keyctl restrict_keyring.
Thanks,
diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c index 4fefb219bfdc8..aba7113d86c76 100644 --- a/crypto/asymmetric_keys/public_key.c +++ b/crypto/asymmetric_keys/public_key.c @@ -325,6 +325,21 @@ int public_key_verify_signature(const struct public_key *pkey, BUG_ON(!sig); BUG_ON(!sig->s);
- /*
* The signature's claimed public key algorithm *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)
return -EINVAL;This seem incorrect too, as sig->pkey_algo could be NULL for direct signature verification calls. For example, for keyctl pkey_verify.
We can make it optional if some callers aren't providing it. Of course, such callers wouldn't be able to verify ECDSA signatures.
- Eric