Netdev List
 help / color / mirror / Atom feed
* [PATCH net v2] calipso: update payload_len when removing the CALIPSO option
@ 2026-10-08 15:28 Joas Antonio dos Santos
       [not found] ` <sashiko-outbox-165773@kernel.org>
  0 siblings, 1 reply; 3+ messages in thread
From: Joas Antonio dos Santos @ 2026-10-08 15:28 UTC (permalink / raw)
  To: Paul Moore
  Cc: Eric Dumazet, David S. Miller, Jakub Kicinski, Paolo Abeni,
	Simon Horman, netdev, linux-security-module, alice.kernel

calipso_skbuff_delattr() removes the CALIPSO option from the hop-by-hop
extension header, or the whole extension header if CALIPSO is its only
option, and pulls the skb by the removed length.  It never updates
ipv6hdr->payload_len, so after a successful removal the header still
claims the original length.

calipso_skbuff_setattr() already adjusts payload_len by the length it
adds; the delete path was missed.

This is reached on the forward path when a packet carrying CALIPSO is
sent to a destination mapped to an unlabeled NetLabel domain.  The
forwarded packet then has a payload_len larger than its real payload
by the removed length (8 to 264 bytes), and the receiver drops it as
truncated.  So removing the label on the forward path has been broken
for every packet that is not resegmented later (SYN, UDP, ICMPv6...).

Adjust payload_len after the header is moved.  BIG TCP packets carry
payload_len == 0 (see ipv6_set_payload_len()), and ipv6_payload_len()
then falls back to skb->len, so leave them alone.

Tested with client, router and server network namespaces on one kernel
(arm64, QEMU): SELinux permissive with a minimal mdp-generated policy
(network_peer_controls=1), "netlabelctl calipso add pass doi:16", the
default NetLabel mapping left unlabeled, IPv6 forwarding on the router.
The client sends 5 UDP datagrams with a CALIPSO hop-by-hop option
(DOI 16, set through IPV6_HOPOPTS) to the server through the router:

  before: router Ip6OutForwDatagrams 5, server receives 0,
          server Ip6InTruncatedPkts 5
  after:  router Ip6OutForwDatagrams 5, server receives 5,
          server Ip6InTruncatedPkts 0

Fixes: 2917f57b6bc1 ("calipso: Allow the lsm to label the skbuff directly.")
Signed-off-by: Joas Antonio dos Santos <joasantonio108@gmail.com>
Assisted-by: Claude:claude-opus-5-5
---
Changes in v2:
- Skip the adjustment for BIG TCP packets (payload_len == 0), as
  suggested by Eric Dumazet.
- Exercised the forward path and described the test in the changelog.
- calipso_skbuff_setattr() has the same BIG TCP issue with its
  htons(payload + len_delta); I will send that as a separate patch.

Reproducer for the forward test (as run on 602042bf29f6, arm64, QEMU,
CONFIG_SECURITY_SELINUX=y, CONFIG_NETLABEL=y, CONFIG_IPV6=y):

  # minimal policy from the kernel tree; an older checkpolicy may need
  # all policycap lines except network_peer_controls removed
  # scripts/selinux/mdp/mdp is built by 'make' with SELinux enabled
  scripts/selinux/mdp/mdp -m policy.conf file_contexts
  checkpolicy -U allow -M -o policy.33 policy.conf

  # boot with lsm=selinux enforcing=0, then:
  cat policy.33 > /sys/fs/selinux/load
  netlabelctl calipso add pass doi:16     # default mapping stays unlabeled

  ip netns add C; ip netns add R; ip netns add S
  ip link add cr type veth peer name rc
  ip link add rs type veth peer name sr
  ip link set cr netns C; ip link set rc netns R
  ip link set rs netns R; ip link set sr netns S
  ip -n C -6 addr add fd00:1::2/64 dev cr nodad; ip -n C link set cr up
  ip -n R -6 addr add fd00:1::1/64 dev rc nodad; ip -n R link set rc up
  ip -n R -6 addr add fd00:2::1/64 dev rs nodad; ip -n R link set rs up
  ip -n S -6 addr add fd00:2::2/64 dev sr nodad; ip -n S link set sr up
  ip -n C -6 route add default via fd00:1::1
  ip -n S -6 route add default via fd00:2::1
  ip netns exec R sh -c 'echo 1 > /proc/sys/net/ipv6/conf/all/forwarding'

  # a UDP listener on [::]:9999 in S (I used a small recv loop)
  ip netns exec C ./calsend fd00:2::2
  ip netns exec S grep -E 'Ip6InTruncatedPkts|Udp6InDatagrams' /proc/net/snmp6

calsend.c (UDP with a CALIPSO hop-by-hop option, DOI 16, valid CRC):

  /* Send UDP datagrams carrying a CALIPSO hop-by-hop option (DOI 16,
   * level 0, no categories) with a valid CRC. */
  #include <arpa/inet.h>
  #include <netinet/in.h>
  #include <stdint.h>
  #include <stdio.h>
  #include <string.h>
  #include <sys/socket.h>
  #include <unistd.h>

  static uint16_t crc_ccitt(uint16_t crc, const uint8_t *p, size_t len)
  {
  	while (len--) {
  		crc ^= *p++;
  		for (int i = 0; i < 8; i++)
  			crc = (crc & 1) ? (crc >> 1) ^ 0x8408 : crc >> 1;
  	}
  	return crc;
  }

  int main(int argc, char **argv)
  {
  	uint8_t hop[16] = {
  		0, 1,			/* next header (set by kernel), len 16 */
  		0x07, 8,		/* CALIPSO, option data length 8 */
  		0, 0, 0, 16,		/* DOI 16 */
  		0,			/* compartment length */
  		0,			/* sensitivity level */
  		0, 0,			/* CRC */
  		1, 2, 0, 0,		/* PadN */
  	};
  	uint8_t *opt = hop + 2;
  	uint16_t crc = ~crc_ccitt(0xffff, opt, 10);

  	opt[8] = crc & 0xff;
  	opt[9] = crc >> 8;

  	struct sockaddr_in6 a = { .sin6_family = AF_INET6, .sin6_port = htons(9999) };
  	inet_pton(AF_INET6, argv[1], &a.sin6_addr);
  	int s = socket(AF_INET6, SOCK_DGRAM, 0);
  	if (setsockopt(s, IPPROTO_IPV6, IPV6_HOPOPTS, hop, sizeof(hop))) {
  		perror("IPV6_HOPOPTS");
  		return 1;
  	}
  	for (int i = 0; i < 5; i++) {
  		sendto(s, "calipso", 7, 0, (void *)&a, sizeof(a));
  		usleep(200000);
  	}
  	printf("CALTEST: sent 5 UDP datagrams with CALIPSO to %s\n", argv[1]);
  	return 0;
  }

 net/ipv6/calipso.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/ipv6/calipso.c b/net/ipv6/calipso.c
index c6a34334e..5a42db4f8 100644
--- a/net/ipv6/calipso.c
+++ b/net/ipv6/calipso.c
@@ -1428,6 +1428,10 @@ static int calipso_skbuff_delattr(struct sk_buff *skb)
 		skb_pull(skb, delta);
 		memmove((char *)ip6_hdr + delta, ip6_hdr, size);
 		skb_reset_network_header(skb);
+		ip6_hdr = ipv6_hdr(skb);
+		/* BIG TCP packets have payload_len == 0 */
+		if (ip6_hdr->payload_len)
+			be16_add_cpu(&ip6_hdr->payload_len, -delta);
 	}
 
 	return 0;
-- 
2.43.0


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

* Re: [PATCH net v2] calipso: update payload_len when removing the CALIPSO option
       [not found]   ` <CAHC9VhS+bdL_YnLQtd0sPmKKfy9BvS-LWV_6EhAuu4qfUFe_mw@mail.gmail.com>
@ 2026-10-15 10:47     ` Paolo Abeni
  2026-10-15 23:14       ` Joas Antonio dos Santos
  0 siblings, 1 reply; 3+ messages in thread
From: Paolo Abeni @ 2026-10-15 10:47 UTC (permalink / raw)
  To: Paul Moore, sashiko-reviews
  Cc: linux-security-module, Joas Antonio dos Santos, network dev

On 10/14/26 16:02, Paul Moore wrote:
> On Fri, Oct 9, 2026 at 11:32 AM <sashiko-bot@kernel.org> wrote:
>>
>> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>> - [High] ipv6: calipso_skbuff_delattr corrupts MAC header on bridged packets
>> --
>>
>> commit 9797bee2d14350b4afedb726df3172b004b7badd
>> Author: Joas Antonio dos Santos <joasantonio108@gmail.com>
>>
>> calipso: update payload_len when removing the CALIPSO option
>>
>> This commit updates calipso_skbuff_delattr() to adjust the IPv6 payload
>> length after removing a CALIPSO option. The patch prevents forwarded packets
>> from being dropped as truncated by subtracting the removed length, while
>> leaving BIG TCP packets unchanged.
>>
>>> diff --git a/net/ipv6/calipso.c b/net/ipv6/calipso.c
>>> index c6a34334e657e..5a42db4f87472 100644
>>> --- a/net/ipv6/calipso.c
>>> +++ b/net/ipv6/calipso.c
>>> @@ -1428,6 +1428,10 @@ static int calipso_skbuff_delattr(struct sk_buff *skb)
>>>                skb_pull(skb, delta);
>>>                memmove((char *)ip6_hdr + delta, ip6_hdr, size);
>>>                skb_reset_network_header(skb);
>>
>> [Severity: High]
>> When a bridged IPv6 packet with a CALIPSO option is forwarded, br_netfilter
>> feeds it to the NF_INET_FORWARD hook where SELinux may call
>> calipso_skbuff_delattr() in net/ipv6/calipso.c. Does shifting the IPv6 header
>> forward by delta bytes here without repositioning the MAC header create a gap
>> that corrupts the MAC header on bridged packets?
> 
> I haven't chased down the bridging code to confirm if the problem
> identified by Sashiko is true, but I do think that in the bridging
> case we don't want to change the label on the packet as we might do in
> the routing case (depending on the runtime config).
> 
> This would need to be tested, but I believe we could simply add a
> nf_bridge_info_exists() check to selinux_ip_forward() prior to calling
> into selinux_netlbl_skbuff_setsid().  We would need to add dummy
> nf_bridge_info_exists() when CONFIG_BRIDGE_NETFILTER=n to
> netfilter_bridge.h but I can't imagine the netdev folks being opposed
> to such a change; I'm sure they will let us know either way.
The above looks like a pre-exiting issue, the patch itself looks
correct, right?

/P


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

* Re: [PATCH net v2] calipso: update payload_len when removing the CALIPSO option
  2026-10-15 10:47     ` Paolo Abeni
@ 2026-10-15 23:14       ` Joas Antonio dos Santos
  0 siblings, 0 replies; 3+ messages in thread
From: Joas Antonio dos Santos @ 2026-10-15 23:14 UTC (permalink / raw)
  To: Paolo Abeni
  Cc: Paul Moore, sashiko-reviews, linux-security-module, network dev

On Thu, 15 Oct 2026, Paolo Abeni wrote:
> The above looks like a pre-existing issue, the patch itself looks
> correct, right?

Hi Paolo, Paul,

Yes, the issue is pre-existing; the delattr patch doesn't change it.
The skb_pull()/memmove()/skb_reset_network_header() sequence quoted
by Sashiko predates this patch, which only adjusts payload_len after
it.

I tested Paul's idea on top of both CALIPSO patches (QEMU, veth +
br0 with bridge-nf-call-ip6tables=1, SELinux + NetLabel CALIPSO
DOI 16):

- without it, a bridged CALIPSO packet towards an unlabeled
  destination reaches the peer with a corrupted Ethernet header
  (garbage dst MAC, ethertype 0x0000) and is dropped; an unlabeled
  packet towards a CALIPSO-mapped destination never reaches the peer.
- with the nf_bridge_info_exists() check in selinux_ip_forward()
  (plus a stub returning false for CONFIG_BRIDGE_NETFILTER=n),
  5/5 packets are delivered unmodified in both cases.
- the routed path is unchanged: delattr 5/5, setattr still adds the
  option 5/5. W=1 is clean with BRIDGE_NETFILTER=y and =n.

Paul, if you like I can send it as a separate selinux patch with your
Suggested-by.

Thanks,
Joas

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

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

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-08 15:28 [PATCH net v2] calipso: update payload_len when removing the CALIPSO option Joas Antonio dos Santos
     [not found] ` <sashiko-outbox-165773@kernel.org>
     [not found]   ` <CAHC9VhS+bdL_YnLQtd0sPmKKfy9BvS-LWV_6EhAuu4qfUFe_mw@mail.gmail.com>
2026-10-15 10:47     ` Paolo Abeni
2026-10-15 23:14       ` Joas Antonio dos Santos

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