KEYS: be careful with error codes in public_key_verify_signature()
In public_key_verify_signature(), if akcipher_request_alloc() fails, we return -ENOMEM. But that error code was set 25 lines above, and by accident someone could easily insert new code in between that assigns to 'ret', which would introduce a signature verification bypass. Make the code clearer by moving the -ENOMEM down to where it is used. Additionally, the callers of public_key_verify_signature() only consider a negative return value to be an error. This means that if any positive return value is accidentally introduced deeper in the call stack (e.g. 'return EBADMSG' instead of 'return -EBADMSG' somewhere in RSA), signature verification will be bypassed. Make things more robust by having public_key_verify_signature() warn about positive errors and translate them into -EINVAL. Signed-off-by: Eric Biggers <ebiggers@google.com> Signed-off-by: David Howells <dhowells@redhat.com>
This commit is contained in:
parent
a80745a6de
commit
72f9a07b6b
1 changed files with 5 additions and 2 deletions
|
@ -73,7 +73,7 @@ int public_key_verify_signature(const struct public_key *pkey,
|
||||||
char alg_name_buf[CRYPTO_MAX_ALG_NAME];
|
char alg_name_buf[CRYPTO_MAX_ALG_NAME];
|
||||||
void *output;
|
void *output;
|
||||||
unsigned int outlen;
|
unsigned int outlen;
|
||||||
int ret = -ENOMEM;
|
int ret;
|
||||||
|
|
||||||
pr_devel("==>%s()\n", __func__);
|
pr_devel("==>%s()\n", __func__);
|
||||||
|
|
||||||
|
@ -99,6 +99,7 @@ int public_key_verify_signature(const struct public_key *pkey,
|
||||||
if (IS_ERR(tfm))
|
if (IS_ERR(tfm))
|
||||||
return PTR_ERR(tfm);
|
return PTR_ERR(tfm);
|
||||||
|
|
||||||
|
ret = -ENOMEM;
|
||||||
req = akcipher_request_alloc(tfm, GFP_KERNEL);
|
req = akcipher_request_alloc(tfm, GFP_KERNEL);
|
||||||
if (!req)
|
if (!req)
|
||||||
goto error_free_tfm;
|
goto error_free_tfm;
|
||||||
|
@ -127,7 +128,7 @@ int public_key_verify_signature(const struct public_key *pkey,
|
||||||
* signature and returns that to us.
|
* signature and returns that to us.
|
||||||
*/
|
*/
|
||||||
ret = crypto_wait_req(crypto_akcipher_verify(req), &cwait);
|
ret = crypto_wait_req(crypto_akcipher_verify(req), &cwait);
|
||||||
if (ret < 0)
|
if (ret)
|
||||||
goto out_free_output;
|
goto out_free_output;
|
||||||
|
|
||||||
/* Do the actual verification step. */
|
/* Do the actual verification step. */
|
||||||
|
@ -142,6 +143,8 @@ error_free_req:
|
||||||
error_free_tfm:
|
error_free_tfm:
|
||||||
crypto_free_akcipher(tfm);
|
crypto_free_akcipher(tfm);
|
||||||
pr_devel("<==%s() = %d\n", __func__, ret);
|
pr_devel("<==%s() = %d\n", __func__, ret);
|
||||||
|
if (WARN_ON_ONCE(ret > 0))
|
||||||
|
ret = -EINVAL;
|
||||||
return ret;
|
return ret;
|
||||||
}
|
}
|
||||||
EXPORT_SYMBOL_GPL(public_key_verify_signature);
|
EXPORT_SYMBOL_GPL(public_key_verify_signature);
|
||||||
|
|
Loading…
Reference in a new issue