From a18187c643f1d28215856a773a4de4f87d7d39bd Mon Sep 17 00:00:00 2001 From: Alexander Scheel Date: Tue, 10 Jan 2023 10:04:30 -0500 Subject: [PATCH] Correctly distinguish empty issuer names in PKI (#18466) * Correctly distinguish empty issuer names When using client.Logical().JSONMergePatch(...) with an empty issuer name, patch incorrectly reports: > issuer name contained invalid characters In this case, both the error in getIssuerName(...) is incorrect and patch should allow setting an empty issuer name explicitly. Signed-off-by: Alexander Scheel * Add changelog Signed-off-by: Alexander Scheel * Add tests Signed-off-by: Alexander Scheel Signed-off-by: Alexander Scheel --- builtin/logical/pki/backend_test.go | 10 ++++++++++ builtin/logical/pki/path_fetch_issuers.go | 2 +- builtin/logical/pki/util.go | 12 +++++++----- changelog/18466.txt | 3 +++ 4 files changed, 21 insertions(+), 6 deletions(-) create mode 100644 changelog/18466.txt diff --git a/builtin/logical/pki/backend_test.go b/builtin/logical/pki/backend_test.go index 22b4cc5c4..e34ac07c3 100644 --- a/builtin/logical/pki/backend_test.go +++ b/builtin/logical/pki/backend_test.go @@ -5082,6 +5082,16 @@ func TestPerIssuerAIA(t *testing.T) { require.Equal(t, leafCert.IssuingCertificateURL, []string{"https://example.com/ca", "https://backup.example.com/ca"}) require.Equal(t, leafCert.OCSPServer, []string{"https://example.com/ocsp", "https://backup.example.com/ocsp"}) require.Equal(t, leafCert.CRLDistributionPoints, []string{"https://example.com/crl", "https://backup.example.com/crl"}) + + // Validate that we can set an issuer name and remove it. + _, err = CBPatch(b, s, "issuer/default", map[string]interface{}{ + "issuer_name": "my-issuer", + }) + require.NoError(t, err) + _, err = CBPatch(b, s, "issuer/default", map[string]interface{}{ + "issuer_name": "", + }) + require.NoError(t, err) } func TestIssuersWithoutCRLBits(t *testing.T) { diff --git a/builtin/logical/pki/path_fetch_issuers.go b/builtin/logical/pki/path_fetch_issuers.go index f50be774d..ae3710dfc 100644 --- a/builtin/logical/pki/path_fetch_issuers.go +++ b/builtin/logical/pki/path_fetch_issuers.go @@ -550,7 +550,7 @@ func (b *backend) pathPatchIssuer(ctx context.Context, req *logical.Request, dat var newName string if ok { newName, err = getIssuerName(sc, data) - if err != nil && err != errIssuerNameInUse { + if err != nil && err != errIssuerNameInUse && err != errIssuerNameIsEmpty { // If the error is name already in use, and the new name is the // old name for this issuer, we're not actually updating the // issuer name (or causing a conflict) -- so don't err out. Other diff --git a/builtin/logical/pki/util.go b/builtin/logical/pki/util.go index 23c3111fb..a71a4d017 100644 --- a/builtin/logical/pki/util.go +++ b/builtin/logical/pki/util.go @@ -28,9 +28,10 @@ const ( ) var ( - nameMatcher = regexp.MustCompile("^" + framework.GenericNameRegex(issuerRefParam) + "$") - errIssuerNameInUse = errutil.UserError{Err: "issuer name already in use"} - errKeyNameInUse = errutil.UserError{Err: "key name already in use"} + nameMatcher = regexp.MustCompile("^" + framework.GenericNameRegex(issuerRefParam) + "$") + errIssuerNameInUse = errutil.UserError{Err: "issuer name already in use"} + errIssuerNameIsEmpty = errutil.UserError{Err: "expected non-empty issuer name"} + errKeyNameInUse = errutil.UserError{Err: "key name already in use"} ) func serialFromCert(cert *x509.Certificate) string { @@ -159,11 +160,12 @@ func getIssuerName(sc *storageContext, data *framework.FieldData) (string, error issuerNameIface, ok := data.GetOk("issuer_name") if ok { issuerName = strings.TrimSpace(issuerNameIface.(string)) - + if len(issuerName) == 0 { + return issuerName, errIssuerNameIsEmpty + } if strings.ToLower(issuerName) == defaultRef { return issuerName, errutil.UserError{Err: "reserved keyword 'default' can not be used as issuer name"} } - if !nameMatcher.MatchString(issuerName) { return issuerName, errutil.UserError{Err: "issuer name contained invalid characters"} } diff --git a/changelog/18466.txt b/changelog/18466.txt new file mode 100644 index 000000000..220e058a1 --- /dev/null +++ b/changelog/18466.txt @@ -0,0 +1,3 @@ +```release-note:bug +secrets/pki: Allow patching issuer to set an empty issuer name. +```