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

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

[Hailey Mothershead]

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 <hailmo@amazon.com>
---
 crypto/ecdh.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/crypto/ecdh.c b/crypto/ecdh.c
index 80afee3234fb..71599cadf0bc 100644
--- a/crypto/ecdh.c
+++ b/crypto/ecdh.c
@@ -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;
 }
 
-- 
2.40.1

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

[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 <hailmo@amazon.com>
---
 crypto/aead.c   | 2 +-
 crypto/cipher.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/crypto/aead.c b/crypto/aead.c
index 16991095270d..2592d5375de5 100644
--- a/crypto/aead.c
+++ b/crypto/aead.c
@@ -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;
 }
 
diff --git a/crypto/cipher.c b/crypto/cipher.c
index b47141ed4a9f..efb87fa417e7 100644
--- a/crypto/cipher.c
+++ b/crypto/cipher.c
@@ -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;
 
 }
-- 
2.40.1

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

[Herbert Xu]

On Thu, Apr 11, 2024 at 11:51:57PM +0000, Hailey Mothershead wrote:
> 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 <hailmo@amazon.com>
> ---
>  crypto/aead.c   | 2 +-
>  crypto/cipher.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/crypto/aead.c b/crypto/aead.c
> index 16991095270d..2592d5375de5 100644
> --- a/crypto/aead.c
> +++ b/crypto/aead.c
> @@ -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.

> diff --git a/crypto/cipher.c b/crypto/cipher.c
> index b47141ed4a9f..efb87fa417e7 100644
> --- a/crypto/cipher.c
> +++ b/crypto/cipher.c
> @@ -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,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

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

[Herbert Xu]

On Thu, Apr 11, 2024 at 11:51:56PM +0000, Hailey Mothershead wrote:
>
> @@ -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.
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt