Add role parameter to restrict issuance of wildcard certificates (#14238)

* Add new AllowWildcardCertificate field to PKI role

This field allows the PKI role to control whether or not issuance of
wildcard certificates are allowed. We default (both on migration and
new role creation) to the less secure true value for backwards
compatibility with existing Vault versions.

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

* Refactor sanitizedName to reducedName

Per comment, this variable name was confusing during the reproduction
and subsequent fix of the earlier vulnerability and associated bug
report. Because the common name isn't necessarily _sanitized_ in any way
(and indeed must be considered in relation to other parts or the whole),
but portions of the entire name are removed, reducedName appears to make
the most sense.

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

* Enforce AllowWildcardCertificates during issuance

This commit adds the bulk of correctly validating wildcard certificate
Common Names during issuance according to RFC 6125 Section 6.4.3
semantics. As part of this, support for RFC 2818-conforming wildcard
certificates (wherein there are almost no restrictions on issuance) has
been removed.

Note that this flag does take precedence over AllowAnyName, giving a
little more safety in wildcard issuance in this case.

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

* Update test cases to conform with RFC 6125

Test cases 19, 70+71, and 83+84 didn't conform with the RFC 6125, and so
should've been rejected under strict conformance. For 70+71 and 83+84,
we previously conditioned around the value of AllowSubdomains (allowing
issuance when true), but they likely should've been rejected either way.

Additionally, update the notes about globs matching wildcard
certificates to notate this is indeed the case.

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

* Check AllowWildcardCertifciates in issuance tests

This allows for regression tests to cover the new
AllowWildcardCertificate conditional. We add additional test cases
ensuring that wildcard issuance is properly forbidden in all relevant
scenarios, while allowing the existing test cases to validate that
wildcard status doesn't affect non-wildcard certificates.

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

* Add Wildcard allowance during signing operations

When using sign-verbatim, sign-intermediate, or getting certificate
generation parameters, set AllowWildcardCertificates to mirror existing
policies.

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

* Add changelog entry

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
This commit is contained in:
Alexander Scheel 2022-02-24 07:41:56 -06:00 committed by GitHub
parent c77952b1a6
commit 11c5068533
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 368 additions and 213 deletions

View File

@ -823,7 +823,10 @@ func generateRoleSteps(t *testing.T, useCSRs bool) []logicaltest.TestStep {
KeyType: "rsa",
KeyBits: 2048,
RequireCN: true,
AllowWildcardCertificates: new(bool),
}
*roleVals.AllowWildcardCertificates = true
issueVals := certutil.IssueData{}
ret := []logicaltest.TestStep{}
@ -3934,6 +3937,7 @@ type IssuanceRegression struct {
AllowGlobDomains MultiBool
AllowSubdomains MultiBool
AllowLocalhost MultiBool
AllowWildcardCertificates MultiBool
CommonName string
Issued bool
}
@ -3944,14 +3948,15 @@ 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() {
role := fmt.Sprintf("issuance-regression-%d-bare-%v-glob-%v-subdomains-%v-localhost-%v", index, AllowBareDomains, AllowGlobDomains, AllowSubdomains, AllowLocalhost)
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,
@ -3978,6 +3983,7 @@ func RoleIssuanceRegressionHelper(t *testing.T, client *api.Client, index int, t
}
}
}
}
return tested
}
@ -3985,144 +3991,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)
/* 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) != 86 {
if len(testCases) != 95 {
t.Fatalf("misnumbered test case entries will make it hard to find bugs: %v", len(testCases))
}

View File

@ -49,6 +49,7 @@ func (b *backend) getGenerationParams(
AllowLocalhost: true,
AllowAnyName: true,
AllowIPSANs: true,
AllowWildcardCertificates: new(bool),
EnforceHostnames: false,
AllowedURISANs: []string{"*"},
AllowedOtherSANs: []string{"*"},
@ -61,6 +62,7 @@ func (b *backend) getGenerationParams(
StreetAddress: data.Get("street_address").([]string),
PostalCode: data.Get("postal_code").([]string),
}
*role.AllowWildcardCertificates = true
var err error
if role.KeyBits, role.SignatureBits, err = certutil.ValidateDefaultOrValueKeyTypeSignatureLength(role.KeyType, role.KeyBits, role.SignatureBits); err != nil {

View File

@ -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}
)
@ -204,8 +225,16 @@ func validateURISAN(b *backend, data *inputBundle, uri string) bool {
// 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
@ -215,39 +244,102 @@ 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 {
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(sanitizedName)
converted, err := p.ToASCII(reducedName)
if err != nil {
return name
}
@ -256,7 +348,15 @@ func validateNames(b *backend, data *inputBundle, names []string) string {
}
}
// Self-explanatory
// 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, but validations from EnforceHostnames and
// AllowWildcardCertificates take precedence.
if data.role.AllowAnyName {
continue
}
@ -278,8 +378,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
@ -287,14 +387,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
}
}
@ -316,15 +416,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
}
}
@ -360,8 +460,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
}

View File

@ -127,6 +127,7 @@ func (b *backend) pathSignVerbatim(ctx context.Context, req *logical.Request, da
AllowLocalhost: true,
AllowAnyName: true,
AllowIPSANs: true,
AllowWildcardCertificates: new(bool),
EnforceHostnames: false,
KeyType: "any",
UseCSRCommonName: true,
@ -139,6 +140,7 @@ func (b *backend) pathSignVerbatim(ctx context.Context, req *logical.Request, da
ExtKeyUsage: data.Get("ext_key_usage").([]string),
ExtKeyUsageOIDs: data.Get("ext_key_usage_oids").([]string),
}
*entry.AllowWildcardCertificates = true
*entry.GenerateLease = false

View File

@ -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
@ -472,6 +480,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 {
@ -571,6 +588,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),
AllowedURISANsTemplate: data.Get("allowed_uri_sans_template").(bool),
EnforceHostnames: data.Get("enforce_hostnames").(bool),
@ -652,6 +670,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 {
@ -760,6 +787,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"`
@ -812,6 +840,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,
"allowed_uri_sans_template": r.AllowedURISANsTemplate,
"enforce_hostnames": r.EnforceHostnames,

View File

@ -280,6 +280,7 @@ func (b *backend) pathCASignIntermediate(ctx context.Context, req *logical.Reque
AllowLocalhost: true,
AllowAnyName: true,
AllowIPSANs: true,
AllowWildcardCertificates: new(bool),
EnforceHostnames: false,
KeyType: "any",
AllowedOtherSANs: []string{"*"},
@ -288,6 +289,7 @@ func (b *backend) pathCASignIntermediate(ctx context.Context, req *logical.Reque
AllowExpirationPastCA: true,
NotAfter: data.Get("not_after").(string),
}
*role.AllowWildcardCertificates = true
if cn := data.Get("common_name").(string); len(cn) == 0 {
role.UseCSRCommonName = true

3
changelog/14238.txt Normal file
View File

@ -0,0 +1,3 @@
```release-note:improvement
secrets/pki: Restrict issuance of wildcard certificates via role parameter (`allow_wildcard_certificates`)
```