[PATCH net 0/3] net: don't redirect decrypted skbs to ingress, psp time-wait validator

[PATCH net 0/3] net: don't redirect decrypted skbs to ingress, psp time-wait validator

[Todd Freed]

Three fixes that are relevant for the net-next series, "psp: introduce
socket-less associations". These were previously patches 6-8 of its v1:

  https://lore.kernel.org/netdev/20261111005533.6436-1-toddfreed@meta.com

The first two patches prevent sending decrypted skbs from egress to
ingress, since the meaning of skb->decrypted depends on the direction.
This affects the packets of kTLS and PSP device offload sockets.

The third patch makes psp_twsk_init() give a validator only to the
time-wait sockets of PSP connections with a Tx key, instead of to every
TCP time-wait socket. It has no effect today, but the net-next series
will rely on the presence of the validator to check if the socket has
an encryption offload.

Testing, on two hosts with mlx5 (ConnectX-7), with and without the
patches: an egress rule on a connection's flow sends its packets to the
device's ingress, with mirred (mirror and redirect) or bpf
(bpf_clone_redirect() and bpf_redirect() with BPF_F_INGRESS), for a PSP
device offload socket, a kTLS device offload socket and a plain TCP
socket.

Without the patches, the PSP and kTLS sockets' packets arrive on ingress
in all four cases. With them, none do: mirrored and cloned packets still
go out, encrypted by the device, and redirected ones are dropped. The
plain TCP socket's packets arrive on ingress in all four cases, with and
without the patches. The PSP and plain TCP cases give the same results
on netdevsim.

drivers/net psp.py and hw/tls_hw_offload.py pass, with and without the
patches, as does tc-testing mirred.json on netdevsim (39 pass, 7 skipped
by their dependsOn probes). Each patch builds without warnings (gcc and
clang, allmodconfig, W=1).

Todd Freed (3):
  net/sched: act_mirred: don't send decrypted skbs from egress to
    ingress
  bpf: don't redirect decrypted skbs from egress to ingress
  psp: only give time-wait sockets of PSP connections a validator

 net/core/filter.c      | 10 ++++++++++
 net/psp/psp_sock.c     |  7 +++++--
 net/sched/act_mirred.c |  6 ++++++
 3 files changed, 21 insertions(+), 2 deletions(-)


base-commit: ead6fa9a0ab194f02a2f36d634b193e9c5f9a1c6
-- 
2.53.1-Meta

[PATCH net 1/3] net/sched: act_mirred: don't send decrypted skbs from egress to ingress

[Todd Freed]

On transmit, skb->decrypted means that the device must encrypt the
packet, e.g. a packet of a kTLS or PSP device offload socket.

On receive, skb->decrypted means that the device decrypted the packet,
and kTLS device offload trusts it as such (tls_device_decrypted()).

A packet which mirred redirects or mirrors from egress to ingress keeps
the bit. On ingress, it then claims to have been decrypted by the device,
although it was never encrypted, and its payload, which was to be
encrypted before leaving the host, could then be forwarded in cleartext.

Clearing skb->decrypted would not work, because that is what indicates
intent to encrypt the packet.

Don't send decrypted skbs from egress to ingress: a redirected one is
dropped, a mirrored one isn't copied.

Fixes: 414776621d10 ("net/tls: prevent skb_orphan() from leaking TLS plain text with offload")
Signed-off-by: Todd Freed <toddfreed@meta.com>
---
 net/sched/act_mirred.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
index 1c98e4d81ebe..52c3eeab381a 100644
--- a/net/sched/act_mirred.c
+++ b/net/sched/act_mirred.c
@@ -285,6 +285,12 @@ static int tcf_mirred_to_dev(struct sk_buff *skb, struct tcf_mirred *m,
 		goto err_cant_do;
 	}
 
+	/* On transmit, decrypted asks the device to encrypt the packet. Sent
+	 * to ingress, the packet would read as decrypted by the device.
+	 */
+	if (unlikely(want_ingress && !at_ingress && skb_is_decrypted(skb)))
+		goto err_cant_do;
+
 	/* we could easily avoid the clone only if called by ingress and clsact;
 	 * since we can't easily detect the clsact caller, skip clone only for
 	 * ingress - that covers the TC S/W datapath.
-- 
2.53.1-Meta

[PATCH net 2/3] bpf: don't redirect decrypted skbs from egress to ingress

[Todd Freed]

On transmit, skb->decrypted means that the device must encrypt the
packet, e.g. a packet of a kTLS or PSP device offload socket.

On receive, skb->decrypted means that the device decrypted the packet,
and kTLS device offload trusts it as such (tls_device_decrypted()).

bpf_redirect() and bpf_clone_redirect() with BPF_F_INGRESS send the
packet to a device's receive path with the bit still set, where it claims
to have been decrypted by the device, although it was never encrypted.

In addition, the redirect drops the metadata dst (skb_scrub_packet()),
which will hold the Tx association for socket-less PSP offload in the
net-next series "psp: introduce socket-less associations".

Drop decrypted skbs which bpf redirects from egress to ingress, as mirred
does since the previous patch.

Fixes: 414776621d10 ("net/tls: prevent skb_orphan() from leaking TLS plain text with offload")
Signed-off-by: Todd Freed <toddfreed@meta.com>
---
 net/core/filter.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/net/core/filter.c b/net/core/filter.c
index 946b07a17fce..defbef3a9f75 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -2226,6 +2226,16 @@ static int __bpf_redirect_common(struct sk_buff *skb, struct net_device *dev,
 static int __bpf_redirect(struct sk_buff *skb, struct net_device *dev,
 			  u32 flags)
 {
+	/*
+	 * On transmit, decrypted asks the device to encrypt the packet. Sent
+	 * to ingress, the packet would read as decrypted by the device.
+	 */
+	if (unlikely((flags & BPF_F_INGRESS) && !skb_at_tc_ingress(skb) &&
+		     skb_is_decrypted(skb))) {
+		kfree_skb(skb);
+		return -EINVAL;
+	}
+
 	if (dev_is_mac_header_xmit(dev))
 		return __bpf_redirect_common(skb, dev, flags);
 	else
-- 
2.53.1-Meta

[PATCH net 3/3] psp: only give time-wait sockets of PSP connections a validator

[Todd Freed]

psp_twsk_init() sets tw_validate_xmit_skb on TCP time-wait sockets.
Today, that becomes set whether or not the socket has PSP offload, which
is not needed but is harmless. psp_validate_xmit() passes such packets.

But a validator makes a socket look like it has an encryption offload of
its own. The tc psp action in the net-next series "psp: introduce
socket-less associations" relies on the presence of the validator to
determine whether the socket has an encryption offload.

Set the validator only when the socket has a PSP association with a Tx
key, as psp_sock_set_tx_key() does for full sockets.

Fixes: 6b46ca260e22 ("net: psp: add socket security association code")
Signed-off-by: Todd Freed <toddfreed@meta.com>
---
 net/psp/psp_sock.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/net/psp/psp_sock.c b/net/psp/psp_sock.c
index a6b1c42dd626..1df65505f1c6 100644
--- a/net/psp/psp_sock.c
+++ b/net/psp/psp_sock.c
@@ -314,10 +314,13 @@ void psp_twsk_init(struct inet_timewait_sock *tw, const struct sock *sk)
 {
 	struct psp_assoc *pas = psp_sk_assoc(sk);
 
-	if (pas)
+	if (pas) {
 		refcount_inc(&pas->refcnt);
+		/* like the full socket case in psp_sock_set_tx_key() */
+		if (pas->tx.spi)
+			tw->tw_validate_xmit_skb = psp_validate_xmit;
+	}
 	rcu_assign_pointer(tw->psp_assoc, pas);
-	tw->tw_validate_xmit_skb = psp_validate_xmit;
 }
 
 void psp_twsk_assoc_free(struct inet_timewait_sock *tw)
-- 
2.53.1-Meta