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: ddf151af6c650160f2583f66c07d41ed18c7ae5e
GitHub-Pull-Request: golang/net#156
Reviewed-on: https://go-review.googlesource.com/c/net/+/448156
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Mateusz Poliwczak <mpoliwczak34@gmail.com>
diff --git a/dns/dnsmessage/message.go b/dns/dnsmessage/message.go
index ffdf19d..69c611b 100644
--- a/dns/dnsmessage/message.go
+++ b/dns/dnsmessage/message.go
@@ -263,6 +263,7 @@
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 @@
//
// 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 @@
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) 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 @@
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 {
diff --git a/dns/dnsmessage/message_test.go b/dns/dnsmessage/message_test.go
index 3cddfca..ef5326d 100644
--- a/dns/dnsmessage/message_test.go
+++ b/dns/dnsmessage/message_test.go
@@ -212,25 +212,28 @@
}
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 @@
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)
}
}
}
