[PATCH nf-next 0/2] netfilter: don't send decrypted skbs in clear text

[PATCH nf-next 0/2] netfilter: don't send decrypted skbs in clear text

[Todd Freed]

skb->decrypted marks a packet which the device decrypted on receive, or
must encrypt on transmit (tls device offload, psp). since this series in
net, the ip stack doesn't fragment such packets, and sends no icmp error
about a received one. an error about a locally originated one is only
sent if it is routed to a local address.

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

netfilter does both itself: the bridge refragments the packets which
bridge conntrack reassembled, and the reject code builds its own tcp
resets, and for the netdev and bridge families its own icmp errors. this
series applies the same rules there.

the psp socket-less associations series for net-next, which follows
this one, makes forwarding and bridging packets which a psp device
decrypted an intended use.

this series doesn't depend on the net series or the net-next series,
and doesn't conflict with them.

testing: on netdevsim psp, with the net series applied, nft reject of
psp socket traffic, with tcp reset and icmpx, for ipv4 and ipv6:

 - in an inet input chain and a netdev ingress chain, of the decrypted
   packets a host receives. without patch 2, the tcp resets and the
   netdev icmp errors are sent in the clear. with patch 2, they are not
   sent.
 - in an inet output chain, of the packets a host sends. with and
   without this series, tcp resets and icmp errors are sent to the local
   socket.

patch 1 is build-tested only

Todd Freed (2):
  netfilter: bridge: don't refragment decrypted skbs
  netfilter: reject: don't reply to decrypted skbs

 net/bridge/netfilter/nf_conntrack_bridge.c |  6 +++++
 net/ipv4/netfilter/nf_reject_ipv4.c        | 26 ++++++++++++++++++++
 net/ipv6/netfilter.c                       |  6 +++++
 net/ipv6/netfilter/nf_reject_ipv6.c        | 28 ++++++++++++++++++++++
 4 files changed, 66 insertions(+)


base-commit: 734bef548a96e8774d6eef5dfa2026c6edc4e80b
-- 
2.53.0-Meta

[PATCH nf-next v1 1/2] netfilter: bridge: don't refragment decrypted skbs

[Todd Freed]

on transmit, skb->decrypted means that the device must encrypt the skb
or drop it. this applies to psp offload and ktls offload. those
offloads cannot support fragments. they need the packet's l4 header, and
non-initial fragments do not have it. therefore, the ip stack doesn't
fragment decrypted skbs.

"net: don't fragment decrypted skbs" (net) drops decrypted skbs in
ip_do_fragment() and ip6_fragment(), instead of fragmenting them.

nf_conntrack_bridge refragments the packets it reassembled with its own
functions, which don't have that check: nf_br_ip_fragment() for ipv4,
and br_ip6_fragment() for ipv6. br_netfilter uses ip_do_fragment() and
ip6_fragment(), and is covered by the net patch.

a packet which a psp device decrypted, and which is bridged without
being re-encrypted, gets there if the peer sends ip fragments inside
psp: bridge conntrack reassembles them into one decrypted skb. its
fragments couldn't be encrypted, and the ones built from scratch don't
carry the decrypted mark, so they'd leave the bridge port in clear text.

drop decrypted skbs in nf_br_ip_fragment() and br_ip6_fragment(), as
the ip stack does.

Signed-off-by: Todd Freed <toddfreed@meta.com>
---
 net/bridge/netfilter/nf_conntrack_bridge.c | 6 ++++++
 net/ipv6/netfilter.c                       | 6 ++++++
 2 files changed, 12 insertions(+)

diff --git a/net/bridge/netfilter/nf_conntrack_bridge.c b/net/bridge/netfilter/nf_conntrack_bridge.c
index b5444335b86f..0069734853fd 100644
--- a/net/bridge/netfilter/nf_conntrack_bridge.c
+++ b/net/bridge/netfilter/nf_conntrack_bridge.c
@@ -38,6 +38,12 @@ static int nf_br_ip_fragment(struct net *net, struct sock *sk,
 	struct iphdr *iph;
 	int err = 0;
 
+	/* PSP and TLS offload need the packet's L4 header, which non-initial
+	 * fragments do not have.
+	 */
+	if (unlikely(skb_is_decrypted(skb)))
+		goto blackhole;
+
 	/* for offloaded checksums cleanup checksum before fragmentation */
 	if (skb->ip_summed == CHECKSUM_PARTIAL) {
 		err = ip_check_csum_start(skb);
diff --git a/net/ipv6/netfilter.c b/net/ipv6/netfilter.c
index a7025ec87035..e538f1c8d602 100644
--- a/net/ipv6/netfilter.c
+++ b/net/ipv6/netfilter.c
@@ -124,6 +124,12 @@ int br_ip6_fragment(struct net *net, struct sock *sk, struct sk_buff *skb,
 	int hroom, err = 0;
 	__be32 frag_id;
 
+	/* PSP and TLS offload need the packet's L4 header, which non-initial
+	 * fragments do not have.
+	 */
+	if (unlikely(skb_is_decrypted(skb)))
+		goto blackhole;
+
 	err = ip6_find_1stfragopt(skb, &prevhdr);
 	if (err < 0)
 		goto blackhole;
-- 
2.53.0-Meta

[PATCH nf-next v1 2/2] netfilter: reject: don't reply to decrypted skbs

[Todd Freed]

on receive, skb->decrypted means that the device decrypted the skb (tls
device offload, psp). for a decrypted skb, any part of the packet which
had been encrypted should not be sent back onto the network in
cleartext.

"ipv4: icmp: don't quote decrypted payload in icmp errors" and
"ipv6: icmpv6: don't quote decrypted payload in icmpv6 errors" (net)
make the ip stack send no icmp error about a received decrypted skb.
tls and psp skbs can't be told apart reliably, since
skb_scrub_packet() drops SKB_EXT_PSP and keeps skb->decrypted, so the
rule covers both.

the reject code builds its own replies, without icmp_send() and
icmpv6_send():

 - nf_reject_skb_v4_unreach() and nf_reject_skb_v6_unreach(), used by
   the reject expression in netdev and bridge chains, quote up to 536
   and 1220 bytes of the packet.
 - the tcp resets of nf_reject_skb_v4_tcp_reset(),
   nf_reject_skb_v6_tcp_reset(), nf_send_reset() and nf_send_reset6()
   carry the ports and sequence numbers of the packet's tcp header,
   which are encrypted with psp.

apply the same rule here:

 - nf_reject_skb_v4_tcp_reset(), nf_reject_skb_v4_unreach() and their
   ipv6 versions only get received skbs: the bridge and netdev reject
   expressions are limited to the prerouting, input and ingress hooks.
   don't send a reply for a decrypted skb.
 - nf_send_reset() and nf_send_reset6() also get locally originated
   skbs, from a reject rule in the output chain. don't send a reset
   about a received decrypted skb. for a locally originated, decrypted
   skb, only send the reset if it is routed to a local address
   (RTCF_LOCAL, RTF_LOCAL), as icmp_send() and icmpv6_send() do.

a reject rule then drops a received decrypted packet without a reply, as
it does a packet it can't reply to, and conntrack isn't moved to closing
either, as with a drop rule. nf_send_unreach() and nf_send_unreach6()
use icmp_send() and icmpv6_send(), and get the rule from the patches in
net.

Signed-off-by: Todd Freed <toddfreed@meta.com>
---
 net/ipv4/netfilter/nf_reject_ipv4.c | 26 ++++++++++++++++++++++++++
 net/ipv6/netfilter/nf_reject_ipv6.c | 28 ++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+)

diff --git a/net/ipv4/netfilter/nf_reject_ipv4.c b/net/ipv4/netfilter/nf_reject_ipv4.c
index 7cb33af671e2..1ad5ada45536 100644
--- a/net/ipv4/netfilter/nf_reject_ipv4.c
+++ b/net/ipv4/netfilter/nf_reject_ipv4.c
@@ -59,6 +59,12 @@ struct sk_buff *nf_reject_skb_v4_tcp_reset(struct net *net,
 	if (!nf_reject_iphdr_validate(oldskb))
 		return NULL;
 
+	/* Only received skbs get here (bridge, netdev ingress): see
+	 * nf_send_reset()
+	 */
+	if (unlikely(skb_is_decrypted(oldskb)))
+		return NULL;
+
 	oth = nf_reject_ip_tcphdr_get(oldskb, &_oth, hook);
 	if (!oth)
 		return NULL;
@@ -118,6 +124,12 @@ struct sk_buff *nf_reject_skb_v4_unreach(struct net *net,
 	if (!nf_reject_iphdr_validate(oldskb))
 		return NULL;
 
+	/* Only received skbs get here (bridge, netdev ingress): see
+	 * nf_send_reset()
+	 */
+	if (unlikely(skb_is_decrypted(oldskb)))
+		return NULL;
+
 	/* IP header checks: fragment. */
 	if (ip_hdr(oldskb)->frag_off & htons(IP_OFFSET))
 		return NULL;
@@ -287,6 +299,16 @@ void nf_send_reset(struct net *net, struct sock *sk, struct sk_buff *oldskb,
 	if (skb_rtable(oldskb)->rt_flags & (RTCF_BROADCAST | RTCF_MULTICAST))
 		return;
 
+	/* Don't send the decrypted part of an skb onto the network. A
+	 * received skb was decrypted by the device with crypto offload, which
+	 * in some cases encrypts the transport header. The different offloads
+	 * can't be reliably distinguished, so don't reply at all. A locally
+	 * originated skb to be encrypted gets a reply if the reply is routed to
+	 * a local address.
+	 */
+	if (unlikely(skb_is_decrypted(oldskb) && oldskb->skb_iif))
+		return;
+
 	nskb = alloc_skb(sizeof(struct iphdr) + sizeof(struct tcphdr) +
 			 LL_MAX_HEADER, GFP_ATOMIC);
 	if (!nskb)
@@ -304,6 +326,10 @@ void nf_send_reset(struct net *net, struct sock *sk, struct sk_buff *oldskb,
 	if (ip_route_me_harder(net, sk, nskb, RTN_UNSPEC))
 		goto free_nskb;
 
+	if (unlikely(skb_is_decrypted(oldskb)) &&
+	    !(skb_rtable(nskb)->rt_flags & RTCF_LOCAL))
+		goto free_nskb;
+
 	/* "Never happens" */
 	if (nskb->len > dst4_mtu(skb_dst(nskb)))
 		goto free_nskb;
diff --git a/net/ipv6/netfilter/nf_reject_ipv6.c b/net/ipv6/netfilter/nf_reject_ipv6.c
index 5e08d8529cc8..de815538ebf1 100644
--- a/net/ipv6/netfilter/nf_reject_ipv6.c
+++ b/net/ipv6/netfilter/nf_reject_ipv6.c
@@ -84,6 +84,12 @@ struct sk_buff *nf_reject_skb_v6_tcp_reset(struct net *net,
 	if (!nf_reject_ip6hdr_validate(oldskb))
 		return NULL;
 
+	/* Only received skbs get here (bridge, netdev ingress): see
+	 * nf_send_reset6()
+	 */
+	if (unlikely(skb_is_decrypted(oldskb)))
+		return NULL;
+
 	oth = nf_reject_ip6_tcphdr_get(oldskb, &_oth, &otcplen, hook);
 	if (!oth)
 		return NULL;
@@ -144,6 +150,12 @@ struct sk_buff *nf_reject_skb_v6_unreach(struct net *net,
 	if (!nf_reject_ip6hdr_validate(oldskb))
 		return NULL;
 
+	/* Only received skbs get here (bridge, netdev ingress): see
+	 * nf_send_reset6()
+	 */
+	if (unlikely(skb_is_decrypted(oldskb)))
+		return NULL;
+
 	/* Don't reply to ICMPV6_DEST_UNREACH with ICMPV6_DEST_UNREACH */
 	if (nf_skb_is_icmp6_unreach(oldskb))
 		return NULL;
@@ -331,6 +343,16 @@ void nf_send_reset6(struct net *net, struct sock *sk, struct sk_buff *oldskb,
 	if (!otcph)
 		return;
 
+	/* Don't send the decrypted part of an skb onto the network. A
+	 * received skb was decrypted by the device with crypto offload, which
+	 * in some cases encrypts the transport header. The different offloads
+	 * can't be reliably distinguished, so don't reply at all. A locally
+	 * originated skb to be encrypted gets a reply if the reply is routed to
+	 * a local address.
+	 */
+	if (unlikely(skb_is_decrypted(oldskb) && oldskb->skb_iif))
+		return;
+
 	memset(&fl6, 0, sizeof(fl6));
 	fl6.flowi6_proto = IPPROTO_TCP;
 	fl6.saddr = oip6h->daddr;
@@ -359,6 +381,12 @@ void nf_send_reset6(struct net *net, struct sock *sk, struct sk_buff *oldskb,
 	if (IS_ERR(dst))
 		return;
 
+	if (unlikely(skb_is_decrypted(oldskb)) &&
+	    !(dst_rt6_info(dst)->rt6i_flags & RTF_LOCAL)) {
+		dst_release(dst);
+		return;
+	}
+
 	nskb = alloc_skb(LL_MAX_HEADER + sizeof(struct ipv6hdr) +
 			 sizeof(struct tcphdr) + dst->trailer_len,
 			 GFP_ATOMIC);
-- 
2.53.0-Meta

Re: [PATCH nf-next 0/2] netfilter: don't send decrypted skbs in clear text

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.