From b9d8fec927ef3cd157e6a0956f5ec89f6891ed27 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Mon, 3 Jun 2019 15:17:01 -0700 Subject: net/tls: don't look for decrypted frames on non-offloaded sockets If the RX config of a TLS socket is SW, there is no point iterating over the fragments and checking if frame is decrypted. It will always be fully encrypted. Note that in fully encrypted case the function doesn't actually touch any offload-related state, so it's safe to call for TLS_SW, today. Soon we will introduce code which can only be called for offloaded contexts. Signed-off-by: Jakub Kicinski Reviewed-by: Dirk van der Merwe Signed-off-by: David S. Miller --- net/tls/tls_sw.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'net/tls/tls_sw.c') diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c index 960494f437ac..f833407c789f 100644 --- a/net/tls/tls_sw.c +++ b/net/tls/tls_sw.c @@ -1492,9 +1492,11 @@ static int decrypt_skb_update(struct sock *sk, struct sk_buff *skb, if (!ctx->decrypted) { #ifdef CONFIG_TLS_DEVICE - err = tls_device_decrypted(sk, skb); - if (err < 0) - return err; + if (tls_ctx->rx_conf == TLS_HW) { + err = tls_device_decrypted(sk, skb); + if (err < 0) + return err; + } #endif /* Still not decrypted after tls_device */ if (!ctx->decrypted) { -- cgit v1.2.1 From fb0f886fa265f265ad126fc7cd7e8ec51e2f770f Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Mon, 3 Jun 2019 15:17:05 -0700 Subject: net/tls: don't pass version to tls_advance_record_sn() All callers pass prot->version as the last parameter of tls_advance_record_sn(), yet tls_advance_record_sn() itself needs a pointer to prot. Pass prot from callers. Signed-off-by: Jakub Kicinski Reviewed-by: Dirk van der Merwe Signed-off-by: David S. Miller --- net/tls/tls_sw.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) (limited to 'net/tls/tls_sw.c') diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c index f833407c789f..bef71e54fad0 100644 --- a/net/tls/tls_sw.c +++ b/net/tls/tls_sw.c @@ -534,7 +534,7 @@ static int tls_do_encryption(struct sock *sk, /* Unhook the record from context if encryption is not failure */ ctx->open_rec = NULL; - tls_advance_record_sn(sk, &tls_ctx->tx, prot->version); + tls_advance_record_sn(sk, prot, &tls_ctx->tx); return rc; } @@ -1486,7 +1486,6 @@ static int decrypt_skb_update(struct sock *sk, struct sk_buff *skb, struct tls_context *tls_ctx = tls_get_ctx(sk); struct tls_sw_context_rx *ctx = tls_sw_ctx_rx(tls_ctx); struct tls_prot_info *prot = &tls_ctx->prot_info; - int version = prot->version; struct strp_msg *rxm = strp_msg(skb); int pad, err = 0; @@ -1504,8 +1503,8 @@ static int decrypt_skb_update(struct sock *sk, struct sk_buff *skb, async); if (err < 0) { if (err == -EINPROGRESS) - tls_advance_record_sn(sk, &tls_ctx->rx, - version); + tls_advance_record_sn(sk, prot, + &tls_ctx->rx); return err; } @@ -1520,7 +1519,7 @@ static int decrypt_skb_update(struct sock *sk, struct sk_buff *skb, rxm->full_len -= pad; rxm->offset += prot->prepend_size; rxm->full_len -= prot->overhead_size; - tls_advance_record_sn(sk, &tls_ctx->rx, version); + tls_advance_record_sn(sk, prot, &tls_ctx->rx); ctx->decrypted = true; ctx->saved_data_ready(sk); } else { -- cgit v1.2.1 From 89fec474fa1ab2c754e48d29e1081a2c2bd22dc6 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Mon, 10 Jun 2019 21:40:00 -0700 Subject: net/tls: pass record number as a byte array TLS offload code casts record number to a u64. The buffer should be aligned to 8 bytes, but its actually a __be64, and the rest of the TLS code treats it as big int. Make the offload callbacks take a byte array, drivers can make the choice to do the ugly cast if they want to. Prepare for copying the record number onto the stack by defining a constant for max size of the byte array. Signed-off-by: Jakub Kicinski Reviewed-by: Dirk van der Merwe Signed-off-by: David S. Miller --- net/tls/tls_sw.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'net/tls/tls_sw.c') diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c index bef71e54fad0..c1d22290f1d0 100644 --- a/net/tls/tls_sw.c +++ b/net/tls/tls_sw.c @@ -2015,8 +2015,7 @@ static int tls_read_size(struct strparser *strp, struct sk_buff *skb) goto read_failure; } #ifdef CONFIG_TLS_DEVICE - handle_device_resync(strp->sk, TCP_SKB_CB(skb)->seq + rxm->offset, - *(u64*)tls_ctx->rx.rec_seq); + handle_device_resync(strp->sk, TCP_SKB_CB(skb)->seq + rxm->offset); #endif return data_len + TLS_HEADER_SIZE; @@ -2283,8 +2282,9 @@ int tls_set_sw_offload(struct sock *sk, struct tls_context *ctx, int tx) goto free_priv; } - /* Sanity-check the IV size for stack allocations. */ - if (iv_size > MAX_IV_SIZE || nonce_size > MAX_IV_SIZE) { + /* Sanity-check the sizes for stack allocations. */ + if (iv_size > MAX_IV_SIZE || nonce_size > MAX_IV_SIZE || + rec_seq_size > TLS_MAX_REC_SEQ_SIZE) { rc = -EINVAL; goto free_priv; } -- cgit v1.2.1 From fe58a5a02cd9f49d5868539b4146ec1e5e5176e4 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Mon, 10 Jun 2019 21:40:01 -0700 Subject: net/tls: rename handle_device_resync() handle_device_resync() doesn't describe the function very well. The function checks if resync should be issued upon parsing of a new record. Signed-off-by: Jakub Kicinski Reviewed-by: Dirk van der Merwe Signed-off-by: David S. Miller --- net/tls/tls_sw.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'net/tls/tls_sw.c') diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c index c1d22290f1d0..bc3a1b188d4a 100644 --- a/net/tls/tls_sw.c +++ b/net/tls/tls_sw.c @@ -2015,7 +2015,8 @@ static int tls_read_size(struct strparser *strp, struct sk_buff *skb) goto read_failure; } #ifdef CONFIG_TLS_DEVICE - handle_device_resync(strp->sk, TCP_SKB_CB(skb)->seq + rxm->offset); + tls_device_rx_resync_new_rec(strp->sk, + TCP_SKB_CB(skb)->seq + rxm->offset); #endif return data_len + TLS_HEADER_SIZE; -- cgit v1.2.1 From f953d33ba1225d68cf8790b4706d8c4410b15926 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Mon, 10 Jun 2019 21:40:02 -0700 Subject: net/tls: add kernel-driven TLS RX resync TLS offload device may lose sync with the TCP stream if packets arrive out of order. Drivers can currently request a resync at a specific TCP sequence number. When a record is found starting at that sequence number kernel will inform the device of the corresponding record number. This requires the device to constantly scan the stream for a known pattern (constant bytes of the header) after sync is lost. This patch adds an alternative approach which is entirely under the control of the kernel. Kernel tracks records it had to fully decrypt, even though TLS socket is in TLS_HW mode. If multiple records did not have any decrypted parts - it's a pretty strong indication that the device is out of sync. We choose the min number of fully encrypted records to be 2, which should hopefully be more than will get retransmitted at a time. After kernel decides the device is out of sync it schedules a resync request. If the TCP socket is empty the resync gets performed immediately. If socket is not empty we leave the record parser to resync when next record comes. Before resync in message parser we peek at the TCP socket and don't attempt the sync if the socket already has some of the next record queued. On resync failure (encrypted data continues to flow in) we retry with exponential backoff, up to once every 128 records (with a 16k record thats at most once every 2M of data). Signed-off-by: Jakub Kicinski Reviewed-by: Dirk van der Merwe Signed-off-by: David S. Miller --- net/tls/tls_sw.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'net/tls/tls_sw.c') diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c index bc3a1b188d4a..533eaa4826e5 100644 --- a/net/tls/tls_sw.c +++ b/net/tls/tls_sw.c @@ -2015,7 +2015,7 @@ static int tls_read_size(struct strparser *strp, struct sk_buff *skb) goto read_failure; } #ifdef CONFIG_TLS_DEVICE - tls_device_rx_resync_new_rec(strp->sk, + tls_device_rx_resync_new_rec(strp->sk, data_len + TLS_HEADER_SIZE, TCP_SKB_CB(skb)->seq + rxm->offset); #endif return data_len + TLS_HEADER_SIZE; -- cgit v1.2.1 From 13aecb17acabc2a92187d08f7ca93bb8aad62c6f Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Thu, 4 Jul 2019 14:50:36 -0700 Subject: net/tls: fix poll ignoring partially copied records David reports that RPC applications which use epoll() occasionally get stuck, and that TLS ULP causes the kernel to not wake applications, even though read() will return data. This is indeed true. The ctx->rx_list which holds partially copied records is not consulted when deciding whether socket is readable. Note that SO_RCVLOWAT with epoll() is and has always been broken for kernel TLS. We'd need to parse all records from the TCP layer, instead of just the first one. Fixes: 692d7b5d1f91 ("tls: Fix recvmsg() to be able to peek across multiple records") Reported-by: David Beckett Signed-off-by: Jakub Kicinski Reviewed-by: Dirk van der Merwe Signed-off-by: David S. Miller --- net/tls/tls_sw.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'net/tls/tls_sw.c') diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c index 455a782c7658..e2385183526e 100644 --- a/net/tls/tls_sw.c +++ b/net/tls/tls_sw.c @@ -1958,7 +1958,8 @@ bool tls_sw_stream_read(const struct sock *sk) ingress_empty = list_empty(&psock->ingress_msg); rcu_read_unlock(); - return !ingress_empty || ctx->recv_pkt; + return !ingress_empty || ctx->recv_pkt || + !skb_queue_empty(&ctx->rx_list); } static int tls_read_size(struct strparser *strp, struct sk_buff *skb) -- cgit v1.2.1