On Mon, Feb 21, 2022 at 02:46:26AM +0100, Jarkko Sakkinen wrote:
On Mon, Jan 31, 2022 at 04:34:14PM -0800, Eric Biggers wrote:
From: Eric Biggers ebiggers@google.com
It is insecure to allow arbitrary hash algorithms and signature encodings to be used with arbitrary signature algorithms. Notably, ECDSA, ECRDSA, and SM2 all sign/verify raw hash values and don't disambiguate between different hash algorithms like RSA PKCS#1 v1.5 padding does. Therefore, they need to be restricted to certain sets of hash algorithms (ideally just one, but in practice small sets are used). Additionally, the encoding is an integral part of modern signature algorithms, and is not supposed to vary.
Therefore, tighten the checks of hash_algo and encoding done by software_key_determine_akcipher().
Also rearrange the parameters to software_key_determine_akcipher() to put the public_key first, as this is the most important parameter and it often determines everything else.
Fixes: 299f561a6693 ("x509: Add support for parsing x509 certs with ECDSA keys") Fixes: 215525639631 ("X.509: support OSCCA SM2-with-SM3 certificate verification") Fixes: 0d7a78643f69 ("crypto: ecrdsa - add EC-RDSA (GOST 34.10) algorithm") Cc: stable@vger.kernel.org Signed-off-by: Eric Biggers ebiggers@google.com
crypto/asymmetric_keys/public_key.c | 111 +++++++++++++++++++--------- 1 file changed, 76 insertions(+), 35 deletions(-)
diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c index aba7113d86c76..a603ee8afdb8d 100644 --- a/crypto/asymmetric_keys/public_key.c +++ b/crypto/asymmetric_keys/public_key.c @@ -60,39 +60,83 @@ static void public_key_destroy(void *payload0, void *payload3) } /*
- Determine the crypto algorithm name.
- Given a public_key, and an encoding and hash_algo to be used for signing
- and/or verification with that key, determine the name of the corresponding
*/
- akcipher algorithm. Also check that encoding and hash_algo are allowed.
-static -int software_key_determine_akcipher(const char *encoding,
const char *hash_algo,const struct public_key *pkey,char alg_name[CRYPTO_MAX_ALG_NAME])+static int +software_key_determine_akcipher(const struct public_key *pkey,
const char *encoding, const char *hash_algo,char alg_name[CRYPTO_MAX_ALG_NAME])Why is changing parameter order necessary?
It's mentioned in the commit message. It's obviously not necessary but this way makes much more sense IMO.
- Eric