diff --git a/builtin/logical/pki/backend_test.go b/builtin/logical/pki/backend_test.go index 550b1dbf5025..9b269f8c3071 100644 --- a/builtin/logical/pki/backend_test.go +++ b/builtin/logical/pki/backend_test.go @@ -692,12 +692,15 @@ func generateCSRSteps(t *testing.T, caCert, caKey string, intdata, reqdata map[s // Generates steps to test out various role permutations func generateRoleSteps(t *testing.T, useCSRs bool) []logicaltest.TestStep { roleVals := roleEntry{ - MaxTTL: 12 * time.Hour, - KeyType: "rsa", - KeyBits: 2048, - RequireCN: true, - SignatureBits: 256, + MaxTTL: 12 * time.Hour, + KeyType: "rsa", + KeyBits: 2048, + RequireCN: true, + SignatureBits: 256, + AllowWildcardCertificates: new(bool), } + *roleVals.AllowWildcardCertificates = true + issueVals := certutil.IssueData{} ret := []logicaltest.TestStep{} @@ -3294,13 +3297,14 @@ func (o MultiBool) ToValues() []bool { } type IssuanceRegression struct { - AllowedDomains []string - AllowBareDomains MultiBool - AllowGlobDomains MultiBool - AllowSubdomains MultiBool - AllowLocalhost MultiBool - CommonName string - Issued bool + AllowedDomains []string + AllowBareDomains MultiBool + AllowGlobDomains MultiBool + AllowSubdomains MultiBool + AllowLocalhost MultiBool + AllowWildcardCertificates MultiBool + CommonName string + Issued bool } func RoleIssuanceRegressionHelper(t *testing.T, client *api.Client, index int, test IssuanceRegression) int { @@ -3309,36 +3313,38 @@ func RoleIssuanceRegressionHelper(t *testing.T, client *api.Client, index int, t for _, AllowGlobDomains := range test.AllowGlobDomains.ToValues() { for _, AllowSubdomains := range test.AllowSubdomains.ToValues() { for _, AllowLocalhost := range test.AllowLocalhost.ToValues() { + for _, AllowWildcardCertificates := range test.AllowWildcardCertificates.ToValues() { + role := fmt.Sprintf("issuance-regression-%d-bare-%v-glob-%v-subdomains-%v-localhost-%v-wildcard-%v", index, AllowBareDomains, AllowGlobDomains, AllowSubdomains, AllowLocalhost, AllowWildcardCertificates) + resp, err := client.Logical().Write("pki/roles/"+role, map[string]interface{}{ + "allowed_domains": test.AllowedDomains, + "allow_bare_domains": AllowBareDomains, + "allow_glob_domains": AllowGlobDomains, + "allow_subdomains": AllowSubdomains, + "allow_localhost": AllowLocalhost, + "allow_wildcard_certificates": AllowWildcardCertificates, + // TODO: test across this vector as well. Currently certain wildcard + // matching is broken with it enabled (such as x*x.foo). + "enforce_hostnames": false, + "key_type": "ec", + "key_bits": 256, + }) + if err != nil { + t.Fatal(err) + } - role := fmt.Sprintf("issuance-regression-%d-bare-%v-glob-%v-subdomains-%v-localhost-%v", index, AllowBareDomains, AllowGlobDomains, AllowSubdomains, AllowLocalhost) - resp, err := client.Logical().Write("pki/roles/"+role, map[string]interface{}{ - "allowed_domains": test.AllowedDomains, - "allow_bare_domains": AllowBareDomains, - "allow_glob_domains": AllowGlobDomains, - "allow_subdomains": AllowSubdomains, - "allow_localhost": AllowLocalhost, - // TODO: test across this vector as well. Currently certain wildcard - // matching is broken with it enabled (such as x*x.foo). - "enforce_hostnames": false, - "key_type": "ec", - "key_bits": 256, - }) - if err != nil { - t.Fatal(err) - } + resp, err = client.Logical().Write("pki/issue/"+role, map[string]interface{}{ + "common_name": test.CommonName, + }) - resp, err = client.Logical().Write("pki/issue/"+role, map[string]interface{}{ - "common_name": test.CommonName, - }) + haveErr := err != nil || resp == nil + expectErr := !test.Issued - haveErr := err != nil || resp == nil - expectErr := !test.Issued + if haveErr != expectErr { + t.Fatalf("issuance regression test [%d] failed: haveErr: %v, expectErr: %v, err: %v, resp: %v, test case: %v, role: %v", index, haveErr, expectErr, err, resp, test, role) + } - if haveErr != expectErr { - t.Fatalf("issuance regression test [%d] failed: haveErr: %v, expectErr: %v, err: %v, resp: %v, test case: %v, role: %v", index, haveErr, expectErr, err, resp, test, role) + tested += 1 } - - tested += 1 } } } @@ -3350,144 +3356,155 @@ func RoleIssuanceRegressionHelper(t *testing.T, client *api.Client, index int, t func TestBackend_Roles_IssuanceRegression(t *testing.T) { // Regression testing of role's issuance policy. testCases := []IssuanceRegression{ - // allowed, bare, glob, subdomains, localhost, cn, issued + // allowed, bare, glob, subdomains, localhost, wildcards, cn, issued + // === Globs not allowed but used === // // Allowed contains globs, but globbing not allowed, resulting in all // issuances failing. Note that tests against issuing a wildcard with // a bare domain will be covered later. - /* 0 */ {[]string{"*.*.foo"}, MAny, MFalse, MAny, MAny, "baz.fud.bar.foo", false}, - /* 1 */ {[]string{"*.*.foo"}, MAny, MFalse, MAny, MAny, "*.fud.bar.foo", false}, - /* 2 */ {[]string{"*.*.foo"}, MAny, MFalse, MAny, MAny, "fud.bar.foo", false}, - /* 3 */ {[]string{"*.*.foo"}, MAny, MFalse, MAny, MAny, "*.bar.foo", false}, - /* 4 */ {[]string{"*.*.foo"}, MAny, MFalse, MAny, MAny, "bar.foo", false}, - /* 5 */ {[]string{"*.*.foo"}, MAny, MFalse, MAny, MAny, "*.foo", false}, - /* 6 */ {[]string{"*.foo"}, MAny, MFalse, MAny, MAny, "foo", false}, - /* 7 */ {[]string{"*.foo"}, MAny, MFalse, MAny, MAny, "baz.fud.bar.foo", false}, - /* 8 */ {[]string{"*.foo"}, MAny, MFalse, MAny, MAny, "*.fud.bar.foo", false}, - /* 9 */ {[]string{"*.foo"}, MAny, MFalse, MAny, MAny, "fud.bar.foo", false}, - /* 10 */ {[]string{"*.foo"}, MAny, MFalse, MAny, MAny, "*.bar.foo", false}, - /* 11 */ {[]string{"*.foo"}, MAny, MFalse, MAny, MAny, "bar.foo", false}, - /* 12 */ {[]string{"*.foo"}, MAny, MFalse, MAny, MAny, "foo", false}, - + /* 0 */ {[]string{"*.*.foo"}, MAny, MFalse, MAny, MAny, MAny, "baz.fud.bar.foo", false}, + /* 1 */ {[]string{"*.*.foo"}, MAny, MFalse, MAny, MAny, MAny, "*.fud.bar.foo", false}, + /* 2 */ {[]string{"*.*.foo"}, MAny, MFalse, MAny, MAny, MAny, "fud.bar.foo", false}, + /* 3 */ {[]string{"*.*.foo"}, MAny, MFalse, MAny, MAny, MAny, "*.bar.foo", false}, + /* 4 */ {[]string{"*.*.foo"}, MAny, MFalse, MAny, MAny, MAny, "bar.foo", false}, + /* 5 */ {[]string{"*.*.foo"}, MAny, MFalse, MAny, MAny, MAny, "*.foo", false}, + /* 6 */ {[]string{"*.foo"}, MAny, MFalse, MAny, MAny, MAny, "foo", false}, + /* 7 */ {[]string{"*.foo"}, MAny, MFalse, MAny, MAny, MAny, "baz.fud.bar.foo", false}, + /* 8 */ {[]string{"*.foo"}, MAny, MFalse, MAny, MAny, MAny, "*.fud.bar.foo", false}, + /* 9 */ {[]string{"*.foo"}, MAny, MFalse, MAny, MAny, MAny, "fud.bar.foo", false}, + /* 10 */ {[]string{"*.foo"}, MAny, MFalse, MAny, MAny, MAny, "*.bar.foo", false}, + /* 11 */ {[]string{"*.foo"}, MAny, MFalse, MAny, MAny, MAny, "bar.foo", false}, + /* 12 */ {[]string{"*.foo"}, MAny, MFalse, MAny, MAny, MAny, "foo", false}, + + // === Localhost sanity === // // Localhost forbidden, not matching allowed domains -> not issued - /* 13 */ {[]string{"*.*.foo"}, MAny, MAny, MAny, MFalse, "localhost", false}, + /* 13 */ {[]string{"*.*.foo"}, MAny, MAny, MAny, MFalse, MAny, "localhost", false}, // Localhost allowed, not matching allowed domains -> issued - /* 14 */ {[]string{"*.*.foo"}, MAny, MAny, MAny, MTrue, "localhost", true}, + /* 14 */ {[]string{"*.*.foo"}, MAny, MAny, MAny, MTrue, MAny, "localhost", true}, // Localhost allowed via allowed domains (and bare allowed), not by AllowLocalhost -> issued - /* 15 */ {[]string{"localhost"}, MTrue, MAny, MAny, MFalse, "localhost", true}, + /* 15 */ {[]string{"localhost"}, MTrue, MAny, MAny, MFalse, MAny, "localhost", true}, // Localhost allowed via allowed domains (and bare not allowed), not by AllowLocalhost -> not issued - /* 16 */ {[]string{"localhost"}, MFalse, MAny, MAny, MFalse, "localhost", false}, - // Localhost allowed via allowed domains, and by AllowLocalhost -> issued - /* 17 */ {[]string{"localhost"}, MAny, MAny, MAny, MTrue, "localhost", true}, + /* 16 */ {[]string{"localhost"}, MFalse, MAny, MAny, MFalse, MAny, "localhost", false}, + // Localhost allowed via allowed domains (but bare not allowed), and by AllowLocalhost -> issued + /* 17 */ {[]string{"localhost"}, MFalse, MAny, MAny, MTrue, MAny, "localhost", true}, + // === Bare wildcard issuance == // // allowed_domains contains one or more wildcards and bare domains allowed, // resulting in the cert being issued. - /* 18 */ {[]string{"*.foo"}, MTrue, MAny, MAny, MAny, "*.foo", true}, - /* 19 */ {[]string{"*.*.foo"}, MTrue, MAny, MAny, MAny, "*.*.foo", true}, + /* 18 */ {[]string{"*.foo"}, MTrue, MAny, MAny, MAny, MTrue, "*.foo", true}, + /* 19 */ {[]string{"*.*.foo"}, MTrue, MAny, MAny, MAny, MAny, "*.*.foo", false}, // Does not conform to RFC 6125 // === Double Leading Glob Testing === // - // Allowed contains globs, but glob allowed so certain matches work. // The value of bare and localhost does not impact these results. - /* 20 */ {[]string{"*.*.foo"}, MAny, MTrue, MFalse, MAny, "baz.fud.bar.foo", true}, // glob domains allow infinite subdomains - /* 21 */ {[]string{"*.*.foo"}, MAny, MTrue, MFalse, MAny, "*.fud.bar.foo", true}, // ???? does glob domain allow wildcard of subdomains? - /* 22 */ {[]string{"*.*.foo"}, MAny, MTrue, MFalse, MAny, "fud.bar.foo", true}, - /* 23 */ {[]string{"*.*.foo"}, MAny, MTrue, MFalse, MAny, "*.bar.foo", true}, // Regression fix: Vault#13530 - /* 24 */ {[]string{"*.*.foo"}, MAny, MTrue, MFalse, MAny, "bar.foo", false}, - /* 25 */ {[]string{"*.*.foo"}, MAny, MTrue, MFalse, MAny, "*.foo", false}, - /* 26 */ {[]string{"*.*.foo"}, MAny, MTrue, MFalse, MAny, "foo", false}, + /* 20 */ {[]string{"*.*.foo"}, MAny, MTrue, MFalse, MAny, MAny, "baz.fud.bar.foo", true}, // glob domains allow infinite subdomains + /* 21 */ {[]string{"*.*.foo"}, MAny, MTrue, MFalse, MAny, MTrue, "*.fud.bar.foo", true}, // glob domain allows wildcard of subdomains + /* 22 */ {[]string{"*.*.foo"}, MAny, MTrue, MFalse, MAny, MAny, "fud.bar.foo", true}, + /* 23 */ {[]string{"*.*.foo"}, MAny, MTrue, MFalse, MAny, MTrue, "*.bar.foo", true}, // Regression fix: Vault#13530 + /* 24 */ {[]string{"*.*.foo"}, MAny, MTrue, MFalse, MAny, MAny, "bar.foo", false}, + /* 25 */ {[]string{"*.*.foo"}, MAny, MTrue, MFalse, MAny, MAny, "*.foo", false}, + /* 26 */ {[]string{"*.*.foo"}, MAny, MTrue, MFalse, MAny, MAny, "foo", false}, // Allowed contains globs, but glob and subdomain both work, so we expect // wildcard issuance to work as well. The value of bare and localhost does // not impact these results. - /* 27 */ {[]string{"*.*.foo"}, MAny, MTrue, MTrue, MAny, "baz.fud.bar.foo", true}, - /* 28 */ {[]string{"*.*.foo"}, MAny, MTrue, MTrue, MAny, "*.fud.bar.foo", true}, - /* 29 */ {[]string{"*.*.foo"}, MAny, MTrue, MTrue, MAny, "fud.bar.foo", true}, - /* 30 */ {[]string{"*.*.foo"}, MAny, MTrue, MTrue, MAny, "*.bar.foo", true}, // Regression fix: Vault#13530 - /* 31 */ {[]string{"*.*.foo"}, MAny, MTrue, MTrue, MAny, "bar.foo", false}, - /* 32 */ {[]string{"*.*.foo"}, MAny, MTrue, MTrue, MAny, "*.foo", false}, - /* 33 */ {[]string{"*.*.foo"}, MAny, MTrue, MTrue, MAny, "foo", false}, + /* 27 */ {[]string{"*.*.foo"}, MAny, MTrue, MTrue, MAny, MAny, "baz.fud.bar.foo", true}, + /* 28 */ {[]string{"*.*.foo"}, MAny, MTrue, MTrue, MAny, MTrue, "*.fud.bar.foo", true}, + /* 29 */ {[]string{"*.*.foo"}, MAny, MTrue, MTrue, MAny, MAny, "fud.bar.foo", true}, + /* 30 */ {[]string{"*.*.foo"}, MAny, MTrue, MTrue, MAny, MTrue, "*.bar.foo", true}, // Regression fix: Vault#13530 + /* 31 */ {[]string{"*.*.foo"}, MAny, MTrue, MTrue, MAny, MAny, "bar.foo", false}, + /* 32 */ {[]string{"*.*.foo"}, MAny, MTrue, MTrue, MAny, MAny, "*.foo", false}, + /* 33 */ {[]string{"*.*.foo"}, MAny, MTrue, MTrue, MAny, MAny, "foo", false}, // === Single Leading Glob Testing === // - // Allowed contains globs, but glob allowed so certain matches work. // The value of bare and localhost does not impact these results. - /* 34 */ {[]string{"*.foo"}, MAny, MTrue, MFalse, MAny, "baz.fud.bar.foo", true}, // glob domains allow infinite subdomains - /* 35 */ {[]string{"*.foo"}, MAny, MTrue, MFalse, MAny, "*.fud.bar.foo", true}, // ???? does glob domain allow wildcard of subdomains? - /* 36 */ {[]string{"*.foo"}, MAny, MTrue, MFalse, MAny, "fud.bar.foo", true}, // glob domains allow infinite subdomains - /* 37 */ {[]string{"*.foo"}, MAny, MTrue, MFalse, MAny, "*.bar.foo", true}, // ???? does glob domain allow wildcards of subdomains? - /* 38 */ {[]string{"*.foo"}, MAny, MTrue, MFalse, MAny, "bar.foo", true}, - /* 39 */ {[]string{"*.foo"}, MAny, MTrue, MFalse, MAny, "foo", false}, + /* 34 */ {[]string{"*.foo"}, MAny, MTrue, MFalse, MAny, MAny, "baz.fud.bar.foo", true}, // glob domains allow infinite subdomains + /* 35 */ {[]string{"*.foo"}, MAny, MTrue, MFalse, MAny, MTrue, "*.fud.bar.foo", true}, // glob domain allows wildcard of subdomains + /* 36 */ {[]string{"*.foo"}, MAny, MTrue, MFalse, MAny, MAny, "fud.bar.foo", true}, // glob domains allow infinite subdomains + /* 37 */ {[]string{"*.foo"}, MAny, MTrue, MFalse, MAny, MTrue, "*.bar.foo", true}, // glob domain allows wildcards of subdomains + /* 38 */ {[]string{"*.foo"}, MAny, MTrue, MFalse, MAny, MAny, "bar.foo", true}, + /* 39 */ {[]string{"*.foo"}, MAny, MTrue, MFalse, MAny, MAny, "foo", false}, // Allowed contains globs, but glob and subdomain both work, so we expect // wildcard issuance to work as well. The value of bare and localhost does // not impact these results. - /* 40 */ {[]string{"*.foo"}, MAny, MTrue, MTrue, MAny, "baz.fud.bar.foo", true}, - /* 41 */ {[]string{"*.foo"}, MAny, MTrue, MTrue, MAny, "*.fud.bar.foo", true}, - /* 42 */ {[]string{"*.foo"}, MAny, MTrue, MTrue, MAny, "fud.bar.foo", true}, - /* 43 */ {[]string{"*.foo"}, MAny, MTrue, MTrue, MAny, "*.bar.foo", true}, - /* 44 */ {[]string{"*.foo"}, MAny, MTrue, MTrue, MAny, "bar.foo", true}, - /* 45 */ {[]string{"*.foo"}, MAny, MTrue, MTrue, MAny, "foo", false}, + /* 40 */ {[]string{"*.foo"}, MAny, MTrue, MTrue, MAny, MAny, "baz.fud.bar.foo", true}, + /* 41 */ {[]string{"*.foo"}, MAny, MTrue, MTrue, MAny, MTrue, "*.fud.bar.foo", true}, + /* 42 */ {[]string{"*.foo"}, MAny, MTrue, MTrue, MAny, MAny, "fud.bar.foo", true}, + /* 43 */ {[]string{"*.foo"}, MAny, MTrue, MTrue, MAny, MTrue, "*.bar.foo", true}, + /* 44 */ {[]string{"*.foo"}, MAny, MTrue, MTrue, MAny, MAny, "bar.foo", true}, + /* 45 */ {[]string{"*.foo"}, MAny, MTrue, MTrue, MAny, MAny, "foo", false}, // === Only base domain name === // - // Allowed contains only domain components, but subdomains not allowed. This // results in most issuances failing unless we allow bare domains, in which // case only the final issuance for "foo" will succeed. - /* 46 */ {[]string{"foo"}, MAny, MAny, MFalse, MAny, "baz.fud.bar.foo", false}, - /* 47 */ {[]string{"foo"}, MAny, MAny, MFalse, MAny, "*.fud.bar.foo", false}, - /* 48 */ {[]string{"foo"}, MAny, MAny, MFalse, MAny, "fud.bar.foo", false}, - /* 49 */ {[]string{"foo"}, MAny, MAny, MFalse, MAny, "*.bar.foo", false}, - /* 50 */ {[]string{"foo"}, MAny, MAny, MFalse, MAny, "bar.foo", false}, - /* 51 */ {[]string{"foo"}, MAny, MAny, MFalse, MAny, "*.foo", false}, - /* 52 */ {[]string{"foo"}, MFalse, MAny, MFalse, MAny, "foo", false}, - /* 53 */ {[]string{"foo"}, MTrue, MAny, MFalse, MAny, "foo", true}, + /* 46 */ {[]string{"foo"}, MAny, MAny, MFalse, MAny, MAny, "baz.fud.bar.foo", false}, + /* 47 */ {[]string{"foo"}, MAny, MAny, MFalse, MAny, MAny, "*.fud.bar.foo", false}, + /* 48 */ {[]string{"foo"}, MAny, MAny, MFalse, MAny, MAny, "fud.bar.foo", false}, + /* 49 */ {[]string{"foo"}, MAny, MAny, MFalse, MAny, MAny, "*.bar.foo", false}, + /* 50 */ {[]string{"foo"}, MAny, MAny, MFalse, MAny, MAny, "bar.foo", false}, + /* 51 */ {[]string{"foo"}, MAny, MAny, MFalse, MAny, MAny, "*.foo", false}, + /* 52 */ {[]string{"foo"}, MFalse, MAny, MFalse, MAny, MAny, "foo", false}, + /* 53 */ {[]string{"foo"}, MTrue, MAny, MFalse, MAny, MAny, "foo", true}, // Allowed contains only domain components, and subdomains are now allowed. // This results in most issuances succeeding, with the exception of the // base foo, which is still governed by base's value. - /* 54 */ {[]string{"foo"}, MAny, MAny, MTrue, MAny, "baz.fud.bar.foo", true}, - /* 55 */ {[]string{"foo"}, MAny, MAny, MTrue, MAny, "*.fud.bar.foo", true}, - /* 56 */ {[]string{"foo"}, MAny, MAny, MTrue, MAny, "fud.bar.foo", true}, - /* 57 */ {[]string{"foo"}, MAny, MAny, MTrue, MAny, "*.bar.foo", true}, - /* 58 */ {[]string{"foo"}, MAny, MAny, MTrue, MAny, "bar.foo", true}, - /* 59 */ {[]string{"foo"}, MAny, MAny, MTrue, MAny, "*.foo", true}, - /* 60 */ {[]string{"foo"}, MAny, MAny, MTrue, MAny, "x*x.foo", true}, // internal wildcards should be allowed per RFC 6125/6.4.3 - /* 61 */ {[]string{"foo"}, MAny, MAny, MTrue, MAny, "*x.foo", true}, // prefix wildcards should be allowed per RFC 6125/6.4.3 - /* 62 */ {[]string{"foo"}, MAny, MAny, MTrue, MAny, "x*.foo", true}, // suffix wildcards should be allowed per RFC 6125/6.4.3 - /* 63 */ {[]string{"foo"}, MFalse, MAny, MTrue, MAny, "foo", false}, - /* 64 */ {[]string{"foo"}, MTrue, MAny, MTrue, MAny, "foo", true}, + /* 54 */ {[]string{"foo"}, MAny, MAny, MTrue, MAny, MAny, "baz.fud.bar.foo", true}, + /* 55 */ {[]string{"foo"}, MAny, MAny, MTrue, MAny, MTrue, "*.fud.bar.foo", true}, + /* 56 */ {[]string{"foo"}, MAny, MAny, MTrue, MAny, MAny, "fud.bar.foo", true}, + /* 57 */ {[]string{"foo"}, MAny, MAny, MTrue, MAny, MTrue, "*.bar.foo", true}, + /* 58 */ {[]string{"foo"}, MAny, MAny, MTrue, MAny, MAny, "bar.foo", true}, + /* 59 */ {[]string{"foo"}, MAny, MAny, MTrue, MAny, MTrue, "*.foo", true}, + /* 60 */ {[]string{"foo"}, MAny, MAny, MTrue, MAny, MTrue, "x*x.foo", true}, // internal wildcards should be allowed per RFC 6125/6.4.3 + /* 61 */ {[]string{"foo"}, MAny, MAny, MTrue, MAny, MTrue, "*x.foo", true}, // prefix wildcards should be allowed per RFC 6125/6.4.3 + /* 62 */ {[]string{"foo"}, MAny, MAny, MTrue, MAny, MTrue, "x*.foo", true}, // suffix wildcards should be allowed per RFC 6125/6.4.3 + /* 63 */ {[]string{"foo"}, MFalse, MAny, MTrue, MAny, MAny, "foo", false}, + /* 64 */ {[]string{"foo"}, MTrue, MAny, MTrue, MAny, MAny, "foo", true}, // === Internal Glob Matching === // // Basic glob matching requirements - /* 65 */ {[]string{"x*x.foo"}, MAny, MTrue, MAny, MAny, "xerox.foo", true}, - /* 66 */ {[]string{"x*x.foo"}, MAny, MTrue, MAny, MAny, "xylophone.files.pyrex.foo", true}, // globs can match across subdomains - /* 67 */ {[]string{"x*x.foo"}, MAny, MTrue, MAny, MAny, "xercex.bar.foo", false}, // x.foo isn't matched - /* 68 */ {[]string{"x*x.foo"}, MAny, MTrue, MAny, MAny, "bar.foo", false}, // x*x isn't matched. - /* 69 */ {[]string{"x*x.foo"}, MAny, MTrue, MAny, MAny, "*.foo", false}, // unrelated wildcard - /* 70 */ {[]string{"x*x.foo"}, MAny, MTrue, MFalse, MAny, "*.x*x.foo", false}, // ???? double wildcard doesn't match glob without subdomains enabled - /* 71 */ {[]string{"x*x.foo"}, MAny, MTrue, MTrue, MAny, "*.x*x.foo", true}, // ???? as above, but with subdomains enabled - /* 72 */ {[]string{"x*x.foo"}, MAny, MTrue, MAny, MAny, "*.xyx.foo", false}, // ???? single wildcard matching glob fails (even with subdomains=true) + /* 65 */ {[]string{"x*x.foo"}, MAny, MTrue, MAny, MAny, MAny, "xerox.foo", true}, + /* 66 */ {[]string{"x*x.foo"}, MAny, MTrue, MAny, MAny, MAny, "xylophone.files.pyrex.foo", true}, // globs can match across subdomains + /* 67 */ {[]string{"x*x.foo"}, MAny, MTrue, MAny, MAny, MAny, "xercex.bar.foo", false}, // x.foo isn't matched + /* 68 */ {[]string{"x*x.foo"}, MAny, MTrue, MAny, MAny, MAny, "bar.foo", false}, // x*x isn't matched. + /* 69 */ {[]string{"x*x.foo"}, MAny, MTrue, MAny, MAny, MAny, "*.foo", false}, // unrelated wildcard + /* 70 */ {[]string{"x*x.foo"}, MAny, MTrue, MAny, MAny, MAny, "*.x*x.foo", false}, // Does not conform to RFC 6125 + /* 71 */ {[]string{"x*x.foo"}, MAny, MTrue, MAny, MAny, MAny, "*.xyx.foo", false}, // Globs and Subdomains do not layer per docs. // Various requirements around x*x.foo wildcard matching. - /* 73 */ {[]string{"x*x.foo"}, MFalse, MFalse, MAny, MAny, "x*x.foo", false}, // base disabled, shouldn't match wildcard - /* 74 */ {[]string{"x*x.foo"}, MFalse, MTrue, MAny, MAny, "x*x.foo", true}, // base disallowed, but globbing allowed and should match - /* 75 */ {[]string{"x*x.foo"}, MTrue, MAny, MAny, MAny, "x*x.foo", true}, // base allowed, should match wildcard + /* 72 */ {[]string{"x*x.foo"}, MFalse, MFalse, MAny, MAny, MAny, "x*x.foo", false}, // base disabled, shouldn't match wildcard + /* 73 */ {[]string{"x*x.foo"}, MFalse, MTrue, MAny, MAny, MTrue, "x*x.foo", true}, // base disallowed, but globbing allowed and should match + /* 74 */ {[]string{"x*x.foo"}, MTrue, MAny, MAny, MAny, MTrue, "x*x.foo", true}, // base allowed, should match wildcard // Basic glob matching requirements with internal dots. - /* 76 */ {[]string{"x.*.x.foo"}, MAny, MTrue, MAny, MAny, "xerox.foo", false}, // missing dots - /* 77 */ {[]string{"x.*.x.foo"}, MAny, MTrue, MAny, MAny, "x.ero.x.foo", true}, - /* 78 */ {[]string{"x.*.x.foo"}, MAny, MTrue, MAny, MAny, "xylophone.files.pyrex.foo", false}, // missing dots - /* 79 */ {[]string{"x.*.x.foo"}, MAny, MTrue, MAny, MAny, "x.ylophone.files.pyre.x.foo", true}, // globs can match across subdomains - /* 80 */ {[]string{"x.*.x.foo"}, MAny, MTrue, MAny, MAny, "xercex.bar.foo", false}, // x.foo isn't matched - /* 81 */ {[]string{"x.*.x.foo"}, MAny, MTrue, MAny, MAny, "bar.foo", false}, // x.*.x isn't matched. - /* 82 */ {[]string{"x.*.x.foo"}, MAny, MTrue, MAny, MAny, "*.foo", false}, // unrelated wildcard - /* 83 */ {[]string{"x.*.x.foo"}, MAny, MTrue, MFalse, MAny, "*.x.*.x.foo", false}, // ???? double wildcard doesn't match glob without subdomains enabled - /* 84 */ {[]string{"x.*.x.foo"}, MAny, MTrue, MTrue, MAny, "*.x.*.x.foo", true}, // ???? as above, but with subdomains enabled - /* 85 */ {[]string{"x.*.x.foo"}, MAny, MTrue, MAny, MAny, "*.x.y.x.foo", false}, // ???? single wildcard with internal glob match fails (even with subdomains=true) - } - - if len(testCases) != 86 { + /* 75 */ {[]string{"x.*.x.foo"}, MAny, MTrue, MAny, MAny, MAny, "xerox.foo", false}, // missing dots + /* 76 */ {[]string{"x.*.x.foo"}, MAny, MTrue, MAny, MAny, MAny, "x.ero.x.foo", true}, + /* 77 */ {[]string{"x.*.x.foo"}, MAny, MTrue, MAny, MAny, MAny, "xylophone.files.pyrex.foo", false}, // missing dots + /* 78 */ {[]string{"x.*.x.foo"}, MAny, MTrue, MAny, MAny, MAny, "x.ylophone.files.pyre.x.foo", true}, // globs can match across subdomains + /* 79 */ {[]string{"x.*.x.foo"}, MAny, MTrue, MAny, MAny, MAny, "xercex.bar.foo", false}, // x.foo isn't matched + /* 80 */ {[]string{"x.*.x.foo"}, MAny, MTrue, MAny, MAny, MAny, "bar.foo", false}, // x.*.x isn't matched. + /* 81 */ {[]string{"x.*.x.foo"}, MAny, MTrue, MAny, MAny, MAny, "*.foo", false}, // unrelated wildcard + /* 82 */ {[]string{"x.*.x.foo"}, MAny, MTrue, MAny, MAny, MAny, "*.x.*.x.foo", false}, // Does not conform to RFC 6125 + /* 83 */ {[]string{"x.*.x.foo"}, MAny, MTrue, MAny, MAny, MAny, "*.x.y.x.foo", false}, // Globs and Subdomains do not layer per docs. + + // === Wildcard restriction testing === // + /* 84 */ {[]string{"*.foo"}, MAny, MTrue, MFalse, MAny, MFalse, "*.fud.bar.foo", false}, // glob domain allows wildcard of subdomains + /* 85 */ {[]string{"*.foo"}, MAny, MTrue, MFalse, MAny, MFalse, "*.bar.foo", false}, // glob domain allows wildcards of subdomains + /* 86 */ {[]string{"foo"}, MAny, MAny, MTrue, MAny, MFalse, "*.fud.bar.foo", false}, + /* 87 */ {[]string{"foo"}, MAny, MAny, MTrue, MAny, MFalse, "*.bar.foo", false}, + /* 88 */ {[]string{"foo"}, MAny, MAny, MTrue, MAny, MFalse, "*.foo", false}, + /* 89 */ {[]string{"foo"}, MAny, MAny, MTrue, MAny, MFalse, "x*x.foo", false}, + /* 90 */ {[]string{"foo"}, MAny, MAny, MTrue, MAny, MFalse, "*x.foo", false}, + /* 91 */ {[]string{"foo"}, MAny, MAny, MTrue, MAny, MFalse, "x*.foo", false}, + /* 92 */ {[]string{"x*x.foo"}, MTrue, MAny, MAny, MAny, MFalse, "x*x.foo", false}, + /* 93 */ {[]string{"*.foo"}, MFalse, MFalse, MAny, MAny, MAny, "*.foo", false}, // Bare and globs forbidden despite (potentially) allowing wildcards. + /* 94 */ {[]string{"x.*.x.foo"}, MAny, MAny, MAny, MAny, MAny, "x.*.x.foo", false}, // Does not conform to RFC 6125 + } + + if len(testCases) != 95 { t.Fatalf("misnumbered test case entries will make it hard to find bugs: %v", len(testCases)) } diff --git a/builtin/logical/pki/ca_util.go b/builtin/logical/pki/ca_util.go index ea6b083c7eed..0db79c102df3 100644 --- a/builtin/logical/pki/ca_util.go +++ b/builtin/logical/pki/ca_util.go @@ -30,25 +30,27 @@ func (b *backend) getGenerationParams( } role = &roleEntry{ - TTL: time.Duration(data.Get("ttl").(int)) * time.Second, - KeyType: data.Get("key_type").(string), - KeyBits: data.Get("key_bits").(int), - SignatureBits: data.Get("signature_bits").(int), - AllowLocalhost: true, - AllowAnyName: true, - AllowIPSANs: true, - EnforceHostnames: false, - AllowedURISANs: []string{"*"}, - AllowedOtherSANs: []string{"*"}, - AllowedSerialNumbers: []string{"*"}, - OU: data.Get("ou").([]string), - Organization: data.Get("organization").([]string), - Country: data.Get("country").([]string), - Locality: data.Get("locality").([]string), - Province: data.Get("province").([]string), - StreetAddress: data.Get("street_address").([]string), - PostalCode: data.Get("postal_code").([]string), + TTL: time.Duration(data.Get("ttl").(int)) * time.Second, + KeyType: data.Get("key_type").(string), + KeyBits: data.Get("key_bits").(int), + SignatureBits: data.Get("signature_bits").(int), + AllowLocalhost: true, + AllowAnyName: true, + AllowIPSANs: true, + AllowWildcardCertificates: new(bool), + EnforceHostnames: false, + AllowedURISANs: []string{"*"}, + AllowedOtherSANs: []string{"*"}, + AllowedSerialNumbers: []string{"*"}, + OU: data.Get("ou").([]string), + Organization: data.Get("organization").([]string), + Country: data.Get("country").([]string), + Locality: data.Get("locality").([]string), + Province: data.Get("province").([]string), + StreetAddress: data.Get("street_address").([]string), + PostalCode: data.Get("postal_code").([]string), } + *role.AllowWildcardCertificates = true if role.KeyType == "rsa" && role.KeyBits < 2048 { errorResp = logical.ErrorResponse("RSA keys < 2048 bits are unsafe and not supported") diff --git a/builtin/logical/pki/cert_util.go b/builtin/logical/pki/cert_util.go index 205174aab956..bc022009bab6 100644 --- a/builtin/logical/pki/cert_util.go +++ b/builtin/logical/pki/cert_util.go @@ -38,11 +38,32 @@ type inputBundle struct { } var ( + // labelRegex is a single label from a valid domain name and was extracted + // from hostnameRegex below for use in leftWildLabelRegex, without any + // label separators (`.`). + labelRegex = `([a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9\-]*[a-zA-Z0-9])` + // A note on hostnameRegex: although we set the StrictDomainName option // when doing the idna conversion, this appears to only affect output, not // input, so it will allow e.g. host^123.example.com straight through. So // we still need to use this to check the output. - hostnameRegex = regexp.MustCompile(`^(\*\.)?(([a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9\-]*[a-zA-Z0-9])\.)*([A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9\-]*[A-Za-z0-9])\.?$`) + hostnameRegex = regexp.MustCompile(`^(\*\.)?(` + labelRegex + `\.)*` + labelRegex + `\.?$`) + + // Left Wildcard Label Regex is equivalent to a single domain label + // component from hostnameRegex above, but with additional wildcard + // characters added. There are four possibilities here: + // + // 1. Entire label is a wildcard, + // 2. Wildcard exists at the start, + // 3. Wildcard exists at the end, + // 4. Wildcard exists in the middle. + allWildRegex = `\*` + startWildRegex = `\*` + labelRegex + endWildRegex = labelRegex + `\*` + middleWildRegex = labelRegex + `\*` + labelRegex + leftWildLabelRegex = regexp.MustCompile(`^(` + allWildRegex + `|` + startWildRegex + `|` + endWildRegex + `|` + middleWildRegex + `)$`) + + // OIDs for X.509 certificate extensions used below. oidExtensionBasicConstraints = []int{2, 5, 29, 19} oidExtensionSubjectAltName = []int{2, 5, 29, 17} ) @@ -181,8 +202,16 @@ func fetchCertBySerial(ctx context.Context, req *logical.Request, prefix, serial // If one does not pass, it is returned in the string argument. func validateNames(b *backend, data *inputBundle, names []string) string { for _, name := range names { - sanitizedName := name - emailDomain := sanitizedName + // Previously, reducedName was called sanitizedName but this made + // little sense under the previous interpretation of wildcards, + // leading to two bugs in this implementation. We presently call it + // "reduced" to indicate that it is still untrusted input (potentially + // different from the bare Common Name entry we're validating), it + // might have been modified such as by the removal of wildcard labels + // or the email prefix. + reducedName := name + emailDomain := reducedName + wildcardLabel := "" isEmail := false isWildcard := false @@ -192,48 +221,119 @@ 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(sanitizedName, "@") { - splitEmail := strings.Split(sanitizedName, "@") + if strings.Contains(reducedName, "@") { + splitEmail := strings.Split(reducedName, "@") if len(splitEmail) != 2 { return name } - sanitizedName = splitEmail[1] + reducedName = splitEmail[1] emailDomain = splitEmail[1] isEmail = true } - // If we have an asterisk as the first part of the domain name, mark it - // as wildcard and set the sanitized name to the remainder of the - // domain - if strings.HasPrefix(sanitizedName, "*.") { - sanitizedName = sanitizedName[2:] + // Per RFC 6125 Section 6.4.3, and explicitly contradicting the earlier + // RFC 2818 which no modern client will validate against, there are two + // main types of wildcards, each with a single wildcard specifier (`*`, + // functionally different from the `*` used as a glob from the + // AllowGlobDomains parsing path) in the left-most label: + // + // 1. Entire label is a single wildcard character (most common and + // well-supported), + // 2. Part of the label contains a single wildcard character (e.g. per + /// RFC 6125: baz*.example.net, *baz.example.net, or b*z.example.net). + // + // We permit issuance of both but not the older RFC 2818 style under + // the new AllowWildcardCertificates option. However, anything with a + // glob character is technically a wildcard. + if strings.Contains(reducedName, "*") { + // Regardless of later rejections below, this common name contains + // a wildcard character and is thus technically a wildcard name. isWildcard = true + + // Additionally, if AllowWildcardCertificates is explicitly + // forbidden, it takes precedence over AllowAnyName, thus we should + // reject the name now. + // + // We expect the role to have been correctly migrated but guard for + // safety. + if data.role.AllowWildcardCertificates != nil && !*data.role.AllowWildcardCertificates { + return name + } + + if strings.Count(reducedName, "*") > 1 { + // As mentioned above, only one wildcard character is permitted + // under RFC 6125 semantics. + return name + } + + // Split the Common Name into two parts: a left-most label and the + // remaining segments (if present). + splitLabels := strings.SplitN(reducedName, ".", 2) + if len(splitLabels) != 2 { + // We've been given a single-part domain name that consists + // entirely of a wildcard. This is a little tricky to handle, + // but EnforceHostnames validates both the wildcard-containing + // label and the reduced name, but _only_ the latter if it is + // non-empty. This allows us to still validate the only label + // component matches hostname expectations still. + wildcardLabel = splitLabels[0] + reducedName = "" + } else { + // We have a (at least) two label domain name. But before we can + // update our names, we need to validate the wildcard ended up + // in the segment we expected it to. While this is (kinda) + // validated under EnforceHostnames's leftWildLabelRegex, we + // still need to validate it in the non-enforced mode. + // + // By validated assumption above, we know there's strictly one + // wildcard in this domain so we only need to check the wildcard + // label or the reduced name (as one is equivalent to the other). + // Because we later assume reducedName _lacks_ wildcard segments, + // we validate that. + wildcardLabel = splitLabels[0] + reducedName = splitLabels[1] + if strings.Contains(reducedName, "*") { + return name + } + } } // Email addresses using wildcard domain names do not make sense + // in a Common Name field. if isEmail && isWildcard { return name } // AllowAnyName is checked after this because EnforceHostnames still - // applies when allowing any name. Also, we check the sanitized name to + // applies when allowing any name. Also, we check the reduced name to // ensure that we are not either checking a full email address or a // wildcard prefix. if data.role.EnforceHostnames { - p := idna.New( - idna.StrictDomainName(true), - idna.VerifyDNSLength(true), - ) - converted, err := p.ToASCII(sanitizedName) - if err != nil { - return name + if reducedName != "" { + // See note above about splitLabels having only one segment + // and setting reducedName to the empty string. + p := idna.New( + idna.StrictDomainName(true), + idna.VerifyDNSLength(true), + ) + converted, err := p.ToASCII(reducedName) + if err != nil { + return name + } + if !hostnameRegex.MatchString(converted) { + return name + } } - if !hostnameRegex.MatchString(converted) { + + // When a wildcard is specified, we additionally need to validate + // the label with the wildcard is correctly formed. + if isWildcard && !leftWildLabelRegex.MatchString(wildcardLabel) { return name } } - // Self-explanatory + // Self-explanatory, but validations from EnforceHostnames and + // AllowWildcardCertificates take precedence. if data.role.AllowAnyName { continue } @@ -255,8 +355,8 @@ func validateNames(b *backend, data *inputBundle, names []string) string { // Variances are noted in-line if data.role.AllowLocalhost { - if sanitizedName == "localhost" || - sanitizedName == "localdomain" || + if reducedName == "localhost" || + reducedName == "localdomain" || (isEmail && emailDomain == "localhost") || (isEmail && emailDomain == "localdomain") { continue @@ -264,14 +364,14 @@ func validateNames(b *backend, data *inputBundle, names []string) string { if data.role.AllowSubdomains { // It is possible, if unlikely, to have a subdomain of "localhost" - if strings.HasSuffix(sanitizedName, ".localhost") || - (isWildcard && sanitizedName == "localhost") { + if strings.HasSuffix(reducedName, ".localhost") || + (isWildcard && reducedName == "localhost") { continue } // A subdomain of "localdomain" is also not entirely uncommon - if strings.HasSuffix(sanitizedName, ".localdomain") || - (isWildcard && sanitizedName == "localdomain") { + if strings.HasSuffix(reducedName, ".localdomain") || + (isWildcard && reducedName == "localdomain") { continue } } @@ -293,15 +393,15 @@ func validateNames(b *backend, data *inputBundle, names []string) string { // Compare the sanitized name against the hostname // portion of the email address in the broken // display name - if strings.HasSuffix(sanitizedName, "."+splitDisplay[1]) { + if strings.HasSuffix(reducedName, "."+splitDisplay[1]) { continue } } } } - if strings.HasSuffix(sanitizedName, "."+data.req.DisplayName) || - (isWildcard && sanitizedName == data.req.DisplayName) { + if strings.HasSuffix(reducedName, "."+data.req.DisplayName) || + (isWildcard && reducedName == data.req.DisplayName) { continue } } @@ -337,8 +437,8 @@ func validateNames(b *backend, data *inputBundle, names []string) string { } if data.role.AllowSubdomains { - if strings.HasSuffix(sanitizedName, "."+currDomain) || - (isWildcard && strings.EqualFold(sanitizedName, currDomain)) { + if strings.HasSuffix(reducedName, "."+currDomain) || + (isWildcard && strings.EqualFold(reducedName, currDomain)) { valid = true break } diff --git a/builtin/logical/pki/path_issue_sign.go b/builtin/logical/pki/path_issue_sign.go index ce3bcd2cd4b1..5b00bbcabf02 100644 --- a/builtin/logical/pki/path_issue_sign.go +++ b/builtin/logical/pki/path_issue_sign.go @@ -154,20 +154,22 @@ func (b *backend) pathSignVerbatim(ctx context.Context, req *logical.Request, da } entry := &roleEntry{ - AllowLocalhost: true, - AllowAnyName: true, - AllowIPSANs: true, - EnforceHostnames: false, - KeyType: "any", - UseCSRCommonName: true, - UseCSRSANs: true, - AllowedURISANs: []string{"*"}, - AllowedSerialNumbers: []string{"*"}, - GenerateLease: new(bool), - KeyUsage: data.Get("key_usage").([]string), - ExtKeyUsage: data.Get("ext_key_usage").([]string), - ExtKeyUsageOIDs: data.Get("ext_key_usage_oids").([]string), + AllowLocalhost: true, + AllowAnyName: true, + AllowIPSANs: true, + AllowWildcardCertificates: new(bool), + EnforceHostnames: false, + KeyType: "any", + UseCSRCommonName: true, + UseCSRSANs: true, + AllowedURISANs: []string{"*"}, + AllowedSerialNumbers: []string{"*"}, + GenerateLease: new(bool), + KeyUsage: data.Get("key_usage").([]string), + ExtKeyUsage: data.Get("ext_key_usage").([]string), + ExtKeyUsageOIDs: data.Get("ext_key_usage_oids").([]string), } + *entry.AllowWildcardCertificates = true *entry.GenerateLease = false diff --git a/builtin/logical/pki/path_roles.go b/builtin/logical/pki/path_roles.go index 4cfa666cfcc4..9c929563fe9a 100644 --- a/builtin/logical/pki/path_roles.go +++ b/builtin/logical/pki/path_roles.go @@ -107,6 +107,14 @@ can include glob patterns, e.g. "ftp*.example.com". See the documentation for more information.`, }, + "allow_wildcard_certificates": { + Type: framework.TypeBool, + Description: `If set, allows certificates with wildcards in +the common name to be issued, conforming to RFC 6125's Section 6.4.3; e.g., +"*.example.net" or "b*z.example.net". See the documentation for more +information.`, + }, + "allow_any_name": { Type: framework.TypeBool, Description: `If set, clients can request certificates for @@ -458,6 +466,15 @@ func (b *backend) getRole(ctx context.Context, s logical.Storage, n string) (*ro result.AllowedBaseDomain = "" modified = true } + if result.AllowWildcardCertificates == nil { + // While not the most secure default, when AllowWildcardCertificates isn't + // explicitly specified in the stored Role, we automatically upgrade it to + // true to preserve compatibility with previous versions of Vault. Once this + // field is set, this logic will not be triggered any more. + result.AllowWildcardCertificates = new(bool) + *result.AllowWildcardCertificates = true + modified = true + } // Upgrade generate_lease in role if result.GenerateLease == nil { @@ -557,6 +574,7 @@ func (b *backend) pathRoleCreate(ctx context.Context, req *logical.Request, data AllowBareDomains: data.Get("allow_bare_domains").(bool), AllowSubdomains: data.Get("allow_subdomains").(bool), AllowGlobDomains: data.Get("allow_glob_domains").(bool), + AllowWildcardCertificates: new(bool), // Handled specially below AllowAnyName: data.Get("allow_any_name").(bool), EnforceHostnames: data.Get("enforce_hostnames").(bool), AllowIPSANs: data.Get("allow_ip_sans").(bool), @@ -644,6 +662,15 @@ func (b *backend) pathRoleCreate(ctx context.Context, req *logical.Request, data } } + allow_wildcard_certificates, present := data.GetOk("allow_wildcard_certificates") + if !present { + // While not the most secure default, when AllowWildcardCertificates isn't + // explicitly specified in the request, we automatically set it to true to + // preserve compatibility with previous versions of Vault. + allow_wildcard_certificates = true + } + *entry.AllowWildcardCertificates = allow_wildcard_certificates.(bool) + // Store it jsonEntry, err := logical.StorageEntryJSON("role/"+name, entry) if err != nil { @@ -752,6 +779,7 @@ type roleEntry struct { AllowTokenDisplayName bool `json:"allow_token_displayname" mapstructure:"allow_token_displayname"` AllowSubdomains bool `json:"allow_subdomains" mapstructure:"allow_subdomains"` AllowGlobDomains bool `json:"allow_glob_domains" mapstructure:"allow_glob_domains"` + AllowWildcardCertificates *bool `json:"allow_wildcard_certificates,omitempty" mapstructure:"allow_wildcard_certificates"` AllowAnyName bool `json:"allow_any_name" mapstructure:"allow_any_name"` EnforceHostnames bool `json:"enforce_hostnames" mapstructure:"enforce_hostnames"` AllowIPSANs bool `json:"allow_ip_sans" mapstructure:"allow_ip_sans"` @@ -803,6 +831,7 @@ func (r *roleEntry) ToResponseData() map[string]interface{} { "allow_token_displayname": r.AllowTokenDisplayName, "allow_subdomains": r.AllowSubdomains, "allow_glob_domains": r.AllowGlobDomains, + "allow_wildcard_certificates": r.AllowWildcardCertificates, "allow_any_name": r.AllowAnyName, "enforce_hostnames": r.EnforceHostnames, "allow_ip_sans": r.AllowIPSANs, diff --git a/builtin/logical/pki/path_root.go b/builtin/logical/pki/path_root.go index 773444245971..f8213494f814 100644 --- a/builtin/logical/pki/path_root.go +++ b/builtin/logical/pki/path_root.go @@ -267,23 +267,25 @@ func (b *backend) pathCASignIntermediate(ctx context.Context, req *logical.Reque } role := &roleEntry{ - OU: data.Get("ou").([]string), - Organization: data.Get("organization").([]string), - Country: data.Get("country").([]string), - Locality: data.Get("locality").([]string), - Province: data.Get("province").([]string), - StreetAddress: data.Get("street_address").([]string), - PostalCode: data.Get("postal_code").([]string), - TTL: time.Duration(data.Get("ttl").(int)) * time.Second, - AllowLocalhost: true, - AllowAnyName: true, - AllowIPSANs: true, - EnforceHostnames: false, - KeyType: "any", - AllowedURISANs: []string{"*"}, - AllowedSerialNumbers: []string{"*"}, - AllowExpirationPastCA: true, - } + OU: data.Get("ou").([]string), + Organization: data.Get("organization").([]string), + Country: data.Get("country").([]string), + Locality: data.Get("locality").([]string), + Province: data.Get("province").([]string), + StreetAddress: data.Get("street_address").([]string), + PostalCode: data.Get("postal_code").([]string), + TTL: time.Duration(data.Get("ttl").(int)) * time.Second, + AllowLocalhost: true, + AllowAnyName: true, + AllowIPSANs: true, + AllowWildcardCertificates: new(bool), + EnforceHostnames: false, + KeyType: "any", + AllowedURISANs: []string{"*"}, + AllowedSerialNumbers: []string{"*"}, + AllowExpirationPastCA: true, + } + *role.AllowWildcardCertificates = true if cn := data.Get("common_name").(string); len(cn) == 0 { role.UseCSRCommonName = true diff --git a/changelog/14238.txt b/changelog/14238.txt new file mode 100644 index 000000000000..8c794eb53ec8 --- /dev/null +++ b/changelog/14238.txt @@ -0,0 +1,3 @@ +```release-note:improvement +secrets/pki: Restrict issuance of wildcard certificates via role parameter (`allow_wildcard_certificates`) +``` diff --git a/website/content/api-docs/secret/pki.mdx b/website/content/api-docs/secret/pki.mdx index db0240ac8e15..c471a72bbc86 100644 --- a/website/content/api-docs/secret/pki.mdx +++ b/website/content/api-docs/secret/pki.mdx @@ -751,10 +751,27 @@ request is denied. certificates for `localhost` as one of the requested common names. This is useful for testing and to allow clients on a single host to talk securely. -- `allowed_domains` `(list: [])` – Specifies the domains of the role. This is - used with the `allow_bare_domains` and `allow_subdomains` options. +~> **Note**: This strictly applies to `localhost` and `localdomain` when this + option is enabled. Additionally, even if this option is disabled, if either + name is included in `allowed_domains`, the match rules for that option + could permit issuance of a certificate for `localhost`. -- `allowed_domains_template` `()bool: false)` – When set, `allowed_domains` +- `allowed_domains` `(list: [])` – Specifies the domains of the role. This is + used with the `allow_bare_domains`, `allow_subdomains`, and `allow_glob_domains` + options to determine the type of matching between these domains and the + values of common name, DNS-typed SAN entries, and Email-typed SAN entries. + When `allow_any_name` is used, this attribute has no effect. + +~> **Note**: The three options `allow_bare_domains`, `allow_subdomains`, and + `allow_glob_domains` are each independent of each other. That is, at least + one type of allowed matching must describe the relationship between the + `allowed_domains` list and the names on the issued certificate. For example, + given `allowed_domain=foo.*.example.com` and `allow_subdomains=true` and + `allow_glob_domains=true`, a request for `bar.foo.baz.example.com` won't + be permitted, even though it `foo.baz.example.com` matches the glob + `foo.*.example.com` and `bar` is a subdomain of that. + +- `allowed_domains_template` `(bool: false)` – When set, `allowed_domains` may contain templates, as with [ACL Path Templating](/docs/concepts/policies). - `allow_bare_domains` `(bool: false)` – Specifies if clients can request @@ -762,23 +779,51 @@ request is denied. configured domain set with `allowed_domains` is `example.com`, this allows clients to actually request a certificate containing the name `example.com` as one of the DNS values on the final certificate. In some scenarios, this can be - considered a security risk. + considered a security risk. Note that when an `allowed_domain` field contains + a potential wildcard character (for example, `allowed_domains=*.example.com`) + and `allow_bare_domains` and `allow_wildcard_certificates` are both enabled, + issuance of a wildcard certificate for `*.example.com` will be permitted. - `allow_subdomains` `(bool: false)` – Specifies if clients can request certificates with CNs that are subdomains of the CNs allowed by the other role options. _This includes wildcard subdomains._ For example, an `allowed_domains` value of `example.com` with this option set to true will - allow `foo.example.com` and `bar.example.com` as well as `*.example.com`. This - is redundant when using the `allow_any_name` option. + allow `foo.example.com` and `bar.example.com` as well as `*.example.com`. To + restrict issuance of wildcards by this option, see `allow_wildcard_certificates` + below. This option is redundant when using the `allow_any_name` option. - `allow_glob_domains` `(bool: false)` - Allows names specified in `allowed_domains` to contain glob patterns (e.g. `ftp*.example.com`). Clients will be allowed to request certificates with names matching the glob patterns. +~> **Note**: These globs behave like shell-style globs and can match + across multiple domain parts. For example, `allowed_domains=*.example.com` + with `allow_glob_domains` enabled will match not only `foo.example.com` but + also `baz.bar.foo.example.com`. + +~> **Warning**: Glob patterns will match wildcard domains and permit their + issuance unless otherwise restricted by `allow_wildcard_certificates`. For + instance, with `allowed_domains=*.*.example.com` and both `allow_glob_domains` + and `allow_wildcard_certificates` enabled, we will permit the issuance of + a wildcard certificate for `*.foo.example.com`. + +- `allow_wildcard_certificates` `(bool: true)` - Allows the issuance of + certificates with [RFC 6125](https://tools.ietf.org/html/rfc6125) wildcards + in the CN field. When set to `false`, this prevents wildcards from being + issued even if they would've been allowed by an option above. We support + the following four wildcard types: + + - `*.example.com`, a single wildcard as the entire left-most label, + - `foo*.example.com`, a single suffixed wildcard in the left-most label, + - `*foo.example.com`, a single prefixed wildcard in the left-most label, and + - `f*o.example.com`, a single interior wildcard in the left-most label. + - `allow_any_name` `(bool: false)` – Specifies if clients can request any CN. Useful in some circumstances, but make sure you understand whether it is - appropriate for your installation before enabling it. + appropriate for your installation before enabling it. Note that both + `enforce_hostnames` and `allow_wildcard_certificates` are still checked, + which may introduce limitations on issuance with this option. - `enforce_hostnames` `(bool: true)` – Specifies if only valid host names are allowed for CNs, DNS SANs, and the host part of email addresses.