stable.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 1/2] KEYS: asymmetric: enforce that sig algo matches key algo
       [not found] <20220208052448.409152-1-ebiggers@kernel.org>
@ 2022-02-08  5:24 ` Eric Biggers
  2022-02-08  8:50   ` Vitaly Chikunov
  2022-02-08  5:24 ` [PATCH v2 2/2] KEYS: asymmetric: properly validate hash_algo and encoding Eric Biggers
  1 sibling, 1 reply; 4+ messages in thread
From: Eric Biggers @ 2022-02-08  5:24 UTC (permalink / raw)
  To: keyrings, Jarkko Sakkinen, David Howells
  Cc: linux-crypto, linux-integrity, Stefan Berger, Gilad Ben-Yossef,
	Tianjia Zhang, Vitaly Chikunov, Mimi Zohar, stable

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.  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: stable@vger.kernel.org
Tested-by: Stefan Berger <stefanb@linux.ibm.com>
Tested-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 crypto/asymmetric_keys/pkcs7_verify.c    |  6 ------
 crypto/asymmetric_keys/public_key.c      | 15 +++++++++++++++
 crypto/asymmetric_keys/x509_public_key.c |  6 ------
 3 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/crypto/asymmetric_keys/pkcs7_verify.c b/crypto/asymmetric_keys/pkcs7_verify.c
index 0b4d07aa8811..f94a1d1ad3a6 100644
--- a/crypto/asymmetric_keys/pkcs7_verify.c
+++ b/crypto/asymmetric_keys/pkcs7_verify.c
@@ -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;
 	}
diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c
index 4fefb219bfdc..e36213945686 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);
 
+	/*
+	 * 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);
diff --git a/crypto/asymmetric_keys/x509_public_key.c b/crypto/asymmetric_keys/x509_public_key.c
index fe14cae115b5..71cc1738fbfd 100644
--- a/crypto/asymmetric_keys/x509_public_key.c
+++ b/crypto/asymmetric_keys/x509_public_key.c
@@ -128,12 +128,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) {
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH v2 2/2] KEYS: asymmetric: properly validate hash_algo and encoding
       [not found] <20220208052448.409152-1-ebiggers@kernel.org>
  2022-02-08  5:24 ` [PATCH v2 1/2] KEYS: asymmetric: enforce that sig algo matches key algo Eric Biggers
@ 2022-02-08  5:24 ` Eric Biggers
  2022-02-08  9:26   ` Vitaly Chikunov
  1 sibling, 1 reply; 4+ messages in thread
From: Eric Biggers @ 2022-02-08  5:24 UTC (permalink / raw)
  To: keyrings, Jarkko Sakkinen, David Howells
  Cc: linux-crypto, linux-integrity, Stefan Berger, Gilad Ben-Yossef,
	Tianjia Zhang, Vitaly Chikunov, Mimi Zohar, stable

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
Tested-by: Stefan Berger <stefanb@linux.ibm.com>
Tested-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
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 e36213945686..7c9e6be35c30 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])
 {
 	int n;
 
-	if (strcmp(encoding, "pkcs1") == 0) {
-		/* The data wangled by the RSA algorithm is typically padded
-		 * and encoded in some manner, such as EMSA-PKCS1-1_5 [RFC3447
-		 * sec 8.2].
+	if (!encoding)
+		return -EINVAL;
+
+	if (strcmp(pkey->pkey_algo, "rsa") == 0) {
+		/*
+		 * RSA signatures usually use EMSA-PKCS1-1_5 [RFC3447 sec 8.2].
+		 */
+		if (strcmp(encoding, "pkcs1") == 0) {
+			if (!hash_algo)
+				n = snprintf(alg_name, CRYPTO_MAX_ALG_NAME,
+					     "pkcs1pad(%s)",
+					     pkey->pkey_algo);
+			else
+				n = snprintf(alg_name, CRYPTO_MAX_ALG_NAME,
+					     "pkcs1pad(%s,%s)",
+					     pkey->pkey_algo, hash_algo);
+			return n >= CRYPTO_MAX_ALG_NAME ? -EINVAL : 0;
+		}
+		if (strcmp(encoding, "raw") != 0)
+			return -EINVAL;
+		/*
+		 * Raw RSA cannot differentiate between different hash
+		 * algorithms.
+		 */
+		if (hash_algo)
+			return -EINVAL;
+	} else if (strncmp(pkey->pkey_algo, "ecdsa", 5) == 0) {
+		if (strcmp(encoding, "x962") != 0)
+			return -EINVAL;
+		/*
+		 * ECDSA signatures are taken over a raw hash, so they don't
+		 * differentiate between different hash algorithms.  That means
+		 * that the verifier should hard-code a specific hash algorithm.
+		 * Unfortunately, in practice ECDSA is used with multiple SHAs,
+		 * so we have to allow all of them and not just one.
 		 */
 		if (!hash_algo)
-			n = snprintf(alg_name, CRYPTO_MAX_ALG_NAME,
-				     "pkcs1pad(%s)",
-				     pkey->pkey_algo);
-		else
-			n = snprintf(alg_name, CRYPTO_MAX_ALG_NAME,
-				     "pkcs1pad(%s,%s)",
-				     pkey->pkey_algo, hash_algo);
-		return n >= CRYPTO_MAX_ALG_NAME ? -EINVAL : 0;
-	}
-
-	if (strcmp(encoding, "raw") == 0 ||
-	    strcmp(encoding, "x962") == 0) {
-		strcpy(alg_name, pkey->pkey_algo);
-		return 0;
+			return -EINVAL;
+		if (strcmp(hash_algo, "sha1") != 0 &&
+		    strcmp(hash_algo, "sha224") != 0 &&
+		    strcmp(hash_algo, "sha256") != 0 &&
+		    strcmp(hash_algo, "sha384") != 0 &&
+		    strcmp(hash_algo, "sha512") != 0)
+			return -EINVAL;
+	} else if (strcmp(pkey->pkey_algo, "sm2") == 0) {
+		if (strcmp(encoding, "raw") != 0)
+			return -EINVAL;
+		if (!hash_algo)
+			return -EINVAL;
+		if (strcmp(hash_algo, "sm3") != 0)
+			return -EINVAL;
+	} else if (strcmp(pkey->pkey_algo, "ecrdsa") == 0) {
+		if (strcmp(encoding, "raw") != 0)
+			return -EINVAL;
+		if (!hash_algo)
+			return -EINVAL;
+		if (strcmp(hash_algo, "streebog256") != 0 &&
+		    strcmp(hash_algo, "streebog512") != 0)
+			return -EINVAL;
+	} else {
+		/* Unknown public key algorithm */
+		return -ENOPKG;
 	}
-
-	return -ENOPKG;
+	if (strscpy(alg_name, pkey->pkey_algo, CRYPTO_MAX_ALG_NAME) < 0)
+		return -EINVAL;
+	return 0;
 }
 
 static u8 *pkey_pack_u32(u8 *dst, u32 val)
@@ -113,9 +157,8 @@ static int software_key_query(const struct kernel_pkey_params *params,
 	u8 *key, *ptr;
 	int ret, len;
 
-	ret = software_key_determine_akcipher(params->encoding,
-					      params->hash_algo,
-					      pkey, alg_name);
+	ret = software_key_determine_akcipher(pkey, params->encoding,
+					      params->hash_algo, alg_name);
 	if (ret < 0)
 		return ret;
 
@@ -179,9 +222,8 @@ static int software_key_eds_op(struct kernel_pkey_params *params,
 
 	pr_devel("==>%s()\n", __func__);
 
-	ret = software_key_determine_akcipher(params->encoding,
-					      params->hash_algo,
-					      pkey, alg_name);
+	ret = software_key_determine_akcipher(pkey, params->encoding,
+					      params->hash_algo, alg_name);
 	if (ret < 0)
 		return ret;
 
@@ -340,9 +382,8 @@ int public_key_verify_signature(const struct public_key *pkey,
 			return -EKEYREJECTED;
 	}
 
-	ret = software_key_determine_akcipher(sig->encoding,
-					      sig->hash_algo,
-					      pkey, alg_name);
+	ret = software_key_determine_akcipher(pkey, sig->encoding,
+					      sig->hash_algo, alg_name);
 	if (ret < 0)
 		return ret;
 
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v2 1/2] KEYS: asymmetric: enforce that sig algo matches key algo
  2022-02-08  5:24 ` [PATCH v2 1/2] KEYS: asymmetric: enforce that sig algo matches key algo Eric Biggers
@ 2022-02-08  8:50   ` Vitaly Chikunov
  0 siblings, 0 replies; 4+ messages in thread
From: Vitaly Chikunov @ 2022-02-08  8:50 UTC (permalink / raw)
  To: Eric Biggers
  Cc: keyrings, Jarkko Sakkinen, David Howells, linux-crypto,
	linux-integrity, Stefan Berger, Gilad Ben-Yossef, Tianjia Zhang,
	Mimi Zohar, stable

On Mon, Feb 07, 2022 at 09:24:47PM -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.  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: stable@vger.kernel.org
> Tested-by: Stefan Berger <stefanb@linux.ibm.com>
> Tested-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
> Signed-off-by: Eric Biggers <ebiggers@google.com>

Reviewed-by: Vitaly Chikunov <vt@altlinux.org>

Thanks,

> ---
>  crypto/asymmetric_keys/pkcs7_verify.c    |  6 ------
>  crypto/asymmetric_keys/public_key.c      | 15 +++++++++++++++
>  crypto/asymmetric_keys/x509_public_key.c |  6 ------
>  3 files changed, 15 insertions(+), 12 deletions(-)
> 
> diff --git a/crypto/asymmetric_keys/pkcs7_verify.c b/crypto/asymmetric_keys/pkcs7_verify.c
> index 0b4d07aa8811..f94a1d1ad3a6 100644
> --- a/crypto/asymmetric_keys/pkcs7_verify.c
> +++ b/crypto/asymmetric_keys/pkcs7_verify.c
> @@ -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;
>  	}
> diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c
> index 4fefb219bfdc..e36213945686 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);
>  
> +	/*
> +	 * 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);
> diff --git a/crypto/asymmetric_keys/x509_public_key.c b/crypto/asymmetric_keys/x509_public_key.c
> index fe14cae115b5..71cc1738fbfd 100644
> --- a/crypto/asymmetric_keys/x509_public_key.c
> +++ b/crypto/asymmetric_keys/x509_public_key.c
> @@ -128,12 +128,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) {
> -- 
> 2.35.1

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2 2/2] KEYS: asymmetric: properly validate hash_algo and encoding
  2022-02-08  5:24 ` [PATCH v2 2/2] KEYS: asymmetric: properly validate hash_algo and encoding Eric Biggers
@ 2022-02-08  9:26   ` Vitaly Chikunov
  0 siblings, 0 replies; 4+ messages in thread
From: Vitaly Chikunov @ 2022-02-08  9:26 UTC (permalink / raw)
  To: Eric Biggers
  Cc: keyrings, Jarkko Sakkinen, David Howells, linux-crypto,
	linux-integrity, Stefan Berger, Gilad Ben-Yossef, Tianjia Zhang,
	Mimi Zohar, stable

On Mon, Feb 07, 2022 at 09:24:48PM -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
> Tested-by: Stefan Berger <stefanb@linux.ibm.com>
> Tested-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
> Signed-off-by: Eric Biggers <ebiggers@google.com>

Reviewed-by: Vitaly Chikunov <vt@altlinux.org>

Small question below.

> ---
>  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 e36213945686..7c9e6be35c30 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])
>  {
>  	int n;
>  
> -	if (strcmp(encoding, "pkcs1") == 0) {
> -		/* The data wangled by the RSA algorithm is typically padded
> -		 * and encoded in some manner, such as EMSA-PKCS1-1_5 [RFC3447
> -		 * sec 8.2].
> +	if (!encoding)
> +		return -EINVAL;
> +
> +	if (strcmp(pkey->pkey_algo, "rsa") == 0) {
> +		/*
> +		 * RSA signatures usually use EMSA-PKCS1-1_5 [RFC3447 sec 8.2].
> +		 */
> +		if (strcmp(encoding, "pkcs1") == 0) {
> +			if (!hash_algo)
> +				n = snprintf(alg_name, CRYPTO_MAX_ALG_NAME,
> +					     "pkcs1pad(%s)",
> +					     pkey->pkey_algo);
> +			else
> +				n = snprintf(alg_name, CRYPTO_MAX_ALG_NAME,
> +					     "pkcs1pad(%s,%s)",
> +					     pkey->pkey_algo, hash_algo);
> +			return n >= CRYPTO_MAX_ALG_NAME ? -EINVAL : 0;
> +		}
> +		if (strcmp(encoding, "raw") != 0)
> +			return -EINVAL;

Should raw RSA be allowed at all? I understand it was there before, but
I wonder what is the use case.

Thanks,

> +		/*
> +		 * Raw RSA cannot differentiate between different hash
> +		 * algorithms.
> +		 */
> +		if (hash_algo)
> +			return -EINVAL;
> +	} else if (strncmp(pkey->pkey_algo, "ecdsa", 5) == 0) {
> +		if (strcmp(encoding, "x962") != 0)
> +			return -EINVAL;
> +		/*
> +		 * ECDSA signatures are taken over a raw hash, so they don't
> +		 * differentiate between different hash algorithms.  That means
> +		 * that the verifier should hard-code a specific hash algorithm.
> +		 * Unfortunately, in practice ECDSA is used with multiple SHAs,
> +		 * so we have to allow all of them and not just one.
>  		 */
>  		if (!hash_algo)
> -			n = snprintf(alg_name, CRYPTO_MAX_ALG_NAME,
> -				     "pkcs1pad(%s)",
> -				     pkey->pkey_algo);
> -		else
> -			n = snprintf(alg_name, CRYPTO_MAX_ALG_NAME,
> -				     "pkcs1pad(%s,%s)",
> -				     pkey->pkey_algo, hash_algo);
> -		return n >= CRYPTO_MAX_ALG_NAME ? -EINVAL : 0;
> -	}
> -
> -	if (strcmp(encoding, "raw") == 0 ||
> -	    strcmp(encoding, "x962") == 0) {
> -		strcpy(alg_name, pkey->pkey_algo);
> -		return 0;
> +			return -EINVAL;
> +		if (strcmp(hash_algo, "sha1") != 0 &&
> +		    strcmp(hash_algo, "sha224") != 0 &&
> +		    strcmp(hash_algo, "sha256") != 0 &&
> +		    strcmp(hash_algo, "sha384") != 0 &&
> +		    strcmp(hash_algo, "sha512") != 0)
> +			return -EINVAL;
> +	} else if (strcmp(pkey->pkey_algo, "sm2") == 0) {
> +		if (strcmp(encoding, "raw") != 0)
> +			return -EINVAL;
> +		if (!hash_algo)
> +			return -EINVAL;
> +		if (strcmp(hash_algo, "sm3") != 0)
> +			return -EINVAL;
> +	} else if (strcmp(pkey->pkey_algo, "ecrdsa") == 0) {
> +		if (strcmp(encoding, "raw") != 0)
> +			return -EINVAL;
> +		if (!hash_algo)
> +			return -EINVAL;
> +		if (strcmp(hash_algo, "streebog256") != 0 &&
> +		    strcmp(hash_algo, "streebog512") != 0)
> +			return -EINVAL;
> +	} else {
> +		/* Unknown public key algorithm */
> +		return -ENOPKG;
>  	}
> -
> -	return -ENOPKG;
> +	if (strscpy(alg_name, pkey->pkey_algo, CRYPTO_MAX_ALG_NAME) < 0)
> +		return -EINVAL;
> +	return 0;
>  }
>  
>  static u8 *pkey_pack_u32(u8 *dst, u32 val)
> @@ -113,9 +157,8 @@ static int software_key_query(const struct kernel_pkey_params *params,
>  	u8 *key, *ptr;
>  	int ret, len;
>  
> -	ret = software_key_determine_akcipher(params->encoding,
> -					      params->hash_algo,
> -					      pkey, alg_name);
> +	ret = software_key_determine_akcipher(pkey, params->encoding,
> +					      params->hash_algo, alg_name);
>  	if (ret < 0)
>  		return ret;
>  
> @@ -179,9 +222,8 @@ static int software_key_eds_op(struct kernel_pkey_params *params,
>  
>  	pr_devel("==>%s()\n", __func__);
>  
> -	ret = software_key_determine_akcipher(params->encoding,
> -					      params->hash_algo,
> -					      pkey, alg_name);
> +	ret = software_key_determine_akcipher(pkey, params->encoding,
> +					      params->hash_algo, alg_name);
>  	if (ret < 0)
>  		return ret;
>  
> @@ -340,9 +382,8 @@ int public_key_verify_signature(const struct public_key *pkey,
>  			return -EKEYREJECTED;
>  	}
>  
> -	ret = software_key_determine_akcipher(sig->encoding,
> -					      sig->hash_algo,
> -					      pkey, alg_name);
> +	ret = software_key_determine_akcipher(pkey, sig->encoding,
> +					      sig->hash_algo, alg_name);
>  	if (ret < 0)
>  		return ret;
>  
> -- 
> 2.35.1

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2022-02-08  9:26 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20220208052448.409152-1-ebiggers@kernel.org>
2022-02-08  5:24 ` [PATCH v2 1/2] KEYS: asymmetric: enforce that sig algo matches key algo Eric Biggers
2022-02-08  8:50   ` Vitaly Chikunov
2022-02-08  5:24 ` [PATCH v2 2/2] KEYS: asymmetric: properly validate hash_algo and encoding Eric Biggers
2022-02-08  9:26   ` Vitaly Chikunov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).