Skip to contentSkip to conversation

KEYS: be careful with error codes in public_key_verify_signature()

PATCH New Eric Biggers 1 participant 1 message Nov 27, 2017

Subsystems (3)
Crypto API Keys/keyrings Asymmetric keys
lore
Share & tools
lore

Eric Biggers

author
#0 lore raw

PATCH 1/1 KEYS: be careful with error codes in public_key_verify_signature()

From: Eric Biggers <[email protected]>

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 <[email protected]>
---
 crypto/asymmetric_keys/public_key.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)
crypto/asymmetric_keys/public_key.c +5 -2
index bc3035ef27a2..de996586762a 100644
@@ -73,7 +73,7 @@ int public_key_verify_signature(const struct public_key *pkey,
 	char alg_name_buf[CRYPTO_MAX_ALG_NAME];
 	void *output;
 	unsigned int outlen;
-	int ret = -ENOMEM;
+	int ret;
 
 	pr_devel("=>%s()\n", __func__);
 

@@ -99,6 +99,7 @@ int public_key_verify_signature(const struct public_key *pkey,
 	if (IS_ERR(tfm))
 		return PTR_ERR(tfm);
 
+	ret = -ENOMEM;
 	req = akcipher_request_alloc(tfm, GFP_KERNEL);
 	if (!req)
 		goto error_free_tfm;

@@ -127,7 +128,7 @@ int public_key_verify_signature(const struct public_key *pkey,
 	 * signature and returns that to us.
 	 */
 	ret = crypto_wait_req(crypto_akcipher_verify(req), &cwait);
-	if (ret < 0)
+	if (ret)
 		goto out_free_output;
 
 	/* Do the actual verification step. */

@@ -142,6 +143,8 @@ int public_key_verify_signature(const struct public_key *pkey,
 error_free_tfm:
 	crypto_free_akcipher(tfm);
 	pr_devel("<=%s() = %d\n", __func__, ret);
+	if (WARN_ON_ONCE(ret > 0))
+		ret = -EINVAL;
 	return ret;
 }
 EXPORT_SYMBOL_GPL(public_key_verify_signature);
Signature · 1 line
2.15.0