netdev - Re: [PATCH RFC net-next] net/tls: Implement getsockopt SOL_TLS 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: <20200828095223.21d07617@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Date:   Fri, 28 Aug 2020 09:52:23 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     Yutaro Hayakawa <yhayakawa3720@...il.com>
Cc:     netdev@...r.kernel.org, michio.honda@...ac.uk
Subject: Re: [PATCH RFC net-next] net/tls: Implement getsockopt SOL_TLS
 TLS_RX

On Tue, 18 Aug 2020 14:12:24 +0000 Yutaro Hayakawa wrote:

> @@ -352,7 +352,11 @@ static int do_tls_getsockopt_tx(struct sock *sk, char __user *optval,
>  	}
>  
>  	/* get user crypto info */
> -	crypto_info = &ctx->crypto_send.info;
> +	if (tx) {
> +		crypto_info = &ctx->crypto_send.info;
> +	} else {
> +		crypto_info = &ctx->crypto_recv.info;
> +	}

No need for parenthesis, if both branches have one line.

>  
>  	if (!TLS_CRYPTO_INFO_READY(crypto_info)) {
>  		rc = -EBUSY;
> @@ -378,11 +382,19 @@ static int do_tls_getsockopt_tx(struct sock *sk, char __user *optval,
>  			goto out;
>  		}
>  		lock_sock(sk);
> -		memcpy(crypto_info_aes_gcm_128->iv,
> -		       ctx->tx.iv + TLS_CIPHER_AES_GCM_128_SALT_SIZE,
> -		       TLS_CIPHER_AES_GCM_128_IV_SIZE);
> -		memcpy(crypto_info_aes_gcm_128->rec_seq, ctx->tx.rec_seq,
> -		       TLS_CIPHER_AES_GCM_128_REC_SEQ_SIZE);
> +		if (tx) {
> +			memcpy(crypto_info_aes_gcm_128->iv,
> +			       ctx->tx.iv + TLS_CIPHER_AES_GCM_128_SALT_SIZE,
> +			       TLS_CIPHER_AES_GCM_128_IV_SIZE);
> +			memcpy(crypto_info_aes_gcm_128->rec_seq, ctx->tx.rec_seq,
> +			       TLS_CIPHER_AES_GCM_128_REC_SEQ_SIZE);
> +		} else {
> +			memcpy(crypto_info_aes_gcm_128->iv,
> +			       ctx->rx.iv + TLS_CIPHER_AES_GCM_128_SALT_SIZE,
> +			       TLS_CIPHER_AES_GCM_128_IV_SIZE);
> +			memcpy(crypto_info_aes_gcm_128->rec_seq, ctx->rx.rec_seq,
> +			       TLS_CIPHER_AES_GCM_128_REC_SEQ_SIZE);
> +		}

Instead of all the duplication choose the right struct cipher_context
above, like we do for crypto_info.

>  		release_sock(sk);
>  		if (copy_to_user(optval,
>  				 crypto_info_aes_gcm_128,
> @@ -402,11 +414,19 @@ static int do_tls_getsockopt_tx(struct sock *sk, char __user *optval,
>  			goto out;
>  		}
>  		lock_sock(sk);
> -		memcpy(crypto_info_aes_gcm_256->iv,
> -		       ctx->tx.iv + TLS_CIPHER_AES_GCM_256_SALT_SIZE,
> -		       TLS_CIPHER_AES_GCM_256_IV_SIZE);
> -		memcpy(crypto_info_aes_gcm_256->rec_seq, ctx->tx.rec_seq,
> -		       TLS_CIPHER_AES_GCM_256_REC_SEQ_SIZE);
> +		if (tx) {
> +			memcpy(crypto_info_aes_gcm_256->iv,
> +			       ctx->tx.iv + TLS_CIPHER_AES_GCM_256_SALT_SIZE,
> +			       TLS_CIPHER_AES_GCM_256_IV_SIZE);
> +			memcpy(crypto_info_aes_gcm_256->rec_seq, ctx->tx.rec_seq,
> +			       TLS_CIPHER_AES_GCM_256_REC_SEQ_SIZE);
> +		} else {
> +			memcpy(crypto_info_aes_gcm_256->iv,
> +			       ctx->rx.iv + TLS_CIPHER_AES_GCM_256_SALT_SIZE,
> +			       TLS_CIPHER_AES_GCM_256_IV_SIZE);
> +			memcpy(crypto_info_aes_gcm_256->rec_seq, ctx->rx.rec_seq,
> +			       TLS_CIPHER_AES_GCM_256_REC_SEQ_SIZE);
> +		}

ditto.

>  		release_sock(sk);
>  		if (copy_to_user(optval,
>  				 crypto_info_aes_gcm_256,