Backport of PKI: Add a new leaf_not_after_behavior value to force erroring in all circumstances into release/1.18.x

builtin/logical/pki/backend_test.go

@@ -7224,6 +7224,94 @@ func TestGenerateRootCAWithAIA(t *testing.T) {
 	requireSuccessNonNilResponse(t, resp, err, "expected root generation to succeed")
 }
 
+// TestIssuance_AlwaysEnforceErr validates that we properly return an error in all request
+// types that go beyond the issuer's NotAfter
+func TestIssuance_AlwaysEnforceErr(t *testing.T) {
+	t.Parallel()
+	b, s := CreateBackendWithStorage(t)
+
+	resp, err := CBWrite(b, s, "root/generate/internal", map[string]interface{}{
+		"common_name": "root myvault.com",
+		"key_type":    "ec",
+		"ttl":         "10h",
+		"issuer_name": "root-ca",
+		"key_name":    "root-key",
+	})
+	requireSuccessNonNilResponse(t, resp, err, "expected root generation to succeed")
+
+	resp, err = CBPatch(b, s, "issuer/root-ca", map[string]interface{}{
+		"leaf_not_after_behavior": "always_enforce_err",
+	})
+	requireSuccessNonNilResponse(t, resp, err, "failed updating root issuer with always_enforce_err")
+
+	resp, err = CBWrite(b, s, "roles/test-role", map[string]interface{}{
+		"allow_any_name":         true,
+		"key_type":               "ec",
+		"allowed_serial_numbers": "*",
+	})
+
+	expectedErrContains := "cannot satisfy request, as TTL would result in notAfter"
+
+	// Make sure we fail on CA issuance requests now
+	t.Run("ca-issuance", func(t *testing.T) {
+		resp, err = CBWrite(b, s, "intermediate/generate/internal", map[string]interface{}{
+			"common_name": "myint.com",
+		})
+		requireSuccessNonNilResponse(t, resp, err, "failed generating intermediary CSR")
+		requireFieldsSetInResp(t, resp, "csr")
+		csr := resp.Data["csr"]
+
+		_, err = CBWrite(b, s, "issuer/root-ca/sign-intermediate", map[string]interface{}{
+			"csr":            csr,
+			"use_csr_values": true,
+			"ttl":            "60h",
+		})
+		require.ErrorContains(t, err, expectedErrContains, "sign-intermediate should have failed as root issuer leaf behavior is set to always_enforce_err")
+
+		// Make sure it works if we are under
+		resp, err = CBWrite(b, s, "issuer/root-ca/sign-intermediate", map[string]interface{}{
+			"csr":            csr,
+			"use_csr_values": true,
+			"ttl":            "30m",
+		})
+		requireSuccessNonNilResponse(t, resp, err, "sign-intermediate should have passed with a lower TTL value and always_enforce_err")
+	})
+
+	// Make sure we fail on leaf csr signing leaf as we always did for 'err'
+	t.Run("sign-leaf-csr", func(t *testing.T) {
+		_, csrPem := generateTestCsr(t, certutil.ECPrivateKey, 256)
+
+		resp, err = CBWrite(b, s, "issuer/root-ca/sign/test-role", map[string]interface{}{
+			"ttl": "60h",
+			"csr": csrPem,
+		})
+		require.ErrorContains(t, err, expectedErrContains, "expected error from sign csr got: %v", resp)
+
+		// Make sure it works if we are under
+		resp, err = CBWrite(b, s, "issuer/root-ca/sign/test-role", map[string]interface{}{
+			"ttl": "30m",
+			"csr": csrPem,
+		})
+		requireSuccessNonNilResponse(t, resp, err, "sign should have succeeded with a lower TTL and always_enforce_err")
+	})
+
+	// Make sure we fail on leaf csr signing leaf as we always did for 'err'
+	t.Run("issue-leaf-csr", func(t *testing.T) {
+		resp, err = CBWrite(b, s, "issuer/root-ca/issue/test-role", map[string]interface{}{
+			"ttl":         "60h",
+			"common_name": "leaf.example.com",
+		})
+		require.ErrorContains(t, err, expectedErrContains, "expected error from issue got: %v", resp)
+
+		// Make sure it works if we are under
+		resp, err = CBWrite(b, s, "issuer/root-ca/issue/test-role", map[string]interface{}{
+			"ttl":         "30m",
+			"common_name": "leaf.example.com",
+		})
+		requireSuccessNonNilResponse(t, resp, err, "issue should have worked with a lower TTL and always_enforce_err")
+	})
+}
+
 var (
 	initTest  sync.Once
 	rsaCAKey  string

builtin/logical/pki/issuing/issue_common.go

@@ -1008,7 +1008,7 @@ func ApplyIssuerLeafNotAfterBehavior(caSign *certutil.CAInfoBundle, notAfter tim
 			// Explicitly do nothing.
 		case certutil.TruncateNotAfterBehavior:
 			notAfter = caSign.Certificate.NotAfter
-		case certutil.ErrNotAfterBehavior:
+		case certutil.ErrNotAfterBehavior, certutil.AlwaysEnforceErr:
 			fallthrough
 		default:
 			return time.Time{}, errutil.UserError{Err: fmt.Sprintf(

builtin/logical/pki/path_acme_order.go

@@ -529,7 +529,8 @@ func issueCertFromCsr(ac *acmeContext, csr *x509.CertificateRequest) (*certutil.
 	}
 
 	// ACME issued cert will override the TTL values to truncate to the issuer's
-	// expiration if we go beyond, no matter the setting
+	// expiration if we go beyond, no matter the setting.
+	// Note that if set to certutil.AlwaysEnforceErr we will error out
 	if signingBundle.LeafNotAfterBehavior == certutil.ErrNotAfterBehavior {
 		signingBundle.LeafNotAfterBehavior = certutil.TruncateNotAfterBehavior
 	}

builtin/logical/pki/path_acme_test.go

@@ -710,6 +710,85 @@ func TestAcmeConfigChecksPublicAcmeEnv(t *testing.T) {
 	require.NoError(t, err)
 }
 
+// TestAcmeHonorsAlwaysEnforceErr verifies that we get an error and not truncated if the issuer's
+// leaf_not_after_behavior is set to always_enforce_err
+func TestAcmeHonorsAlwaysEnforceErr(t *testing.T) {
+	t.Parallel()
+
+	cluster, client, _ := setupAcmeBackend(t)
+	defer cluster.Cleanup()
+
+	testCtx, cancel := context.WithTimeout(context.Background(), 5*time.Minute)
+	defer cancel()
+
+	mount := "pki"
+	resp, err := client.Logical().WriteWithContext(context.Background(), mount+"/issuers/generate/intermediate/internal",
+		map[string]interface{}{
+			"key_name":    "short-key",
+			"key_type":    "ec",
+			"common_name": "test.com",
+		})
+	require.NoError(t, err, "failed creating intermediary CSR")
+	intermediateCSR := resp.Data["csr"].(string)
+
+	// Sign the intermediate CSR using /pki
+	resp, err = client.Logical().Write(mount+"/issuer/root-ca/sign-intermediate", map[string]interface{}{
+		"csr":     intermediateCSR,
+		"ttl":     "10m",
+		"max_ttl": "1h",
+	})
+	require.NoError(t, err, "failed signing intermediary CSR")
+	intermediateCertPEM := resp.Data["certificate"].(string)
+
+	// Configure the intermediate cert as the CA in /pki2
+	resp, err = client.Logical().Write(mount+"/issuers/import/cert", map[string]interface{}{
+		"pem_bundle": intermediateCertPEM,
+	})
+	require.NoError(t, err, "failed importing intermediary cert")
+	importedIssuersRaw := resp.Data["imported_issuers"].([]interface{})
+	require.Len(t, importedIssuersRaw, 1)
+	shortCaUuid := importedIssuersRaw[0].(string)
+
+	_, err = client.Logical().Write(mount+"/issuer/"+shortCaUuid, map[string]interface{}{
+		"leaf_not_after_behavior": "always_enforce_err",
+		"issuer_name":             "short-ca",
+	})
+	require.NoError(t, err, "failed updating issuer name")
+
+	baseAcmeURL := "/v1/pki/issuer/short-ca/acme/"
+	accountKey, err := rsa.GenerateKey(rand.Reader, 2048)
+	require.NoError(t, err, "failed creating rsa key")
+
+	acmeClient := getAcmeClientForCluster(t, cluster, baseAcmeURL, accountKey)
+
+	// Create new account
+	t.Logf("Testing register on %s", baseAcmeURL)
+	acct, err := acmeClient.Register(testCtx, &acme.Account{}, func(tosURL string) bool { return true })
+	require.NoError(t, err, "failed registering account")
+
+	// Create an order
+	t.Logf("Testing Authorize Order on %s", baseAcmeURL)
+	identifiers := []string{"*.localdomain"}
+	order, err := acmeClient.AuthorizeOrder(testCtx, []acme.AuthzID{
+		{Type: "dns", Value: identifiers[0]},
+	})
+	require.NoError(t, err, "failed creating order")
+
+	// HACK: Update authorization/challenge to completed as we can't really do it properly in this workflow
+	//       test.
+	markAuthorizationSuccess(t, client, acmeClient, acct, order)
+
+	// Build a proper CSR, with the correct name and signed with a different key works.
+	goodCr := &x509.CertificateRequest{DNSNames: []string{identifiers[0]}}
+	csrKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
+	require.NoError(t, err, "failed generated key for CSR")
+	csr, err := x509.CreateCertificateRequest(rand.Reader, goodCr, csrKey)
+	require.NoError(t, err, "failed generating csr")
+
+	_, _, err = acmeClient.CreateOrderCert(testCtx, order.FinalizeURL, csr, true)
+	require.ErrorContains(t, err, "cannot satisfy request, as TTL would result in notAfter", "failed finalizing order")
+}
+
 // TestAcmeTruncatesToIssuerExpiry make sure that if the selected issuer's expiry is shorter than the
 // CSR's selected TTL value in ACME and the issuer's leaf_not_after_behavior setting is set to Err,
 // we will override the configured behavior and truncate to the issuer's NotAfter

builtin/logical/pki/path_fetch_issuers.go

@@ -529,6 +529,8 @@ func (b *backend) pathUpdateIssuer(ctx context.Context, req *logical.Request, da
 	switch rawLeafBehavior {
 	case "err":
 		newLeafBehavior = certutil.ErrNotAfterBehavior
+	case "always_enforce_err":
+		newLeafBehavior = certutil.AlwaysEnforceErr
 	case "truncate":
 		newLeafBehavior = certutil.TruncateNotAfterBehavior
 	case "permit":
@@ -797,6 +799,8 @@ func (b *backend) pathPatchIssuer(ctx context.Context, req *logical.Request, dat
 		switch rawLeafBehavior {
 		case "err":
 			newLeafBehavior = certutil.ErrNotAfterBehavior
+		case "always_enforce_err":
+			newLeafBehavior = certutil.AlwaysEnforceErr
 		case "truncate":
 			newLeafBehavior = certutil.TruncateNotAfterBehavior
 		case "permit":

builtin/logical/pki/path_root.go

@@ -383,8 +383,10 @@ func (b *backend) pathIssuerSignIntermediate(ctx context.Context, req *logical.R
 		// Since we are signing an intermediate, we will by default truncate the
 		// signed intermediary in order to generate a valid intermediary chain. This
 		// was changed in 1.17.x as the default prior was PermitNotAfterBehavior
-		warnAboutTruncate = true
-		signingBundle.LeafNotAfterBehavior = certutil.TruncateNotAfterBehavior
+		if signingBundle.LeafNotAfterBehavior != certutil.AlwaysEnforceErr {
+			warnAboutTruncate = true
+			signingBundle.LeafNotAfterBehavior = certutil.TruncateNotAfterBehavior
+		}
 	}
 
 	useCSRValues := data.Get("use_csr_values").(bool)

changelog/28907.txt

@@ -0,0 +1,3 @@
+```release-note:improvement
+secret/pki: Introduce a new value `always_enforce_err` within `leaf_not_after_behavior` to force the error in all circumstances such as CA issuance and ACME requests if requested TTL values are beyond the issuer's NotAfter.
+```

sdk/helper/certutil/certutil_test.go

@@ -916,6 +916,10 @@ func TestNotAfterValues(t *testing.T) {
 	if PermitNotAfterBehavior != 2 {
 		t.Fatalf("Expected PermitNotAfterBehavior=%v to have value 2", PermitNotAfterBehavior)
 	}
+
+	if AlwaysEnforceErr != 3 {
+		t.Fatalf("Expected AlwaysEnforceErr=%v to have value 3", AlwaysEnforceErr)
+	}
 }
 
 func TestSignatureAlgorithmRoundTripping(t *testing.T) {

sdk/helper/certutil/types.go

@@ -708,9 +708,11 @@ const (
 	ErrNotAfterBehavior NotAfterBehavior = iota
 	TruncateNotAfterBehavior
 	PermitNotAfterBehavior
+	AlwaysEnforceErr
 )
 
 var notAfterBehaviorNames = map[NotAfterBehavior]string{
+	AlwaysEnforceErr:         "always_enforce_err",
 	ErrNotAfterBehavior:      "err",
 	TruncateNotAfterBehavior: "truncate",
 	PermitNotAfterBehavior:   "permit",

ui/app/models/pki/issuer.js

@@ -63,7 +63,7 @@ export default class PkiIssuerModel extends Model {
       'What happens when a leaf certificate is issued, but its NotAfter field (and therefore its expiry date) exceeds that of this issuer.',
     docLink: '/vault/api-docs/secret/pki#update-issuer',
     editType: 'yield',
-    valueOptions: ['err', 'truncate', 'permit'],
+    valueOptions: ['always_enforce_err', 'err', 'truncate', 'permit'],
   })
   leafNotAfterBehavior;
 

ui/lib/pki/addon/components/page/pki-issuer-edit.hbs

@@ -39,8 +39,13 @@
               >
                 {{#each field.options.valueOptions as |value|}}
                   <option selected={{eq @model.leafNotAfterBehavior value}} value={{value}}>
-                    {{capitalize (if (eq value "err") "error" value)}}
-                    if the computed NotAfter exceeds that of this issuer
+                    {{#if (eq value "always_enforce_err")}}
+                      Error if the computed NotAfter exceeds that of this issuer in all circumstances (leaf, CA issuance and
+                      ACME)
+                    {{else}}
+                      {{capitalize (if (eq value "err") "error" value)}}
+                      if the computed NotAfter exceeds that of this issuer
+                    {{/if}}
                   </option>
                 {{/each}}
               </select>

ui/tests/integration/components/pki/page/pki-issuer-edit-test.js

@@ -79,7 +79,7 @@ module('Integration | Component | pki | Page::PkiIssuerEditPage::PkiIssuerEdit',
     assert
       .dom(selectors.leafOption)
       .hasText(
-        'Error if the computed NotAfter exceeds that of this issuer',
+        'Error if the computed NotAfter exceeds that of this issuer in all circumstances (leaf, CA issuance and ACME)',
         'Correct text renders for leaf option'
       );
     assert.dom(selectors.usageCert).isChecked('Usage issuing certificates is checked');

website/content/api-docs/secret/pki/index.mdx

@@ -1086,7 +1086,9 @@ have access.**
    extension is missing from the CSR.
 
 - `enforce_leaf_not_after_behavior` `(bool: false)` - If true, do not apply the default truncate
-  behavior to the issued CA certificate, instead defer to the issuer's configured `leaf_not_after_behavior`
+  behavior to the issued CA certificate, instead defer to the issuer's configured `leaf_not_after_behavior`.
+  If an issuer's `leaf_not_after_behavior` is set to `always_enforce_err`, this flag is not required if
+  the desired behavior is to error out on requests who's TTL extends beyond the issuer's NotAfter.
 
 - `ttl` `(string: "")` - Specifies the requested Time To Live. Cannot be greater
   than the engine's `max_ttl` value. If not provided, the engine's `ttl` value
@@ -2608,8 +2610,12 @@ do so, import a new issuer and a new `issuer_id` will be assigned.
 
 - `leaf_not_after_behavior` `(string: "err")` - Behavior of a leaf's
   `NotAfter` field during issuance. Valid options are:
-
+  - `always_enforce_err` overrides all hardcoded behaviors to enforce an
+    error if any requested TTL is beyond the issuer. This applies to CA issuance,
+    and ACME issuance, along with the normal err on leaf certificates through Vault's API. (Available from 1.18.2+)
   - `err`, to error if the computed `NotAfter` exceeds that of this issuer;
+     - **Note** for CA issuance and ACME issuance this behavior is overridden
+       with truncate behavior, use `always_enforce_err` to disable these overrides
   - `truncate` to silently truncate the requested `NotAfter` value to that
     of this issuer; or
   - `permit` to allow this issuance to succeed with a `NotAfter` value