Skip to content

Commit 4f9d0a5

Browse files
JAORMXclaude
andcommitted
Make conntrack bidirectional for established flows
The conntrack fast path only stored the direction that first matched a rule, so return traffic was recognised but the original direction was not. When a DNS-based dynamic rule expired (60 s TTL), outbound packets for established TCP connections failed the IsEstablished check because the inbound direction was never tracked, causing frame drops until the next DNS re-resolution. Track both directions in the IsEstablished path so established connections survive dynamic rule expiry as long as traffic flows bidirectionally. This matches standard stateful firewall semantics (iptables/nftables conntrack, pf). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent de0cd2d commit 4f9d0a5

2 files changed

Lines changed: 189 additions & 0 deletions

File tree

net/firewall/filter.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,15 @@ func NewFilterWithDynamic(rules []Rule, defaultAction Action, dr *DynamicRules)
6060
// 3. Default action -- returned when no rule matches.
6161
func (f *Filter) Verdict(dir Direction, hdr *PacketHeader) Action {
6262
// Fast path: established connections are always allowed.
63+
// Track the current direction so that both sides of the flow are
64+
// recorded. Without this, only the direction that first matched a
65+
// rule is stored, making conntrack effectively unidirectional.
66+
// When a dynamic rule expires (e.g. DNS TTL), the original
67+
// direction would fail the IsEstablished check because the return
68+
// direction was never tracked. By tracking here we keep both
69+
// directions alive as long as traffic flows bidirectionally.
6370
if f.conntrack.IsEstablished(dir, hdr) {
71+
f.conntrack.Track(dir, hdr)
6472
return Allow
6573
}
6674

net/firewall/filter_test.go

Lines changed: 181 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -298,6 +298,187 @@ func TestFilter_DynamicRules_TracksAllowedConnection(t *testing.T) {
298298
assert.Equal(t, Allow, got)
299299
}
300300

301+
func TestFilter_EstablishedConnectionSurvivesDynamicRuleExpiry(t *testing.T) {
302+
t.Parallel()
303+
304+
// Simulate the real-world scenario: a dynamic rule (from DNS snooping)
305+
// allows egress, the connection is established with bidirectional
306+
// traffic, the dynamic rule expires, and subsequent packets in both
307+
// directions must still be allowed via conntrack.
308+
309+
now := time.Now()
310+
dr := NewDynamicRules()
311+
dr.now = func() time.Time { return now }
312+
313+
_, cidr, err := net.ParseCIDR("203.0.113.50/32")
314+
require.NoError(t, err)
315+
316+
// Short TTL to simulate DNS-based dynamic rule expiry.
317+
dr.Add(Rule{
318+
Direction: Egress,
319+
Action: Allow,
320+
Protocol: 6,
321+
DstCIDR: *cidr,
322+
DstPort: 443,
323+
}, 60*time.Second)
324+
325+
f := NewFilterWithDynamic(nil, Deny, dr)
326+
f.conntrack.now = func() time.Time { return now }
327+
328+
egressHdr := &PacketHeader{
329+
SrcIP: [4]byte{10, 0, 0, 1},
330+
DstIP: [4]byte{203, 0, 113, 50},
331+
Protocol: 6,
332+
SrcPort: 44444,
333+
DstPort: 443,
334+
}
335+
ingressHdr := &PacketHeader{
336+
SrcIP: [4]byte{203, 0, 113, 50},
337+
DstIP: [4]byte{10, 0, 0, 1},
338+
Protocol: 6,
339+
SrcPort: 443,
340+
DstPort: 44444,
341+
}
342+
343+
// 1. Outbound allowed by dynamic rule, tracked in conntrack.
344+
assert.Equal(t, Allow, f.Verdict(Egress, egressHdr))
345+
346+
// 2. Return traffic allowed via conntrack (reverse lookup).
347+
assert.Equal(t, Allow, f.Verdict(Ingress, ingressHdr))
348+
349+
// 3. Advance time past the dynamic rule TTL.
350+
now = now.Add(61 * time.Second)
351+
352+
// Verify the dynamic rule is actually expired.
353+
_, matched := dr.Match(Egress, egressHdr)
354+
assert.False(t, matched, "dynamic rule should have expired")
355+
356+
// 4. Outbound must still be allowed via bidirectional conntrack.
357+
assert.Equal(t, Allow, f.Verdict(Egress, egressHdr),
358+
"egress should be allowed via conntrack after dynamic rule expires")
359+
360+
// 5. Inbound must still be allowed.
361+
assert.Equal(t, Allow, f.Verdict(Ingress, ingressHdr),
362+
"ingress should be allowed via conntrack after dynamic rule expires")
363+
}
364+
365+
func TestFilter_EstablishedConntrack_RefreshesTimestamp(t *testing.T) {
366+
t.Parallel()
367+
368+
// Verify that bidirectional conntrack entries refresh their timestamps,
369+
// keeping long-lived connections alive across multiple sweep cycles.
370+
371+
now := time.Now()
372+
dr := NewDynamicRules()
373+
dr.now = func() time.Time { return now }
374+
375+
_, cidr, err := net.ParseCIDR("203.0.113.50/32")
376+
require.NoError(t, err)
377+
378+
dr.Add(Rule{
379+
Direction: Egress,
380+
Action: Allow,
381+
Protocol: 6,
382+
DstCIDR: *cidr,
383+
DstPort: 443,
384+
}, 60*time.Second)
385+
386+
f := NewFilterWithDynamic(nil, Deny, dr)
387+
f.conntrack.now = func() time.Time { return now }
388+
389+
egressHdr := &PacketHeader{
390+
SrcIP: [4]byte{10, 0, 0, 1},
391+
DstIP: [4]byte{203, 0, 113, 50},
392+
Protocol: 6,
393+
SrcPort: 44444,
394+
DstPort: 443,
395+
}
396+
ingressHdr := &PacketHeader{
397+
SrcIP: [4]byte{203, 0, 113, 50},
398+
DstIP: [4]byte{10, 0, 0, 1},
399+
Protocol: 6,
400+
SrcPort: 443,
401+
DstPort: 44444,
402+
}
403+
404+
// Establish the connection while the dynamic rule is alive.
405+
assert.Equal(t, Allow, f.Verdict(Egress, egressHdr))
406+
assert.Equal(t, Allow, f.Verdict(Ingress, ingressHdr))
407+
408+
// Expire the dynamic rule.
409+
now = now.Add(61 * time.Second)
410+
411+
// Simulate a long-lived connection by sending packets every 2 minutes
412+
// for 10 minutes. Each packet should refresh the conntrack entry,
413+
// preventing the 5-minute TTL from expiring.
414+
for i := 0; i < 5; i++ {
415+
now = now.Add(2 * time.Minute)
416+
f.conntrack.sweep()
417+
418+
assert.Equal(t, Allow, f.Verdict(Egress, egressHdr),
419+
"egress should be allowed at iteration %d (t+%dm)", i, 2*(i+1))
420+
assert.Equal(t, Allow, f.Verdict(Ingress, ingressHdr),
421+
"ingress should be allowed at iteration %d (t+%dm)", i, 2*(i+1))
422+
}
423+
}
424+
425+
func TestFilter_NoReturnTraffic_ConntrackExpires(t *testing.T) {
426+
t.Parallel()
427+
428+
// Verify that a connection with no return traffic eventually has its
429+
// conntrack entry expire. The fix should NOT keep one-directional
430+
// flows alive indefinitely.
431+
432+
now := time.Now()
433+
dr := NewDynamicRules()
434+
dr.now = func() time.Time { return now }
435+
436+
_, cidr, err := net.ParseCIDR("203.0.113.50/32")
437+
require.NoError(t, err)
438+
439+
dr.Add(Rule{
440+
Direction: Egress,
441+
Action: Allow,
442+
Protocol: 6,
443+
DstCIDR: *cidr,
444+
DstPort: 443,
445+
}, 60*time.Second)
446+
447+
f := NewFilterWithDynamic(nil, Deny, dr)
448+
f.conntrack.now = func() time.Time { return now }
449+
450+
egressHdr := &PacketHeader{
451+
SrcIP: [4]byte{10, 0, 0, 1},
452+
DstIP: [4]byte{203, 0, 113, 50},
453+
Protocol: 6,
454+
SrcPort: 44444,
455+
DstPort: 443,
456+
}
457+
ingressHdr := &PacketHeader{
458+
SrcIP: [4]byte{203, 0, 113, 50},
459+
DstIP: [4]byte{10, 0, 0, 1},
460+
Protocol: 6,
461+
SrcPort: 443,
462+
DstPort: 44444,
463+
}
464+
465+
// Outbound allowed by dynamic rule.
466+
assert.Equal(t, Allow, f.Verdict(Egress, egressHdr))
467+
468+
// No return traffic — server never responded.
469+
// Expire the dynamic rule and advance past TCP conntrack TTL.
470+
now = now.Add(6 * time.Minute)
471+
f.conntrack.sweep()
472+
473+
// Outbound should now be denied (conntrack expired, dynamic rule gone).
474+
assert.Equal(t, Deny, f.Verdict(Egress, egressHdr),
475+
"egress should be denied after conntrack expires with no return traffic")
476+
477+
// Inbound should also be denied.
478+
assert.Equal(t, Deny, f.Verdict(Ingress, ingressHdr),
479+
"ingress should be denied after conntrack expires with no return traffic")
480+
}
481+
301482
func TestFilter_StartExpiry_StopsOnCancel(t *testing.T) {
302483
t.Parallel()
303484

0 commit comments

Comments
 (0)