diff --git a/netstack/src/filter/conntrack.rs b/netstack/src/filter/conntrack.rs index a4aea7a736..31277d1fb4 100644 --- a/netstack/src/filter/conntrack.rs +++ b/netstack/src/filter/conntrack.rs @@ -20,8 +20,15 @@ use core::hash::{Hash, Hasher}; use smoltcp::time::{Duration, Instant}; use smoltcp::wire::IpAddress; -fn is_syn(ctx: &PacketContext) -> bool { - ctx.packet.len() >= 34 && (ctx.packet[33] & 0x02) != 0 && (ctx.packet[33] & 0x10) == 0 +fn is_syn(ctx: &PacketContext, l3num: u8) -> bool { + // TCP flags byte is at offset 13 in the TCP header. + // TCP header starts after the IP header: 20 bytes for IPv4, 40 for IPv6. + let tcp_offset = if l3num == 4 { 20 } else { 40 }; + if ctx.packet.len() <= tcp_offset + 13 { + return false; + } + let flags = ctx.packet[tcp_offset + 13]; + (flags & 0x02) != 0 && (flags & 0x10) == 0 } /// ICMP echo request detection (Type 8 for ICMPv4, Type 128 for ICMPv6). @@ -181,7 +188,7 @@ impl ConntrackTable { return ConnState::New; } - if ctx.protocol == 6 && is_syn(ctx) && self.check_syn_limit(ctx.src_addr, now) { + if ctx.protocol == 6 && is_syn(ctx, l3num) && self.check_syn_limit(ctx.src_addr, now) { self.over_limit_count = self.over_limit_count.saturating_add(1); return ConnState::OverLimit; } @@ -447,19 +454,33 @@ mod tests { p } - fn make_ctx<'a>(protocol: u8, packet: &'a [u8]) -> PacketContext<'a> { - PacketContext { - hook: Hook::InputLocal, - in_dev: None, - out_dev: None, - src_addr: IpAddress::v4(10, 0, 0, 1), - dst_addr: IpAddress::v4(10, 0, 0, 2), - protocol, - src_port: Some(1234), - dst_port: Some(80), - packet, - } +fn make_ctx<'a>(protocol: u8, packet: &'a [u8]) -> PacketContext<'a> { + PacketContext { + hook: Hook::InputLocal, + in_dev: None, + out_dev: None, + src_addr: IpAddress::v4(10, 0, 0, 1), + dst_addr: IpAddress::v4(10, 0, 0, 2), + protocol, + src_port: Some(1234), + dst_port: Some(80), + packet, } +} + +fn make_v6_ctx<'a>(protocol: u8, packet: &'a [u8]) -> PacketContext<'a> { + PacketContext { + hook: Hook::InputLocal, + in_dev: None, + out_dev: None, + src_addr: IpAddress::v6(0xfe80, 0, 0, 0, 0, 0, 0, 1), + dst_addr: IpAddress::v6(0xfe80, 0, 0, 0, 0, 0, 0, 2), + protocol, + src_port: Some(1234), + dst_port: Some(80), + packet, + } +} #[test] fn syn_limit_triggers_after_threshold() { @@ -543,4 +564,30 @@ mod tests { assert_eq!(over_limit, 0, "Non-SYN TCP packets must not trigger SYN flood limit"); } + + #[test] + fn ipv6_syn_detection_works() { + // Regression test for the IPv4-only `is_syn()` bug. + // Before fix: offset 33 was hardcoded, reading IPv6 header + // bytes instead of TCP flags (which are at offset 53 for IPv6). + let mut ct = ConntrackTable::new(); + let mut p = vec![0u8; 80]; + // IPv6 header: version 6 at byte 0 (0x60), next header = 6 (TCP) at byte 6 + p[0] = 0x60; + p[6] = 6; // next header = TCP + // TCP header at offset 40, flags at offset 53 + p[53] = 0x02; // SYN + let now = Instant::from_secs(0); + // The l3num is inferred from ctx.src_addr (v6 → l3num=6). + let mut over_limit_count = 0; + for _i in 0..150 { + let ctx = make_v6_ctx(6, &p); + if ct.track(&ctx, now) == ConnState::OverLimit { + over_limit_count += 1; + } + } + assert!(over_limit_count >= 50, + "IPv6 SYN must trigger rate limit after threshold; got {}", + over_limit_count); + } } diff --git a/netstack/src/scheme/netfilter.rs b/netstack/src/scheme/netfilter.rs index f0b0b45a7e..59a154de8b 100644 --- a/netstack/src/scheme/netfilter.rs +++ b/netstack/src/scheme/netfilter.rs @@ -253,17 +253,17 @@ impl NetFilterFile { let path = self.path.clone(); let mut table = table.borrow_mut(); - if path == "/rule/add" { + if path == "rule/add" { let rule = parse_rule(line) .map_err(|e| SyscallError::new(syscall::EINVAL))?; let id = table.add(rule); log::info!("netfilter: added rule id={}", id); - } else if path == "/nat/add" { + } else if path == "nat/add" { let nat_rule = parse_nat_rule(line) .map_err(|_| SyscallError::new(syscall::EINVAL))?; let id = table.nat_table.add(nat_rule); log::info!("netfilter: added NAT rule id={}", id); - } else if path == "/nat/del" { + } else if path == "nat/del" { let id: u32 = line .parse() .map_err(|_| SyscallError::new(syscall::EINVAL))?; @@ -271,21 +271,13 @@ impl NetFilterFile { return Err(SyscallError::new(syscall::ENOENT)); } log::info!("netfilter: removed NAT rule id={}", id); - } else if let Some(rest) = path.strip_prefix("/rule/del/") { - let id: u32 = rest - .parse() - .map_err(|_| SyscallError::new(syscall::EINVAL))?; - if !table.remove(id) { - return Err(SyscallError::new(syscall::ENOENT)); - } - log::info!("netfilter: removed rule id={}", id); - } else if path == "/reset" { + } else if path == "reset" { *table = FilterTable::new(); log::info!("netfilter: table reset to defaults"); - } else if path == "/counters/reset" { + } else if path == "counters/reset" { table.reset_counters(); log::info!("netfilter: counters reset (rules preserved)"); - } else if let Some(rest) = path.strip_prefix("/policy/") { + } else if let Some(rest) = path.strip_prefix("policy/") { let hook = parse_hook_name(rest).ok_or(SyscallError::new(syscall::EINVAL))?; let verdict = match line.to_uppercase().as_str() { "ACCEPT" => Verdict::Accept, @@ -294,7 +286,7 @@ impl NetFilterFile { }; table.set_default_policy(hook, verdict); log::info!("netfilter: set {} policy to {}", rest, verdict.name()); - } else if path == "/rule/del" { + } else if path == "rule/del" { let id: u32 = line .parse() .map_err(|_| SyscallError::new(syscall::EINVAL))?;