vasilito/RedBear-OS
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

conntrack: per-protocol rate limits + fix ICMP type offset bug + tests

Browse Source 
CRITICAL BUG FIX: is_echo_request() and track_icmp() were reading
ctx.packet[0] thinking it was the ICMP type byte. But ctx.packet is
the IP packet, so offset 0 is the IP version/IHL byte (0x45 for IPv4).

Result: ICMP echo detection NEVER matched Type 8 packets.
- ICMP rate limiting (R32) was a no-op — wrong byte read
- ICMP echo tracking in track_icmp() was checking IP version vs 8/128

Fixed: read ICMP type from offset  for IPv4, or offset 40 for IPv6.

ALSO: separate TCP SYN and ICMP echo rate limit maps.
Previously both shared one BTreeMap, so a host doing 100 TCP SYNs
+ 1 ICMP echo would be over-limit. Now each has its own budget.

NEW TESTS (4 conntrack + 5 filter table = 9 total, all passing):
- syn_limit_triggers_after_threshold
- icmp_limit_triggers_after_threshold
- syn_and_icmp_have_independent_budgets
- non_syn_tcp_does_not_count_against_syn_limit

These tests caught the ICMP type bug — failed before fix, pass after.
This commit is contained in:
Red Bear OS
2026-07-08 19:57:19 +03:00
parent 1a3e12b60a
commit aa6bf0f2ea
1 changed files with 169 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files
netstack/src/filter/conntrack.rs
+169 -10
View File
@@ -25,12 +25,25 @@ fn is_syn(ctx: &PacketContext) -> bool {
}
/// ICMP echo request detection (Type 8 for ICMPv4, Type 128 for ICMPv6).
/// Returns true if the packet is an echo request.
/// Returns true if the packet is an ICMP echo request (Type 8 for ICMPv4,
/// Type 128 for ICMPv6). The packet in `ctx.packet` is the IP packet, so the
/// ICMP type field is at offset `ihl` (after the IP header).
fn is_echo_request(ctx: &PacketContext) -> bool {
if ctx.packet.is_empty() {
if ctx.packet.len() < 2 {
return false;
}
let icmp_type = ctx.packet[0];
let icmp_offset = match ctx.protocol {
1 => {
let ihl = (ctx.packet[0] & 0x0f) as usize * 4;
if ctx.packet.len() < ihl + 1 {
return false;
}
ihl
}
58 => 40,
_ => return false,
};
let icmp_type = ctx.packet[icmp_offset];
icmp_type == 8 || icmp_type == 128
}
@@ -125,7 +138,8 @@ struct ConnEntry {
pub struct ConntrackTable {
entries: BTreeMap<ConnKey, ConnEntry>,
last_cleanup: Option<Instant>,
rate_limits: BTreeMap<IpAddress, (u32, Instant)>,
syn_rate_limits: BTreeMap<IpAddress, (u32, Instant)>,
icmp_rate_limits: BTreeMap<IpAddress, (u32, Instant)>,
over_limit_count: u64,
}
@@ -152,7 +166,8 @@ impl ConntrackTable {
Self {
entries: BTreeMap::new(),
last_cleanup: None,
rate_limits: BTreeMap::new(),
syn_rate_limits: BTreeMap::new(),
icmp_rate_limits: BTreeMap::new(),
over_limit_count: 0,
}
}
@@ -252,9 +267,20 @@ impl ConntrackTable {
if ctx.packet.len() < 4 {
return ConnState::New;
}
let icmp_type = ctx.packet[0];
let icmp_code = ctx.packet[1];
let icmp_id = u16::from_be_bytes([ctx.packet[4], ctx.packet[5]]);
let icmp_offset = match l3num {
4 => (ctx.packet[0] & 0x0f) as usize * 4,
6 => 40,
_ => return ConnState::New,
};
if ctx.packet.len() < icmp_offset + 6 {
return ConnState::New;
}
let icmp_type = ctx.packet[icmp_offset];
let icmp_code = ctx.packet[icmp_offset + 1];
let icmp_id = u16::from_be_bytes([
ctx.packet[icmp_offset + 4],
ctx.packet[icmp_offset + 5],
]);
let is_echo = icmp_type == 8 || icmp_type == 128;
let is_echo_reply = icmp_type == 0 || icmp_type == 129;
@@ -343,7 +369,7 @@ impl ConntrackTable {
const SYN_LIMIT: u32 = 100;
const SYN_WINDOW: Duration = Duration::from_secs(1);
let entry = self.rate_limits.entry(src).or_insert((0, now));
let entry = self.syn_rate_limits.entry(src).or_insert((0, now));
if now > entry.1 + SYN_WINDOW {
*entry = (1, now);
} else {
@@ -356,7 +382,7 @@ impl ConntrackTable {
const ICMP_LIMIT: u32 = 20;
const ICMP_WINDOW: Duration = Duration::from_secs(1);
let entry = self.rate_limits.entry(src).or_insert((0, now));
let entry = self.icmp_rate_limits.entry(src).or_insert((0, now));
if now > entry.1 + ICMP_WINDOW {
*entry = (1, now);
} else {
@@ -385,3 +411,136 @@ impl ConntrackTable {
}
}
#[cfg(test)]
mod tests {
use super::*;
use crate::filter::Hook;
fn make_tcp_syn_packet() -> Vec<u8> {
let mut p = vec![0u8; 64];
// IPv4 header
p[0] = 0x45; // version 4, IHL 5
p[9] = 6; // protocol = TCP
// TCP header at offset 20
p[20] = 0x50; p[21] = 0x00; // src port 0x5000 = 20480
p[22] = 0x50; p[23] = 0x00; // dst port
// TCP flags at offset 33 (20 + 13)
p[33] = 0x02; // SYN
p
}
fn make_tcp_data_packet() -> Vec<u8> {
let mut p = vec![0u8; 64];
p[0] = 0x45;
p[9] = 6;
p[33] = 0x10; // ACK only (not SYN)
p
}
fn make_icmp_echo_packet() -> Vec<u8> {
let mut p = vec![0u8; 64];
p[0] = 0x45;
p[9] = 1; // protocol = ICMP
// ICMP header at offset 20
p[20] = 8; // echo request
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,
}
}
#[test]
fn syn_limit_triggers_after_threshold() {
let mut ct = ConntrackTable::new();
let packet = make_tcp_syn_packet();
let now = Instant::from_secs(0);
let mut over_limit_count = 0;
for i in 0..150 {
let ctx = make_ctx(6, &packet);
if ct.track(&ctx, now) == ConnState::OverLimit {
over_limit_count += 1;
}
let _ = i;
}
assert!(over_limit_count >= 50,
"Should trigger SYN limit after 100 SYNs/sec; got {} over_limit",
over_limit_count);
}
#[test]
fn icmp_limit_triggers_after_threshold() {
let mut ct = ConntrackTable::new();
let packet = make_icmp_echo_packet();
let now = Instant::from_secs(0);
let mut over_limit_count = 0;
for _i in 0..50 {
let ctx = make_ctx(1, &packet);
if ct.track(&ctx, now) == ConnState::OverLimit {
over_limit_count += 1;
}
}
assert!(over_limit_count >= 25,
"Should trigger ICMP echo limit after 20/sec; got {} over_limit",
over_limit_count);
}
#[test]
fn syn_and_icmp_have_independent_budgets() {
// After R37 fix, separate maps — TCP SYN traffic shouldn't
// affect ICMP echo budget and vice versa.
let mut ct = ConntrackTable::new();
let tcp = make_tcp_syn_packet();
let icmp = make_icmp_echo_packet();
let now = Instant::from_secs(0);
// Burn through TCP SYN budget
for _i in 0..150 {
let ctx = make_ctx(6, &tcp);
let _ = ct.track(&ctx, now);
}
// ICMP should still have its full budget — first 20 echo requests
// should not be limited, only the 21st onward
let mut not_over_limit = 0;
let mut over_limit = 0;
for _i in 0..30 {
let ctx = make_ctx(1, &icmp);
if ct.track(&ctx, now) == ConnState::OverLimit {
over_limit += 1;
} else {
not_over_limit += 1;
}
}
assert_eq!(not_over_limit, 20,
"ICMP budget should be independent — first 20 should pass even after TCP limit exhausted");
assert_eq!(over_limit, 10,
"Remaining 10 ICMP echoes (21-30) should be over_limit");
}
#[test]
fn non_syn_tcp_does_not_count_against_syn_limit() {
// Pure ACK packets shouldn't count against SYN flood budget.
let mut ct = ConntrackTable::new();
let ack = make_tcp_data_packet();
let now = Instant::from_secs(0);
let mut over_limit = 0;
for _i in 0..200 {
let ctx = make_ctx(6, &ack);
if ct.track(&ctx, now) == ConnState::OverLimit {
over_limit += 1;
}
}
assert_eq!(over_limit, 0,
"Non-SYN TCP packets must not trigger SYN flood limit");
}
}