Add a test for forbiddenLabel and make it stricter.

registration-authority.go

@@ -11,6 +11,7 @@ import (
 	"fmt"
 	"github.com/bifurcation/gose"
 	"regexp"
+	"strings"
 )
 
 // All of the fields in RegistrationAuthorityImpl need to be
@@ -26,17 +27,42 @@ func NewRegistrationAuthorityImpl() RegistrationAuthorityImpl {
 }
 
 func forbiddenIdentifier(id string) bool {
-	// XXX Flesh this out, and add real policy.  Only rough checks for now
-
-	// If it contains characters not allowed in a domain name ...
-	match, err := regexp.MatchString("[^a-zA-Z0-9.-]", id)
-	if (err != nil) || match {
+	// A DNS label is a part separated by dots, e.g. www.foo.net has labels
+	// "www", "foo", and "net".
+	const maxLabels = 10
+	labels := strings.SplitN(id, ".", maxLabels + 1)
+	if len(labels) < 2 || len(labels) > maxLabels {
 		return true
 	}
 
-	// If it is entirely numeric (like an IP address) ...
-	match, err = regexp.MatchString("[^0-9.]", id)
-	if (err != nil) || !match {
+	for _, label := range labels {
+		// DNS defines max label length as 63 characters. Some implementations allow
+		// more, but we will be conservative.
+		if len(label) < 1 || len(label) > 63 {
+			return true
+		}
+		// Only alphanumerics and dash are allowed in identifiers.
+		// TODO: Before identifiers reach this function, do lowercasing.
+		matched, err := regexp.MatchString("[^a-zA-Z0-9-]", id)
+		if (err != nil) || matched {
+			return true
+		}
+
+		// A label cannot begin with a hyphen (-)
+		if label[0] == '-' {
+			return true
+		}
+
+		// Punycode labels are not yet allowed. May allow in future after looking at
+		// homoglyph mitigations.
+		if label[0:4] == "xn--" {
+			return true
+		}
+	}
+
+	// Forbid identifiers that are entirely numeric like an IP address.
+	matched, err := regexp.MatchString("[^0-9.]", id)
+	if err != nil || !matched {
 		return true
 	}
 

registration-authority_test.go

@@ -0,0 +1,77 @@
+// Copyright 2014 ISRG.  All rights reserved
+// This Source Code Form is subject to the terms of the Mozilla Public
+// License, v. 2.0. If a copy of the MPL was not distributed with this
+// file, You can obtain one at http://mozilla.org/MPL/2.0/.
+
+package boulder
+
+import (
+  "testing"
+)
+
+func TestForbiddenIdentifier(t *testing.T) {
+  shouldBeAccepted := []string{
+    "www.zombo.com",
+    "zombo.com",
+    "www.163.com", // Technically disallowed (all-numeric label) but actually common.
+    "163.com",
+    "zom-bo.com",
+    "zombo-.com",
+    "www.zom-bo.com",
+    "www.zombo-.com",
+  }
+  shouldBeForbidden := []string{
+    "127.0.0.1",
+    "10.0.0.10",
+    "192.168.1.1",
+    "123.45.78.12",
+    "",
+    "0",
+    "1",
+    "*",
+    "**",
+    "*.*",
+    "zombo*com",
+    "*.com",
+    "*.zombo.com",
+    ".",
+    "..",
+    "a..",
+    "..a",
+    ".a.",
+    ".....",
+    "www.zombo_com.com",
+    "\uFEFF", // Byte order mark
+    "\uFEFFwww.zombo.com",
+    "www.zömbo.com", // No non-ASCII for now.
+    "xn--hmr.net", // No punycode for now.
+    "www.xn--hmr.net",
+    "www.zom\u202Ebo.com", // Right-to-Left Override
+    "\u202Ewww.zombo.com",
+    "www.zom\u200Fbo.com", // Right-to-Left Mark
+    "\u200Fwww.zombo.com",
+    // 6 * 26 characters = too long for DNS label (max 63).
+    "www.abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyz.com",
+    // Labels can't start with dash.
+    "www.-ombo.com",
+    // Underscores are technically disallowed in DNS. Some DNS
+    // implementations accept them but we will be conservative.
+    "www.zom_bo.com",
+    // All-numeric final label not okay.
+    "www.zombo.163",
+    "zombocom",
+    "a.b.c.d.e.f.g.h.i.j.k", // Too many DNS labels
+  }
+
+  for _, identifier := range shouldBeForbidden {
+    if ! forbiddenIdentifier(identifier) {
+      t.Error("Identifier was not correctly forbidden: ", identifier)
+    }
+  }
+
+  for _, identifier := range shouldBeAccepted {
+    if ! forbiddenIdentifier(identifier) {
+      t.Error("Identifier was incorrectly forbidden: ", identifier)
+    }
+  }
+}