Vault 6122 pki role issuer name validation (#15473)

* Add a warning when Issuing Certificate set on a role does not resolve.

* Ivanka's requests - add a warning on deleting issuer or changing it's name.

* reduce number of roles to iterate through; only verify roles after migration.  ignore roles deleted behind our back.

builtin/logical/pki/path_fetch_issuers.go

@@ -248,7 +248,9 @@ func (b *backend) pathUpdateIssuer(ctx context.Context, req *logical.Request, da
 
 	modified := false
 
+	var oldName string
 	if newName != issuer.Name {
+		oldName = issuer.Name
 		issuer.Name = newName
 		modified = true
 	}
@@ -309,7 +311,12 @@ func (b *backend) pathUpdateIssuer(ctx context.Context, req *logical.Request, da
 		}
 	}
 
-	return respondReadIssuer(issuer)
+	response, err := respondReadIssuer(issuer)
+	if newName != oldName {
+		addWarningOnDereferencing(oldName, response, ctx, req.Storage)
+	}
+
+	return response, err
 }
 
 func (b *backend) pathGetRawIssuer(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) {
@@ -399,15 +406,25 @@ func (b *backend) pathDeleteIssuer(ctx context.Context, req *logical.Request, da
 		return logical.ErrorResponse("unable to resolve issuer id for reference: " + issuerName), nil
 	}
 
+	var response *logical.Response
+	response = &logical.Response{}
+
+	issuer, err := fetchIssuerById(ctx, req.Storage, ref)
+	if err != nil {
+		return nil, err
+	}
+	if issuer.Name != "" {
+		addWarningOnDereferencing(issuer.Name, response, ctx, req.Storage)
+	}
+	addWarningOnDereferencing(string(issuer.ID), response, ctx, req.Storage)
+
 	wasDefault, err := deleteIssuer(ctx, req.Storage, ref)
 	if err != nil {
 		return nil, err
 	}
-
-	var response *logical.Response
 	if wasDefault {
-		response = &logical.Response{}
 		response.AddWarning(fmt.Sprintf("Deleted issuer %v (via issuer_ref %v); this was configured as the default issuer. Operations without an explicit issuer will not work until a new default is configured.", ref, issuerName))
+		addWarningOnDereferencing(defaultRef, response, ctx, req.Storage)
 	}
 
 	// Since we've deleted an issuer, the chains might've changed. Call the
@@ -422,6 +439,21 @@ func (b *backend) pathDeleteIssuer(ctx context.Context, req *logical.Request, da
 	return response, nil
 }
 
+func addWarningOnDereferencing(name string, resp *logical.Response, ctx context.Context, s logical.Storage) {
+	timeout, inUseBy, err := checkForRolesReferencing(name, ctx, s)
+	if err != nil || timeout {
+		if inUseBy == 0 {
+			resp.AddWarning(fmt.Sprint("Unable to check if any roles referenced this issuer by ", name))
+		} else {
+			resp.AddWarning(fmt.Sprint("The name ", name, " was in use by at least ", inUseBy, " roles"))
+		}
+	} else {
+		if inUseBy > 0 {
+			resp.AddWarning(fmt.Sprint(inUseBy, " roles reference ", name))
+		}
+	}
+}
+
 const (
 	pathGetIssuerHelpSyn  = `Fetch a single issuer certificate.`
 	pathGetIssuerHelpDesc = `

builtin/logical/pki/path_roles.go

@@ -726,6 +726,25 @@ func (b *backend) pathRoleCreate(ctx context.Context, req *logical.Request, data
 	if len(entry.Issuer) == 0 {
 		entry.Issuer = defaultRef
 	}
+	// Check that the issuers reference set resolves to something
+	if !b.useLegacyBundleCaStorage() {
+		issuerId, err := resolveIssuerReference(ctx, req.Storage, entry.Issuer)
+		if err != nil {
+			if issuerId == IssuerRefNotFound {
+				if resp == nil {
+					resp = &logical.Response{}
+				}
+				if entry.Issuer == defaultRef {
+					resp.AddWarning("Issuing Certificate was set to default, but no default issuing certificate (configurable at /config/issuers) is currently set")
+				} else {
+					resp.AddWarning(fmt.Sprintf("Issuing Certificate was set to %s but no issuing certificate currently has that name", entry.Issuer))
+				}
+			} else {
+				return nil, err
+			}
+		}
+
+	}
 
 	// Store it
 	jsonEntry, err := logical.StorageEntryJSON("role/"+name, entry)

builtin/logical/pki/storage.go

@@ -27,6 +27,9 @@ const (
 
 	// Used as a quick sanity check for a reference id lookups...
 	uuidLength = 36
+
+	maxRolesToScanOnIssuerChange = 100
+	maxRolesToFindOnIssuerChange = 10
 )
 
 type keyID string
@@ -910,3 +913,39 @@ func isKeyInUse(keyId string, ctx context.Context, s logical.Storage) (inUse boo
 
 	return false, "", nil
 }
+
+func checkForRolesReferencing(issuerId string, ctx context.Context, storage logical.Storage) (timeout bool, inUseBy int32, err error) {
+	roleEntries, err := storage.List(ctx, "role/")
+	if err != nil {
+		return false, 0, err
+	}
+
+	inUseBy = 0
+	checkedRoles := 0
+
+	for _, roleName := range roleEntries {
+		entry, err := storage.Get(ctx, "role/"+roleName)
+		if err != nil {
+			return false, 0, err
+		}
+		if entry != nil { // If nil, someone deleted an entry since we haven't taken a lock here so just continue
+			var role roleEntry
+			err = entry.DecodeJSON(&role)
+			if err != nil {
+				return false, inUseBy, err
+			}
+			if role.Issuer == issuerId {
+				inUseBy = inUseBy + 1
+				if inUseBy >= maxRolesToFindOnIssuerChange {
+					return true, inUseBy, nil
+				}
+			}
+		}
+		checkedRoles = checkedRoles + 1
+		if checkedRoles >= maxRolesToScanOnIssuerChange {
+			return true, inUseBy, nil
+		}
+	}
+
+	return false, inUseBy, nil
+}