cmd/tailscale: extend hostname validation (#7678)

In addition to checking the total hostname length, validate characters used in each DNS label and label length.

Updates https://github.com/tailscale/corp/issues/10012

Signed-off-by: Anton Tolchanov <anton@tailscale.com>

cmd/tailscale/cli/cli_test.go

@@ -621,9 +621,16 @@ func TestPrefsFromUpArgs(t *testing.T) {
 		{
 			name: "error_long_hostname",
 			args: upArgsT{
-				hostname: strings.Repeat("a", 300),
+				hostname: strings.Repeat(strings.Repeat("a", 63)+".", 4),
 			},
-			wantErr: `hostname too long: 300 bytes (max 256)`,
+			wantErr: `"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" is too long to be a DNS name`,
+		},
+		{
+			name: "error_long_label",
+			args: upArgsT{
+				hostname: strings.Repeat("a", 64) + ".example.com",
+			},
+			wantErr: `"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" is not a valid DNS label`,
 		},
 		{
 			name: "error_linux_netfilter_empty",

cmd/tailscale/cli/up.go

@@ -34,6 +34,7 @@ import (
 	"tailscale.com/tailcfg"
 	"tailscale.com/types/logger"
 	"tailscale.com/types/preftype"
+	"tailscale.com/util/dnsname"
 	"tailscale.com/version"
 	"tailscale.com/version/distro"
 )
@@ -320,8 +321,8 @@ func prefsFromUpArgs(upArgs upArgsT, warnf logger.Logf, st *ipnstate.Status, goo
 		}
 	}
 
-	if len(upArgs.hostname) > 256 {
+	if err := dnsname.ValidHostname(upArgs.hostname); upArgs.hostname != "" && err != nil {
-		return nil, fmt.Errorf("hostname too long: %d bytes (max 256)", len(upArgs.hostname))
+		return nil, err
 	}
 
 	prefs := ipn.NewPrefs()

util/dnsname/dnsname.go

@@ -214,6 +214,21 @@ func FirstLabel(hostname string) string {
 	return first
 }
 
+// ValidHostname checks if a string is a valid hostname.
+func ValidHostname(hostname string) error {
+	fqdn, err := ToFQDN(hostname)
+	if err != nil {
+		return err
+	}
+
+	for _, label := range strings.Split(fqdn.WithoutTrailingDot(), ".") {
+		if err := ValidLabel(label); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
 var separators = map[byte]bool{
 	' ': true,
 	'.': true,

util/dnsname/dnsname_test.go

@@ -185,6 +185,31 @@ func TestTrimSuffix(t *testing.T) {
 	}
 }
 
+func TestValidHostname(t *testing.T) {
+	tests := []struct {
+		hostname string
+		wantErr  string
+	}{
+		{"example", ""},
+		{"example.com", ""},
+		{" example", `must start with a letter or number`},
+		{"example-.com", `must end with a letter or number`},
+		{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"},
+		{"www.what🤦lol.example.com", "contains invalid character"},
+	}
+
+	for _, test := range tests {
+		t.Run(test.hostname, func(t *testing.T) {
+			err := ValidHostname(test.hostname)
+			if (err == nil) != (test.wantErr == "") || (err != nil && !strings.Contains(err.Error(), test.wantErr)) {
+				t.Fatalf("ValidHostname(%s)=%v; expected %v", test.hostname, err, test.wantErr)
+			}
+		})
+	}
+}
+
 var sinkFQDN FQDN
 
 func BenchmarkToFQDN(b *testing.B) {