diff --git a/builtin/logical/pki/cert_util_test.go b/builtin/logical/pki/cert_util_test.go index 30f0f71c7715..90b8a12f438f 100644 --- a/builtin/logical/pki/cert_util_test.go +++ b/builtin/logical/pki/cert_util_test.go @@ -8,6 +8,7 @@ import ( "crypto/x509" "crypto/x509/pkix" "fmt" + "github.com/hashicorp/vault/sdk/helper/testhelpers/schema" "net" "net/url" "os" @@ -1165,3 +1166,115 @@ func testParseCsrToFields(t *testing.T, issueTime time.Time, tt *parseCertificat t.Errorf("testParseCertificateToFields() diff: %s", strings.ReplaceAll(strings.Join(diff, "\n"), "map", "\nmap")) } } + +// TestVerify_chained_name_constraints verifies that we perform name constraints certificate validation using the +// entire CA chain. +// +// This test constructs a root CA that +// - allows: .example.com +// - excludes: bad.example.com +// +// and an intermediate that +// - forbids alsobad.example.com +// +// It verifies that the intermediate +// - can issue certs like good.example.com +// - rejects names like notanexample.com since they are not in the namespace of names permitted by the root CA +// - rejects bad.example.com, since the root CA excludes it +// - rejects alsobad.example.com, since the intermediate CA excludes it. +func TestVerify_chained_name_constraints(t *testing.T) { + t.Parallel() + bRoot, sRoot := CreateBackendWithStorage(t) + + //////////////////////////////////////////////////////////////////////////////////////////////////////////////////// + // Setup + + var bInt *backend + var sInt logical.Storage + { + resp, err := CBWrite(bRoot, sRoot, "root/generate/internal", map[string]interface{}{ + "ttl": "40h", + "common_name": "myvault.com", + "permitted_dns_domains": ".example.com", + "excluded_dns_domains": "bad.example.com", + }) + require.NoError(t, err) + require.NotNil(t, resp) + + // Create the CSR + bInt, sInt = CreateBackendWithStorage(t) + resp, err = CBWrite(bInt, sInt, "intermediate/generate/internal", map[string]interface{}{ + "common_name": "myint.com", + }) + require.NoError(t, err) + schema.ValidateResponse(t, schema.GetResponseSchema(t, bRoot.Route("intermediate/generate/internal"), logical.UpdateOperation), resp, true) + csr := resp.Data["csr"] + + // Sign the CSR + resp, err = CBWrite(bRoot, sRoot, "root/sign-intermediate", map[string]interface{}{ + "common_name": "myint.com", + "csr": csr, + "ttl": "60h", + "excluded_dns_domains": "alsobad.example.com", + }) + require.NoError(t, err) + require.NotNil(t, resp) + + // Import the New Signed Certificate into the Intermediate Mount. + // Note that we append the root CA certificate to the signed intermediate, so that + // the entire chain is stored by set-signed. + resp, err = CBWrite(bInt, sInt, "intermediate/set-signed", map[string]interface{}{ + "certificate": strings.Join(resp.Data["ca_chain"].([]string), "\n"), + }) + require.NoError(t, err) + + // Create a Role in the Intermediate Mount + resp, err = CBWrite(bInt, sInt, "roles/test", map[string]interface{}{ + "allow_bare_domains": true, + "allow_subdomains": true, + "allow_any_name": true, + }) + require.NoError(t, err) + require.NotNil(t, resp) + } + + //////////////////////////////////////////////////////////////////////////////////////////////////////////////////// + // Tests + + testCases := []struct { + commonName string + wantError string + }{ + { + commonName: "good.example.com", + }, + { + commonName: "notanexample.com", + wantError: "should not be permitted by root CA", + }, + { + commonName: "bad.example.com", + wantError: "should be rejected by the root CA", + }, + { + commonName: "alsobad.example.com", + wantError: "should be rejected by the intermediate CA", + }, + } + + for _, tc := range testCases { + t.Run(tc.commonName, func(t *testing.T) { + resp, err := CBWrite(bInt, sInt, "issue/test", map[string]any{ + "common_name": tc.commonName, + }) + if tc.wantError != "" { + require.Error(t, err, tc.wantError) + require.ErrorContains(t, err, "certificate is not authorized to sign for this name") + require.Nil(t, resp) + } else { + require.NoError(t, err) + require.NoError(t, resp.Error()) + } + }) + } +} diff --git a/builtin/logical/pki/issuing/cert_verify.go b/builtin/logical/pki/issuing/cert_verify.go index f17cd7de087d..a97f67bc5d43 100644 --- a/builtin/logical/pki/issuing/cert_verify.go +++ b/builtin/logical/pki/issuing/cert_verify.go @@ -4,6 +4,7 @@ package issuing import ( + "bytes" "context" "fmt" "os" @@ -42,26 +43,35 @@ func VerifyCertificate(ctx context.Context, storage logical.Storage, issuerId Is return nil } - certChainPool := ctx509.NewCertPool() + rootCertPool := ctx509.NewCertPool() + intermediateCertPool := ctx509.NewCertPool() + for _, certificate := range parsedBundle.CAChain { cert, err := convertCertificate(certificate.Bytes) if err != nil { return err } - certChainPool.AddCert(cert) + if bytes.Equal(cert.RawIssuer, cert.RawSubject) { + rootCertPool.AddCert(cert) + } else { + intermediateCertPool.AddCert(cert) + } + } + if len(rootCertPool.Subjects()) < 1 { + // Alright, this is weird, since we don't have the root CA, we'll treat the intermediate as + // the root, otherwise we'll get a "x509: certificate signed by unknown authority" error. + rootCertPool, intermediateCertPool = intermediateCertPool, rootCertPool } - - // Validation Code, assuming we need to validate the entire chain of constraints // Note that we use github.com/google/certificate-transparency-go/x509 to perform certificate verification, // since that library provides options to disable checks that the standard library does not. options := ctx509.VerifyOptions{ - Intermediates: nil, // We aren't verifying the chain here, this would do more work - Roots: certChainPool, + Roots: rootCertPool, + Intermediates: intermediateCertPool, CurrentTime: time.Time{}, KeyUsages: nil, - MaxConstraintComparisions: 0, // This means infinite + MaxConstraintComparisions: 0, // Use the library's 'sensible default' DisableTimeChecks: true, DisableEKUChecks: true, DisableCriticalExtensionChecks: false, diff --git a/changelog/29255.txt b/changelog/29255.txt new file mode 100644 index 000000000000..2d9eda7b28af --- /dev/null +++ b/changelog/29255.txt @@ -0,0 +1,3 @@ +```release-note:bug +secrets/pki: Fix a bug that prevented the full CA chain to be used when enforcing name constraints. +``` diff --git a/ui/lib/core/addon/components/policy-example.hbs b/ui/lib/core/addon/components/policy-example.hbs index 638089f7522c..4c9ac3759871 100644 --- a/ui/lib/core/addon/components/policy-example.hbs +++ b/ui/lib/core/addon/components/policy-example.hbs @@ -6,15 +6,16 @@
{{#if (eq @policyType "acl")}}

- ACL Policies are written in Hashicorp Configuration Language ( - HCL + ACL Policies + are written in Hashicorp Configuration Language ( + HCL ) or JSON and describe which paths in Vault a user or machine is allowed to access. Here is an example policy:

{{else if (eq @policyType "rgp")}}

Role Governing Policies (RGPs) are tied to client tokens or identities which is similar to - ACL policies. They use - Sentinel + ACL policies. They use + Sentinel as a language framework to enable fine-grained policy decisions.

@@ -31,9 +32,9 @@ Endpoint Governing Policies (EGPs) are tied to particular paths (e.g. aws/creds/ ) instead of tokens. They use - Sentinel + Sentinel as a language to access - properties + properties of the incoming requests.

@@ -55,8 +56,11 @@ More information about {{uppercase @policyType}} policies can be found - + here. - +

\ No newline at end of file diff --git a/ui/tests/acceptance/open-api-path-help-test.js b/ui/tests/acceptance/open-api-path-help-test.js index e48316d9fe31..621d36326a84 100644 --- a/ui/tests/acceptance/open-api-path-help-test.js +++ b/ui/tests/acceptance/open-api-path-help-test.js @@ -80,7 +80,7 @@ function secretEngineHelper(test, secretEngine) { assert.deepEqual( Object.keys(result).sort(), Object.keys(expected).sort(), - `getProps returns expected attributes for ${modelName}` + `getProps returns expected attributes for ${modelName} (help url: "${helpUrl}")` ); Object.keys(expected).forEach((attrName) => { assert.deepEqual(result[attrName], expected[attrName], `${attrName} attribute details match`); diff --git a/ui/tests/helpers/openapi/expected-secret-attrs.js b/ui/tests/helpers/openapi/expected-secret-attrs.js index 1fb27c2eba76..c72160e1a963 100644 --- a/ui/tests/helpers/openapi/expected-secret-attrs.js +++ b/ui/tests/helpers/openapi/expected-secret-attrs.js @@ -1234,6 +1234,34 @@ const pki = { label: 'Exclude Common Name from Subject Alternative Names (SANs)', type: 'boolean', }, + excludedDnsDomains: { + editType: 'stringArray', + fieldGroup: 'default', + helpText: + 'Domains for which this certificate is not allowed to sign or issue child certificates (see https://tools.ietf.org/html/rfc5280#section-4.2.1.10).', + label: 'Excluded DNS Domains', + }, + excludedEmailAddresses: { + editType: 'stringArray', + fieldGroup: 'default', + helpText: + 'Email addresses for which this certificate is not allowed to sign or issue child certificates (see https://tools.ietf.org/html/rfc5280#section-4.2.1.10).', + label: 'Excluded email addresses', + }, + excludedIpRanges: { + editType: 'stringArray', + fieldGroup: 'default', + helpText: + 'IP ranges for which this certificate is not allowed to sign or issue child certificates (see https://tools.ietf.org/html/rfc5280#section-4.2.1.10). Ranges must be specified in the notation of IP address and prefix length, like "192.0.2.0/24" or "2001:db8::/32", as defined in RFC 4632 and RFC 4291.', + label: 'Excluded IP ranges', + }, + excludedUriDomains: { + editType: 'stringArray', + fieldGroup: 'default', + helpText: + 'URI domains for which this certificate is not allowed to sign or issue child certificates (see https://tools.ietf.org/html/rfc5280#section-4.2.1.10).', + label: 'Excluded URI domains', + }, format: { editType: 'string', helpText: @@ -1312,6 +1340,27 @@ const pki = { fieldGroup: 'default', label: 'Permitted DNS Domains', }, + permittedEmailAddresses: { + editType: 'stringArray', + fieldGroup: 'default', + helpText: + 'Email addresses for which this certificate is allowed to sign or issue child certificates (see https://tools.ietf.org/html/rfc5280#section-4.2.1.10).', + label: 'Permitted email adresses', + }, + permittedIpRanges: { + editType: 'stringArray', + fieldGroup: 'default', + helpText: + 'IP ranges for which this certificate is allowed to sign or issue child certificates (see https://tools.ietf.org/html/rfc5280#section-4.2.1.10). Ranges must be specified in the notation of IP address and prefix length, like "192.0.2.0/24" or "2001:db8::/32", as defined in RFC 4632 and RFC 4291.', + label: 'Permitted IP ranges', + }, + permittedUriDomains: { + editType: 'stringArray', + fieldGroup: 'default', + helpText: + 'URI domains for which this certificate is allowed to sign or issue child certificates (see https://tools.ietf.org/html/rfc5280#section-4.2.1.10).', + label: 'Permitted URI domains', + }, postalCode: { editType: 'stringArray', helpText: 'If set, Postal Code will be set to this value.', diff --git a/ui/tests/integration/components/policy-example-test.js b/ui/tests/integration/components/policy-example-test.js index 37659b0585c8..831f5c659fd2 100644 --- a/ui/tests/integration/components/policy-example-test.js +++ b/ui/tests/integration/components/policy-example-test.js @@ -50,7 +50,7 @@ module('Integration | Component | policy-example', function (hooks) { assert .dom(SELECTORS.policyDescription('rgp')) .hasText( - 'Role Governing Policies (RGPs) are tied to client tokens or identities which is similar to ACL policies . They use Sentinel as a language framework to enable fine-grained policy decisions.' + 'Role Governing Policies (RGPs) are tied to client tokens or identities which is similar to ACL policies. They use Sentinel as a language framework to enable fine-grained policy decisions.' ); });