netdev - RE: [PATCH net 3/3] tls: zero the crypto information from tls

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <DB7PR04MB425294A2EA0DA1170CC2C33E8B020@DB7PR04MB4252.eurprd04.prod.outlook.com>
Date:   Wed, 5 Sep 2018 13:48:48 +0000
From:   Vakul Garg <vakul.garg@....com>
To:     Sabrina Dubroca <sd@...asysnail.net>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
CC:     Boris Pismenny <borisp@...lanox.com>,
        Ilya Lesokhin <ilyal@...lanox.com>,
        Aviad Yehezkel <aviadye@...lanox.com>,
        Dave Watson <davejwatson@...com>
Subject: RE: [PATCH net 3/3] tls: zero the crypto information from tls_context
 before freeing



> -----Original Message-----
> From: netdev-owner@...r.kernel.org <netdev-owner@...r.kernel.org> On
> Behalf Of Sabrina Dubroca
> Sent: Wednesday, September 5, 2018 6:52 PM
> To: netdev@...r.kernel.org
> Cc: Sabrina Dubroca <sd@...asysnail.net>; Boris Pismenny
> <borisp@...lanox.com>; Ilya Lesokhin <ilyal@...lanox.com>; Aviad
> Yehezkel <aviadye@...lanox.com>; Dave Watson <davejwatson@...com>
> Subject: [PATCH net 3/3] tls: zero the crypto information from tls_context
> before freeing
> 
> This contains key material in crypto_send_aes_gcm_128 and
> crypto_recv_aes_gcm_128.
> 
> Fixes: 3c4d7559159b ("tls: kernel TLS support")
> Signed-off-by: Sabrina Dubroca <sd@...asysnail.net>
> ---
>  include/net/tls.h  |  1 +
>  net/tls/tls_main.c | 14 ++++++++++++--
>  2 files changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/include/net/tls.h b/include/net/tls.h index
> d5c683e8bb22..2010d23112f9 100644
> --- a/include/net/tls.h
> +++ b/include/net/tls.h
> @@ -180,6 +180,7 @@ struct tls_context {
>  		struct tls_crypto_info crypto_recv;
>  		struct tls12_crypto_info_aes_gcm_128
> crypto_recv_aes_gcm_128;
>  	};
> +	char tls_crypto_ctx_end[0];

This makes the key zeroization dependent upon the position of unions in structure.
Can you zeroise the crypto_send, crypto_recv separately using two memzero_explicit calls? 

> 
>  	struct list_head list;
>  	struct net_device *netdev;
> diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c index
> 0d432d025471..d3a57c0b2182 100644
> --- a/net/tls/tls_main.c
> +++ b/net/tls/tls_main.c
> @@ -241,6 +241,16 @@ static void tls_write_space(struct sock *sk)
>  	ctx->sk_write_space(sk);
>  }
> 
> +static void tls_ctx_free(struct tls_context *ctx) {
> +	if (!ctx)
> +		return;
> +
> +	memzero_explicit(&ctx->crypto_send,
> +			 offsetof(struct tls_context, tls_crypto_ctx_end));
> +	kfree(ctx);
> +}
> +
>  static void tls_sk_proto_close(struct sock *sk, long timeout)  {
>  	struct tls_context *ctx = tls_get_ctx(sk); @@ -294,7 +304,7 @@
> static void tls_sk_proto_close(struct sock *sk, long timeout)  #else
>  	{
>  #endif
> -		kfree(ctx);
> +		tls_ctx_free(ctx);
>  		ctx = NULL;
>  	}
> 
> @@ -305,7 +315,7 @@ static void tls_sk_proto_close(struct sock *sk, long
> timeout)
>  	 * for sk->sk_prot->unhash [tls_hw_unhash]
>  	 */
>  	if (free_ctx)
> -		kfree(ctx);
> +		tls_ctx_free(ctx);
>  }
> 
>  static int do_tls_getsockopt_tx(struct sock *sk, char __user *optval,
> --
> 2.18.0