dns: @hnw alternate proposal (#193)

* fix(dns): resolve CNAME chains and prevent invalid IP parsing - Add automated in-band CNAME chain resolution to extract final A/AAAA IP addresses. - Replace `MaxResponseAnswers` with `MaxIPs` and `MaxCNAMEs` to explicitly bound resource decoding and prevent memory exhaustion. - Bound CNAME chain traversal to prevent infinite loops from cyclic records. * refactor(dns): eagerly decode CNAME target into r.data, drop target field Address review feedback on #189: - Remove Resource.target: Resource.Decode expands CNAME RDATA in place into r.data (uncompressed wire format) while the full message is still available and updates header.Length, storing the name bytes exactly once. - Add Resource.CNAMEView returning a length-bounded view of the expanded target; Message.WriteAnswers uses it. - Rename matchesHost to ResourceHeader.pertainsTo. - Make ResolveConfig.MaxCNAMEs explicit: drop the silent default in Client.StartResolve and let callers opt in (x/xnet). - Reduce test suite to regression coverage only. refs #189 * dns: simplify @hnw function and additional fixes --------- Co-authored-by: Yoshio HANAWA <y@hnw.jp>
2026-09-01 04:19:05 +00:00 · 2026-08-28 21:39:12 -03:00
parent ab9d3ee691
commit 75f1e02a20
9 changed files with 369 additions and 56 deletions
@@ -25,7 +25,9 @@ type ResolveConfig struct {
 	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.
+	// DNS response. If zero it defaults to the number of Questions. Answers
+	// are decoded in wire order regardless of type, so a response resolved
+	// through CNAMEs needs room for the CNAME records as well as the addresses.
 	MaxResponseAnswers uint16
 }

@@ -37,7 +39,7 @@ 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
@@ -197,6 +197,8 @@ const (
 	TypeALL   Type = 255 // ALL
 )

+func (tp Type) IsIPAddr() bool { return tp == TypeA || tp == TypeAAAA }
+
 // A Class is a type of network.
 type Class uint16

@@ -99,6 +99,12 @@ func NamesEqual(a, b Name) bool {
 	return internal.BytesEqual(a.data, b.data)
 }

+// NamesEqualFold reports whether two DNS names are equal under ASCII case
+// folding, which is how DNS labels compare per RFC 1035 section 2.3.3.
+func NamesEqualFold(a, b Name) bool {
+	return internal.BytesEqualFoldASCII(a.data, b.data)
+}
+
 type ZFlags uint16

 func NewResource(name Name, typ Type, class Class, ttl uint32, data []byte) Resource {
@@ -299,27 +305,57 @@ func (m *Message) AppendTo(buf []byte, txid uint16, flags HeaderFlags) (_ []byte
 	return buf, nil
 }

+// WriteAnswers writes the addresses answering host into dst, following the
+// CNAME chain rooted at host. It returns the number of addresses written.
 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
+	// Each round resolves one CNAME, which consumes an answer. Bounding the
+	// walk by the answer count is thus enough to reach the addresses, and
+	// terminates on cyclic chains.
+	var alias Name // Canonical name reached so far; zero means host itself.
+	for range m.Answers {
+		var next Name
+		for i := range m.Answers {
+			ans := &m.Answers[i]
+			if !ans.header.ownedBy(alias, host) {
+				continue
+			}
+			switch {
+			case ans.header.Type.IsIPAddr():
+				if int(n) >= len(dst) {
+					return n, lneto.ErrExhausted
+				}
+				addr, ok := netip.AddrFromSlice(ans.RawData())
+				if !ok {
+					err = lneto.ErrInvalidAddr
+					continue
+				}
+				dst[n] = addr
+				n++
+			case ans.header.Type == TypeCNAME:
+				if cname := ans.CNAMEView(); cname.Len() != 0 {
+					next = cname
+				}
+			}
 		}
-		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 n > 0 || next.Len() == 0 {
+			break
 		}
+		alias = next
 	}
 	return n, err
 }

+// ownedBy reports whether the record's owner name is the name being resolved:
+// the alias reached by following CNAMEs, or host at the root of the chain.
+func (h *ResourceHeader) ownedBy(alias Name, host string) bool {
+	if alias.Len() == 0 {
+		return h.Name.EqualString(host)
+	}
+	// Fold: the server chooses the case of both the CNAME target and the owner
+	// name of the records it aliases, and may randomize it (DNS 0x20).
+	return NamesEqualFold(h.Name, alias)
+}
+
 func (m *Message) Len() uint16 {
 	return SizeHeader + m.lenResources()
 }
@@ -411,6 +447,15 @@ func (r *Resource) RawData() []byte {
 	return r.data[:length]
 }

+// CNAMEView returns the canonical name held by a CNAME record, aliasing the
+// Resource's buffer. It returns a zero Name for any other record type.
+func (r *Resource) CNAMEView() Name {
+	if r.header.Type != TypeCNAME {
+		return Name{}
+	}
+	return Name{data: r.RawData()}
+}
+
 func (q *Question) Reset() {
 	q.Name.Reset()
 	*q = Question{Name: q.Name} // Reuse Name's buffer.
@@ -460,8 +505,19 @@ func (r *Resource) Decode(b []byte, off uint16) (uint16, error) {
 	if r.header.Length > uint16(len(b[off:])) {
 		return off, errResourceLen
 	}
-	r.data = append(r.data[:0], b[off:off+r.header.Length]...)
-	return off + r.header.Length, nil
+	end := off + r.header.Length
+	if r.header.Type == TypeCNAME {
+		// CNAME data is a name which may use message compression. Expand it now
+		// since r.data is detached from b, leaving pointers unresolvable later.
+		cname := Name{data: r.data[:0]}
+		if _, derr := cname.Decode(b, off); derr == nil {
+			r.data = cname.data
+			r.header.Length = uint16(len(r.data))
+			return end, nil
+		}
+	}
+	r.data = append(r.data[:0], b[off:end]...)
+	return end, nil
 }

 func (r *Resource) appendTo(buf []byte) (_ []byte, err error) {
@@ -239,6 +239,152 @@ 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,
+		MaxResponseAnswers: 6,
+	})
+	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
+// 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: "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,
+		},
+		{
+			name: "CNAME target case differs from owner name",
+			host: "a.com",
+			// A server picks the case of both the CNAME target and the owner
+			// name of the record it aliases, and may randomize it (DNS 0x20),
+			// so the two must compare under ASCII case folding.
+			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 A IN ttl=10 rdlen=4 1.2.3.4.
+				0x01, 'b', 0x03, 'c', 'o', 'm', 0x00, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00, 0x00, 0x0a, 0x00, 0x04, 0x01, 0x02, 0x03, 0x04,
+			},
+			want: []netip.Addr{netip.AddrFrom4([4]byte{1, 2, 3, 4})},
+		},
+	}
+	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)
@@ -254,7 +400,7 @@ func TestClient_ReceivesDNSResponse(t *testing.T) {
 	tests := []struct {
 		name        string
 		responseIPs [][4]byte
-		wantAnswers int
+		wantAnswers int // Addresses returned by ResponseAnswerLookup and copied by ResponseCopyTo.
 	}{
 		{name: "single_answer", responseIPs: allIPs[:1], wantAnswers: 1},
 		{name: "multiple_answers", responseIPs: allIPs[:4], wantAnswers: 4},
@@ -312,7 +458,7 @@ func TestClient_ReceivesDNSResponse(t *testing.T) {
 			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++ {