From caaefa00a0605834279bc01ec43b9e48e8588d72 Mon Sep 17 00:00:00 2001
From: David Anderson <danderson@tailscale.com>
Date: Mon, 31 May 2021 19:31:29 -0700
Subject: [PATCH] util/dnsname: don't validate the contents of DNS labels.

DNS names consist of labels, but outside of length limits, DNS
itself permits any content within the labels. Some records require
labels to conform to hostname limitations (which is what we implemented
before), but not all.

Fixes #2024.

Signed-off-by: David Anderson <danderson@tailscale.com>
---
 util/dnsname/dnsname.go      | 40 +++++++++++++++---------------------
 util/dnsname/dnsname_test.go |  1 +
 2 files changed, 17 insertions(+), 24 deletions(-)

diff --git a/util/dnsname/dnsname.go b/util/dnsname/dnsname.go
index 905e90f5f..6ed85c84f 100644
--- a/util/dnsname/dnsname.go
+++ b/util/dnsname/dnsname.go
@@ -48,21 +48,6 @@ func ToFQDN(s string) (FQDN, error) {
 	return FQDN(s + "."), nil
 }
 
-func validLabel(s string) bool {
-	if len(s) == 0 || len(s) > maxLabelLength {
-		return false
-	}
-	if !isalphanum(s[0]) || !isalphanum(s[len(s)-1]) {
-		return false
-	}
-	for i := 1; i < len(s)-1; i++ {
-		if !isalphanum(s[i]) && s[i] != '-' {
-			return false
-		}
-	}
-	return true
-}
-
 // WithTrailingDot returns f as a string, with a trailing dot.
 func (f FQDN) WithTrailingDot() string {
 	return string(f)
@@ -120,23 +105,30 @@ func isValidFQDN(s string) bool {
 			continue
 		}
 		label := s[st:i]
-		if len(label) == 0 || len(label) > maxLabelLength {
-			return false
-		}
-		if !isalphanum(label[0]) || !isalphanum(label[len(label)-1]) {
+		if !validLabel(label) {
 			return false
 		}
-		for j := 1; j < len(label)-1; j++ {
-			if !isalphanum(label[j]) && label[j] != '-' {
-				return false
-			}
-		}
 		st = i + 1
 	}
 
 	return true
 }
 
+func validLabel(s string) bool {
+	// You might be tempted to do further validation of the
+	// contents of labels here, based on the hostname rules in RFC
+	// 1123. However, DNS labels are not always subject to
+	// hostname rules. In general, they can contain any non-zero
+	// byte sequence, even though in practice a more restricted
+	// set is used.
+	//
+	// See https://github.com/tailscale/tailscale/issues/2024 for more.
+	if len(s) == 0 || len(s) > maxLabelLength {
+		return false
+	}
+	return true
+}
+
 // 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 {
diff --git a/util/dnsname/dnsname_test.go b/util/dnsname/dnsname_test.go
index df3ea30b6..4867df2ac 100644
--- a/util/dnsname/dnsname_test.go
+++ b/util/dnsname/dnsname_test.go
@@ -24,6 +24,7 @@ func TestFQDN(t *testing.T) {
 		{".foo.com", "foo.com.", false, 2},
 		{"com", "com.", false, 1},
 		{"www.tailscale.com", "www.tailscale.com.", false, 3},
+		{"_ssh._tcp.tailscale.com", "_ssh._tcp.tailscale.com.", false, 4},
 		{"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.com", "", true, 0},
 		{strings.Repeat("aaaaa.", 60) + "com", "", true, 0},
 		{"foo..com", "", true, 0},