util/dnsname: do not allow labels starting with a digitadrian/stricter-labels

This disallows labels that start with digits (as per RFC 1035 section
2.3.1). The invalid character error text is also improved so that UTF-8
sequences are properly copied to the error rather than an encoding of
the first byte.

Updates #13858

Change-Id: Ib3fea46559918e7bba6b11c1ca33ede61755bd52
Signed-off-by: Adrian Dewhurst <adrian@tailscale.com>

2 files changed, 57 insertions, 5 deletions

diff --git a/util/dnsname/dnsname.go b/util/dnsname/dnsname.go
index dde0baaed..e0c1c2a69 100644
--- a/util/dnsname/dnsname.go
+++ b/util/dnsname/dnsname.go
@@ -102,8 +102,8 @@ func ValidLabel(label string) error {
 	if len(label) > maxLabelLength {
 		return fmt.Errorf("%q is too long, max length is %d bytes", label, maxLabelLength)
 	}
-	if !isalphanum(label[0]) {
-		return fmt.Errorf("%q is not a valid DNS label: must start with a letter or number", label)
+	if !isalpha(label[0]) {
+		return fmt.Errorf("%q is not a valid DNS label: must start with a letter", label)
 	}
 	if !isalphanum(label[len(label)-1]) {
 		return fmt.Errorf("%q is not a valid DNS label: must end with a letter or number", label)
@@ -111,9 +111,9 @@ func ValidLabel(label string) error {
 	if len(label) < 2 {
 		return nil
 	}
-	for i := 1; i < len(label)-1; i++ {
+	for i, c := range label {
 		if !isdnschar(label[i]) {
-			return fmt.Errorf("%q is not a valid DNS label: contains invalid character %q", label, label[i])
+			return fmt.Errorf("%q is not a valid DNS label: contains invalid character %q", label, c)
 		}
 	}
 	return nil
diff --git a/util/dnsname/dnsname_test.go b/util/dnsname/dnsname_test.go
index 719e28be3..27ca4e8d6 100644
--- a/util/dnsname/dnsname_test.go
+++ b/util/dnsname/dnsname_test.go
@@ -192,8 +192,9 @@ func TestValidHostname(t *testing.T) {
 	}{
 		{"example", ""},
 		{"example.com", ""},
-		{" example", `must start with a letter or number`},
+		{" example", `must start with a letter`},
 		{"example-.com", `must end with a letter or number`},
+		{"www.2example.com", `must start with a letter`},
 		{strings.Repeat("a", 63), ""},
 		{strings.Repeat("a", 64), `is too long, max length is 63 bytes`},
 		{strings.Repeat(strings.Repeat("a", 63)+".", 4), "is too long to be a DNS name"},
@@ -230,3 +231,54 @@ func BenchmarkToFQDN(b *testing.B) {
 		})
 	}
 }
+
+func TestValidLabel(t *testing.T) {
+	tests := []struct {
+		label   string
+		wantErr string
+	}{
+		{"", "empty DNS label"},
+		{strings.Repeat("a", 63), ""},
+		{strings.Repeat("a", 64), "is too long, max length is 63 bytes"},
+		{"a", ""},
+		{"A", ""},
+		{"1", "must start with a letter"},
+		{"-", "must start with a letter"},
+		{"$", "must start with a letter"},
+		{"$abc", "must start with a letter"},
+		{"1abc", "must start with a letter"},
+		{"-abc", "must start with a letter"},
+		{"az", ""},
+		{"aZ", ""},
+		{"Az", ""},
+		{"A5", ""},
+		{"A-", "must end with a letter or number"},
+		{"A$", "must end with a letter or number"},
+		{"abcd", ""},
+		{"aBcD", ""},
+		{"aBc9", ""},
+		{"aBc$", "must end with a letter or number"},
+		{"aBc-", "must end with a letter or number"},
+		{"1Bcd", "must start with a letter"},
+		{"-Bcd", "must start with a letter"},
+		{"%Bcd", "must start with a letter"},
+		{"A--d", ""},
+		{"A---", "must end with a letter or number"},
+		{"A234", ""},
+		{"A^34", "contains invalid character '^'"},
+		{"a.b", "contains invalid character '.'"},
+		{"what🤦lol", "contains invalid character '🤦'"},
+		{"truncated\xf0\x90\x8dz", "contains invalid character '\ufffd'"},
+		{"invalid\xe2\x28\xa1z", "contains invalid character '\ufffd'"},
+		{"overlong\xc1\x81z", "contains invalid character '\ufffd'"},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.label, func(t *testing.T) {
+			err := ValidLabel(tt.label)
+			if (err == nil) != (tt.wantErr == "") || (err != nil && !strings.HasSuffix(err.Error(), tt.wantErr)) {
+				t.Fatalf("ValidLabel(%s)=%v; expected %v", tt.label, err, tt.wantErr)
+			}
+		})
+	}
+}