[PATCH net-next] Crypto/chtls: add/delete TLS header in driver

[PATCH net-next] Crypto/chtls: add/delete TLS header in driver

[Rohit Maheshwari]

Kernel TLS forms TLS header in kernel during encryption and removes
while decryption before giving packet back to user application. The
similar logic is introduced in chtls code as well.

Signed-off-by: Vinay Kumar Yadav <vinay.yadav@chelsio.com>
Signed-off-by: Rohit Maheshwari <rohitm@chelsio.com>
---
 drivers/crypto/chelsio/chtls/chtls_io.c | 41 ++++++++++++++++---------
 net/tls/tls_main.c                      |  1 +
 2 files changed, 28 insertions(+), 14 deletions(-)

diff --git a/drivers/crypto/chelsio/chtls/chtls_io.c b/drivers/crypto/chelsio/chtls/chtls_io.c
index 5cf9b021220b..7fdb17cde506 100644
--- a/drivers/crypto/chelsio/chtls/chtls_io.c
+++ b/drivers/crypto/chelsio/chtls/chtls_io.c
@@ -902,14 +902,6 @@ static int chtls_skb_copy_to_page_nocache(struct sock *sk,
 	return 0;
 }
 
-/* Read TLS header to find content type and data length */
-static int tls_header_read(struct tls_hdr *thdr, struct iov_iter *from)
-{
-	if (copy_from_iter(thdr, sizeof(*thdr), from) != sizeof(*thdr))
-		return -EFAULT;
-	return (__force int)cpu_to_be16(thdr->length);
-}
-
 static int csk_mem_free(struct chtls_dev *cdev, struct sock *sk)
 {
 	return (cdev->max_host_sndbuf - sk->sk_wmem_queued);
@@ -1022,15 +1014,20 @@ int chtls_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
 			goto wait_for_sndbuf;
 
 		if (is_tls_tx(csk) && !csk->tlshws.txleft) {
-			struct tls_hdr hdr;
+			unsigned char record_type = TLS_RECORD_TYPE_DATA;
 
-			recordsz = tls_header_read(&hdr, &msg->msg_iter);
-			size -= TLS_HEADER_LENGTH;
-			copied += TLS_HEADER_LENGTH;
+			if (unlikely(msg->msg_controllen)) {
+				err = tls_proccess_cmsg(sk, msg, &record_type);
+				if (err)
+					goto out_err;
+			}
+
+			recordsz = size;
 			csk->tlshws.txleft = recordsz;
-			csk->tlshws.type = hdr.type;
+			csk->tlshws.type = record_type;
+
 			if (skb)
-				ULP_SKB_CB(skb)->ulp.tls.type = hdr.type;
+				ULP_SKB_CB(skb)->ulp.tls.type = record_type;
 		}
 
 		if (!skb || (ULP_SKB_CB(skb)->flags & ULPCB_FLAG_NO_APPEND) ||
@@ -1521,6 +1518,22 @@ static int chtls_pt_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
 				}
 			}
 		}
+		/* Set record type if not already done. For a non-data record,
+		 * do not proceed if record type could not be copied.
+		 */
+		if (ULP_SKB_CB(skb)->flags & ULPCB_FLAG_TLS_HDR) {
+			struct tls_hdr *thdr = (struct tls_hdr *)skb->data;
+			int cerr = 0;
+
+			cerr = put_cmsg(msg, SOL_TLS, TLS_GET_RECORD_TYPE,
+					sizeof(thdr->type), &thdr->type);
+
+			if (cerr && thdr->type != TLS_RECORD_TYPE_DATA)
+				return -EIO;
+			/*  don't send tls header, skip copy */
+			goto skip_copy;
+		}
+
 		if (skb_copy_datagram_msg(skb, offset, msg, avail)) {
 			if (!copied) {
 				copied = -EFAULT;
diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c
index 82225bcc1117..c338b203ce8f 100644
--- a/net/tls/tls_main.c
+++ b/net/tls/tls_main.c
@@ -195,6 +195,7 @@ int tls_proccess_cmsg(struct sock *sk, struct msghdr *msg,
 
 	return rc;
 }
+EXPORT_SYMBOL(tls_proccess_cmsg);
 
 int tls_push_partial_record(struct sock *sk, struct tls_context *ctx,
 			    int flags)
-- 
2.18.1

Re: [PATCH net-next] Crypto/chtls: add/delete TLS header in driver

[Jakub Kicinski]

On Wed, 18 Mar 2020 19:03:04 +0530 Rohit Maheshwari wrote:
> @@ -1022,15 +1014,20 @@ int chtls_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
>  			goto wait_for_sndbuf;
>  
>  		if (is_tls_tx(csk) && !csk->tlshws.txleft) {
> -			struct tls_hdr hdr;
> +			unsigned char record_type = TLS_RECORD_TYPE_DATA;
>  
> -			recordsz = tls_header_read(&hdr, &msg->msg_iter);
> -			size -= TLS_HEADER_LENGTH;
> -			copied += TLS_HEADER_LENGTH;
> +			if (unlikely(msg->msg_controllen)) {
> +				err = tls_proccess_cmsg(sk, msg, &record_type);

This is for the TOE TLS offload, right?

Could you open code this in your driver? This function calls
tls_handle_open_record(), which should be fine with the code as is,
but someone may make an assumption that it's no called for TOE and
break your offload.

Given it's impossible to test the offloads without HW today, I'd 
rather not mix the TOE with the other TLS types..

> +				if (err)
> +					goto out_err;
> +			}
> +
> +			recordsz = size;
>  			csk->tlshws.txleft = recordsz;
> -			csk->tlshws.type = hdr.type;
> +			csk->tlshws.type = record_type;
> +
>  			if (skb)
> -				ULP_SKB_CB(skb)->ulp.tls.type = hdr.type;
> +				ULP_SKB_CB(skb)->ulp.tls.type = record_type;
>  		}
>  
>  		if (!skb || (ULP_SKB_CB(skb)->flags & ULPCB_FLAG_NO_APPEND) ||