From 25364ebb61c9515fad15e2ce9a605b9aabfb6256 Mon Sep 17 00:00:00 2001 From: hc-github-team-secure-vault-core <82990506+hc-github-team-secure-vault-core@users.noreply.github.com> Date: Wed, 14 Jun 2023 14:08:57 -0400 Subject: [PATCH] backport of commit 052719b9a824da4ee368c3d982070ab290846970 (#21226) Co-authored-by: Alexander Scheel --- builtin/logical/pki/backend_test.go | 46 +++++++++++++++++++++++------ builtin/logical/pki/cert_util.go | 21 ++++++++++++- changelog/21209.txt | 3 ++ 3 files changed, 60 insertions(+), 10 deletions(-) create mode 100644 changelog/21209.txt diff --git a/builtin/logical/pki/backend_test.go b/builtin/logical/pki/backend_test.go index 6e7791993..da843f2a6 100644 --- a/builtin/logical/pki/backend_test.go +++ b/builtin/logical/pki/backend_test.go @@ -6125,22 +6125,23 @@ func TestPKI_TemplatedAIAs(t *testing.T) { _, err = CBWrite(b, s, "config/urls", aiaData) require.NoError(t, err) - // But root generation will fail. + // Root generation should succeed, but without AIA info. rootData := map[string]interface{}{ "common_name": "Long-Lived Root X1", "issuer_name": "long-root-x1", "key_type": "ec", } - _, err = CBWrite(b, s, "root/generate/internal", rootData) - require.Error(t, err) - require.Contains(t, err.Error(), "unable to parse AIA URL") + resp, err = CBWrite(b, s, "root/generate/internal", rootData) + require.NoError(t, err) + _, err = CBDelete(b, s, "root") + require.NoError(t, err) - // Clearing the config and regenerating the root should succeed. + // Clearing the config and regenerating the root should still succeed. _, err = CBWrite(b, s, "config/urls", map[string]interface{}{ - "crl_distribution_points": "", - "issuing_certificates": "", - "ocsp_servers": "", - "enable_templating": false, + "crl_distribution_points": "{{cluster_path}}/issuer/my-root-id/crl/der", + "issuing_certificates": "{{cluster_aia_path}}/issuer/my-root-id/der", + "ocsp_servers": "{{cluster_path}}/ocsp", + "enable_templating": true, }) require.NoError(t, err) resp, err = CBWrite(b, s, "root/generate/internal", rootData) @@ -7098,6 +7099,33 @@ func TestPatchIssuer(t *testing.T) { } } +func TestGenerateRootCAWithAIA(t *testing.T) { + // Generate a root CA at /pki-root + b_root, s_root := CreateBackendWithStorage(t) + + // Setup templated AIA information + _, err := CBWrite(b_root, s_root, "config/cluster", map[string]interface{}{ + "path": "https://localhost:8200", + "aia_path": "https://localhost:8200", + }) + require.NoError(t, err, "failed to write AIA settings") + + _, err = CBWrite(b_root, s_root, "config/urls", map[string]interface{}{ + "crl_distribution_points": "{{cluster_path}}/issuer/{{issuer_id}}/crl/der", + "issuing_certificates": "{{cluster_aia_path}}/issuer/{{issuer_id}}/der", + "ocsp_servers": "{{cluster_path}}/ocsp", + "enable_templating": true, + }) + require.NoError(t, err, "failed to write AIA settings") + + // Write a root issuer, this should succeed. + resp, err := CBWrite(b_root, s_root, "root/generate/exported", map[string]interface{}{ + "common_name": "root myvault.com", + "key_type": "ec", + }) + requireSuccessNonNilResponse(t, resp, err, "expected root generation to succeed") +} + var ( initTest sync.Once rsaCAKey string diff --git a/builtin/logical/pki/cert_util.go b/builtin/logical/pki/cert_util.go index 1795fcab8..e6f49f1a3 100644 --- a/builtin/logical/pki/cert_util.go +++ b/builtin/logical/pki/cert_util.go @@ -773,7 +773,26 @@ func generateCert(sc *storageContext, uris, err := entries.toURLEntries(sc, issuerID("")) if err != nil { - return nil, nil, errutil.InternalError{Err: fmt.Sprintf("unable to parse AIA URL information: %v\nUsing templated AIA URL's {{issuer_id}} field when generating root certificates is not supported.", err)} + // When generating root issuers, don't err on missing issuer + // ID; there is little value in including AIA info on a root, + // as this info would point back to itself; though RFC 5280 is + // a touch vague on this point, this seems to be consensus + // from public CAs such as DigiCert Global Root G3, ISRG Root + // X1, and others. + // + // This is a UX bug if we do err here, as it requires AIA + // templating to not include issuer id (a best practice for + // child certs issued from root and intermediate mounts + // however), and setting this before root generation (or, on + // root renewal) could cause problems. + if _, nonEmptyIssuerErr := entries.toURLEntries(sc, issuerID("empty-issuer-id")); nonEmptyIssuerErr != nil { + return nil, nil, errutil.InternalError{Err: fmt.Sprintf("unable to parse AIA URL information: %v\nUsing templated AIA URL's {{issuer_id}} field when generating root certificates is not supported.", err)} + } + + uris = &certutil.URLEntries{} + + msg := "When generating root CA, found global AIA configuration with issuer_id template unsuitable for root generation. This AIA configuration has been ignored. To include AIA on this root CA, set the global AIA configuration to not include issuer_id and instead to refer to a static issuer name." + warnings = append(warnings, msg) } data.Params.URLs = uris diff --git a/changelog/21209.txt b/changelog/21209.txt new file mode 100644 index 000000000..31ddf413c --- /dev/null +++ b/changelog/21209.txt @@ -0,0 +1,3 @@ +```release-note:change +secrets/pki: Allow issuance of root CAs without AIA, when templated AIA information includes issuer_id. +```