dnsname,tailcfg: add hostname sanitation logic to node display names (#1304)

Signed-off-by: Sonia Appasamy <sonia@tailscale.com>

cmd/tailscale/cli/status.go

@@ -216,13 +216,11 @@ func peerActive(ps *ipnstate.PeerStatus) bool {
 }
 
 func dnsOrQuoteHostname(st *ipnstate.Status, ps *ipnstate.PeerStatus) string {
-	if i := strings.Index(ps.DNSName, "."); i != -1 && dnsname.HasSuffix(ps.DNSName, st.MagicDNSSuffix) {
+	baseName := dnsname.TrimSuffix(ps.DNSName, st.MagicDNSSuffix)
-		return ps.DNSName[:i]
+	if baseName != "" {
+		return baseName
 	}
-	if ps.DNSName != "" {
+	return fmt.Sprintf("(%q)", dnsname.SanitizeHostname(ps.HostName))
-		return strings.TrimRight(ps.DNSName, ".")
-	}
-	return fmt.Sprintf("(%q)", strings.ReplaceAll(ps.SimpleHostName(), " ", "_"))
 }
 
 func ownerLogin(st *ipnstate.Status, ps *ipnstate.PeerStatus) string {

ipn/ipnstate/ipnstate.go

@@ -98,14 +98,6 @@ type PeerStatus struct {
 	InEngine bool
 }
 
-// SimpleHostName returns a potentially simplified version of ps.HostName for display purposes.
-func (ps *PeerStatus) SimpleHostName() string {
-	n := ps.HostName
-	n = strings.TrimSuffix(n, ".local")
-	n = strings.TrimSuffix(n, ".localdomain")
-	return n
-}
-
 type StatusBuilder struct {
 	mu     sync.Mutex
 	locked bool
@@ -323,11 +315,8 @@ table tbody tr:nth-child(even) td { background-color: #f5f5f5; }
 			}
 		}
 
-		hostName := ps.SimpleHostName()
+		hostName := dnsname.SanitizeHostname(ps.HostName)
-		dnsName := strings.TrimRight(ps.DNSName, ".")
+		dnsName := dnsname.TrimSuffix(ps.DNSName, st.MagicDNSSuffix)
-		if i := strings.Index(dnsName, "."); i != -1 && dnsname.HasSuffix(dnsName, st.MagicDNSSuffix) {
-			dnsName = dnsName[:i]
-		}
 		if strings.EqualFold(dnsName, hostName) || ps.UserID != st.Self.UserID {
 			hostName = ""
 		}

tailcfg/tailcfg.go

@@ -243,16 +243,8 @@ func (n *Node) DisplayNames(forOwner bool) (name, hostIfDifferent string) {
 // fields: n.ComputedName, n.computedHostIfDifferent, and
 // n.ComputedNameWithHost.
 func (n *Node) InitDisplayNames(networkMagicDNSSuffix string) {
-	dnsName := n.Name
+	name := dnsname.TrimSuffix(n.Name, networkMagicDNSSuffix)
-	if dnsName != "" {
+	hostIfDifferent := dnsname.SanitizeHostname(n.Hostinfo.Hostname)
-		dnsName = strings.TrimRight(dnsName, ".")
-		if i := strings.Index(dnsName, "."); i != -1 && dnsname.HasSuffix(dnsName, networkMagicDNSSuffix) {
-			dnsName = dnsName[:i]
-		}
-	}
-
-	name := dnsName
-	hostIfDifferent := n.Hostinfo.Hostname
 
 	if strings.EqualFold(name, hostIfDifferent) {
 		hostIfDifferent = ""

util/dnsname/dnsname.go

@@ -7,13 +7,120 @@ package dnsname
 
 import "strings"
 
-// HasSuffix reports whether the provided DNS name ends with the
+var separators = map[byte]bool{
-// component(s) in suffix, ignoring any trailing dots.
+	' ': true,
+	'.': true,
+	'@': true,
+	'_': true,
+}
+
+func islower(c byte) bool {
+	return 'a' <= c && c <= 'z'
+}
+
+func isupper(c byte) bool {
+	return 'A' <= c && c <= 'Z'
+}
+
+func isalpha(c byte) bool {
+	return islower(c) || isupper(c)
+}
+
+func isalphanum(c byte) bool {
+	return isalpha(c) || ('0' <= c && c <= '9')
+}
+
+func isdnschar(c byte) bool {
+	return isalphanum(c) || c == '-'
+}
+
+func tolower(c byte) byte {
+	if isupper(c) {
+		return c + 'a' - 'A'
+	} else {
+		return c
+	}
+}
+
+// maxLabelLength is the maximal length of a label permitted by RFC 1035.
+const maxLabelLength = 63
+
+// SanitizeLabel takes a string intended to be a DNS name label
+// and turns it into a valid name label according to RFC 1035.
+func SanitizeLabel(label string) string {
+	var sb strings.Builder // TODO: don't allocate in common case where label is already fine
+	start, end := 0, len(label)
+
+	// This is technically stricter than necessary as some characters may be dropped,
+	// but labels have no business being anywhere near this long in any case.
+	if end > maxLabelLength {
+		end = maxLabelLength
+	}
+
+	// A label must start with a letter or number...
+	for ; start < end; start++ {
+		if isalphanum(label[start]) {
+			break
+		}
+	}
+
+	// ...and end with a letter or number.
+	for ; start < end; end-- {
+		// This is safe because (start < end) implies (end >= 1).
+		if isalphanum(label[end-1]) {
+			break
+		}
+	}
+
+	for i := start; i < end; i++ {
+		// Consume a separator only if we are not at a boundary:
+		// then we can turn it into a hyphen without breaking the rules.
+		boundary := (i == start) || (i == end-1)
+		if !boundary && separators[label[i]] {
+			sb.WriteByte('-')
+		} else if isdnschar(label[i]) {
+			sb.WriteByte(tolower(label[i]))
+		}
+	}
+
+	return sb.String()
+}
+
+// HasSuffix reports whether the provided name ends with the
+// component(s) in suffix, ignoring any trailing or leading dots.
 //
 // If suffix is the empty string, HasSuffix always reports false.
 func HasSuffix(name, suffix string) bool {
 	name = strings.TrimSuffix(name, ".")
 	suffix = strings.TrimSuffix(suffix, ".")
+	suffix = strings.TrimPrefix(suffix, ".")
 	nameBase := strings.TrimSuffix(name, suffix)
 	return len(nameBase) < len(name) && strings.HasSuffix(nameBase, ".")
 }
+
+// TrimSuffix trims any trailing dots from a name and removes the
+// suffix ending if present. The name will never be returned with
+// a trailing dot, even after trimming.
+func TrimSuffix(name, suffix string) string {
+	if HasSuffix(name, suffix) {
+		name = strings.TrimSuffix(name, ".")
+		suffix = strings.Trim(suffix, ".")
+		name = strings.TrimSuffix(name, suffix)
+	}
+	return strings.TrimSuffix(name, ".")
+}
+
+// TrimCommonSuffixes returns hostname with some common suffixes removed.
+func TrimCommonSuffixes(hostname string) string {
+	hostname = strings.TrimSuffix(hostname, ".local")
+	hostname = strings.TrimSuffix(hostname, ".localdomain")
+	hostname = strings.TrimSuffix(hostname, ".lan")
+	return hostname
+}
+
+// SanitizeHostname turns hostname into a valid name label according
+// to RFC 1035.
+func SanitizeHostname(hostname string) string {
+	hostname = TrimCommonSuffixes(hostname)
+	return SanitizeLabel(hostname)
+}

util/dnsname/dnsname_test.go

@@ -4,7 +4,60 @@
 
 package dnsname
 
-import "testing"
+import (
+	"strings"
+	"testing"
+)
+
+func TestSanitizeLabel(t *testing.T) {
+	tests := []struct {
+		name string
+		in   string
+		want string
+	}{
+		{"empty", "", ""},
+		{"space", " ", ""},
+		{"upper", "OBERON", "oberon"},
+		{"mixed", "Avery's iPhone 4(SE)", "averys-iphone-4se"},
+		{"dotted", "mon.ipn.dev", "mon-ipn-dev"},
+		{"email", "admin@example.com", "admin-example-com"},
+		{"boudary", ".bound.ary.", "bound-ary"},
+		{"bad_trailing", "a-", "a"},
+		{"bad_leading", "-a", "a"},
+		{"bad_both", "-a-", "a"},
+		{
+			"overlong",
+			strings.Repeat("test.", 20),
+			"test-test-test-test-test-test-test-test-test-test-test-test-tes",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := SanitizeLabel(tt.in)
+			if got != tt.want {
+				t.Errorf("want %q; got %q", tt.want, got)
+			}
+		})
+	}
+}
+
+func TestTrimCommonSuffixes(t *testing.T) {
+	tests := []struct {
+		hostname string
+		want     string
+	}{
+		{"computer.local", "computer"},
+		{"computer.localdomain", "computer"},
+		{"computer.lan", "computer"},
+		{"computer.mynetwork", "computer.mynetwork"},
+	}
+	for _, tt := range tests {
+		got := TrimCommonSuffixes(tt.hostname)
+		if got != tt.want {
+			t.Errorf("TrimCommonSuffixes(%q) = %q; want %q", tt.hostname, got, tt.want)
+		}
+	}
+}
 
 func TestHasSuffix(t *testing.T) {
 	tests := []struct {
@@ -14,6 +67,7 @@ func TestHasSuffix(t *testing.T) {
 		{"foo.com", "com", true},
 		{"foo.com.", "com", true},
 		{"foo.com.", "com.", true},
+		{"foo.com", ".com", true},
 
 		{"", "", false},
 		{"foo.com.", "", false},
@@ -26,3 +80,25 @@ func TestHasSuffix(t *testing.T) {
 		}
 	}
 }
+
+func TestTrimSuffix(t *testing.T) {
+	tests := []struct {
+		name   string
+		suffix string
+		want   string
+	}{
+		{"foo.magicdnssuffix.", "magicdnssuffix", "foo"},
+		{"foo.magicdnssuffix", "magicdnssuffix", "foo"},
+		{"foo.magicdnssuffix", ".magicdnssuffix", "foo"},
+		{"foo.anothersuffix", "magicdnssuffix", "foo.anothersuffix"},
+		{"foo.anothersuffix.", "magicdnssuffix", "foo.anothersuffix"},
+		{"a.b.c.d", "c.d", "a.b"},
+		{"name.", "foo", "name"},
+	}
+	for _, tt := range tests {
+		got := TrimSuffix(tt.name, tt.suffix)
+		if got != tt.want {
+			t.Errorf("TrimSuffix(%q, %q) = %q; want %q", tt.name, tt.suffix, got, tt.want)
+		}
+	}
+}