Skip to content

Commit e4307b2

Browse files
JAORMXclaude
andcommitted
Filter DNS Answer A records by owner name
Previously, ParseDNSResponse collected every A record in the Answer section and associated them with the question name. An attacker controlling an allowed zone could return additional A records with different owner names — for internal IPs, cloud metadata endpoints, or any IP — and those would be installed into dynamic egress rules under the question's qname, silently broadening the allowlist. Collect only A records whose owner name is reachable from qname via the response's own CNAME chain. A multi-pass walk bounded by the Answer count handles legitimate CNAME chains while rejecting injected out-of-bailiwick records and detecting CNAME loops without infinite iteration. Also normalize the returned qname (lower-cased, trailing-dot stripped) via normalizeDNSName so downstream policy matching sees a canonical form; the policy matcher was already normalizing internally so behavior is preserved. Tests cover: legitimate multi-A response, out-of-bailiwick A record dropped, CNAME chain followed, and mixed response with both valid CNAME-A and injected unrelated-A dropped. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent af82a30 commit e4307b2

2 files changed

Lines changed: 141 additions & 2 deletions

File tree

net/egress/dns.go

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,13 +63,48 @@ func BuildNXDOMAIN(txnID uint16, qname string, qtype uint16) ([]byte, error) {
6363

6464
// ParseDNSResponse extracts the question name, A-record IPs, and minimum
6565
// TTL from a DNS response payload.
66+
//
67+
// Only A records whose owner name is reachable from the question via the
68+
// response's own CNAME chain are returned. An answer with an unrelated
69+
// owner name — for example, an injected out-of-bailiwick record — is
70+
// discarded. Without this filter, a compromised allowed zone could
71+
// smuggle arbitrary IPs into dynamic egress rules by returning them in
72+
// the Answer section under any name.
73+
//
74+
// The returned qname is normalized (lower-cased, trailing dot stripped).
6675
func ParseDNSResponse(payload []byte) (qname string, ips []net.IP, ttl uint32, err error) {
6776
var msg dns.Msg
6877
if err := msg.Unpack(payload); err != nil {
6978
return "", nil, 0, fmt.Errorf("unpack DNS response: %w", err)
7079
}
7180
if len(msg.Question) > 0 {
72-
qname = msg.Question[0].Name
81+
qname = normalizeDNSName(msg.Question[0].Name)
82+
}
83+
84+
// Build the set of owner names reachable from qname via CNAMEs in this
85+
// response. Multi-pass until no new names are added; bounded by the
86+
// Answer count so CNAME loops cannot cause infinite iteration.
87+
validNames := map[string]struct{}{qname: {}}
88+
for i := 0; i < len(msg.Answer); i++ {
89+
progress := false
90+
for _, rr := range msg.Answer {
91+
cn, ok := rr.(*dns.CNAME)
92+
if !ok {
93+
continue
94+
}
95+
owner := normalizeDNSName(cn.Hdr.Name)
96+
if _, have := validNames[owner]; !have {
97+
continue
98+
}
99+
target := normalizeDNSName(cn.Target)
100+
if _, already := validNames[target]; !already {
101+
validNames[target] = struct{}{}
102+
progress = true
103+
}
104+
}
105+
if !progress {
106+
break
107+
}
73108
}
74109

75110
var minTTL uint32
@@ -79,6 +114,9 @@ func ParseDNSResponse(payload []byte) (qname string, ips []net.IP, ttl uint32, e
79114
if !ok {
80115
continue
81116
}
117+
if _, ok := validNames[normalizeDNSName(a.Hdr.Name)]; !ok {
118+
continue
119+
}
82120
ips = append(ips, a.A)
83121
if first || a.Hdr.Ttl < minTTL {
84122
minTTL = a.Hdr.Ttl

net/egress/dns_test.go

Lines changed: 102 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,13 +146,114 @@ func TestParseDNSResponse(t *testing.T) {
146146

147147
qname, ips, ttl, err := ParseDNSResponse(payload)
148148
require.NoError(t, err)
149-
assert.Equal(t, "example.com.", qname)
149+
assert.Equal(t, "example.com", qname)
150150
assert.Len(t, ips, 2)
151151
assert.Equal(t, "93.184.216.34", ips[0].String())
152152
assert.Equal(t, "93.184.216.35", ips[1].String())
153153
assert.Equal(t, uint32(60), ttl) // minimum TTL
154154
}
155155

156+
func TestParseDNSResponse_DropsOutOfBailiwickAnswers(t *testing.T) {
157+
t.Parallel()
158+
159+
// Question for example.com; attacker-controlled response slips an A
160+
// record with a different owner name (typical out-of-bailiwick injection
161+
// attempt to smuggle an internal IP into dynamic rules).
162+
msg := &dns.Msg{
163+
MsgHdr: dns.MsgHdr{Id: 0x1234, Response: true},
164+
Question: []dns.Question{
165+
{Name: "example.com.", Qtype: dns.TypeA, Qclass: dns.ClassINET},
166+
},
167+
Answer: []dns.RR{
168+
&dns.A{
169+
Hdr: dns.RR_Header{Name: "example.com.", Rrtype: dns.TypeA, Class: dns.ClassINET, Ttl: 300},
170+
A: net.ParseIP("93.184.216.34"),
171+
},
172+
&dns.A{
173+
Hdr: dns.RR_Header{Name: "internal.local.", Rrtype: dns.TypeA, Class: dns.ClassINET, Ttl: 300},
174+
A: net.ParseIP("169.254.169.254"),
175+
},
176+
},
177+
}
178+
payload, err := msg.Pack()
179+
require.NoError(t, err)
180+
181+
qname, ips, _, err := ParseDNSResponse(payload)
182+
require.NoError(t, err)
183+
assert.Equal(t, "example.com", qname)
184+
require.Len(t, ips, 1)
185+
assert.Equal(t, "93.184.216.34", ips[0].String())
186+
}
187+
188+
func TestParseDNSResponse_FollowsCNAMEChain(t *testing.T) {
189+
t.Parallel()
190+
191+
// Legitimate CNAME chain: example.com -> cdn.example.net -> 1.2.3.4.
192+
// The A record's owner matches the CNAME target, which is reachable
193+
// from the question via the chain, so the IP is accepted.
194+
msg := &dns.Msg{
195+
MsgHdr: dns.MsgHdr{Id: 0x2345, Response: true},
196+
Question: []dns.Question{
197+
{Name: "example.com.", Qtype: dns.TypeA, Qclass: dns.ClassINET},
198+
},
199+
Answer: []dns.RR{
200+
&dns.CNAME{
201+
Hdr: dns.RR_Header{Name: "example.com.", Rrtype: dns.TypeCNAME, Class: dns.ClassINET, Ttl: 300},
202+
Target: "cdn.example.net.",
203+
},
204+
&dns.A{
205+
Hdr: dns.RR_Header{Name: "cdn.example.net.", Rrtype: dns.TypeA, Class: dns.ClassINET, Ttl: 300},
206+
A: net.ParseIP("1.2.3.4"),
207+
},
208+
},
209+
}
210+
payload, err := msg.Pack()
211+
require.NoError(t, err)
212+
213+
qname, ips, _, err := ParseDNSResponse(payload)
214+
require.NoError(t, err)
215+
assert.Equal(t, "example.com", qname)
216+
require.Len(t, ips, 1)
217+
assert.Equal(t, "1.2.3.4", ips[0].String())
218+
}
219+
220+
func TestParseDNSResponse_DropsUnreachableCNAMEA(t *testing.T) {
221+
t.Parallel()
222+
223+
// An A record whose owner is NOT reachable via any CNAME chain from
224+
// the question must be dropped even if another A record from the same
225+
// name-chain is valid.
226+
msg := &dns.Msg{
227+
MsgHdr: dns.MsgHdr{Id: 0x3456, Response: true},
228+
Question: []dns.Question{
229+
{Name: "example.com.", Qtype: dns.TypeA, Qclass: dns.ClassINET},
230+
},
231+
Answer: []dns.RR{
232+
&dns.CNAME{
233+
Hdr: dns.RR_Header{Name: "example.com.", Rrtype: dns.TypeCNAME, Class: dns.ClassINET, Ttl: 300},
234+
Target: "cdn.example.net.",
235+
},
236+
// A record for an unrelated name slipped into the Answer section.
237+
&dns.A{
238+
Hdr: dns.RR_Header{Name: "attacker.example.net.", Rrtype: dns.TypeA, Class: dns.ClassINET, Ttl: 300},
239+
A: net.ParseIP("10.0.0.1"),
240+
},
241+
// Legitimate A record at the CNAME target.
242+
&dns.A{
243+
Hdr: dns.RR_Header{Name: "cdn.example.net.", Rrtype: dns.TypeA, Class: dns.ClassINET, Ttl: 300},
244+
A: net.ParseIP("1.2.3.4"),
245+
},
246+
},
247+
}
248+
payload, err := msg.Pack()
249+
require.NoError(t, err)
250+
251+
_, ips, _, err := ParseDNSResponse(payload)
252+
require.NoError(t, err)
253+
require.Len(t, ips, 1)
254+
assert.Equal(t, "1.2.3.4", ips[0].String())
255+
}
256+
156257
func TestParseDNSResponse_NoARecords(t *testing.T) {
157258
t.Parallel()
158259

0 commit comments

Comments
 (0)