From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [IPv6:2a0f:8001:1:32::40]) by lore.proxmox.com (Postfix) with ESMTPS id 816831FF0AE for ; Tue, 27 Oct 2026 09:55:51 +0100 (CET) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 2B733214CC; Tue, 27 Oct 2026 09:55:45 +0100 (CET) From: Hannes Laimer To: pve-devel@lists.proxmox.com Subject: [PATCH proxmox-ebpf v3 03/45] policy: add a check for non-first fragments Date: Tue, 27 Oct 2026 09:54:45 +0100 Message-ID: <20261027085527.702738-4-h.laimer@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20261027085527.702738-1-h.laimer@proxmox.com> References: <20261027085527.702738-1-h.laimer@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1793091336477 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.338 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: TD7YAVLZPGAVYH46WIFMVRZKBM3KRLZW X-Message-ID-Hash: TD7YAVLZPGAVYH46WIFMVRZKBM3KRLZW X-MailFrom: h.laimer@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: A non-first fragment has no transport header, so no check matched it, and a pair allowed only through narrowed rules dropped every later fragment. Add a check that matches non-first fragments of a specific (or any) protocol. Signed-off-by: Hannes Laimer --- src/policy/bpf/policy.bpf.c | 26 +++++++++++---- src/policy/bpf/types.h | 3 ++ src/policy/mod.rs | 63 ++++++++++++++++++++++--------------- src/policy/types.rs | 4 +++ tests/policy.rs | 62 ++++++++++++++++++++++++++++++++++++ 5 files changed, 127 insertions(+), 31 deletions(-) diff --git a/src/policy/bpf/policy.bpf.c b/src/policy/bpf/policy.bpf.c index 53acd35cc6df..fcf513bb13de 100644 --- a/src/policy/bpf/policy.bpf.c +++ b/src/policy/bpf/policy.bpf.c @@ -82,7 +82,10 @@ int tc_tap_ingress(struct __sk_buff *skb) { return TC_ACT_UNSPEC; } -// What a check matches on. `matchable` is set once the parse reached the L4 header. +// What a check matches on. `matchable` is set once the parse reached the L4 header. `fragment` +// marks a non-first fragment, which has a protocol and no L4 header. `unreadable` marks a frame +// the parse could not read to its L4 header, such as one cut short or behind a second tag in the +// frame data. struct frame { __u16 eth_proto; __u16 sport; @@ -95,6 +98,7 @@ struct frame { __u8 has_ports; __u8 has_icmp; __u8 unreadable; + __u8 fragment; }; static __always_inline int is_ext_header(__u8 proto) { @@ -114,7 +118,11 @@ static __always_inline int ipv6_l4(struct __sk_buff *skb, struct frame *f, __u32 return 0; } // the offset sits in the top 13 bits of the second halfword - if (hdr[2] || (hdr[3] & 0xf8)) return 0; + if (hdr[2] || (hdr[3] & 0xf8)) { + f->fragment = 1; + f->ip_proto = hdr[0]; + return 0; + } *off += 8; } else { if (bpf_skb_load_bytes(skb, *off, hdr, 2) < 0) { @@ -206,10 +214,12 @@ static __always_inline void parse_frame(struct __sk_buff *skb, struct frame *f) f->unreadable = 1; return; } - // only the first fragment carries the L4 header - if (bpf_ntohs(ip.frag_off) & IPV4_FRAG_OFFSET) return; f->family = FAMILY_V4; f->ip_proto = ip.protocol; + if (bpf_ntohs(ip.frag_off) & IPV4_FRAG_OFFSET) { + f->fragment = 1; + return; + } off += (__u32)ip.ihl * 4; } else if (f->eth_proto == ETHERTYPE_IPV6) { struct ipv6hdr ip6; @@ -229,9 +239,13 @@ static __always_inline void parse_frame(struct __sk_buff *skb, struct frame *f) } static __always_inline int check_matches(const struct frame *f, const struct check_value *c) { - if (!f->matchable) return 0; + if (c->match_flags & CHECK_FRAGMENT) { + if (!f->fragment) return 0; + } else if (!f->matchable) { + return 0; + } if (c->family && c->family != f->family) return 0; - if (c->ip_proto != f->ip_proto) return 0; + if ((c->match_flags & CHECK_HAS_PROTO) && c->ip_proto != f->ip_proto) return 0; if (c->match_flags & CHECK_HAS_SPORT) { if (!f->has_ports || f->sport < c->sport_lo || f->sport > c->sport_hi) return 0; diff --git a/src/policy/bpf/types.h b/src/policy/bpf/types.h index a8b1df66fc41..0bfe1d3b9038 100644 --- a/src/policy/bpf/types.h +++ b/src/policy/bpf/types.h @@ -26,6 +26,9 @@ #define CHECK_HAS_DPORT (1 << 1) #define CHECK_HAS_ICMP_TYPE (1 << 2) #define CHECK_HAS_ICMP_CODE (1 << 3) +// the check is for non-first fragments, which no other check matches +#define CHECK_FRAGMENT (1 << 4) +#define CHECK_HAS_PROTO (1 << 5) struct nic_entry { __u16 id; diff --git a/src/policy/mod.rs b/src/policy/mod.rs index 62ee7576b6fc..e3d3e580f7f3 100644 --- a/src/policy/mod.rs +++ b/src/policy/mod.rs @@ -76,8 +76,11 @@ pub enum Family { /// left out is not matched on. #[derive(Clone, Debug, Default, PartialEq, Eq)] pub struct CheckEntry { - pub ip_proto: u8, + pub ip_proto: Option, pub family: Family, + /// Match non-first fragments instead of frames with an L4 header. A fragment has no L4 header, + /// so a check that names a port or an ICMP field matches nothing. + pub fragment: bool, pub sport: Option<(u16, u16)>, pub dport: Option<(u16, u16)>, pub icmp_type: Option, @@ -88,6 +91,12 @@ pub struct CheckEntry { impl CheckEntry { fn value(&self) -> CheckValue { let mut match_flags = 0; + if self.fragment { + match_flags |= CHECK_FRAGMENT; + } + if self.ip_proto.is_some() { + match_flags |= CHECK_HAS_PROTO; + } if self.sport.is_some() { match_flags |= CHECK_HAS_SPORT; } @@ -103,7 +112,7 @@ impl CheckEntry { let (sport_lo, sport_hi) = self.sport.unwrap_or_default(); let (dport_lo, dport_hi) = self.dport.unwrap_or_default(); CheckValue { - ip_proto: self.ip_proto, + ip_proto: self.ip_proto.unwrap_or_default(), family: self.family as u8, match_flags, allow: self.allow as u8, @@ -681,13 +690,10 @@ mod tests { fn entry(port: u16) -> CheckEntry { CheckEntry { - ip_proto: 6, - family: Family::Any, - sport: None, + ip_proto: Some(6), dport: Some((port, port)), - icmp_type: None, - icmp_code: None, allow: true, + ..Default::default() } } @@ -718,25 +724,19 @@ mod tests { #[test] fn check_entry_bytes() { let plain = CheckEntry { - ip_proto: 17, - family: Family::Any, - sport: None, - dport: None, - icmp_type: None, - icmp_code: None, - allow: false, + ip_proto: Some(17), + ..Default::default() }; - assert_eq!( - bytes(plain.value()), - [17, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0] - ); + let b = bytes(plain.value()); + assert_eq!(b[..4], [17, 0, CHECK_HAS_PROTO, 0]); + assert_eq!(b[4..], [0; 12]); let v4 = CheckEntry { family: Family::V4, allow: true, ..plain.clone() }; - assert_eq!(bytes(v4.value())[..4], [17, 4, 0, 1]); + assert_eq!(bytes(v4.value())[..4], [17, 4, CHECK_HAS_PROTO, 1]); let v6 = CheckEntry { family: Family::V6, ..plain.clone() @@ -748,7 +748,7 @@ mod tests { ..plain.clone() }; let b = bytes(sport.value()); - assert_eq!(b[2], CHECK_HAS_SPORT); + assert_eq!(b[2], CHECK_HAS_PROTO | CHECK_HAS_SPORT); assert_eq!(b[4..6], 1000u16.to_ne_bytes()); assert_eq!(b[6..8], 2000u16.to_ne_bytes()); assert_eq!(b[8..], [0; 8]); @@ -758,27 +758,40 @@ mod tests { ..plain.clone() }; let b = bytes(dport.value()); - assert_eq!(b[2], CHECK_HAS_DPORT); + assert_eq!(b[2], CHECK_HAS_PROTO | CHECK_HAS_DPORT); assert_eq!(b[4..8], [0; 4]); assert_eq!(b[8..10], 22u16.to_ne_bytes()); assert_eq!(b[10..12], 23u16.to_ne_bytes()); let icmp = CheckEntry { - ip_proto: 1, + ip_proto: Some(1), icmp_type: Some(3), icmp_code: Some(4), ..plain.clone() }; let b = bytes(icmp.value()); - assert_eq!(b[2], CHECK_HAS_ICMP_TYPE | CHECK_HAS_ICMP_CODE); + assert_eq!( + b[2], + CHECK_HAS_PROTO | CHECK_HAS_ICMP_TYPE | CHECK_HAS_ICMP_CODE + ); assert_eq!(b[12..], [3, 4, 0, 0]); let code_only = CheckEntry { - ip_proto: 1, + ip_proto: Some(1), icmp_code: Some(0), + ..plain.clone() + }; + assert_eq!( + bytes(code_only.value())[2], + CHECK_HAS_PROTO | CHECK_HAS_ICMP_CODE + ); + + let fragment = CheckEntry { + ip_proto: None, + fragment: true, ..plain }; - assert_eq!(bytes(code_only.value())[2], CHECK_HAS_ICMP_CODE); + assert_eq!(bytes(fragment.value())[..3], [0, 0, CHECK_FRAGMENT]); } #[test] diff --git a/src/policy/types.rs b/src/policy/types.rs index e411ba4ae6f2..c8e97c177625 100644 --- a/src/policy/types.rs +++ b/src/policy/types.rs @@ -25,6 +25,8 @@ pub const CHECK_HAS_SPORT: u8 = 1 << 0; pub const CHECK_HAS_DPORT: u8 = 1 << 1; pub const CHECK_HAS_ICMP_TYPE: u8 = 1 << 2; pub const CHECK_HAS_ICMP_CODE: u8 = 1 << 3; +pub const CHECK_FRAGMENT: u8 = 1 << 4; +pub const CHECK_HAS_PROTO: u8 = 1 << 5; #[repr(C)] #[derive(Copy, Clone, Debug, PartialEq, Eq)] @@ -151,5 +153,7 @@ mod layout { defined("CHECK_HAS_ICMP_CODE"), usize::from(CHECK_HAS_ICMP_CODE) ); + assert_eq!(defined("CHECK_FRAGMENT"), usize::from(CHECK_FRAGMENT)); + assert_eq!(defined("CHECK_HAS_PROTO"), usize::from(CHECK_HAS_PROTO)); } } diff --git a/tests/policy.rs b/tests/policy.rs index c755c2cc5a60..5ca034da5f68 100644 --- a/tests/policy.rs +++ b/tests/policy.rs @@ -51,6 +51,8 @@ struct Check { dport: Option<(u16, u16)>, icmp_type: Option, icmp_code: Option, + fragment: bool, + any_proto: bool, allow: bool, } @@ -80,6 +82,12 @@ impl Check { v.match_flags |= CHECK_HAS_ICMP_CODE; v.icmp_code = code; } + if self.fragment { + v.match_flags |= CHECK_FRAGMENT; + } + if !self.any_proto { + v.match_flags |= CHECK_HAS_PROTO; + } v } } @@ -381,6 +389,60 @@ fn a_later_fragment_matches_nothing() { assert_eq!(run(&fragment), TC_ACT_UNSPEC); } +#[test] +fn a_fragment_check_matches_later_fragments_only() { + let fragments = Check { + fragment: true, + any_proto: true, + allow: true, + ..Default::default() + }; + install(false, &[fragments]); + + let later = ipv4(PROTO_TCP, 185, &tcp(40000, 22)); + assert_eq!(run(ðernet(ETHERTYPE_IPV4, 0, &later)), TC_ACT_UNSPEC); + let mut later = fragment_header(PROTO_UDP, 185); + later.extend_from_slice(&udp(40000, 53)); + assert_eq!( + run(ðernet(ETHERTYPE_IPV6, 0, &ipv6(PROTO_FRAGMENT, &later))), + TC_ACT_UNSPEC + ); + + // the first fragment carries the L4 header, more fragments following or not + let first = ipv4(PROTO_TCP, 0x2000, &tcp(40000, 22)); + assert_eq!(run(ðernet(ETHERTYPE_IPV4, 0, &first)), TC_ACT_SHOT); + assert_eq!(run(&tcp_frame(40000, 22)), TC_ACT_SHOT); + let mut first = fragment_header(PROTO_UDP, 0); + first.extend_from_slice(&udp(40000, 53)); + assert_eq!( + run(ðernet(ETHERTYPE_IPV6, 0, &ipv6(PROTO_FRAGMENT, &first))), + TC_ACT_SHOT + ); +} + +#[test] +fn a_fragment_check_can_name_the_protocol() { + let udp_fragments = Check { + ip_proto: PROTO_UDP, + fragment: true, + allow: true, + ..Default::default() + }; + install(false, &[udp_fragments]); + + let later = |protocol| ethernet(ETHERTYPE_IPV4, 0, &ipv4(protocol, 185, &[0; 8])); + assert_eq!(run(&later(PROTO_UDP)), TC_ACT_UNSPEC); + assert_eq!(run(&later(PROTO_TCP)), TC_ACT_SHOT); + + let later = |protocol| { + let mut payload = fragment_header(protocol, 185); + payload.extend_from_slice(&[0; 8]); + ethernet(ETHERTYPE_IPV6, 0, &ipv6(PROTO_FRAGMENT, &payload)) + }; + assert_eq!(run(&later(PROTO_UDP)), TC_ACT_UNSPEC); + assert_eq!(run(&later(PROTO_TCP)), TC_ACT_SHOT); +} + #[test] fn one_vlan_tag_is_parsed_through() { install( -- 2.47.3