dns/dnsmessage: reject packing of 255B rooted names, reject unpacking of 256B (dns encoded) names

Packing a 255B (rooted) name will create an 256B (dns encoded) name, which is an invalid name. Similar with unpacking, we can't unpack 256B (dns encoded) name, because it is too long.

Change-Id: I17cc93a93a17a879a2a789629e56ad39999da9ac
GitHub-Last-Rev: ddf151a
GitHub-Pull-Request: #156
Reviewed-on: https://go-review.googlesource.com/c/net/+/448156
Reviewed-by: Michael Knyszek <[email protected]>
Reviewed-by: Damien Neil <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Run-TryBot: Mateusz Poliwczak <[email protected]>

Author: mateusz834

dns/dnsmessage/message.go

@@ -263,6 +263,7 @@ var (
 	errNilResouceBody     = errors.New("nil resource body")
 	errResourceLen        = errors.New("insufficient data for resource body length")
 	errSegTooLong         = errors.New("segment length too long")
+	errNameTooLong        = errors.New("name too long")
 	errZeroSegLen         = errors.New("zero length segment")
 	errResTooLong         = errors.New("resource length too long")
 	errTooManyQuestions   = errors.New("too many Questions to pack (>65535)")
@@ -1728,7 +1729,7 @@ const (
 //
 // The provided extRCode must be an extended RCode.
 func (h *ResourceHeader) SetEDNS0(udpPayloadLen int, extRCode RCode, dnssecOK bool) error {
-	h.Name = Name{Data: [nameLen]byte{'.'}, Length: 1} // RFC 6891 section 6.1.2
+	h.Name = Name{Data: [255]byte{'.'}, Length: 1} // RFC 6891 section 6.1.2
 	h.Type = TypeOPT
 	h.Class = Class(udpPayloadLen)
 	h.TTL = uint32(extRCode) >> 4 << 24
@@ -1888,21 +1889,21 @@ func unpackBytes(msg []byte, off int, field []byte) (int, error) {
 	return newOff, nil
 }
 
-const nameLen = 255
+const nonEncodedNameMax = 254
 
 // A Name is a non-encoded domain name. It is used instead of strings to avoid
 // allocations.
 type Name struct {
-	Data   [nameLen]byte // 255 bytes
+	Data   [255]byte
 	Length uint8
 }
 
 // NewName creates a new Name from a string.
 func NewName(name string) (Name, error) {
-	if len(name) > nameLen {
+	n := Name{Length: uint8(len(name))}
+	if len(name) > len(n.Data) {
 		return Name{}, errCalcLen
 	}
-	n := Name{Length: uint8(len(name))}
 	copy(n.Data[:], name)
 	return n, nil
 }
@@ -1936,6 +1937,10 @@ func (n *Name) GoString() string {
 func (n *Name) pack(msg []byte, compression map[string]int, compressionOff int) ([]byte, error) {
 	oldMsg := msg
 
+	if n.Length > nonEncodedNameMax {
+		return nil, errNameTooLong
+	}
+
 	// Add a trailing dot to canonicalize name.
 	if n.Length == 0 || n.Data[n.Length-1] != '.' {
 		return oldMsg, errNonCanonicalName
@@ -2057,8 +2062,8 @@ Loop:
 	if len(name) == 0 {
 		name = append(name, '.')
 	}
-	if len(name) > len(n.Data) {
-		return off, errCalcLen
+	if len(name) > nonEncodedNameMax {
+		return off, errNameTooLong
 	}
 	n.Length = uint8(len(name))
 	if ptr == 0 {

dns/dnsmessage/message_test.go

@@ -212,25 +212,28 @@ func TestName(t *testing.T) {
 }
 
 func TestNamePackUnpack(t *testing.T) {
+	const suffix = ".go.dev."
+	var longDNSPrefix = strings.Repeat("verylongdomainlabel.", 20)
+
 	tests := []struct {
-		in   string
-		want string
-		err  error
+		in  string
+		err error
 	}{
-		{"", "", errNonCanonicalName},
-		{".", ".", nil},
-		{"google..com", "", errNonCanonicalName},
-		{"google.com", "", errNonCanonicalName},
-		{"google..com.", "", errZeroSegLen},
-		{"google.com.", "google.com.", nil},
-		{".google.com.", "", errZeroSegLen},
-		{"www..google.com.", "", errZeroSegLen},
-		{"www.google.com.", "www.google.com.", nil},
+		{"", errNonCanonicalName},
+		{".", nil},
+		{"google..com", errNonCanonicalName},
+		{"google.com", errNonCanonicalName},
+		{"google..com.", errZeroSegLen},
+		{"google.com.", nil},
+		{".google.com.", errZeroSegLen},
+		{"www..google.com.", errZeroSegLen},
+		{"www.google.com.", nil},
+		{in: longDNSPrefix[:254-len(suffix)] + suffix},                      // 254B name, with ending dot.
+		{in: longDNSPrefix[:255-len(suffix)] + suffix, err: errNameTooLong}, // 255B name, with ending dot.
 	}
 
 	for _, test := range tests {
 		in := MustNewName(test.in)
-		want := MustNewName(test.want)
 		buf, err := in.pack(make([]byte, 0, 30), map[string]int{}, 0)
 		if err != test.err {
 			t.Errorf("got %q.pack() = %v, want = %v", test.in, err, test.err)
@@ -253,8 +256,40 @@ func TestNamePackUnpack(t *testing.T) {
 				len(buf),
 			)
 		}
-		if got != want {
-			t.Errorf("unpacking packing of %q: got = %#v, want = %#v", test.in, got, want)
+		if got != in {
+			t.Errorf("unpacking packing of %q: got = %#v, want = %#v", test.in, got, in)
+		}
+	}
+}
+
+func TestNameUnpackTooLongName(t *testing.T) {
+	var suffix = []byte{2, 'g', 'o', 3, 'd', 'e', 'v', 0}
+
+	const label = "longdnslabel"
+	labelBinary := append([]byte{byte(len(label))}, []byte(label)...)
+	var longDNSPrefix = bytes.Repeat(labelBinary, 18)
+	longDNSPrefix = longDNSPrefix[:len(longDNSPrefix):len(longDNSPrefix)]
+
+	prepName := func(length int) []byte {
+		missing := length - (len(longDNSPrefix) + len(suffix) + 1)
+		name := append(longDNSPrefix, byte(missing))
+		name = append(name, bytes.Repeat([]byte{'a'}, missing)...)
+		return append(name, suffix...)
+	}
+
+	tests := []struct {
+		name []byte
+		err  error
+	}{
+		{name: prepName(255)},
+		{name: prepName(256), err: errNameTooLong},
+	}
+
+	for i, test := range tests {
+		var got Name
+		_, err := got.unpack(test.name, 0)
+		if err != test.err {
+			t.Errorf("%v: %v: expected error: %v, got %v", i, test.name, test.err, err)
 		}
 	}
 }