PKI: Change sign-intermediate to truncate notAfter by default (behavior change) (#26796)

* PKI: Change sign-intermediate to truncate notAfter by default

 - The PKI sign-intermediate API allowed an end-user to request a TTL
   value that would extend beyond the signing issuer's notAfter. This would
   generate an invalid CA chain when properly validated.
 - We are now changing the default behavior to truncate the returned certificate
   to the signing issuer's notAfter.
 - End-users can get the old behavior by configuring the signing issuer's
   leaf_not_after_behavior field to permit, and call sign-intermediary
   with the new argument enforce_leaf_not_after_behavior to true. The
   new argument could also be used to enforce an error instead of truncating
   behavior if the signing issuer's leaf_not_after_behavior is set to err.

* Add cl

* Add cl and upgrade note

* Apply suggestions from code review

Co-authored-by: Sarah Chavis <62406755+schavis@users.noreply.github.com>

---------

Co-authored-by: Sarah Chavis <62406755+schavis@users.noreply.github.com>

builtin/logical/pki/backend_test.go

@@ -2539,6 +2539,59 @@ func TestBackend_Root_Idempotency(t *testing.T) {
 	}
 }
 
+// TestBackend_SignIntermediate_EnforceLeafFlag verifies if the flag is true
+// that we will leverage the issuer's configured behavior
+func TestBackend_SignIntermediate_EnforceLeafFlag(t *testing.T) {
+	t.Parallel()
+	b, s := CreateBackendWithStorage(t)
+
+	resp, err := CBWrite(b, s, "root/generate/internal", map[string]interface{}{
+		"ttl":         "40h",
+		"common_name": "myvault.com",
+	})
+	require.NoError(t, err, "failed generating root cert")
+	rootCert := parseCert(t, resp.Data["certificate"].(string))
+
+	_, err = CBWrite(b, s, "issuer/default", map[string]interface{}{
+		"leaf_not_after_behavior": "err",
+	})
+	require.NoError(t, err, "failed updating root issuer cert behavior")
+
+	resp, err = CBWrite(b, s, "intermediate/generate/internal", map[string]interface{}{
+		"common_name": "myint.com",
+	})
+	require.NoError(t, err, "failed generating intermediary CSR")
+	csr := resp.Data["csr"]
+
+	_, err = CBWrite(b, s, "root/sign-intermediate", map[string]interface{}{
+		"common_name":                     "myint.com",
+		"other_sans":                      "1.3.6.1.4.1.311.20.2.3;utf8:caadmin@example.com",
+		"csr":                             csr,
+		"ttl":                             "60h",
+		"enforce_leaf_not_after_behavior": true,
+	})
+	require.Error(t, err, "sign-intermediate should have failed as root issuer leaf behavior is set to err")
+
+	// Now test with permit, the old default behavior
+	_, err = CBWrite(b, s, "issuer/default", map[string]interface{}{
+		"leaf_not_after_behavior": "permit",
+	})
+	require.NoError(t, err, "failed updating root issuer cert behavior to permit")
+
+	resp, err = CBWrite(b, s, "root/sign-intermediate", map[string]interface{}{
+		"common_name":                     "myint.com",
+		"other_sans":                      "1.3.6.1.4.1.311.20.2.3;utf8:caadmin@example.com",
+		"csr":                             csr,
+		"ttl":                             "60h",
+		"enforce_leaf_not_after_behavior": true,
+	})
+	require.NoError(t, err, "failed to sign intermediary CA with permit as issuer")
+	intCert := parseCert(t, resp.Data["certificate"].(string))
+
+	require.Truef(t, rootCert.NotAfter.Before(intCert.NotAfter),
+		"root cert notAfter %v was not before ca cert's notAfter %v", rootCert.NotAfter, intCert.NotAfter)
+}
+
 func TestBackend_SignIntermediate_AllowedPastCAValidity(t *testing.T) {
 	t.Parallel()
 	b_root, s_root := CreateBackendWithStorage(t)
@@ -2546,7 +2599,7 @@ func TestBackend_SignIntermediate_AllowedPastCAValidity(t *testing.T) {
 	var err error
 
 	// Direct issuing from root
-	_, err = CBWrite(b_root, s_root, "root/generate/internal", map[string]interface{}{
+	resp, err := CBWrite(b_root, s_root, "root/generate/internal", map[string]interface{}{
 		"ttl":         "40h",
 		"common_name": "myvault.com",
 	})
@@ -2554,6 +2607,8 @@ func TestBackend_SignIntermediate_AllowedPastCAValidity(t *testing.T) {
 		t.Fatal(err)
 	}
 
+	rootCert := parseCert(t, resp.Data["certificate"].(string))
+
 	_, err = CBWrite(b_root, s_root, "roles/test", map[string]interface{}{
 		"allow_bare_domains": true,
 		"allow_subdomains":   true,
@@ -2563,7 +2618,7 @@ func TestBackend_SignIntermediate_AllowedPastCAValidity(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	resp, err := CBWrite(b_int, s_int, "intermediate/generate/internal", map[string]interface{}{
+	resp, err = CBWrite(b_int, s_int, "intermediate/generate/internal", map[string]interface{}{
 		"common_name": "myint.com",
 	})
 	schema.ValidateResponse(t, schema.GetResponseSchema(t, b_root.Route("intermediate/generate/internal"), logical.UpdateOperation), resp, true)
@@ -2614,6 +2669,9 @@ func TestBackend_SignIntermediate_AllowedPastCAValidity(t *testing.T) {
 	cert := parseCert(t, resp.Data["certificate"].(string))
 	certSkid := certutil.GetHexFormatted(cert.SubjectKeyId, ":")
 	require.Equal(t, intSkid, certSkid)
+
+	require.Equal(t, rootCert.NotAfter, cert.NotAfter, "intermediary cert's NotAfter did not match root cert's NotAfter")
+	require.Contains(t, resp.Warnings, intCaTruncatationWarning, "missing warning about intermediary CA notAfter truncation")
 }
 
 func TestBackend_ConsulSignLeafWithLegacyRole(t *testing.T) {

builtin/logical/pki/path_root.go

@@ -29,6 +29,8 @@ import (
 	"golang.org/x/crypto/ed25519"
 )
 
+const intCaTruncatationWarning = "the signed intermediary CA certificate's notAfter was truncated to the issuer's notAfter"
+
 func pathGenerateRoot(b *backend) *framework.Path {
 	pattern := "root/generate/" + framework.GenericNameRegex("exported")
 
@@ -375,10 +377,14 @@ func (b *backend) pathIssuerSignIntermediate(ctx context.Context, req *logical.R
 		}
 	}
 
-	// Since we are signing an intermediate, we explicitly want to override
-	// the leaf NotAfterBehavior to permit issuing intermediates longer than
-	// the life of this issuer.
-	signingBundle.LeafNotAfterBehavior = certutil.PermitNotAfterBehavior
+	warnAboutTruncate := false
+	if enforceLeafNotAfter := data.Get("enforce_leaf_not_after_behavior").(bool); !enforceLeafNotAfter {
+		// 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
+	}
 
 	useCSRValues := data.Get("use_csr_values").(bool)
 
@@ -418,6 +424,11 @@ func (b *backend) pathIssuerSignIntermediate(ctx context.Context, req *logical.R
 		return nil, err
 	}
 
+	if warnAboutTruncate &&
+		signingBundle.Certificate.NotAfter.Equal(parsedBundle.Certificate.NotAfter) {
+		resp.AddWarning(intCaTruncatationWarning)
+	}
+
 	return resp, nil
 }
 

builtin/logical/pki/path_sign_issuers.go

@@ -36,6 +36,11 @@ func pathSignIntermediate(b *backend) *framework.Path {
 
 func buildPathIssuerSignIntermediateRaw(b *backend, pattern string, displayAttrs *framework.DisplayAttributes) *framework.Path {
 	fields := addIssuerRefField(map[string]*framework.FieldSchema{})
+	fields["enforce_leaf_not_after_behavior"] = &framework.FieldSchema{
+		Type:        framework.TypeBool,
+		Default:     false,
+		Description: "Do not truncate the NotAfter field, use the issuer's configured leaf_not_after_behavior",
+	}
 	path := &framework.Path{
 		Pattern:      pattern,
 		DisplayAttrs: displayAttrs,

changelog/26796.txt

@@ -0,0 +1,3 @@
+```release-note:change
+secrets/pki: sign-intermediate API will truncate notAfter if calculated to go beyond the signing issuer's notAfter. Previously the notAfter was permitted to go beyond leading to invalid chains.
+```

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

@@ -1028,6 +1028,10 @@ when signing an externally-owned intermediate.
    Access to this endpoint should be restricted by policy to only trusted
    operators.
 
+By default, Vault truncates the `notAfter` field of the signed intermediary
+to use the `notAfter` field of the signing issuer, it the computed field for the
+intermediary goes beyond the prescribed length.
+
 | Method | Path                                        | Issuer    |
 | :----- | :------------------------------------------ | :-------  |
 | `POST` | `/pki/root/sign-intermediate`               | `default` |
@@ -1416,6 +1420,9 @@ have access.**
 ~> Note: This value is only used as a default when the `ExtendedKeyUsage`
    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`
+
 - `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
   will be used, which defaults to system values if not explicitly set. See

website/content/docs/upgrading/upgrade-to-1.17.x.mdx

@@ -0,0 +1,39 @@
+---
+layout: docs
+page_title: Upgrade to Vault 1.17.x - Guides
+description: |-
+  Deprecations, important or breaking changes, and remediation recommendations
+  for anyone upgrading to 1.17.x from Vault 1.16.x.
+---
+
+# Overview
+
+The Vault 1.17.x upgrade guide contains information on deprecations, important
+or breaking changes, and remediation recommendations for anyone upgrading from
+Vault 1.16. **Please read carefully**.
+
+## Important changes
+
+### PKI sign-intermediate now truncates notAfter field to signing issuer
+
+Prior to 1.17.x, Vault allowed the calculated sign-intermediate `notAfter` field
+to go beyond the signing issuer `notAfter` field. The extended value lead to a
+CA chain that would not validate properly. As of 1.17.x, Vault truncates the
+intermediary `notAfter` value to the signing issuer `notAfter` if the calculated
+field is greater.
+
+#### How to opt out
+
+You can use the new `enforce_leaf_not_after_behavior` flag on the
+sign-intermediate API along with the `leaf_not_after_behavior` flag for the
+signing issuer to opt out of the truncating behavior.
+
+When you set `enforce_leaf_not_after_behavior` to true, the sign-intermediate
+API uses the `leaf_not_after_behavior` value configured for the signing issuer
+to control truncation the behavior. Setting the issuer `leaf_not_after_behavior`
+field to `permit` and `enforce_leaf_not_after_behavior` to true restores the
+legacy behavior.
+
+## Known issues and workarounds
+
+@include 'known-issues/ocsp-redirect.mdx'

website/data/docs-nav-data.json

@@ -2282,6 +2282,10 @@
         "title": "Upgrade to Raft WAL",
         "path": "upgrading/raft-wal"
       },
+      {
+        "title": "Upgrade to 1.17.x",
+        "path": "upgrading/upgrade-to-1.17.x"
+      },
       {
         "title": "Upgrade to 1.16.x",
         "path": "upgrading/upgrade-to-1.16.x"