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 064DB1FF0DF for ; Thu, 05 Nov 2026 12:13:45 +0100 (CET) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 225E92164C; Thu, 05 Nov 2026 12:13:36 +0100 (CET) From: Hannes Laimer To: pve-devel@lists.proxmox.com Subject: [PATCH proxmox v4 03/45] ebpf: policy: add a check for non-first fragments Date: Thu, 5 Nov 2026 12:12:42 +0100 Message-ID: <20261105111324.706922-4-h.laimer@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20261105111324.706922-1-h.laimer@proxmox.com> References: <20261105111324.706922-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: 1793877210088 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.282 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: TBEGULJGGTFJFBY36OGPRJVBW4RR2PYQ X-Message-ID-Hash: TBEGULJGGTFJFBY36OGPRJVBW4RR2PYQ 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 --- proxmox-ebpf/src/policy/bpf/policy.bpf.c | 28 ++++++++--- proxmox-ebpf/src/policy/bpf/types.h | 3 ++ proxmox-ebpf/src/policy/mod.rs | 63 ++++++++++++++---------- proxmox-ebpf/src/policy/types.rs | 4 ++ proxmox-ebpf/tests/policy.rs | 62 +++++++++++++++++++++++ 5 files changed, 129 insertions(+), 31 deletions(-) diff --git a/proxmox-ebpf/src/policy/bpf/policy.bpf.c b/proxmox-ebpf/src/policy/bpf/policy.bpf.c index ed189a1227e1..f94857a17c18 100644 --- a/proxmox-ebpf/src/policy/bpf/policy.bpf.c +++ b/proxmox-ebpf/src/policy/bpf/policy.bpf.c @@ -75,7 +75,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; @@ -88,6 +91,7 @@ struct frame { __u8 has_ports; __u8 has_icmp; __u8 unreadable; + __u8 fragment; // an authentication header was walked past, so a check for AH matches as well __u8 ah; }; @@ -109,7 +113,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) { @@ -202,10 +210,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 == ETH_P_IPV6) { struct ipv6hdr ip6; @@ -225,9 +235,15 @@ 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 && !(c->ip_proto == PROTO_AH && f->ah)) return 0; + if ((c->match_flags & CHECK_HAS_PROTO) && c->ip_proto != f->ip_proto && + !(c->ip_proto == PROTO_AH && f->ah)) + 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/proxmox-ebpf/src/policy/bpf/types.h b/proxmox-ebpf/src/policy/bpf/types.h index dbeb9bea59db..971af2c1e754 100644 --- a/proxmox-ebpf/src/policy/bpf/types.h +++ b/proxmox-ebpf/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/proxmox-ebpf/src/policy/mod.rs b/proxmox-ebpf/src/policy/mod.rs index 4ca65bb9f595..9a670bb11577 100644 --- a/proxmox-ebpf/src/policy/mod.rs +++ b/proxmox-ebpf/src/policy/mod.rs @@ -74,8 +74,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, @@ -86,6 +89,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; } @@ -101,7 +110,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, @@ -551,13 +560,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() } } @@ -588,25 +594,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() @@ -618,7 +618,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]); @@ -628,27 +628,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/proxmox-ebpf/src/policy/types.rs b/proxmox-ebpf/src/policy/types.rs index e411ba4ae6f2..c8e97c177625 100644 --- a/proxmox-ebpf/src/policy/types.rs +++ b/proxmox-ebpf/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/proxmox-ebpf/tests/policy.rs b/proxmox-ebpf/tests/policy.rs index 11c513c69945..03bcf438850a 100644 --- a/proxmox-ebpf/tests/policy.rs +++ b/proxmox-ebpf/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