Merge branch 'main' into ldap-add-samaccountname-login-option

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())
+			}
+		})
+	}
+}

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,

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.
+```

ui/lib/core/addon/components/policy-example.hbs

@@ -6,15 +6,16 @@
 <div class="has-bottom-margin-s">
   {{#if (eq @policyType "acl")}}
     <p data-test-example-modal-text="acl">
-      ACL Policies are written in Hashicorp Configuration Language (
-      <ExternalLink @href="https://github.com/hashicorp/hcl">HCL</ExternalLink>
+      <Hds::Link::Inline @href={{doc-link "/vault/tutorials/get-started/introduction-policies"}}>ACL Policies</Hds::Link::Inline>
+      are written in Hashicorp Configuration Language (
+      <Hds::Link::Inline @href="https://github.com/hashicorp/hcl">HCL</Hds::Link::Inline>
       ) or JSON and describe which paths in Vault a user or machine is allowed to access. Here is an example policy:
     </p>
   {{else if (eq @policyType "rgp")}}
     <p class="has-bottom-margin-s" data-test-example-modal-text="rgp">
       Role Governing Policies (RGPs) are tied to client tokens or identities which is similar to
-      <DocLink @path="/vault/tutorials/policies/policies">ACL policies</DocLink>. They use
-      <DocLink @path="/vault/docs/enterprise/sentinel">Sentinel</DocLink>
+      <Hds::Link::Inline @href={{doc-link "/vault/tutorials/policies/policies"}}>ACL policies</Hds::Link::Inline>. They use
+      <Hds::Link::Inline @href={{doc-link "/vault/docs/enterprise/sentinel"}}>Sentinel</Hds::Link::Inline>
       as a language framework to enable fine-grained policy decisions.
     </p>
     <p>
@@ -31,9 +32,9 @@
       Endpoint Governing Policies (EGPs) are tied to particular paths (e.g.
       <code class="tag is-marginless is-paddingless">aws/creds/</code>
       ) instead of tokens. They use
-      <ExternalLink @href="https://docs.hashicorp.com/sentinel/language">Sentinel</ExternalLink>
+      <Hds::Link::Inline @href="https://docs.hashicorp.com/sentinel/language">Sentinel</Hds::Link::Inline>
       as a language to access
-      <DocLink @path="/vault/docs/enterprise/sentinel/properties">properties</DocLink>
+      <Hds::Link::Inline @href={{doc-link "/vault/docs/enterprise/sentinel/properties"}}>properties</Hds::Link::Inline>
       of the incoming requests.
     </p>
     <p>
@@ -55,8 +56,11 @@
     More information about
     {{uppercase @policyType}}
     policies can be found
-    <DocLink @path={{get this.moreInformationLinks @policyType}} data-test-example-modal-information-link>
+    <Hds::Link::Inline
+      @href={{doc-link (get this.moreInformationLinks @policyType)}}
+      data-test-example-modal-information-link
+    >
       here.
-    </DocLink>
+    </Hds::Link::Inline>
   </p>
 </div>

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`);

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.',

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.'
       );
   });