[PATCH proxmox v4 03/45] ebpf: policy: add a check for non-first fragments

[Hannes Laimer <h.laimer@proxmox.com>]

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 <h.laimer@proxmox.com>
---
 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<u8>,
     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<u8>,
@@ -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<u8>,
     icmp_code: Option<u8>,
+    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(&ethernet(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(&ethernet(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(&ethernet(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(&ethernet(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