diff mbox

crypto: atmel-aes - fix the keys zeroing on errors

Message ID 20180223090140.16334-1-antoine.tenart@bootlin.com (mailing list archive)
State Accepted
Delegated to: Herbert Xu
Headers show

Commit Message

Antoine Tenart Feb. 23, 2018, 9:01 a.m. UTC
The Atmel AES driver uses memzero_explicit on the keys on error, but the
variable zeroed isn't the right one because of a typo. Fix this by using
the right variable.

Fixes: 89a82ef87e01 ("crypto: atmel-authenc - add support to authenc(hmac(shaX), Y(aes)) modes")
Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
---
 drivers/crypto/atmel-aes.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Tudor Ambarus Feb. 23, 2018, 12:04 p.m. UTC | #1
Thanks Antoine,

On 02/23/2018 11:01 AM, Antoine Tenart wrote:
> The Atmel AES driver uses memzero_explicit on the keys on error, but the
> variable zeroed isn't the right one because of a typo. Fix this by using
> the right variable.
> 
> Fixes: 89a82ef87e01 ("crypto: atmel-authenc - add support to authenc(hmac(shaX), Y(aes)) modes")
> Signed-off-by: Antoine Tenart<antoine.tenart@bootlin.com>

Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>

There are few other places in crypto where we extract the authenc keys
in the same type of local variable, struct crypto_authenc_keys keys, and
we don't zeroize it after use. I think we should fix those cases too.

Bye,
ta
Antoine Tenart Feb. 23, 2018, 12:37 p.m. UTC | #2
Hi Tudor,

On Fri, Feb 23, 2018 at 02:04:33PM +0200, Tudor Ambarus wrote:
> 
> There are few other places in crypto where we extract the authenc keys
> in the same type of local variable, struct crypto_authenc_keys keys, and
> we don't zeroize it after use. I think we should fix those cases too.

You're right, I spotted other places where keys weren't zeroed. I
haven't got the time to do anything about it so far :)

Thanks,
Antoine
Herbert Xu March 2, 2018, 4:44 p.m. UTC | #3
On Fri, Feb 23, 2018 at 10:01:40AM +0100, Antoine Tenart wrote:
> The Atmel AES driver uses memzero_explicit on the keys on error, but the
> variable zeroed isn't the right one because of a typo. Fix this by using
> the right variable.
> 
> Fixes: 89a82ef87e01 ("crypto: atmel-authenc - add support to authenc(hmac(shaX), Y(aes)) modes")
> Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>

Patch applied.  Thanks.
Tudor Ambarus March 21, 2018, 1:15 p.m. UTC | #4
Hi, Antoine,

On 02/23/2018 02:37 PM, Antoine Tenart wrote:
> Hi Tudor,
> 
> On Fri, Feb 23, 2018 at 02:04:33PM +0200, Tudor Ambarus wrote:
>>
>> There are few other places in crypto where we extract the authenc keys
>> in the same type of local variable, struct crypto_authenc_keys keys, and
>> we don't zeroize it after use. I think we should fix those cases too.
> 
> You're right, I spotted other places where keys weren't zeroed. I
> haven't got the time to do anything about it so far :)
> 

Will you send a patch to fix those cases? We will forget about it.
I can do it myself, but it's better to ask first.

best,
ta
Antoine Tenart March 21, 2018, 1:25 p.m. UTC | #5
Hi Tudor,

On Wed, Mar 21, 2018 at 03:15:27PM +0200, Tudor Ambarus wrote:
> On 02/23/2018 02:37 PM, Antoine Tenart wrote:
> > On Fri, Feb 23, 2018 at 02:04:33PM +0200, Tudor Ambarus wrote:
> > > 
> > > There are few other places in crypto where we extract the authenc keys
> > > in the same type of local variable, struct crypto_authenc_keys keys, and
> > > we don't zeroize it after use. I think we should fix those cases too.
> > 
> > You're right, I spotted other places where keys weren't zeroed. I
> > haven't got the time to do anything about it so far :)
> > 
> 
> Will you send a patch to fix those cases? We will forget about it.
> I can do it myself, but it's better to ask first.

The sooner those issues are fixed the better, and I haven't done
anything yet. So yes, if you can, please take care of this :)

Thanks,
Antoine
diff mbox

Patch

diff --git a/drivers/crypto/atmel-aes.c b/drivers/crypto/atmel-aes.c
index 691c6465b71e..8561cce67741 100644
--- a/drivers/crypto/atmel-aes.c
+++ b/drivers/crypto/atmel-aes.c
@@ -2155,7 +2155,7 @@  static int atmel_aes_authenc_setkey(struct crypto_aead *tfm, const u8 *key,
 
 badkey:
 	crypto_aead_set_flags(tfm, CRYPTO_TFM_RES_BAD_KEY_LEN);
-	memzero_explicit(&key, sizeof(keys));
+	memzero_explicit(&keys, sizeof(keys));
 	return -EINVAL;
 }