[Linux-stable-mirror] [PATCH 4/9] X.509: fix BUG_ON() when hash algorithm is unsupported
From: Eric Biggers ebiggers@google.com
The X.509 parser mishandles the case where the certificate's signature's hash algorithm is not available in the crypto API. In this case, x509_get_sig_params() doesn't allocate the cert->sig->digest buffer; this part seems to be intentional. However, public_key_verify_signature() is still called via x509_check_for_self_signed(), which triggers the 'BUG_ON(!sig->digest)'.
Fix this by making public_key_verify_signature() return -ENOPKG if the hash buffer has not been allocated.
Reproducer when all the CONFIG_CRYPTO_SHA512* options are disabled:
openssl req -new -sha512 -x509 -batch -nodes -outform der \ | keyctl padd asymmetric desc @s
Fixes: 6c2dc5ae4ab7 ("X.509: Extract signature digest and make self-signed cert checks earlier") Reported-by: Paolo Valente paolo.valente@linaro.org Cc: Paolo Valente paolo.valente@linaro.org Cc: stable@vger.kernel.org # v4.7+ Signed-off-by: Eric Biggers ebiggers@google.com --- crypto/asymmetric_keys/public_key.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c index de996586762a..e929fe1e4106 100644 --- a/crypto/asymmetric_keys/public_key.c +++ b/crypto/asymmetric_keys/public_key.c @@ -79,9 +79,11 @@ int public_key_verify_signature(const struct public_key *pkey,
BUG_ON(!pkey); BUG_ON(!sig); - BUG_ON(!sig->digest); BUG_ON(!sig->s);
+ if (!sig->digest) + return -ENOPKG; + alg_name = sig->pkey_algo; if (strcmp(sig->pkey_algo, "rsa") == 0) { /* The data wangled by the RSA algorithm is typically padded
Eric Biggers ebiggers3@gmail.com wrote:
The X.509 parser mishandles the case where the certificate's signature's hash algorithm is not available in the crypto API. In this case, x509_get_sig_params() doesn't allocate the cert->sig->digest buffer; this part seems to be intentional.
Well, yes, that would be intentional: we can't digest the digestibles without access to a hash algorithm to do so and we can't allocate a digest buffer without knowing how big it should be.
Fix this by making public_key_verify_signature() return -ENOPKG if the hash buffer has not been allocated.
Hmmm... I'm not sure that this is the right place to do this, since it obscures a potential invalid argument within the kernel.
I'm more inclined that the users of X.509 certs should check x509->unsupported_sig (pkcs7_verify_sig_chain() does this already partially).
David
Hi David,
On Thu, Feb 08, 2018 at 03:07:30PM +0000, David Howells wrote:
Eric Biggers ebiggers3@gmail.com wrote:
The X.509 parser mishandles the case where the certificate's signature's hash algorithm is not available in the crypto API. In this case, x509_get_sig_params() doesn't allocate the cert->sig->digest buffer; this part seems to be intentional.
Well, yes, that would be intentional: we can't digest the digestibles without access to a hash algorithm to do so and we can't allocate a digest buffer without knowing how big it should be.
Fix this by making public_key_verify_signature() return -ENOPKG if the hash buffer has not been allocated.
Hmmm... I'm not sure that this is the right place to do this, since it obscures a potential invalid argument within the kernel.
I'm more inclined that the users of X.509 certs should check x509->unsupported_sig (pkcs7_verify_sig_chain() does this already partially).
Well, either way using BUG_ON() is inappropriate for recoverable problems where an error code can be returned. In this case we can simply indicate that the signature verification failed. Note that unprivileged users can reach this BUG_ON(), and it also appears to be reachable in at least 3 different ways... So it really has to be fixed.
And considering that the ->unsupported_sig and ->unsupported_key flags are almost completely broken anyway, it sounds like there would have to be a second patchset to actually fix them. So I'd rather you just took the more important fixes in patches 1-5 now as-is, and then a patchset to make certificates with unsupported algorithms be accepted in more cases can be done separately.
Thanks,
Eric
linux-stable-mirror@lists.linaro.org
-
David Howells -
Eric Biggers