diff --git a/builtin/logical/pki/cert_util.go b/builtin/logical/pki/cert_util.go index 2a2c9b9b5..77b8c1f97 100644 --- a/builtin/logical/pki/cert_util.go +++ b/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) diff --git a/builtin/logical/pki/cert_util_test.go b/builtin/logical/pki/cert_util_test.go index d457e14da..102fb9487 100644 --- a/builtin/logical/pki/cert_util_test.go +++ b/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) - } } diff --git a/changelog/_10959.txt b/changelog/_10959.txt new file mode 100644 index 000000000..1f9740a7d --- /dev/null +++ b/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 +``` \ No newline at end of file