Make cert domain name validation case insensitive (#10959)

* make cert domain name validation case insensitive

* reafctor TestPki_PermitFQDNs mutliple cases

* TestPki_PermitFQDNS: fail uppercase alt_name

* add change log

* fix tests

* use EqualFold for potential utf-8 string comparison

Co-authored-by: Freyert <Freyert@users.noreply.github.com>

builtin/logical/pki/cert_util.go

@@ -181,7 +181,7 @@ func fetchCertBySerial(ctx context.Context, req *logical.Request, prefix, serial
 func validateNames(b *backend, data *inputBundle, names []string) string {
 	for _, name := range names {
 		sanitizedName := name
-		emailDomain := name
+		emailDomain := sanitizedName
 		isEmail := false
 		isWildcard := false
 
@@ -191,8 +191,8 @@ func validateNames(b *backend, data *inputBundle, names []string) string {
 		// ends up being problematic for users, I guess that could be separated
 		// into dns_names and email_names in the future to be explicit, but I
 		// don't think this is likely.
-		if strings.Contains(name, "@") {
-			splitEmail := strings.Split(name, "@")
+		if strings.Contains(sanitizedName, "@") {
+			splitEmail := strings.Split(sanitizedName, "@")
 			if len(splitEmail) != 2 {
 				return name
 			}
@@ -241,7 +241,7 @@ func validateNames(b *backend, data *inputBundle, names []string) string {
 		// 1) If a role allows a certain class of base (localhost, token
 		// display name, role-configured domains), perform further tests
 		//
-		// 2) If there is a perfect match on either the name itself or it's an
+		// 2) If there is a perfect match on either the sanitized name or it's an
 		// email address with a perfect match on the hostname portion, allow it
 		//
 		// 3) If subdomains are allowed, we check based on the sanitized name;
@@ -254,8 +254,8 @@ func validateNames(b *backend, data *inputBundle, names []string) string {
 		// Variances are noted in-line
 
 		if data.role.AllowLocalhost {
-			if name == "localhost" ||
-				name == "localdomain" ||
+			if sanitizedName == "localhost" ||
+				sanitizedName == "localdomain" ||
 				(isEmail && emailDomain == "localhost") ||
 				(isEmail && emailDomain == "localdomain") {
 				continue
@@ -329,15 +329,15 @@ func validateNames(b *backend, data *inputBundle, names []string) string {
 				// First, allow an exact match of the base domain if that role flag
 				// is enabled
 				if data.role.AllowBareDomains &&
-					(name == currDomain ||
-						(isEmail && emailDomain == currDomain)) {
+					(strings.EqualFold(sanitizedName, currDomain) ||
+						(isEmail && strings.EqualFold(emailDomain, currDomain))) {
 					valid = true
 					break
 				}
 
 				if data.role.AllowSubdomains {
 					if strings.HasSuffix(sanitizedName, "."+currDomain) ||
-						(isWildcard && sanitizedName == currDomain) {
+						(isWildcard && strings.EqualFold(sanitizedName, currDomain)) {
 						valid = true
 						break
 					}
@@ -345,7 +345,7 @@ func validateNames(b *backend, data *inputBundle, names []string) string {
 
 				if data.role.AllowGlobDomains &&
 					strings.Contains(currDomain, "*") &&
-					glob.Glob(currDomain, name) {
+					glob.Glob(currDomain, sanitizedName) {
 					valid = true
 					break
 				}
@@ -803,7 +803,7 @@ func generateCreationBundle(b *backend, data *inputBundle, caSign *certutil.CAIn
 		if csr == nil || !data.role.UseCSRSANs {
 			cnAltRaw, ok := data.apiData.GetOk("alt_names")
 			if ok {
-				cnAlt := strutil.ParseDedupLowercaseAndSortStrings(cnAltRaw.(string), ",")
+				cnAlt := strutil.ParseDedupAndSortStrings(cnAltRaw.(string), ",")
 				for _, v := range cnAlt {
 					if strings.Contains(v, "@") {
 						emailAddresses = append(emailAddresses, v)

builtin/logical/pki/cert_util_test.go

@@ -163,30 +163,58 @@ func TestPki_PermitFQDNs(t *testing.T) {
 	var b backend
 	fields := addCACommonFields(map[string]*framework.FieldSchema{})
 
-	apiData := &framework.FieldData{
-		Schema: fields,
-		Raw: map[string]interface{}{
-			"common_name": "example.com.",
-			"ttl":         3600,
+	cases := map[string]struct {
+		input    *inputBundle
+		expected []string
+	}{
+		"base valid case": {
+			input: &inputBundle{
+				apiData: &framework.FieldData{
+					Schema: fields,
+					Raw: map[string]interface{}{
+						"common_name": "example.com.",
+						"ttl":         3600,
+					},
+				},
+				role: &roleEntry{
+					AllowAnyName:     true,
+					MaxTTL:           3600,
+					EnforceHostnames: true,
+				},
+			},
+			expected: []string{"example.com."},
+		},
+		"case insensitivity validation": {
+			input: &inputBundle{
+				apiData: &framework.FieldData{
+					Schema: fields,
+					Raw: map[string]interface{}{
+						"common_name": "Example.Net",
+						"alt_names":   "eXaMPLe.COM",
+						"ttl":         3600,
+					},
+				},
+				role: &roleEntry{
+					AllowedDomains:   []string{"example.net", "EXAMPLE.COM"},
+					AllowBareDomains: true,
+					MaxTTL:           3600,
+				},
+			},
+			expected: []string{"Example.Net", "eXaMPLe.COM"},
 		},
 	}
-	input := &inputBundle{
-		apiData: apiData,
-		role: &roleEntry{
-			AllowAnyName:     true,
-			MaxTTL:           3600,
-			EnforceHostnames: true,
-		},
-	}
-	cb, err := generateCreationBundle(&b, input, nil, nil)
-	if err != nil {
-		t.Fatalf("Error: %v", err)
+
+	for _, testCase := range cases {
+		cb, err := generateCreationBundle(&b, testCase.input, nil, nil)
+		if err != nil {
+			t.Fatalf("Error: %v", err)
+		}
+
+		actual := cb.Params.DNSNames
+
+		if !reflect.DeepEqual(testCase.expected, actual) {
+			t.Fatalf("Expected %v, got %v", testCase.expected, actual)
+		}
 	}
 
-	expected := []string{"example.com."}
-	actual := cb.Params.DNSNames
-
-	if !reflect.DeepEqual(expected, actual) {
-		t.Fatalf("Expected %v, got %v", expected, actual)
-	}
 }

changelog/_10959.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+secret/pki: use case insensitive domain name comparison as per RFC1035 section 2.3.3
+```