PKI: Allow operators to increase the maximum TTL for ACME issued certificates (#26797)

* PKI: Allow operators to increase the maximum TTL for ACME issued certificates

* Add cl

builtin/logical/pki/path_acme_order.go

@@ -22,8 +22,6 @@ import (
 	"golang.org/x/net/idna"
 )
 
-var maxAcmeCertTTL = 90 * (24 * time.Hour)
-
 func pathAcmeListOrders(b *backend, baseUrl string, opts acmeWrapperOpts) *framework.Path {
 	return patternAcmeListOrders(b, baseUrl+"/orders", opts)
 }
@@ -546,9 +544,16 @@ func issueCertFromCsr(ac *acmeContext, csr *x509.CertificateRequest) (*certutil.
 		return nil, "", fmt.Errorf("failed computing certificate TTL from role/mount: %v: %w", err, ErrMalformed)
 	}
 
-	// Force a maximum 90 day TTL or lower for ACME
-	if time.Now().Add(maxAcmeCertTTL).Before(normalNotAfter) {
-		input.apiData.Raw["ttl"] = maxAcmeCertTTL
+	// We only allow ServerAuth key usage from ACME issued certs
+	// when configuration does not allow usage of ExtKeyusage field.
+	config, err := ac.sc.Backend.GetAcmeState().getConfigWithUpdate(ac.sc)
+	if err != nil {
+		return nil, "", fmt.Errorf("failed to fetch ACME configuration: %w", err)
+	}
+
+	// Force our configured max acme TTL
+	if time.Now().Add(config.MaxTTL).Before(normalNotAfter) {
+		input.apiData.Raw["ttl"] = config.MaxTTL.Seconds()
 	}
 
 	if csr.PublicKeyAlgorithm == x509.UnknownPublicKeyAlgorithm || csr.PublicKey == nil {
@@ -577,13 +582,6 @@ func issueCertFromCsr(ac *acmeContext, csr *x509.CertificateRequest) (*certutil.
 		return nil, "", fmt.Errorf("verification of parsed bundle failed: %w", err)
 	}
 
-	// We only allow ServerAuth key usage from ACME issued certs
-	// when configuration does not allow usage of ExtKeyusage field.
-	config, err := ac.sc.Backend.GetAcmeState().getConfigWithUpdate(ac.sc)
-	if err != nil {
-		return nil, "", fmt.Errorf("failed to fetch ACME configuration: %w", err)
-	}
-
 	if !config.AllowRoleExtKeyUsage {
 		for _, usage := range parsedBundle.Certificate.ExtKeyUsage {
 			if usage != x509.ExtKeyUsageServerAuth {

builtin/logical/pki/path_acme_test.go

@@ -310,7 +310,7 @@ func TestAcmeBasicWorkflow(t *testing.T) {
 			// Make sure the certificate has a NotAfter date of a maximum of 90 days
 			acmeCert, err := x509.ParseCertificate(certs[0])
 			require.NoError(t, err, "failed parsing acme cert bytes")
-			maxAcmeNotAfter := time.Now().Add(maxAcmeCertTTL)
+			maxAcmeNotAfter := time.Now().Add(defaultAcmeMaxTTL)
 			if maxAcmeNotAfter.Before(acmeCert.NotAfter) {
 				require.Fail(t, fmt.Sprintf("certificate has a NotAfter value %v greater than ACME max ttl %v", acmeCert.NotAfter, maxAcmeNotAfter))
 			}
@@ -1394,7 +1394,7 @@ func setupAcmeBackendOnClusterAtPath(t *testing.T, cluster *vault.TestCluster, c
 	return cluster, client, pathConfig
 }
 
-func testAcmeCertSignedByCa(t *testing.T, client *api.Client, derCerts [][]byte, issuerRef string) {
+func testAcmeCertSignedByCa(t *testing.T, client *api.Client, derCerts [][]byte, issuerRef string) *x509.Certificate {
 	t.Helper()
 	require.NotEmpty(t, derCerts)
 	acmeCert, err := x509.ParseCertificate(derCerts[0])
@@ -1418,6 +1418,8 @@ func testAcmeCertSignedByCa(t *testing.T, client *api.Client, derCerts [][]byte,
 	if diffs := deep.Equal(expectedCerts, derCerts); diffs != nil {
 		t.Fatalf("diffs were found between the acme chain returned and the expected value: \n%v", diffs)
 	}
+
+	return acmeCert
 }
 
 // TestAcmeValidationError make sure that we properly return errors on validation errors.
@@ -1613,6 +1615,92 @@ func TestAcmeRevocationAcrossAccounts(t *testing.T) {
 		"revocation time was not greater than 0, cert was not revoked: %v", revocationTimeInt)
 }
 
+// TestAcmeMaxTTL verify that we can update the ACME configuration's max_ttl value and
+// get a certificate that has a higher notAfter beyond the 90 day original limit
+func TestAcmeMaxTTL(t *testing.T) {
+	t.Parallel()
+	cluster, client, _ := setupAcmeBackend(t)
+	defer cluster.Cleanup()
+
+	testCtx, cancel := context.WithTimeout(context.Background(), 5*time.Minute)
+	defer cancel()
+
+	numHours := 140 * 24 // The ACME role has a TTL of 152 days
+	acmeConfig := map[string]interface{}{
+		"enabled":                  true,
+		"allowed_issuers":          "*",
+		"allowed_roles":            "*",
+		"default_directory_policy": "role:acme",
+		"dns_resolver":             "",
+		"eab_policy_name":          "",
+		"max_ttl":                  fmt.Sprintf("%dh", numHours),
+	}
+	_, err := client.Logical().WriteWithContext(testCtx, "pki/config/acme", acmeConfig)
+	require.NoError(t, err, "error configuring acme")
+
+	// First Create Our Client
+	accountKey, err := rsa.GenerateKey(rand.Reader, 2048)
+	require.NoError(t, err, "failed creating rsa key")
+	acmeClient := getAcmeClientForCluster(t, cluster, "/v1/pki/acme/", accountKey)
+
+	discovery, err := acmeClient.Discover(testCtx)
+	require.NoError(t, err, "failed acme discovery call")
+	t.Logf("%v", discovery)
+
+	acct, err := acmeClient.Register(testCtx, &acme.Account{
+		Contact: []string{"mailto:test@example.com"},
+	}, func(tosURL string) bool { return true })
+	require.NoError(t, err, "failed registering account")
+	require.Equal(t, acme.StatusValid, acct.Status)
+	require.Contains(t, acct.Contact, "mailto:test@example.com")
+	require.Len(t, acct.Contact, 1)
+
+	authorizations := []acme.AuthzID{
+		{"dns", "localhost"},
+	}
+	// Create an order
+	identifiers := make([]string, len(authorizations))
+	for index, auth := range authorizations {
+		identifiers[index] = auth.Value
+	}
+
+	createOrder, err := acmeClient.AuthorizeOrder(testCtx, authorizations)
+	require.NoError(t, err, "failed creating order")
+	require.Equal(t, acme.StatusPending, createOrder.Status)
+	require.Empty(t, createOrder.CertURL)
+	require.Equal(t, createOrder.URI+"/finalize", createOrder.FinalizeURL)
+	require.Len(t, createOrder.AuthzURLs, len(authorizations), "expected same number of authzurls as identifiers")
+
+	// HACK: Update authorization/challenge to completed as we can't really do it properly in this workflow
+	//       test.
+	markAuthorizationSuccess(t, client, acmeClient, acct, createOrder)
+
+	// Submit the CSR
+	requestCSR := x509.CertificateRequest{
+		Subject: pkix.Name{CommonName: "localhost"},
+	}
+	csrKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
+	require.NoError(t, err, "failed generated key for CSR")
+	csr, err := x509.CreateCertificateRequest(rand.Reader, &requestCSR, csrKey)
+	require.NoError(t, err, "failed generating csr")
+
+	certs, _, err := acmeClient.CreateOrderCert(testCtx, createOrder.FinalizeURL, csr, true)
+	require.NoError(t, err, "failed finalizing order")
+
+	// Validate we get a signed cert back
+	acmeCert := testAcmeCertSignedByCa(t, client, certs, "int-ca")
+	duration := time.Duration(numHours) * time.Hour
+	maxTTL := time.Now().Add(duration)
+	buffer := time.Duration(24) * time.Hour
+	dayTruncate := time.Duration(24) * time.Hour
+
+	acmeCertNotAfter := acmeCert.NotAfter.Truncate(dayTruncate)
+
+	// Make sure we are in the ballpark of our max_ttl value.
+	require.Greaterf(t, acmeCertNotAfter, maxTTL.Add(-1*buffer), "ACME cert: %v should have been greater than max TTL was %v", acmeCert.NotAfter, maxTTL)
+	require.Less(t, acmeCertNotAfter, maxTTL.Add(buffer), "ACME cert: %v should have been less than max TTL was %v", acmeCert.NotAfter, maxTTL)
+}
+
 func doACMEWorkflow(t *testing.T, vaultClient *api.Client, acmeClient *acme.Client) (*ecdsa.PrivateKey, [][]byte) {
 	testCtx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
 	defer cancel()

builtin/logical/pki/path_config_acme.go

@@ -11,6 +11,7 @@ import (
 	"regexp"
 	"strconv"
 	"strings"
+	"time"
 
 	"github.com/hashicorp/vault/helper/constants"
 	"github.com/hashicorp/vault/sdk/framework"
@@ -23,6 +24,7 @@ const (
 	pathConfigAcmeHelpSyn  = "Configuration of ACME Endpoints"
 	pathConfigAcmeHelpDesc = "Here we configure:\n\nenabled=false, whether ACME is enabled, defaults to false meaning that clusters will by default not get ACME support,\nallowed_issuers=\"default\", which issuers are allowed for use with ACME; by default, this will only be the primary (default) issuer,\nallowed_roles=\"*\", which roles are allowed for use with ACME; by default these will be all roles matching our selection criteria,\ndefault_directory_policy=\"\", either \"forbid\", preventing the default directory from being used at all, \"role:<role_name>\" which is the role to be used for non-role-qualified ACME requests; or \"sign-verbatim\", the default meaning ACME issuance will be equivalent to sign-verbatim.,\ndns_resolver=\"\", which specifies a custom DNS resolver to use for all ACME-related DNS lookups"
 	disableAcmeEnvVar      = "VAULT_DISABLE_PUBLIC_ACME"
+	defaultAcmeMaxTTL      = 90 * (24 * time.Hour)
 )
 
 type acmeConfigEntry struct {
@@ -33,6 +35,7 @@ type acmeConfigEntry struct {
 	DefaultDirectoryPolicy string        `json:"default_directory_policy"`
 	DNSResolver            string        `json:"dns_resolver"`
 	EabPolicyName          EabPolicyName `json:"eab_policy_name"`
+	MaxTTL                 time.Duration `json:"max_ttl"`
 }
 
 var defaultAcmeConfig = acmeConfigEntry{
@@ -43,6 +46,7 @@ var defaultAcmeConfig = acmeConfigEntry{
 	DefaultDirectoryPolicy: "sign-verbatim",
 	DNSResolver:            "",
 	EabPolicyName:          eabPolicyNotRequired,
+	MaxTTL:                 defaultAcmeMaxTTL,
 }
 
 var (
@@ -69,6 +73,11 @@ func (sc *storageContext) getAcmeConfig() (*acmeConfigEntry, error) {
 		return nil, errutil.InternalError{Err: fmt.Sprintf("unable to decode ACME configuration: %v", err)}
 	}
 
+	// Update previous stored configurations to use the default max ttl we used to enforce
+	if mapping.MaxTTL == 0 {
+		mapping.MaxTTL = defaultAcmeMaxTTL
+	}
+
 	return &mapping, nil
 }
 
@@ -129,6 +138,11 @@ func pathAcmeConfig(b *backend) *framework.Path {
 				Description: `Specify the policy to use for external account binding behaviour, 'not-required', 'new-account-required' or 'always-required'`,
 				Default:     "always-required",
 			},
+			"max_ttl": {
+				Type:        framework.TypeDurationSecond,
+				Description: `Specify the maximum TTL for ACME certificates. Role TTL values will be limited to this value`,
+				Default:     defaultAcmeMaxTTL.Seconds(),
+			},
 		},
 
 		Operations: map[logical.Operation]framework.OperationHandler{
@@ -183,6 +197,7 @@ func genResponseFromAcmeConfig(config *acmeConfigEntry, warnings []string) *logi
 			"enabled":                  config.Enabled,
 			"dns_resolver":             config.DNSResolver,
 			"eab_policy":               config.EabPolicyName,
+			"max_ttl":                  config.MaxTTL.Seconds(),
 		},
 		Warnings: warnings,
 	}
@@ -251,6 +266,14 @@ func (b *backend) pathAcmeWrite(ctx context.Context, req *logical.Request, d *fr
 		config.EabPolicyName = eabPolicy.Name
 	}
 
+	if maxTTLRaw, ok := d.GetOk("max_ttl"); ok {
+		maxTTL := time.Second * time.Duration(maxTTLRaw.(int))
+		if maxTTL <= 0 {
+			return nil, fmt.Errorf("invalid max_ttl value, must be greater than 0")
+		}
+		config.MaxTTL = maxTTL
+	}
+
 	// Validate Default Directory Behavior:
 	defaultDirectoryPolicyType, extraInfo, err := getDefaultDirectoryPolicyType(config.DefaultDirectoryPolicy)
 	if err != nil {

changelog/26797.txt

@@ -0,0 +1,3 @@
+```release-note:improvement
+secrets/pki: Add a new ACME configuration parameter that allows increasing the maximum TTL for ACME leaf certificates
+```

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

@@ -369,7 +369,8 @@ $ curl \
     "default_directory_policy": "sign-verbatim",
     "dns_resolver": "",
     "eab_policy": "not-required",
-    "enabled": true
+    "enabled": true,
+    "max_ttl": 776000
   },
 }
 ```
@@ -425,6 +426,10 @@ mount.
  - `enabled` `(bool: false)` - Whether ACME is enabled on this mount. When
    ACME is disabled, all requests to ACME directory URLs will return 404.
 
+ - `max_ttl` `(string: "")` - Specifies the maximum Time To Live provided as a
+   string duration with time suffix. Hour is the largest suffix. If not set,
+   defaults to the previous hard-coded behavior of 90 days (2160 hours).
+
 #### Sample payload
 
 ```

website/content/docs/secrets/pki/considerations.mdx

@@ -272,8 +272,9 @@ certificates on behalf of consumers. Vault's PKI Engine only includes server
 support for ACME; no client functionality has been included.
 
 ~> Note: Vault's PKI ACME server caps the certificate's validity at 90 days
-   maximum, regardless of role and/or global limits. Shorter validity
-   durations can be set via limiting the role's TTL to be under 90 days.
+   maximum by default, overridable using the ACME config max_ttl parameter.
+   Shorter validity durations can be set via limiting the role's TTL to
+   be under the global ACME configured limit.
    Aligning with Let's Encrypt, we do not support the optional `NotBefore`
    and `NotAfter` order request parameters.