Linux Security Modules development
 help / color / mirror / Atom feed
* [PATCH] selinux: don't label bridged packets in selinux_ip_forward()
@ 2026-10-16 14:37 Joas Antonio dos Santos
  2026-10-16 19:11 ` Stephen Smalley
  2026-10-17 14:37 ` sashiko-bot
  0 siblings, 2 replies; 3+ messages in thread
From: Joas Antonio dos Santos @ 2026-10-16 14:37 UTC (permalink / raw)
  To: Paul Moore, Stephen Smalley
  Cc: Ondrej Mosnacek, Paolo Abeni, Pablo Neira Ayuso, Florian Westphal,
	Phil Sutter, selinux, linux-security-module, netfilter-devel,
	coreteam, netdev, linux-kernel

With br_netfilter and bridge-nf-call-ip{,6}tables enabled, bridged IP
packets go through the NF_INET_FORWARD hook, where selinux_ip_forward()
calls selinux_netlbl_skbuff_setsid() and NetLabel may add or remove a
CIPSO or CALIPSO option.  cipso_v4_skbuff_setattr(),
calipso_skbuff_setattr() and calipso_skbuff_delattr() move the IP
header and reset the network header, but the Ethernet header stays
where it was; when the bridge pushes it back in front of the packet it
picks up stale bytes and the frame goes out with a garbage destination
MAC and EtherType.

Labeling a bridged packet based on its IP addresses is not what we want
here anyway, so leave bridged packets alone.  Add a stub of
nf_bridge_info_exists() for CONFIG_BRIDGE_NETFILTER=n.

Tested with QEMU, two veths enslaved to a bridge with br_netfilter in
the middle, SELinux and a NetLabel DOI 16 configuration, 5 UDP packets
per case:

  case                                        before        after
  CALIPSO packet, unlabeled destination       MAC garbage   5/5
  unlabeled IPv6, CALIPSO destination         MAC garbage   5/5
  unlabeled IPv4, CIPSO destination           MAC garbage   5/5

Routed packets are still labeled (an unlabeled packet forwarded towards
a CALIPSO destination gets the option, 5/5).

Fixes: 948bf85c1bc9 ("netlabel: Add functionality to set the security attributes of a packet")
Link: https://lore.kernel.org/netdev/CAHC9VhS+bdL_YnLQtd0sPmKKfy9BvS-LWV_6EhAuu4qfUFe_mw@mail.gmail.com/
Suggested-by: Paul Moore <paul@paul-moore.com>
Signed-off-by: Joas Antonio dos Santos <joasantonio108@gmail.com>
Assisted-by: Claude:claude-opus-5-5
---
 include/linux/netfilter_bridge.h |  5 +++++
 security/selinux/hooks.c         | 13 ++++++++-----
 2 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/include/linux/netfilter_bridge.h b/include/linux/netfilter_bridge.h
index 743475ca7..c1f7c3cb2 100644
--- a/include/linux/netfilter_bridge.h
+++ b/include/linux/netfilter_bridge.h
@@ -79,6 +79,11 @@ static inline bool nf_bridge_in_prerouting(const struct sk_buff *skb)
 }
 #else
 #define br_drop_fake_rtable(skb)	        do { } while (0)
+static inline bool nf_bridge_info_exists(const struct sk_buff *skb)
+{
+	return false;
+}
+
 static inline bool nf_bridge_in_prerouting(const struct sk_buff *skb)
 {
 	return false;
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 30776b010..1e4f02f99 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -49,6 +49,7 @@
 #include <linux/fs_parser.h>
 #include <linux/netfilter_ipv4.h>
 #include <linux/netfilter_ipv6.h>
+#include <linux/netfilter_bridge.h>
 #include <linux/tty.h>
 #include <net/icmp.h>
 #include <net/ip.h>		/* for local_port_range[] */
@@ -6145,11 +6146,13 @@ static unsigned int selinux_ip_forward(void *priv, struct sk_buff *skb,
 				 SECCLASS_PACKET, PACKET__FORWARD_IN, &ad))
 			return NF_DROP;
 
-	if (netlbl_enabled())
-		/* we do this in the FORWARD path and not the POST_ROUTING
-		 * path because we want to make sure we apply the necessary
-		 * labeling before IPsec is applied so we can leverage AH
-		 * protection */
+	/* we do this in the FORWARD path and not the POST_ROUTING path
+	 * because we want to make sure we apply the necessary labeling
+	 * before IPsec is applied so we can leverage AH protection; bridged
+	 * packets are skipped as changing their IP options would leave the
+	 * Ethernet header behind
+	 */
+	if (netlbl_enabled() && !nf_bridge_info_exists(skb))
 		if (selinux_netlbl_skbuff_setsid(skb, family, peer_sid) != 0)
 			return NF_DROP;
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] selinux: don't label bridged packets in selinux_ip_forward()
  2026-10-16 14:37 [PATCH] selinux: don't label bridged packets in selinux_ip_forward() Joas Antonio dos Santos
@ 2026-10-16 19:11 ` Stephen Smalley
  2026-10-17 14:37 ` sashiko-bot
  1 sibling, 0 replies; 3+ messages in thread
From: Stephen Smalley @ 2026-10-16 19:11 UTC (permalink / raw)
  To: Joas Antonio dos Santos
  Cc: Paul Moore, Ondrej Mosnacek, Paolo Abeni, Pablo Neira Ayuso,
	Florian Westphal, Phil Sutter, selinux, linux-security-module,
	netfilter-devel, coreteam, netdev, linux-kernel

On Fri, Oct 16, 2026 at 10:37 AM Joas Antonio dos Santos
<joasantonio108@gmail.com> wrote:
>
> With br_netfilter and bridge-nf-call-ip{,6}tables enabled, bridged IP
> packets go through the NF_INET_FORWARD hook, where selinux_ip_forward()
> calls selinux_netlbl_skbuff_setsid() and NetLabel may add or remove a
> CIPSO or CALIPSO option.  cipso_v4_skbuff_setattr(),
> calipso_skbuff_setattr() and calipso_skbuff_delattr() move the IP
> header and reset the network header, but the Ethernet header stays
> where it was; when the bridge pushes it back in front of the packet it
> picks up stale bytes and the frame goes out with a garbage destination
> MAC and EtherType.
>
> Labeling a bridged packet based on its IP addresses is not what we want
> here anyway, so leave bridged packets alone.  Add a stub of
> nf_bridge_info_exists() for CONFIG_BRIDGE_NETFILTER=n.
>
> Tested with QEMU, two veths enslaved to a bridge with br_netfilter in
> the middle, SELinux and a NetLabel DOI 16 configuration, 5 UDP packets
> per case:
>
>   case                                        before        after
>   CALIPSO packet, unlabeled destination       MAC garbage   5/5
>   unlabeled IPv6, CALIPSO destination         MAC garbage   5/5
>   unlabeled IPv4, CIPSO destination           MAC garbage   5/5
>
> Routed packets are still labeled (an unlabeled packet forwarded towards
> a CALIPSO destination gets the option, 5/5).
>
> Fixes: 948bf85c1bc9 ("netlabel: Add functionality to set the security attributes of a packet")
> Link: https://lore.kernel.org/netdev/CAHC9VhS+bdL_YnLQtd0sPmKKfy9BvS-LWV_6EhAuu4qfUFe_mw@mail.gmail.com/
> Suggested-by: Paul Moore <paul@paul-moore.com>
> Signed-off-by: Joas Antonio dos Santos <joasantonio108@gmail.com>
> Assisted-by: Claude:claude-opus-5-5

Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] selinux: don't label bridged packets in selinux_ip_forward()
  2026-10-16 14:37 [PATCH] selinux: don't label bridged packets in selinux_ip_forward() Joas Antonio dos Santos
  2026-10-16 19:11 ` Stephen Smalley
@ 2026-10-17 14:37 ` sashiko-bot
  1 sibling, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-10-17 14:37 UTC (permalink / raw)
  Cc: selinux, linux-security-module

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] selinux: NetLabel bypass for routed bridge packets in selinux_ip_forward()
--

commit aa9d29a0c0e705f6d7d92b6c1e12ec004af2b214
Author: Joas Antonio dos Santos <joasantonio108@gmail.com>

selinux: don't label bridged packets in selinux_ip_forward()

This commit avoids labeling bridged IP packets in selinux_ip_forward() to
prevent MAC and EtherType corruption when adding or removing CIPSO or
CALIPSO options. It adds nf_bridge_info_exists() to skip bridged packets
since labeling based on IP addresses is unwanted there.

Link: https://lore.kernel.org/netdev/CAHC9VhS+bdL_YnLQtd0sPmKKfy9BvS-LWV_6EhAuu4qfUFe_mw@mail.gmail.com/

> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 18dd28b2bb13d..735e4889bd5b7 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
[ ... ]
> @@ -6021,11 +6022,13 @@ static unsigned int selinux_ip_forward(void *priv, struct sk_buff *skb,
>  				 SECCLASS_PACKET, PACKET__FORWARD_IN, &ad))
>  			return NF_DROP;
>  
> -	if (netlbl_enabled())
> -		/* we do this in the FORWARD path and not the POST_ROUTING
> -		 * path because we want to make sure we apply the necessary
> -		 * labeling before IPsec is applied so we can leverage AH
> -		 * protection */
> +	/* we do this in the FORWARD path and not the POST_ROUTING path
> +	 * because we want to make sure we apply the necessary labeling
> +	 * before IPsec is applied so we can leverage AH protection; bridged
> +	 * packets are skipped as changing their IP options would leave the
> +	 * Ethernet header behind
> +	 */
> +	if (netlbl_enabled() && !nf_bridge_info_exists(skb))
>  		if (selinux_netlbl_skbuff_setsid(skb, family, peer_sid) != 0)
>  			return NF_DROP;

[Severity: High]
Does this logic inadvertently skip NetLabel tagging for packets that are
legitimately routed rather than just bridged?

If a packet arrives on a bridge port, receives the SKB_EXT_BRIDGE_NF extension
in br_nf_pre_routing(), and is destined for the host's MAC address, it is
passed up to the local IP stack via br_pass_frame_up().

Since the bridge extension is not freed on the local input path, the packet
retains it.

When the packet is subsequently L3 forwarded via ip_forward(), the
NF_INET_FORWARD hook calls selinux_ip_forward().

Because nf_bridge_info_exists(skb) evaluates to true for these routed packets,
it appears SELinux will erroneously skip the NetLabel tagging, potentially
allowing a remote or local unprivileged attacker to bypass network security
policies.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/179216146138.25460.7866279004288174372@gmail.com?part=1

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-10-17 14:37 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-16 14:37 [PATCH] selinux: don't label bridged packets in selinux_ip_forward() Joas Antonio dos Santos
2026-10-16 19:11 ` Stephen Smalley
2026-10-17 14:37 ` sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox