Skip to contentSkip to conversation

crypto: ecdh - zeroize crpytographic keys after use

PATCH In review Hailey Mothershead 2 participants 4 messages Apr 11, 2024 – Apr 12, 2024 Crypto API

lore
Share & tools
lore

Hailey Mothershead

author
#0 lore raw

PATCH 1/2 crypto: ecdh - zeroize crpytographic keys after use

Fips 140-3 specifies that Sensitive Security Parameters (SSPs) must be zeroized after use and that overwriting these variables with a new SSP is not sufficient for zeroization. So explicitly zeroize the private key before it is overwritten in ecdh_set_secret.

It also requires that variables used in the creation of SSPs be zeroized once they are no longer in use. Zeroize the public key as it is used in the creation of the shared secret.

Signed-off-by Hailey Mothershead <[email protected]>
---
 crypto/ecdh.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
crypto/ecdh.c +3 -1
index 80afee3234fb..71599cadf0bc 100644
@@ -33,6 +33,8 @@ static int ecdh_set_secret(struct crypto_kpp *tfm, const void *buf,
 	    params.key_size > sizeof(u64) * ctx->ndigits)
 		return -EINVAL;
 
+	memset(ctx->private_key, 0, sizeof(ctx->private_key));
+
 	if (!params.key || !params.key_size)
 		return ecc_gen_privkey(ctx->curve_id, ctx->ndigits,
 				       ctx->private_key);

@@ -111,7 +113,7 @@ static int ecdh_compute_value(struct kpp_request *req)
 free_all:
 	kfree_sensitive(shared_secret);
 free_pubkey:
-	kfree(public_key);
+	kfree_sensitive(public_key);
 	return ret;
 }
Signature · 1 line
2.40.1
author
#1 lore raw

PATCH 2/2 crypto: aead,cipher - zeroize key buffer after use

I.G 9.7.B for FIPS 140-3 specifies that variables temporarily holding cryptographic information should be zeroized once they are no longer needed. Accomplish this by using kfree_sensitive for buffers that previously held the private key.

Signed-off-by Hailey Mothershead <[email protected]>
---
 crypto/aead.c   | 2 +-
 crypto/cipher.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
crypto/aead.c +1 -1
index 16991095270d..2592d5375de5 100644
@@ -36,7 +36,7 @@ static int setkey_unaligned(struct crypto_aead *tfm, const u8 *key,
 	memcpy(alignbuffer, key, keylen);
 	ret = crypto_aead_alg(tfm)->setkey(tfm, alignbuffer, keylen);
 	memset(alignbuffer, 0, keylen);
-	kfree(buffer);
+	kfree_sensitive(buffer);
 	return ret;
 }
crypto/cipher.c +1 -1
index b47141ed4a9f..efb87fa417e7 100644
@@ -35,7 +35,7 @@ static int setkey_unaligned(struct crypto_cipher *tfm, const u8 *key,
 	memcpy(alignbuffer, key, keylen);
 	ret = cia->cia_setkey(crypto_cipher_tfm(tfm), alignbuffer, keylen);
 	memset(alignbuffer, 0, keylen);
-	kfree(buffer);
+	kfree_sensitive(buffer);
 	return ret;
 
 }
Signature · 1 line
2.40.1
maintainer
#2 lore raw

Re: [PATCH 2/2] crypto: aead,cipher - zeroize key buffer after use

Quoted 16 lines · Hailey Mothershead

I.G 9.7.B for FIPS 140-3 specifies that variables temporarily holding cryptographic information should be zeroized once they are no longer needed. Accomplish this by using kfree_sensitive for buffers that previously held the private key.

Signed-off-by Hailey Mothershead <[email protected]>
---
 crypto/aead.c   | 2 +-
 crypto/cipher.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
crypto/aead.c +0 -0
index 16991095270d..2592d5375de5 100644

crypto/aead.c static int setkey_unaligned(struct crypto_aead *tfm, const u8 *key, · line 39

Quoted 5 lines
crypto/aead.c +1 -1
@@ -36,7 +36,7 @@ static int setkey_unaligned(struct crypto_aead *tfm, const u8 *key,
 	memcpy(alignbuffer, key, keylen);
 	ret = crypto_aead_alg(tfm)->setkey(tfm, alignbuffer, keylen);
 	memset(alignbuffer, 0, keylen);
-	kfree(buffer);
+	kfree_sensitive(buffer);

Please remove the now-redundant memset.

crypto/cipher.c static int setkey_unaligned(struct crypto_cipher *tfm, const u8 *key, · line 38

Quoted 10 lines
crypto/cipher.c +1 -1
index b47141ed4a9f..efb87fa417e7 100644
@@ -35,7 +35,7 @@ static int setkey_unaligned(struct crypto_cipher *tfm, const u8 *key,
 	memcpy(alignbuffer, key, keylen);
 	ret = cia->cia_setkey(crypto_cipher_tfm(tfm), alignbuffer, keylen);
 	memset(alignbuffer, 0, keylen);
-	kfree(buffer);
+	kfree_sensitive(buffer);

Ditto.

Thanks,

Signature · 3 lines
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
maintainer
#3 lore raw
In reply to Hailey Mothershead

crypto/ecdh.c static int ecdh_compute_value(struct kpp_request *req) · line 116

Quoted 7 lines · Hailey Mothershead
crypto/ecdh.c +1 -1
@@ -111,7 +113,7 @@ static int ecdh_compute_value(struct kpp_request *req)
 free_all:
 	kfree_sensitive(shared_secret);
 free_pubkey:
-	kfree(public_key);
+	kfree_sensitive(public_key);

It makes no sense to zero the public key. Nack.

Signature · 3 lines
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt