From 2a933c1903437e6689bae415a90a4fbde3e86679 Mon Sep 17 00:00:00 2001
From: Anton Tolchanov <1687799+knyar@users.noreply.github.com>
Date: Mon, 27 Mar 2023 18:21:58 +0100
Subject: [PATCH] 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 | 11 +++++++++--
 cmd/tailscale/cli/up.go       |  5 +++--
 util/dnsname/dnsname.go       | 15 +++++++++++++++
 util/dnsname/dnsname_test.go  | 25 +++++++++++++++++++++++++
 4 files changed, 52 insertions(+), 4 deletions(-)

diff --git a/cmd/tailscale/cli/cli_test.go b/cmd/tailscale/cli/cli_test.go
index 5a495f9d5..309e7ae03 100644
--- a/cmd/tailscale/cli/cli_test.go
+++ b/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",
diff --git a/cmd/tailscale/cli/up.go b/cmd/tailscale/cli/up.go
index 2e0b43e45..92241748b 100644
--- a/cmd/tailscale/cli/up.go
+++ b/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 {
-		return nil, fmt.Errorf("hostname too long: %d bytes (max 256)", len(upArgs.hostname))
+	if err := dnsname.ValidHostname(upArgs.hostname); upArgs.hostname != "" && err != nil {
+		return nil, err
 	}
 
 	prefs := ipn.NewPrefs()
diff --git a/util/dnsname/dnsname.go b/util/dnsname/dnsname.go
index ba5ed2a5a..6481a5867 100644
--- a/util/dnsname/dnsname.go
+++ b/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,
diff --git a/util/dnsname/dnsname_test.go b/util/dnsname/dnsname_test.go
index d8ec4b810..563959d33 100644
--- a/util/dnsname/dnsname_test.go
+++ b/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) {