From 76fb27bea72c472bc21247c9d3a14ee3ef544247 Mon Sep 17 00:00:00 2001 From: Sonia Appasamy Date: Thu, 18 Feb 2021 17:15:38 -0500 Subject: [PATCH] dnsname,tailcfg: add hostname sanitation logic to node display names (#1304) Signed-off-by: Sonia Appasamy --- cmd/tailscale/cli/status.go | 10 ++-- ipn/ipnstate/ipnstate.go | 15 +---- tailcfg/tailcfg.go | 12 +--- util/dnsname/dnsname.go | 111 ++++++++++++++++++++++++++++++++++- util/dnsname/dnsname_test.go | 78 +++++++++++++++++++++++- 5 files changed, 194 insertions(+), 32 deletions(-) diff --git a/cmd/tailscale/cli/status.go b/cmd/tailscale/cli/status.go index dde138fa2..74ae8d1dd 100644 --- a/cmd/tailscale/cli/status.go +++ b/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) { - return ps.DNSName[:i] + baseName := dnsname.TrimSuffix(ps.DNSName, st.MagicDNSSuffix) + if baseName != "" { + return baseName } - if ps.DNSName != "" { - return strings.TrimRight(ps.DNSName, ".") - } - return fmt.Sprintf("(%q)", strings.ReplaceAll(ps.SimpleHostName(), " ", "_")) + return fmt.Sprintf("(%q)", dnsname.SanitizeHostname(ps.HostName)) } func ownerLogin(st *ipnstate.Status, ps *ipnstate.PeerStatus) string { diff --git a/ipn/ipnstate/ipnstate.go b/ipn/ipnstate/ipnstate.go index 6d713bc78..c1232f335 100644 --- a/ipn/ipnstate/ipnstate.go +++ b/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() - dnsName := strings.TrimRight(ps.DNSName, ".") - if i := strings.Index(dnsName, "."); i != -1 && dnsname.HasSuffix(dnsName, st.MagicDNSSuffix) { - dnsName = dnsName[:i] - } + hostName := dnsname.SanitizeHostname(ps.HostName) + dnsName := dnsname.TrimSuffix(ps.DNSName, st.MagicDNSSuffix) if strings.EqualFold(dnsName, hostName) || ps.UserID != st.Self.UserID { hostName = "" } diff --git a/tailcfg/tailcfg.go b/tailcfg/tailcfg.go index 61a63c7b4..6ad7b0c19 100644 --- a/tailcfg/tailcfg.go +++ b/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 - if dnsName != "" { - dnsName = strings.TrimRight(dnsName, ".") - if i := strings.Index(dnsName, "."); i != -1 && dnsname.HasSuffix(dnsName, networkMagicDNSSuffix) { - dnsName = dnsName[:i] - } - } - - name := dnsName - hostIfDifferent := n.Hostinfo.Hostname + name := dnsname.TrimSuffix(n.Name, networkMagicDNSSuffix) + hostIfDifferent := dnsname.SanitizeHostname(n.Hostinfo.Hostname) if strings.EqualFold(name, hostIfDifferent) { hostIfDifferent = "" diff --git a/util/dnsname/dnsname.go b/util/dnsname/dnsname.go index 1488272a4..db9f8b0ff 100644 --- a/util/dnsname/dnsname.go +++ b/util/dnsname/dnsname.go @@ -7,13 +7,120 @@ package dnsname import "strings" -// HasSuffix reports whether the provided DNS name ends with the -// component(s) in suffix, ignoring any trailing dots. +var separators = map[byte]bool{ + ' ': 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) +} diff --git a/util/dnsname/dnsname_test.go b/util/dnsname/dnsname_test.go index da4e51384..166040de0 100644 --- a/util/dnsname/dnsname_test.go +++ b/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) + } + } +}