diff --git a/dns/client.go b/dns/client.go index 0a62847..7cba399 100644 --- a/dns/client.go +++ b/dns/client.go @@ -24,9 +24,8 @@ type ResolveConfig struct { Questions []Question Additional []Resource EnableRecursion bool - // MaxResponseAnswers limits how many answer records are decoded from the - // DNS response. If zero it defaults to the number of Questions. - MaxResponseAnswers uint16 + MaxIPs uint16 + MaxCNAMEs uint16 } func (sudp *Client) Protocol() uint64 { return uint64(lneto.IPProtoUDP) } @@ -37,13 +36,18 @@ func (sudp *Client) ConnectionID() *uint64 { return &sudp.connID } func (c *Client) StartResolve(localPort, txid uint16, cfg ResolveConfig) error { nd := len(cfg.Questions) - if nd > math.MaxUint16 { + if nd > math.MaxUint16 || nd == 0 { return lneto.ErrInvalidConfig } - maxAns := cfg.MaxResponseAnswers - if maxAns == 0 { - maxAns = uint16(nd) + maxIPs := cfg.MaxIPs + if maxIPs == 0 { + maxIPs = uint16(nd) } + maxCNAMEs := cfg.MaxCNAMEs + if maxCNAMEs == 0 { + maxCNAMEs = 16 + } + maxAns := maxIPs + maxCNAMEs c.reset(localPort, txid, CQueryPending, cfg.EnableRecursion) c.msg.LimitResourceDecoding(uint16(nd), maxAns, 0, 0) c.msg.AddQuestions(cfg.Questions) diff --git a/dns/dns.go b/dns/dns.go index ba791fa..d7e1df6 100644 --- a/dns/dns.go +++ b/dns/dns.go @@ -48,6 +48,7 @@ type Question struct { type Resource struct { header ResourceHeader data []byte + target Name } // A ResourceHeader is the header of a DNS resource record. There are @@ -300,22 +301,45 @@ func (m *Message) AppendTo(buf []byte, txid uint16, flags HeaderFlags) (_ []byte } func (m *Message) WriteAnswers(dst []netip.Addr, host string) (n uint16, err error) { - for i := range m.Answers { - if int(n) >= len(dst) { - return n, lneto.ErrExhausted + var alias Name + // Each pass follows at most one alias hop + // The pass limit also bounds CNAME cycles in malformed responses. + for range m.Answers { + var next Name + n = 0 + for i := range m.Answers { + if int(n) >= len(dst) { + break + } + ans := &m.Answers[i] + hdr := ans.Header() + matched := alias.Len() == 0 && hdr.Name.EqualString(host) || + alias.Len() != 0 && NamesEqual(hdr.Name, alias) + if !matched { + continue + } + switch hdr.Type { + case TypeA, TypeAAAA: + var ok bool + dst[n], ok = netip.AddrFromSlice(ans.RawData()) + if !ok { + err = lneto.ErrInvalidAddr + continue + } else { + n++ + } + case TypeCNAME: + if ans.target.Len() != 0 { + next = ans.target + } + } } - ans := &m.Answers[i] - hdr := ans.Header() - if !hdr.Name.EqualString(host) { - continue - } - var ok bool - dst[n], ok = netip.AddrFromSlice(ans.RawData()) - if !ok { - err = lneto.ErrInvalidAddr - } else { - n++ + if next.Len() == 0 || n > 0 { + // Either no new alias was found or addresses were resolved for + // the current name; a name cannot alias and hold records at once. + break } + alias = next } return n, err } @@ -399,6 +423,7 @@ func (h *ResourceHeader) String() string { func (r *Resource) Reset() { r.header.Reset() r.data = r.data[:0] + r.target.Reset() } func (r *Resource) Header() ResourceHeader { return r.header } @@ -460,6 +485,14 @@ func (r *Resource) Decode(b []byte, off uint16) (uint16, error) { if r.header.Length > uint16(len(b[off:])) { return off, errResourceLen } + if r.header.Type == TypeCNAME { + _, err = r.target.Decode(b, off) + if err != nil { + r.target.Reset() // Tolerate undecodable target; CNAME chain lookup skips it. + } + } else { + r.target.Reset() + } r.data = append(r.data[:0], b[off:off+r.header.Length]...) return off + r.header.Length, nil } @@ -775,6 +808,7 @@ func (dst *Question) CopyFrom(q Question) { func (dst *Resource) CopyFrom(r Resource) { dst.header.CopyFrom(r.header) dst.data = append(dst.data[:0], r.data...) + dst.target.CopyFrom(r.target) } // SetA sets an A (IPv4 address) resource record, reusing internal buffers. diff --git a/dns/dns_test.go b/dns/dns_test.go index 75d4a6e..104b128 100644 --- a/dns/dns_test.go +++ b/dns/dns_test.go @@ -239,26 +239,178 @@ func TestDecodeMessage(t *testing.T) { } } +// Regression test for CNAME-following: a response for www.yahoo.co.jp +// contains a CNAME record to edge12.g.yimg.jp (with compressed labels in its +// RDATA) followed by the A record for the canonical name. The CNAME RDATA +// must not be interpreted as an IP address and the A record must be returned. +func TestClient_CNAMEResponse(t *testing.T) { + const hostname = "www.yahoo.co.jp" + const txid = uint16(0x1234) + const clientPort = uint16(54321) + response := []byte{ + // Header: txid 0x1234, QR|RD|RA, QD=1 AN=2 NS=0 AR=0. + 0x12, 0x34, 0x81, 0x80, 0x00, 0x01, 0x00, 0x02, 0x00, 0x00, 0x00, 0x00, + // Question: www.yahoo.co.jp A IN. + 0x03, 'w', 'w', 'w', 0x05, 'y', 'a', 'h', 'o', 'o', 0x02, 'c', 'o', 0x02, 'j', 'p', 0x00, + 0x00, 0x01, 0x00, 0x01, + // Answer 1: (ptr to question) CNAME IN ttl=842 rdlen=16 + // rdata: edge12.g.yimg.jp with "jp" as compression pointer to offset 0x19. + 0xc0, 0x0c, 0x00, 0x05, 0x00, 0x01, 0x00, 0x00, 0x03, 0x4a, 0x00, 0x10, + 0x06, 'e', 'd', 'g', 'e', '1', '2', 0x01, 'g', 0x04, 'y', 'i', 'm', 'g', 0xc0, 0x19, + // Answer 2: (ptr into CNAME rdata) A IN ttl=36 rdlen=4 182.22.23.124. + 0xc0, 0x2d, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00, 0x00, 0x24, 0x00, 0x04, 0xb6, 0x16, 0x17, 0x7c, + } + name := MustNewName(hostname) + var client Client + err := client.StartResolve(clientPort, txid, ResolveConfig{ + Questions: []Question{{ + Name: name, + Type: TypeA, + Class: ClassINET, + }}, + EnableRecursion: true, + MaxIPs: 4, + MaxCNAMEs: 2, + }) + if err != nil { + t.Fatal("failed to start DNS resolve:", err) + } + var queryBuf [512]byte + _, err = client.Encapsulate(queryBuf[:], 0, 0) + if err != nil { + t.Fatal("failed to encapsulate DNS query:", err) + } + if err := client.Demux(response, 0); err != nil { + t.Fatal("failed to demux DNS response:", err) + } + var addrs [4]netip.Addr + n, err := client.ResponseAnswerLookup(addrs[:], hostname) + if err != nil { + t.Fatal("failed to look up DNS response answers:", err) + } + if n != 1 { + t.Fatalf("expected 1 answer, got %d: %v", n, addrs[:n]) + } + if addrs[0] != (netip.AddrFrom4([4]byte{182, 22, 23, 124})) { + t.Fatalf("expected 182.22.23.124, got %v", addrs[0]) + } +} + +// Table-driven tests for Message.WriteAnswers covering answer reordering, +// non-address record types and cyclic CNAME aliases. +func TestMessage_WriteAnswers(t *testing.T) { + tests := []struct { + name string + host string + response []byte + want []netip.Addr + }{ + { + name: "A record before its CNAME", + host: "www.yahoo.co.jp", + // Answer 1 is the A record for edge12.g.yimg.jp, spelled out with + // a trailing compression pointer to "jp" in the question. Answer 2 + // is the CNAME from www.yahoo.co.jp whose RDATA is a single + // backward compression pointer to answer 1's owner name. + response: []byte{ + // Header: txid 0x1234, QR|RD|RA, QD=1 AN=2 NS=0 AR=0. + 0x12, 0x34, 0x81, 0x80, 0x00, 0x01, 0x00, 0x02, 0x00, 0x00, 0x00, 0x00, + // Question: www.yahoo.co.jp A IN. + 0x03, 'w', 'w', 'w', 0x05, 'y', 'a', 'h', 'o', 'o', 0x02, 'c', 'o', 0x02, 'j', 'p', 0x00, + 0x00, 0x01, 0x00, 0x01, + // Answer 1: edge12.g.yimg.jp A IN ttl=36 rdlen=4 182.22.23.124. + 0x06, 'e', 'd', 'g', 'e', '1', '2', 0x01, 'g', 0x04, 'y', 'i', 'm', 'g', 0xc0, 0x19, + 0x00, 0x01, 0x00, 0x01, 0x00, 0x00, 0x00, 0x24, 0x00, 0x04, 0xb6, 0x16, 0x17, 0x7c, + // Answer 2: (ptr to question) CNAME IN ttl=842 rdlen=2, target + // is a pointer to answer 1's owner name at offset 0x21. + 0xc0, 0x0c, 0x00, 0x05, 0x00, 0x01, 0x00, 0x00, 0x03, 0x4a, 0x00, 0x02, 0xc0, 0x21, + }, + want: []netip.Addr{netip.AddrFrom4([4]byte{182, 22, 23, 124})}, + }, + { + name: "ignores non-address records", + host: "example.com", + response: []byte{ + // Header: txid 0x5678, QR|RD|RA, QD=1 AN=2 NS=0 AR=0. + 0x56, 0x78, 0x81, 0x80, 0x00, 0x01, 0x00, 0x02, 0x00, 0x00, 0x00, 0x00, + // Question: example.com A IN. + 0x07, 'e', 'x', 'a', 'm', 'p', 'l', 'e', 0x03, 'c', 'o', 'm', 0x00, + 0x00, 0x01, 0x00, 0x01, + // Answer 1: (ptr to question) TXT IN ttl=60 rdlen=6 "hello". + 0xc0, 0x0c, 0x00, 0x10, 0x00, 0x01, 0x00, 0x00, 0x00, 0x3c, 0x00, 0x06, 0x05, 'h', 'e', 'l', 'l', 'o', + // Answer 2: (ptr to question) A IN ttl=300 rdlen=4 192.0.2.1. + 0xc0, 0x0c, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00, 0x01, 0x2c, 0x00, 0x04, 0xc0, 0x00, 0x02, 0x01, + }, + want: []netip.Addr{netip.AddrFrom4([4]byte{192, 0, 2, 1})}, + }, + { + name: "CNAME cycle terminates", + host: "a.com", + response: []byte{ + // Header: txid 0xabcd, QR|RD|RA, QD=1 AN=2 NS=0 AR=0. + 0xab, 0xcd, 0x81, 0x80, 0x00, 0x01, 0x00, 0x02, 0x00, 0x00, 0x00, 0x00, + // Question: a.com A IN. + 0x01, 'a', 0x03, 'c', 'o', 'm', 0x00, 0x00, 0x01, 0x00, 0x01, + // Answer 1: a.com CNAME b.com. + 0x01, 'a', 0x03, 'c', 'o', 'm', 0x00, 0x00, 0x05, 0x00, 0x01, 0x00, 0x00, 0x00, 0x0a, 0x00, 0x07, 0x01, 'b', 0x03, 'c', 'o', 'm', 0x00, + // Answer 2: b.com CNAME a.com. + 0x01, 'b', 0x03, 'c', 'o', 'm', 0x00, 0x00, 0x05, 0x00, 0x01, 0x00, 0x00, 0x00, 0x0a, 0x00, 0x07, 0x01, 'a', 0x03, 'c', 'o', 'm', 0x00, + }, + want: nil, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var msg Message + msg.LimitResourceDecoding(1, 4, 0, 0) + _, incomplete, err := msg.Decode(tt.response) + if incomplete || err != nil { + t.Fatal("decode:", incomplete, err) + } + var addrs [4]netip.Addr + n, err := msg.WriteAnswers(addrs[:], tt.host) + if err != nil { + t.Fatal("write answers:", err) + } + if n != uint16(len(tt.want)) { + t.Fatalf("expected %d addresses, got %d: %v", len(tt.want), n, addrs[:n]) + } + for i, want := range tt.want { + if addrs[i] != want { + t.Errorf("address %d: expected %v, got %v", i, want, addrs[i]) + } + } + }) + } +} + func TestClient_ReceivesDNSResponse(t *testing.T) { const hostname = "example.com" const txid = uint16(12345) const clientPort = uint16(54321) - const maxAnswers = 4 - allIPs := [5][4]byte{ + const maxIPs = 4 + const maxCNAMEs = 2 + const maxDecoded = maxIPs + maxCNAMEs + allIPs := [8][4]byte{ {192, 0, 2, 1}, {192, 0, 2, 2}, {192, 0, 2, 3}, {192, 0, 2, 4}, {192, 0, 2, 5}, + {192, 0, 2, 6}, + {192, 0, 2, 7}, + {192, 0, 2, 8}, } tests := []struct { name string responseIPs [][4]byte - wantAnswers int + wantDecoded int // Answers decoded (and copied by ResponseCopyTo). + wantAnswers int // Addresses returned by ResponseAnswerLookup. }{ - {name: "single_answer", responseIPs: allIPs[:1], wantAnswers: 1}, - {name: "multiple_answers", responseIPs: allIPs[:4], wantAnswers: 4}, - {name: "answer_limit", responseIPs: allIPs[:5], wantAnswers: maxAnswers}, + {name: "single_answer", responseIPs: allIPs[:1], wantDecoded: 1, wantAnswers: 1}, + {name: "multiple_answers", responseIPs: allIPs[:4], wantDecoded: 4, wantAnswers: 4}, + {name: "answer_limit", responseIPs: allIPs[:5], wantDecoded: 5, wantAnswers: maxIPs}, + {name: "decode_limit", responseIPs: allIPs[:8], wantDecoded: maxDecoded, wantAnswers: maxIPs}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -290,8 +442,9 @@ func TestClient_ReceivesDNSResponse(t *testing.T) { Type: TypeA, Class: ClassINET, }}, - EnableRecursion: true, - MaxResponseAnswers: maxAnswers, + EnableRecursion: true, + MaxIPs: maxIPs, + MaxCNAMEs: maxCNAMEs, }) if err != nil { t.Fatal("failed to start DNS resolve:", err) @@ -307,12 +460,12 @@ func TestClient_ReceivesDNSResponse(t *testing.T) { t.Fatal("failed to demux DNS response:", err) } - var addrs [maxAnswers]netip.Addr + var addrs [maxIPs]netip.Addr answers, err := client.ResponseAnswerLookup(addrs[:], hostname) if err != nil { t.Fatal("failed to look up DNS response answers:", err) } - if answers != uint16(tt.wantAnswers) { + if int(answers) != tt.wantAnswers { t.Fatalf("expected %d answers, got %d", tt.wantAnswers, answers) } for i := 0; i < tt.wantAnswers; i++ { @@ -334,8 +487,8 @@ func TestClient_ReceivesDNSResponse(t *testing.T) { if !done { t.Fatal("expected done=true") } - if len(lookup.Answers) != tt.wantAnswers { - t.Fatalf("expected %d copied answers, got %d", tt.wantAnswers, len(lookup.Answers)) + if len(lookup.Answers) != tt.wantDecoded { + t.Fatalf("expected %d copied answers, got %d", tt.wantDecoded, len(lookup.Answers)) } }) } diff --git a/x/xnet/stack-async.go b/x/xnet/stack-async.go index 62e24fe..3fcda60 100644 --- a/x/xnet/stack-async.go +++ b/x/xnet/stack-async.go @@ -629,8 +629,8 @@ func (s *StackAsync) StartLookupIPType(host string, qtype dns.Type) error { Additional: []dns.Resource{ s.ednsopt, }, - EnableRecursion: true, - MaxResponseAnswers: uint16(len(s.addrbufnip)), + EnableRecursion: true, + MaxIPs: uint16(len(s.addrbufnip)), }) if err != nil { return err diff --git a/x/xnet/xnet_dns_test.go b/x/xnet/xnet_dns_test.go index b6a6a8a..db4582d 100644 --- a/x/xnet/xnet_dns_test.go +++ b/x/xnet/xnet_dns_test.go @@ -156,6 +156,14 @@ func buildDNSResponsePacket(t *testing.T, txid uint16, dstPort uint16, hostname dns.NewResource(name, dns.TypeA, dns.ClassINET, 300, addr.AsSlice()), }, } + return buildDNSMsgResponsePacket(t, txid, dstPort, msg, srcIP, srcMAC, dstIP, dstMAC, buf) +} + +// buildDNSMsgResponsePacket wraps a DNS response message into a complete +// Ethernet+IP+UDP packet with valid checksums. +func buildDNSMsgResponsePacket(t *testing.T, txid uint16, dstPort uint16, msg dns.Message, + srcIP netip.Addr, srcMAC [6]byte, dstIP netip.Addr, dstMAC [6]byte, buf []byte) ([]byte, error) { + t.Helper() // Response flags: QR=1 (response), RD=1 (recursion desired), RA=1 (recursion available). responseFlags := dns.HeaderFlags(1<<15 | 1<<8 | 1<<7) @@ -237,3 +245,97 @@ var errBaseLenDNS = func() error { }() var errInvalidEtherType = errors.New("invalid ethernet type") + +// TestDNS_CNAMEResponse verifies that a DNS response containing a CNAME record +// followed by an A record for the canonical name resolves to the A record's +// address: the CNAME RDATA must not be misinterpreted as an IP address. +func TestDNS_CNAMEResponse(t *testing.T) { + const seed = 9876 + const MTU = ethernet.MaxMTU + + client := new(StackAsync) + dnsServerAddr := netip.AddrFrom4([4]byte{8, 8, 8, 8}) + clientAddr := netip.AddrFrom4([4]byte{10, 0, 0, 100}) + clientMAC := [6]byte{0xde, 0xad, 0xbe, 0xef, 0x00, 0x01} + dnsServerMAC := [6]byte{0x00, 0x11, 0x22, 0x33, 0x44, 0x55} + + err := client.Reset(StackConfig{ + Hostname: "DNSClient", + RandSeed: seed, + StaticAddress4: clientAddr.As4(), + DNSServer: dnsServerAddr, + HardwareAddress: clientMAC, + MTU: uint16(MTU), + }) + if err != nil { + t.Fatal("client Reset failed:", err) + } + client.SetGatewayHardwareAddr(dnsServerMAC) + + const hostname = "www.example.com" + const alias = "cdn.example.net" + wantAddr := netip.MustParseAddr("192.0.2.200") + + err = client.StartLookupIP(hostname) + if err != nil { + t.Fatal("StartLookupIP failed:", err) + } + + const carrierDataSize = ethernet.MaxFrameLength + var buf [carrierDataSize]byte + + // Client sends DNS query for www.example.com. + n, err := client.EgressEthernet(buf[:]) + if err != nil || n == 0 { + t.Fatal("expected DNS query packet from client:", err, n) + } + txid, clientPort, err := extractDNSTxIDAndPort(buf[:n]) + if err != nil { + t.Fatal("failed to extract DNS txid:", err) + } + + // Respond with a CNAME record www.example.com -> cdn.example.net + // followed by the A record for cdn.example.net. + owner, err := dns.NewName(hostname) + if err != nil { + t.Fatal(err) + } + aliasName, err := dns.NewName(alias) + if err != nil { + t.Fatal(err) + } + aliasWire, err := aliasName.AppendTo(nil) + if err != nil { + t.Fatal(err) + } + msg := dns.Message{ + Questions: []dns.Question{{ + Name: owner, + Type: dns.TypeA, + Class: dns.ClassINET, + }}, + Answers: []dns.Resource{ + dns.NewResource(owner, dns.TypeCNAME, dns.ClassINET, 300, aliasWire), + dns.NewResource(aliasName, dns.TypeA, dns.ClassINET, 300, wantAddr.AsSlice()), + }, + } + responsePkt, err := buildDNSMsgResponsePacket(t, txid, clientPort, msg, + dnsServerAddr, dnsServerMAC, clientAddr, clientMAC, buf[:]) + if err != nil { + t.Fatal("failed to build CNAME response packet:", err) + } + if err = client.IngressEthernet(responsePkt); err != nil { + t.Fatal("client Demux of CNAME response failed:", err) + } + + addrs, done, err := client.ResultLookupIP(hostname) + if err != nil { + t.Fatal("ResultLookupIP error:", err) + } + if !done { + t.Fatal("DNS lookup not done after receiving CNAME response") + } + if !slices.Contains(addrs, wantAddr) { + t.Errorf("expected address %s not found in result %v", wantAddr, addrs) + } +}