[PATCH net v2] calipso: update payload_len when removing the CALIPSO option

[PATCH net v2] calipso: update payload_len when removing the CALIPSO option

[Joas Antonio dos Santos]

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

Re: [PATCH net v2] calipso: update payload_len when removing the CALIPSO option

[sashiko-bot]

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?

Because bridged packets bypass L3 routing and do not go through
neigh_output(), the bridge's transmission function br_dev_queue_push_xmit()
unconditionally calls skb_push(skb, ETH_HLEN). Since skb->data is shifted
by delta bytes during the pull, this incorrectly frames the transmitted MAC
header starting delta bytes into the original MAC header.

Could this result in the transmission of gap bytes and the loss of the
destination MAC address, causing severe packet corruption and drops on
L2 forwarded packets?

> +		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;

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