netdev - [PATCH] net: tls: fix tls with sk_redirect using a BPF verdict.

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-next>] [day] [month] [year] [list]

Message-Id: <20220628152505.298790-1-julien.salleyron@gmail.com>
Date:   Tue, 28 Jun 2022 17:25:05 +0200
From:   Julien Salleyron <julien.salleyron@...il.com>
To:     bpf@...r.kernel.org, netdev@...r.kernel.org
Cc:     Julien Salleyron <julien.salleyron@...il.com>,
        Marc Vertes <mvertes@...e.fr>
Subject: [PATCH] net: tls: fix tls with sk_redirect using a BPF verdict.

This patch allows to use KTLS on a socket where we apply sk_redirect using a BPF
verdict program.

Without this patch, we see that the data received after the redirection are
decrypted but with an incorrect offset and length. It seems to us that the
offset and length are correct in the stream-parser data, but finally not applied
in the skb. We have simply applied those values to the skb.

In the case of regular sockets, we saw a big performance improvement from
applying redirect. This is not the case now with KTLS, may be related to the
following point.

It is still necessary to perform a read operation (never triggered) from user
space despite the redirection. It makes no sense, since this read operation is
not necessary on regular sockets without KTLS.

We do not see how to fix this problem without a change of architecture, for
example by performing TLS decrypt directly inside the BPF verdict program.

An example program can be found at
https://github.com/juliens/ktls-bpf_redirect-example/

Co-authored-by: Marc Vertes <mvertes@...e.fr>
---
 net/tls/tls_sw.c                           | 6 ++++++
 tools/testing/selftests/bpf/test_sockmap.c | 8 +++-----
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index 0513f82b8537..a409f8a251db 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -1839,8 +1839,14 @@ int tls_sw_recvmsg(struct sock *sk,
 			if (bpf_strp_enabled) {
 				/* BPF may try to queue the skb */
 				__skb_unlink(skb, &ctx->rx_list);
+
 				err = sk_psock_tls_strp_read(psock, skb);
+
 				if (err != __SK_PASS) {
+                    if (err == __SK_REDIRECT) {
+                        skb->data += rxm->offset;
+                        skb->len = rxm->full_len;
+                    }
 					rxm->offset = rxm->offset + rxm->full_len;
 					rxm->full_len = 0;
 					if (err == __SK_DROP)
diff --git a/tools/testing/selftests/bpf/test_sockmap.c b/tools/testing/selftests/bpf/test_sockmap.c
index 0fbaccdc8861..503e0f3d16a7 100644
--- a/tools/testing/selftests/bpf/test_sockmap.c
+++ b/tools/testing/selftests/bpf/test_sockmap.c
@@ -739,13 +739,11 @@ static int sendmsg_test(struct sockmap_options *opt)
 
 	if (ktls) {
 		/* Redirecting into non-TLS socket which sends into a TLS
-		 * socket is not a valid test. So in this case lets not
-		 * enable kTLS but still run the test.
+		 * socket is not a valid test. So in this case just skip
+		 * the test.
 		 */
 		if (!txmsg_redir || txmsg_ingress) {
-			err = sockmap_init_ktls(opt->verbose, rx_fd);
-			if (err)
-				return err;
+			return 0;
 		}
 		err = sockmap_init_ktls(opt->verbose, c1);
 		if (err)
-- 
2.36.1