Skip to content

Commit 645f2b3

Browse files
JAORMXclaude
andcommitted
Harden DNS egress policy against bypass vectors
- Fail-closed on unparseable DNS queries instead of forwarding them - Reject multi-question DNS queries to prevent policy piggyback bypass - Validate DNS response source IP matches gateway before snooping - Create both TCP+UDP dynamic rules when protocol is unspecified Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 84f4064 commit 645f2b3

7 files changed

Lines changed: 200 additions & 43 deletions

File tree

net/egress/dns.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,9 @@ func ParseDNSQuery(payload []byte) (txnID uint16, qname string, qtype uint16, er
2020
if len(msg.Question) == 0 {
2121
return 0, "", 0, fmt.Errorf("DNS query has no question section")
2222
}
23+
if len(msg.Question) > 1 {
24+
return 0, "", 0, fmt.Errorf("DNS query has %d questions, expected 1", len(msg.Question))
25+
}
2326
return msg.Id, msg.Question[0].Name, msg.Question[0].Qtype, nil
2427
}
2528

net/egress/dns_test.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,24 @@ func TestParseDNSQuery_NoQuestion(t *testing.T) {
4949
assert.Error(t, err)
5050
}
5151

52+
func TestParseDNSQuery_MultipleQuestions(t *testing.T) {
53+
t.Parallel()
54+
55+
msg := &dns.Msg{
56+
MsgHdr: dns.MsgHdr{Id: 1},
57+
Question: []dns.Question{
58+
{Name: "a.com.", Qtype: dns.TypeA, Qclass: dns.ClassINET},
59+
{Name: "b.com.", Qtype: dns.TypeA, Qclass: dns.ClassINET},
60+
},
61+
}
62+
payload, err := msg.Pack()
63+
require.NoError(t, err)
64+
65+
_, _, _, err = ParseDNSQuery(payload)
66+
assert.Error(t, err)
67+
assert.Contains(t, err.Error(), "2 questions")
68+
}
69+
5270
func TestBuildNXDOMAIN(t *testing.T) {
5371
t.Parallel()
5472

net/egress/interceptor.go

Lines changed: 48 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -25,16 +25,20 @@ type DNSInterceptor struct {
2525
policy *Policy
2626
dynamicRules *firewall.DynamicRules
2727
minTTL time.Duration
28+
gatewayIP [4]byte
2829
}
2930

30-
// NewDNSInterceptor creates an interceptor with the given policy and dynamic
31-
// rule set. Dynamic rules created from DNS responses will have at least
32-
// minTTL duration (use 0 for the default of 60 seconds).
33-
func NewDNSInterceptor(policy *Policy, dr *firewall.DynamicRules) *DNSInterceptor {
31+
// NewDNSInterceptor creates an interceptor with the given policy, dynamic
32+
// rule set, and gateway IP. Only DNS responses from the gateway are
33+
// snooped to prevent spoofed responses from creating dynamic rules.
34+
// Dynamic rules created from DNS responses will have at least minTTL
35+
// duration (use 0 for the default of 60 seconds).
36+
func NewDNSInterceptor(policy *Policy, dr *firewall.DynamicRules, gatewayIP [4]byte) *DNSInterceptor {
3437
return &DNSInterceptor{
3538
policy: policy,
3639
dynamicRules: dr,
3740
minTTL: defaultMinTTL,
41+
gatewayIP: gatewayIP,
3842
}
3943
}
4044

@@ -44,14 +48,14 @@ func NewDNSInterceptor(policy *Policy, dr *firewall.DynamicRules) *DNSIntercepto
4448
func (d *DNSInterceptor) HandleEgress(frame []byte, hdr *firewall.PacketHeader) firewall.InterceptResult {
4549
payload, err := ExtractUDPPayload(frame)
4650
if err != nil {
47-
slog.Debug("egress: failed to extract UDP payload", "error", err)
48-
return firewall.InterceptResult{Action: firewall.InterceptForward}
51+
slog.Debug("egress: dropping unparseable UDP frame", "error", err)
52+
return firewall.InterceptResult{Action: firewall.InterceptDrop}
4953
}
5054

5155
txnID, qname, qtype, err := ParseDNSQuery(payload)
5256
if err != nil {
53-
slog.Debug("egress: failed to parse DNS query", "error", err)
54-
return firewall.InterceptResult{Action: firewall.InterceptForward}
57+
slog.Debug("egress: dropping unparseable DNS query", "error", err)
58+
return firewall.InterceptResult{Action: firewall.InterceptDrop}
5559
}
5660

5761
if d.policy.IsAllowed(qname) {
@@ -81,8 +85,16 @@ func (d *DNSInterceptor) HandleEgress(frame []byte, hdr *firewall.PacketHeader)
8185

8286
// HandleIngress snoops an inbound DNS response to learn IP mappings for
8387
// allowed hostnames. For each A record in the response, a dynamic
84-
// firewall rule is created with the TTL from the DNS record.
85-
func (d *DNSInterceptor) HandleIngress(frame []byte, _ *firewall.PacketHeader) {
88+
// firewall rule is created with the TTL from the DNS record. Only
89+
// responses from the gateway IP are processed to prevent spoofed
90+
// responses from injecting dynamic rules.
91+
func (d *DNSInterceptor) HandleIngress(frame []byte, hdr *firewall.PacketHeader) {
92+
if hdr == nil || hdr.SrcIP != d.gatewayIP {
93+
slog.Debug("ingress: ignoring DNS response from non-gateway source",
94+
"src_ip", hdr.SrcIP)
95+
return
96+
}
97+
8698
payload, err := ExtractUDPPayload(frame)
8799
if err != nil {
88100
slog.Debug("ingress: failed to extract UDP payload", "error", err)
@@ -109,10 +121,14 @@ func (d *DNSInterceptor) HandleIngress(frame []byte, _ *firewall.PacketHeader) {
109121

110122
ports, proto := d.policy.HostPorts(qname)
111123

112-
// Default to TCP when protocol is unspecified.
113-
ruleProto := proto
114-
if ruleProto == 0 {
115-
ruleProto = 6
124+
// When protocol is unspecified, create rules for both TCP and UDP
125+
// so that callers who omit the protocol field get full connectivity
126+
// to the resolved IPs rather than TCP-only.
127+
var ruleProtos []uint8
128+
if proto == 0 {
129+
ruleProtos = []uint8{6, 17} // TCP + UDP
130+
} else {
131+
ruleProtos = []uint8{proto}
116132
}
117133

118134
for _, ip := range ips {
@@ -126,33 +142,35 @@ func (d *DNSInterceptor) HandleIngress(frame []byte, _ *firewall.PacketHeader) {
126142
Mask: net.CIDRMask(32, 32),
127143
}
128144

129-
if len(ports) == 0 {
130-
d.dynamicRules.Add(firewall.Rule{
131-
Direction: firewall.Egress,
132-
Action: firewall.Allow,
133-
Protocol: ruleProto,
134-
DstCIDR: cidr,
135-
}, ttl)
136-
slog.Debug("egress: dynamic rule added",
137-
"ip", ip4.String(),
138-
"proto", ruleProto,
139-
"ttl", ttl,
140-
)
141-
} else {
142-
for _, port := range ports {
145+
for _, ruleProto := range ruleProtos {
146+
if len(ports) == 0 {
143147
d.dynamicRules.Add(firewall.Rule{
144148
Direction: firewall.Egress,
145149
Action: firewall.Allow,
146150
Protocol: ruleProto,
147151
DstCIDR: cidr,
148-
DstPort: port,
149152
}, ttl)
150153
slog.Debug("egress: dynamic rule added",
151154
"ip", ip4.String(),
152-
"port", port,
153155
"proto", ruleProto,
154156
"ttl", ttl,
155157
)
158+
} else {
159+
for _, port := range ports {
160+
d.dynamicRules.Add(firewall.Rule{
161+
Direction: firewall.Egress,
162+
Action: firewall.Allow,
163+
Protocol: ruleProto,
164+
DstCIDR: cidr,
165+
DstPort: port,
166+
}, ttl)
167+
slog.Debug("egress: dynamic rule added",
168+
"ip", ip4.String(),
169+
"port", port,
170+
"proto", ruleProto,
171+
"ttl", ttl,
172+
)
173+
}
156174
}
157175
}
158176
}

net/egress/interceptor_test.go

Lines changed: 124 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ func TestDNSInterceptor_AllowedQuery(t *testing.T) {
5757

5858
policy := NewPolicy([]HostSpec{{Name: "api.github.com"}})
5959
dr := firewall.NewDynamicRules()
60-
interceptor := NewDNSInterceptor(policy, dr)
60+
interceptor := NewDNSInterceptor(policy, dr, testDstIP)
6161

6262
frame := buildDNSQueryFrame(testSrcMAC, testDstMAC, testSrcIP, testDstIP, 12345, "api.github.com")
6363
hdr := firewall.ParseHeaders(frame)
@@ -71,7 +71,7 @@ func TestDNSInterceptor_BlockedQuery(t *testing.T) {
7171

7272
policy := NewPolicy([]HostSpec{{Name: "api.github.com"}})
7373
dr := firewall.NewDynamicRules()
74-
interceptor := NewDNSInterceptor(policy, dr)
74+
interceptor := NewDNSInterceptor(policy, dr, testDstIP)
7575

7676
frame := buildDNSQueryFrame(testSrcMAC, testDstMAC, testSrcIP, testDstIP, 12345, "evil.com")
7777
hdr := firewall.ParseHeaders(frame)
@@ -95,7 +95,7 @@ func TestDNSInterceptor_WildcardAllowed(t *testing.T) {
9595

9696
policy := NewPolicy([]HostSpec{{Name: "*.docker.io"}})
9797
dr := firewall.NewDynamicRules()
98-
interceptor := NewDNSInterceptor(policy, dr)
98+
interceptor := NewDNSInterceptor(policy, dr, testDstIP)
9999

100100
frame := buildDNSQueryFrame(testSrcMAC, testDstMAC, testSrcIP, testDstIP, 12345, "registry-1.docker.io")
101101
hdr := firewall.ParseHeaders(frame)
@@ -109,18 +109,18 @@ func TestDNSInterceptor_ResponseSnooping(t *testing.T) {
109109

110110
policy := NewPolicy([]HostSpec{{Name: "api.github.com", Ports: []uint16{443}}})
111111
dr := firewall.NewDynamicRules()
112-
interceptor := NewDNSInterceptor(policy, dr)
112+
interceptor := NewDNSInterceptor(policy, dr, testDstIP)
113113

114114
ips := []net.IP{net.ParseIP("140.82.121.5"), net.ParseIP("140.82.121.6")}
115115
frame := buildDNSResponseFrame(testDstMAC, testSrcMAC, testDstIP, testSrcIP, 12345, "api.github.com", ips, 300)
116116
hdr := firewall.ParseHeaders(frame)
117117

118118
interceptor.HandleIngress(frame, hdr)
119119

120-
// Should have created dynamic rules for both IPs.
121-
assert.Equal(t, 2, dr.Len())
120+
// Protocol unset → TCP+UDP per IP. 2 IPs × 2 protocols = 4 rules.
121+
assert.Equal(t, 4, dr.Len())
122122

123-
// Verify that the rules match egress traffic to the IPs.
123+
// Verify that the rules match TCP egress traffic to the IPs.
124124
hdr1 := &firewall.PacketHeader{
125125
DstIP: [4]byte{140, 82, 121, 5},
126126
Protocol: 6,
@@ -130,6 +130,16 @@ func TestDNSInterceptor_ResponseSnooping(t *testing.T) {
130130
require.True(t, ok)
131131
assert.Equal(t, firewall.Allow, action)
132132

133+
// UDP to the same port should also match.
134+
hdr1udp := &firewall.PacketHeader{
135+
DstIP: [4]byte{140, 82, 121, 5},
136+
Protocol: 17,
137+
DstPort: 443,
138+
}
139+
action, ok = dr.Match(firewall.Egress, hdr1udp)
140+
require.True(t, ok)
141+
assert.Equal(t, firewall.Allow, action)
142+
133143
// Wrong port should not match.
134144
hdr2 := &firewall.PacketHeader{
135145
DstIP: [4]byte{140, 82, 121, 5},
@@ -145,14 +155,17 @@ func TestDNSInterceptor_ResponseSnooping_AllPorts(t *testing.T) {
145155

146156
policy := NewPolicy([]HostSpec{{Name: "example.com"}})
147157
dr := firewall.NewDynamicRules()
148-
interceptor := NewDNSInterceptor(policy, dr)
158+
interceptor := NewDNSInterceptor(policy, dr, testDstIP)
149159

150160
ips := []net.IP{net.ParseIP("93.184.216.34")}
151161
frame := buildDNSResponseFrame(testDstMAC, testSrcMAC, testDstIP, testSrcIP, 12345, "example.com", ips, 60)
152162
hdr := firewall.ParseHeaders(frame)
153163

154164
interceptor.HandleIngress(frame, hdr)
155165

166+
// Protocol unset → TCP+UDP. 1 IP × 2 protocols = 2 rules.
167+
assert.Equal(t, 2, dr.Len())
168+
156169
// Rule should match any port for TCP.
157170
hdr1 := &firewall.PacketHeader{
158171
DstIP: [4]byte{93, 184, 216, 34},
@@ -162,14 +175,24 @@ func TestDNSInterceptor_ResponseSnooping_AllPorts(t *testing.T) {
162175
action, ok := dr.Match(firewall.Egress, hdr1)
163176
require.True(t, ok)
164177
assert.Equal(t, firewall.Allow, action)
178+
179+
// Rule should also match UDP.
180+
hdr1udp := &firewall.PacketHeader{
181+
DstIP: [4]byte{93, 184, 216, 34},
182+
Protocol: 17,
183+
DstPort: 443,
184+
}
185+
action, ok = dr.Match(firewall.Egress, hdr1udp)
186+
require.True(t, ok)
187+
assert.Equal(t, firewall.Allow, action)
165188
}
166189

167190
func TestDNSInterceptor_ResponseSnooping_NotAllowed(t *testing.T) {
168191
t.Parallel()
169192

170193
policy := NewPolicy([]HostSpec{{Name: "allowed.com"}})
171194
dr := firewall.NewDynamicRules()
172-
interceptor := NewDNSInterceptor(policy, dr)
195+
interceptor := NewDNSInterceptor(policy, dr, testDstIP)
173196

174197
// Response for a host not in the policy — should be ignored.
175198
ips := []net.IP{net.ParseIP("1.2.3.4")}
@@ -186,7 +209,7 @@ func TestDNSInterceptor_ResponseNXDOMAINFrameFormat(t *testing.T) {
186209

187210
policy := NewPolicy([]HostSpec{{Name: "allowed.com"}})
188211
dr := firewall.NewDynamicRules()
189-
interceptor := NewDNSInterceptor(policy, dr)
212+
interceptor := NewDNSInterceptor(policy, dr, testDstIP)
190213

191214
frame := buildDNSQueryFrame(testSrcMAC, testDstMAC, testSrcIP, testDstIP, 54321, "blocked.com")
192215
hdr := firewall.ParseHeaders(frame)
@@ -220,3 +243,94 @@ func TestDNSInterceptor_ResponseNXDOMAINFrameFormat(t *testing.T) {
220243
expectedIPLen := uint16(ihl + 8 + len(result.ResponseFrame) - (ipStart + ihl + 8))
221244
assert.Equal(t, expectedIPLen, ipTotalLen)
222245
}
246+
247+
func TestDNSInterceptor_UnparseableQuery_Dropped(t *testing.T) {
248+
t.Parallel()
249+
250+
policy := NewPolicy([]HostSpec{{Name: "allowed.com"}})
251+
dr := firewall.NewDynamicRules()
252+
interceptor := NewDNSInterceptor(policy, dr, testDstIP)
253+
254+
// Build a frame with valid Ethernet+IP+UDP headers but garbage DNS payload.
255+
frame := buildTestUDPFrame(testSrcMAC, testDstMAC, testSrcIP, testDstIP, 12345, 53, []byte{0xDE, 0xAD})
256+
hdr := firewall.ParseHeaders(frame)
257+
258+
result := interceptor.HandleEgress(frame, hdr)
259+
assert.Equal(t, firewall.InterceptDrop, result.Action,
260+
"unparseable DNS queries must be dropped, not forwarded")
261+
}
262+
263+
func TestDNSInterceptor_MultipleQuestions_Dropped(t *testing.T) {
264+
t.Parallel()
265+
266+
policy := NewPolicy([]HostSpec{{Name: "allowed.com"}})
267+
dr := firewall.NewDynamicRules()
268+
interceptor := NewDNSInterceptor(policy, dr, testDstIP)
269+
270+
// Build a DNS query with two questions.
271+
msg := &mdns.Msg{
272+
MsgHdr: mdns.MsgHdr{Id: 0x5678, RecursionDesired: true},
273+
Question: []mdns.Question{
274+
{Name: mdns.Fqdn("allowed.com"), Qtype: mdns.TypeA, Qclass: mdns.ClassINET},
275+
{Name: mdns.Fqdn("evil.com"), Qtype: mdns.TypeA, Qclass: mdns.ClassINET},
276+
},
277+
}
278+
payload, _ := msg.Pack()
279+
frame := buildTestUDPFrame(testSrcMAC, testDstMAC, testSrcIP, testDstIP, 12345, 53, payload)
280+
hdr := firewall.ParseHeaders(frame)
281+
282+
result := interceptor.HandleEgress(frame, hdr)
283+
assert.Equal(t, firewall.InterceptDrop, result.Action,
284+
"queries with multiple questions must be dropped")
285+
}
286+
287+
func TestDNSInterceptor_ResponseFromNonGateway_Ignored(t *testing.T) {
288+
t.Parallel()
289+
290+
policy := NewPolicy([]HostSpec{{Name: "allowed.com"}})
291+
dr := firewall.NewDynamicRules()
292+
gatewayIP := [4]byte{192, 168, 127, 1}
293+
interceptor := NewDNSInterceptor(policy, dr, gatewayIP)
294+
295+
// Build a response that looks like it comes from a non-gateway source.
296+
nonGatewayIP := [4]byte{10, 0, 0, 99}
297+
ips := []net.IP{net.ParseIP("1.2.3.4")}
298+
frame := buildDNSResponseFrame(testDstMAC, testSrcMAC, nonGatewayIP, testSrcIP, 12345, "allowed.com", ips, 300)
299+
hdr := firewall.ParseHeaders(frame)
300+
301+
interceptor.HandleIngress(frame, hdr)
302+
303+
assert.Equal(t, 0, dr.Len(),
304+
"DNS responses from non-gateway sources must not create dynamic rules")
305+
}
306+
307+
func TestDNSInterceptor_ExplicitProtocol_SingleRule(t *testing.T) {
308+
t.Parallel()
309+
310+
// When Protocol is explicitly TCP, only TCP rules should be created.
311+
policy := NewPolicy([]HostSpec{{Name: "example.com", Ports: []uint16{443}, Protocol: 6}})
312+
dr := firewall.NewDynamicRules()
313+
interceptor := NewDNSInterceptor(policy, dr, testDstIP)
314+
315+
ips := []net.IP{net.ParseIP("93.184.216.34")}
316+
frame := buildDNSResponseFrame(testDstMAC, testSrcMAC, testDstIP, testSrcIP, 12345, "example.com", ips, 60)
317+
hdr := firewall.ParseHeaders(frame)
318+
319+
interceptor.HandleIngress(frame, hdr)
320+
321+
// Explicit TCP → 1 rule per IP.
322+
assert.Equal(t, 1, dr.Len())
323+
324+
// TCP should match.
325+
action, ok := dr.Match(firewall.Egress, &firewall.PacketHeader{
326+
DstIP: [4]byte{93, 184, 216, 34}, Protocol: 6, DstPort: 443,
327+
})
328+
require.True(t, ok)
329+
assert.Equal(t, firewall.Allow, action)
330+
331+
// UDP should not match.
332+
_, ok = dr.Match(firewall.Egress, &firewall.PacketHeader{
333+
DstIP: [4]byte{93, 184, 216, 34}, Protocol: 17, DstPort: 443,
334+
})
335+
assert.False(t, ok)
336+
}

net/egress/policy.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ type hostEntry struct {
2424
type HostSpec struct {
2525
Name string // "api.github.com" or "*.docker.io"
2626
Ports []uint16 // empty = all ports
27-
Protocol uint8 // 0 = default (TCP), 6 = TCP, 17 = UDP
27+
Protocol uint8 // 0 = both TCP+UDP, 6 = TCP only, 17 = UDP only
2828
}
2929

3030
// NewPolicy creates a policy from the given host specifications.

0 commit comments

Comments
 (0)