diff --git a/builtin/logical/pki/path_acme_order.go b/builtin/logical/pki/path_acme_order.go index 2574c1c4f8..c0375e21bf 100644 --- a/builtin/logical/pki/path_acme_order.go +++ b/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 { diff --git a/builtin/logical/pki/path_acme_test.go b/builtin/logical/pki/path_acme_test.go index cee8fe3607..c1c751f69d 100644 --- a/builtin/logical/pki/path_acme_test.go +++ b/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() diff --git a/builtin/logical/pki/path_config_acme.go b/builtin/logical/pki/path_config_acme.go index 5125f5c003..ac021a4e9b 100644 --- a/builtin/logical/pki/path_config_acme.go +++ b/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:\" 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 { diff --git a/changelog/26797.txt b/changelog/26797.txt new file mode 100644 index 0000000000..72b6b38024 --- /dev/null +++ b/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 +``` diff --git a/website/content/api-docs/secret/pki.mdx b/website/content/api-docs/secret/pki.mdx index c199b4f40b..b7cd3a85e6 100644 --- a/website/content/api-docs/secret/pki.mdx +++ b/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 ``` diff --git a/website/content/docs/secrets/pki/considerations.mdx b/website/content/docs/secrets/pki/considerations.mdx index e570fb5c9a..0beff30fd9 100644 --- a/website/content/docs/secrets/pki/considerations.mdx +++ b/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.