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 <[email protected]> * Add changelog Signed-off-by: Alexander Scheel <[email protected]> * Add tests Signed-off-by: Alexander Scheel <[email protected]> Signed-off-by: Alexander Scheel <[email protected]>
hashicorp · Jan 14, 2023 · 0ac5d87 · 0ac5d87
1 parent 144de42
commit 0ac5d87
Show file tree

Hide file tree

Showing 4 changed files with 21 additions and 6 deletions.
diff --git 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
@@ -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
@@ -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
@@ -0,0 +1,3 @@
+```release-note:bug
+secrets/pki: Allow patching issuer to set an empty issuer name.
+```