Address some small issues within pki health-check (#19295)

* Address some small issues within pki health-check

 - Notify user yaml output mode is not support with --list argument
 - Output pure JSON in json output mode with --list argument
 - If a checker returns a nil response, convert to an empty slice
 - Add handler for permission errors to too many certs checker
 - Add checks for permission issues within hardware_backed_root and root_issued_leaves

* Identify the role that contained the permission issue in role based checks

 - Augument the role health checks to identify the role(s) that we have
   insufficient permissions to read instead of an overall read failure
 - Treat the failure to list roles as a complete failure for the check

command/healthcheck/healthcheck.go

@@ -119,6 +119,10 @@ func (e *Executor) Execute() (map[string][]*Result, error) {
 			return nil, fmt.Errorf("failed to evaluate %v: %w", checker.Name(), err)
 		}
 
+		if results == nil {
+			results = []*Result{}
+		}
+
 		for _, result := range results {
 			result.Endpoint = e.templatePath(result.Endpoint)
 			result.StatusDisplay = ResultStatusNameMap[result.Status]

command/healthcheck/pki_hardware_backed_root.go

@@ -13,12 +13,14 @@ type HardwareBackedRoot struct {
 
 	UnsupportedVersion bool
 
+	FetchIssues  map[string]*PathFetch
 	IssuerKeyMap map[string]string
 	KeyIsManaged map[string]string
 }
 
 func NewHardwareBackedRootCheck() Check {
 	return &HardwareBackedRoot{
+		FetchIssues:  make(map[string]*PathFetch),
 		IssuerKeyMap: make(map[string]string),
 		KeyIsManaged: make(map[string]string),
 	}
@@ -64,6 +66,7 @@ func (h *HardwareBackedRoot) FetchResources(e *Executor) error {
 			if err != nil {
 				return err
 			}
+			h.FetchIssues[issuer] = ret
 			continue
 		}
 
@@ -83,13 +86,15 @@ func (h *HardwareBackedRoot) FetchResources(e *Executor) error {
 		}
 
 		h.IssuerKeyMap[issuer] = keyId
-		skip, _, keyEntry, err := pkiFetchKeyEntry(e, keyId, func() {
+		skip, ret, keyEntry, err := pkiFetchKeyEntry(e, keyId, func() {
 			h.UnsupportedVersion = true
 		})
 		if skip || err != nil || keyEntry == nil {
 			if err != nil {
 				return err
 			}
+
+			h.FetchIssues[issuer] = ret
 			continue
 		}
 
@@ -112,6 +117,25 @@ func (h *HardwareBackedRoot) Evaluate(e *Executor) (results []*Result, err error
 		return []*Result{&ret}, nil
 	}
 
+	for issuer, fetchPath := range h.FetchIssues {
+		if fetchPath != nil && fetchPath.IsSecretPermissionsError() {
+			delete(h.IssuerKeyMap, issuer)
+			ret := Result{
+				Status:   ResultInsufficientPermissions,
+				Endpoint: fetchPath.Path,
+				Message:  "Without this information, this health check is unable to function.",
+			}
+
+			if e.Client.Token() == "" {
+				ret.Message = "No token available so unable for the endpoint for this mount. " + ret.Message
+			} else {
+				ret.Message = "This token lacks permission for the endpoint for this mount. " + ret.Message
+			}
+
+			results = append(results, &ret)
+		}
+	}
+
 	for name, keyId := range h.IssuerKeyMap {
 		var ret Result
 		ret.Status = ResultInformational

command/healthcheck/pki_role_allows_glob_wildcards.go

@@ -10,14 +10,16 @@ import (
 type RoleAllowsGlobWildcards struct {
 	Enabled            bool
 	UnsupportedVersion bool
-	NoPerms            bool
 
-	RoleEntryMap map[string]map[string]interface{}
+	RoleListFetchIssue *PathFetch
+	RoleFetchIssues    map[string]*PathFetch
+	RoleEntryMap       map[string]map[string]interface{}
 }
 
 func NewRoleAllowsGlobWildcardsCheck() Check {
 	return &RoleAllowsGlobWildcards{
-		RoleEntryMap: make(map[string]map[string]interface{}),
+		RoleFetchIssues: make(map[string]*PathFetch),
+		RoleEntryMap:    make(map[string]map[string]interface{}),
 	}
 }
 
@@ -49,7 +51,7 @@ func (h *RoleAllowsGlobWildcards) FetchResources(e *Executor) error {
 	})
 	if exit || err != nil {
 		if f != nil && f.IsSecretPermissionsError() {
-			h.NoPerms = true
+			h.RoleListFetchIssue = f
 		}
 		return err
 	}
@@ -60,7 +62,7 @@ func (h *RoleAllowsGlobWildcards) FetchResources(e *Executor) error {
 		})
 		if skip || err != nil || entry == nil {
 			if f != nil && f.IsSecretPermissionsError() {
-				h.NoPerms = true
+				h.RoleFetchIssues[role] = f
 			}
 			if err != nil {
 				return err
@@ -84,18 +86,37 @@ func (h *RoleAllowsGlobWildcards) Evaluate(e *Executor) (results []*Result, err
 		}
 		return []*Result{&ret}, nil
 	}
-	if h.NoPerms {
+	if h.RoleListFetchIssue != nil && h.RoleListFetchIssue.IsSecretPermissionsError() {
 		ret := Result{
 			Status:   ResultInsufficientPermissions,
-			Endpoint: "/{{mount}}/roles",
-			Message:  "lacks permission either to list the roles or to read a specific role. This may restrict the ability to fully execute this health check.",
+			Endpoint: h.RoleListFetchIssue.Path,
+			Message:  "lacks permission either to list the roles. This restricts the ability to fully execute this health check.",
 		}
 		if e.Client.Token() == "" {
 			ret.Message = "No token available and so this health check " + ret.Message
 		} else {
 			ret.Message = "This token " + ret.Message
 		}
-		results = append(results, &ret)
+		return []*Result{&ret}, nil
+	}
+
+	for role, fetchPath := range h.RoleFetchIssues {
+		if fetchPath != nil && fetchPath.IsSecretPermissionsError() {
+			delete(h.RoleEntryMap, role)
+			ret := Result{
+				Status:   ResultInsufficientPermissions,
+				Endpoint: fetchPath.Path,
+				Message:  "Without this information, this health check is unable to function.",
+			}
+
+			if e.Client.Token() == "" {
+				ret.Message = "No token available so unable for the endpoint for this mount. " + ret.Message
+			} else {
+				ret.Message = "This token lacks permission the endpoint for this mount. " + ret.Message
+			}
+
+			results = append(results, &ret)
+		}
 	}
 
 	for role, entry := range h.RoleEntryMap {

command/healthcheck/pki_role_allows_localhost.go

@@ -9,14 +9,16 @@ import (
 type RoleAllowsLocalhost struct {
 	Enabled            bool
 	UnsupportedVersion bool
-	NoPerms            bool
 
-	RoleEntryMap map[string]map[string]interface{}
+	RoleListFetchIssue *PathFetch
+	RoleFetchIssues    map[string]*PathFetch
+	RoleEntryMap       map[string]map[string]interface{}
 }
 
 func NewRoleAllowsLocalhostCheck() Check {
 	return &RoleAllowsLocalhost{
-		RoleEntryMap: make(map[string]map[string]interface{}),
+		RoleFetchIssues: make(map[string]*PathFetch),
+		RoleEntryMap:    make(map[string]map[string]interface{}),
 	}
 }
 
@@ -48,7 +50,7 @@ func (h *RoleAllowsLocalhost) FetchResources(e *Executor) error {
 	})
 	if exit || err != nil {
 		if f != nil && f.IsSecretPermissionsError() {
-			h.NoPerms = true
+			h.RoleListFetchIssue = f
 		}
 		return err
 	}
@@ -59,7 +61,7 @@ func (h *RoleAllowsLocalhost) FetchResources(e *Executor) error {
 		})
 		if skip || err != nil || entry == nil {
 			if f != nil && f.IsSecretPermissionsError() {
-				h.NoPerms = true
+				h.RoleFetchIssues[role] = f
 			}
 			if err != nil {
 				return err
@@ -83,18 +85,38 @@ func (h *RoleAllowsLocalhost) Evaluate(e *Executor) (results []*Result, err erro
 		}
 		return []*Result{&ret}, nil
 	}
-	if h.NoPerms {
+
+	if h.RoleListFetchIssue != nil && h.RoleListFetchIssue.IsSecretPermissionsError() {
 		ret := Result{
 			Status:   ResultInsufficientPermissions,
-			Endpoint: "/{{mount}}/roles",
-			Message:  "lacks permission either to list the roles or to read a specific role. This may restrict the ability to fully execute this health check",
+			Endpoint: h.RoleListFetchIssue.Path,
+			Message:  "lacks permission either to list the roles. This restricts the ability to fully execute this health check.",
 		}
 		if e.Client.Token() == "" {
 			ret.Message = "No token available and so this health check " + ret.Message
 		} else {
 			ret.Message = "This token " + ret.Message
 		}
-		results = append(results, &ret)
+		return []*Result{&ret}, nil
+	}
+
+	for role, fetchPath := range h.RoleFetchIssues {
+		if fetchPath != nil && fetchPath.IsSecretPermissionsError() {
+			delete(h.RoleEntryMap, role)
+			ret := Result{
+				Status:   ResultInsufficientPermissions,
+				Endpoint: fetchPath.Path,
+				Message:  "Without this information, this health check is unable to function.",
+			}
+
+			if e.Client.Token() == "" {
+				ret.Message = "No token available so unable for the endpoint for this mount. " + ret.Message
+			} else {
+				ret.Message = "This token lacks permission the endpoint for this mount. " + ret.Message
+			}
+
+			results = append(results, &ret)
+		}
 	}
 
 	for role, entry := range h.RoleEntryMap {

command/healthcheck/pki_role_no_store_false.go

@@ -11,19 +11,20 @@ import (
 type RoleNoStoreFalse struct {
 	Enabled            bool
 	UnsupportedVersion bool
-	NoPerms            bool
 
 	AllowedRoles map[string]bool
 
-	CertCounts   int
-	RoleEntryMap map[string]map[string]interface{}
-	CRLConfig    *PathFetch
+	RoleListFetchIssue *PathFetch
+	RoleFetchIssues    map[string]*PathFetch
+	RoleEntryMap       map[string]map[string]interface{}
+	CRLConfig          *PathFetch
 }
 
 func NewRoleNoStoreFalseCheck() Check {
 	return &RoleNoStoreFalse{
-		AllowedRoles: make(map[string]bool),
-		RoleEntryMap: make(map[string]map[string]interface{}),
+		RoleFetchIssues: make(map[string]*PathFetch),
+		AllowedRoles:    make(map[string]bool),
+		RoleEntryMap:    make(map[string]map[string]interface{}),
 	}
 }
 
@@ -64,7 +65,7 @@ func (h *RoleNoStoreFalse) FetchResources(e *Executor) error {
 	})
 	if exit || err != nil {
 		if f != nil && f.IsSecretPermissionsError() {
-			h.NoPerms = true
+			h.RoleListFetchIssue = f
 		}
 		return err
 	}
@@ -75,7 +76,7 @@ func (h *RoleNoStoreFalse) FetchResources(e *Executor) error {
 		})
 		if skip || err != nil || entry == nil {
 			if f != nil && f.IsSecretPermissionsError() {
-				h.NoPerms = true
+				h.RoleFetchIssues[role] = f
 			}
 			if err != nil {
 				return err
@@ -86,14 +87,6 @@ func (h *RoleNoStoreFalse) FetchResources(e *Executor) error {
 		h.RoleEntryMap[role] = entry
 	}
 
-	exit, _, leaves, err := pkiFetchLeavesList(e, func() {
-		h.UnsupportedVersion = true
-	})
-	if exit || err != nil {
-		return err
-	}
-	h.CertCounts = len(leaves)
-
 	// Check if the issuer is fetched yet.
 	configRet, err := e.FetchIfNotFetched(logical.ReadOperation, "/{{mount}}/config/crl")
 	if err != nil {
@@ -116,18 +109,37 @@ func (h *RoleNoStoreFalse) Evaluate(e *Executor) (results []*Result, err error)
 		return []*Result{&ret}, nil
 	}
 
-	if h.NoPerms {
+	if h.RoleListFetchIssue != nil && h.RoleListFetchIssue.IsSecretPermissionsError() {
 		ret := Result{
 			Status:   ResultInsufficientPermissions,
-			Endpoint: "/{{mount}}/roles",
-			Message:  "lacks permission either to list the roles or to read a specific role. This may restrict the ability to fully execute this health check",
+			Endpoint: h.RoleListFetchIssue.Path,
+			Message:  "lacks permission either to list the roles. This restricts the ability to fully execute this health check.",
 		}
 		if e.Client.Token() == "" {
 			ret.Message = "No token available and so this health check " + ret.Message
 		} else {
 			ret.Message = "This token " + ret.Message
 		}
-		results = append(results, &ret)
+		return []*Result{&ret}, nil
+	}
+
+	for role, fetchPath := range h.RoleFetchIssues {
+		if fetchPath != nil && fetchPath.IsSecretPermissionsError() {
+			delete(h.RoleEntryMap, role)
+			ret := Result{
+				Status:   ResultInsufficientPermissions,
+				Endpoint: fetchPath.Path,
+				Message:  "Without this information, this health check is unable to function.",
+			}
+
+			if e.Client.Token() == "" {
+				ret.Message = "No token available so unable for the endpoint for this mount. " + ret.Message
+			} else {
+				ret.Message = "This token lacks permission the endpoint for this mount. " + ret.Message
+			}
+
+			results = append(results, &ret)
+		}
 	}
 
 	crlAutoRebuild := false

command/healthcheck/pki_root_issued_leaves.go

@@ -14,12 +14,14 @@ type RootIssuedLeaves struct {
 
 	CertsToFetch int
 
+	FetchIssues map[string]*PathFetch
 	RootCertMap map[string]*x509.Certificate
 	LeafCertMap map[string]*x509.Certificate
 }
 
 func NewRootIssuedLeavesCheck() Check {
 	return &RootIssuedLeaves{
+		FetchIssues: make(map[string]*PathFetch),
 		RootCertMap: make(map[string]*x509.Certificate),
 		LeafCertMap: make(map[string]*x509.Certificate),
 	}
@@ -64,9 +66,10 @@ func (h *RootIssuedLeaves) FetchResources(e *Executor) error {
 	}
 
 	for _, issuer := range issuers {
-		skip, _, cert, err := pkiFetchIssuer(e, issuer, func() {
+		skip, pathFetch, cert, err := pkiFetchIssuer(e, issuer, func() {
 			h.UnsupportedVersion = true
 		})
+		h.FetchIssues[issuer] = pathFetch
 		if skip || err != nil {
 			if err != nil {
 				return err
@@ -85,10 +88,15 @@ func (h *RootIssuedLeaves) FetchResources(e *Executor) error {
 		h.RootCertMap[issuer] = cert
 	}
 
-	exit, _, leaves, err := pkiFetchLeavesList(e, func() {
+	exit, f, leaves, err := pkiFetchLeavesList(e, func() {
 		h.UnsupportedVersion = true
 	})
 	if exit || err != nil {
+		if f != nil && f.IsSecretPermissionsError() {
+			for _, issuer := range issuers {
+				h.FetchIssues[issuer] = f
+			}
+		}
 		return err
 	}
 
@@ -130,6 +138,25 @@ func (h *RootIssuedLeaves) Evaluate(e *Executor) (results []*Result, err error)
 		return []*Result{&ret}, nil
 	}
 
+	for issuer, fetchPath := range h.FetchIssues {
+		if fetchPath != nil && fetchPath.IsSecretPermissionsError() {
+			delete(h.RootCertMap, issuer)
+			ret := Result{
+				Status:   ResultInsufficientPermissions,
+				Endpoint: fetchPath.Path,
+				Message:  "Without this information, this health check is unable to function.",
+			}
+
+			if e.Client.Token() == "" {
+				ret.Message = "No token available so unable for the endpoint for this mount. " + ret.Message
+			} else {
+				ret.Message = "This token lacks permission for the endpoint for this mount. " + ret.Message
+			}
+
+			results = append(results, &ret)
+		}
+	}
+
 	issuerHasLeaf := make(map[string]bool)
 	for serial, leaf := range h.LeafCertMap {
 		if len(issuerHasLeaf) == len(h.RootCertMap) {

command/healthcheck/pki_too_many_certs.go

@@ -14,6 +14,7 @@ type TooManyCerts struct {
 	CountWarning  int
 
 	CertCounts int
+	FetchIssue *PathFetch
 }
 
 func NewTooManyCertsCheck() Check {
@@ -60,7 +61,9 @@ func (h *TooManyCerts) FetchResources(e *Executor) error {
 	exit, leavesRet, _, err := pkiFetchLeavesList(e, func() {
 		h.UnsupportedVersion = true
 	})
-	if exit {
+	h.FetchIssue = leavesRet
+
+	if exit || err != nil {
 		return err
 	}
 
@@ -80,6 +83,23 @@ func (h *TooManyCerts) Evaluate(e *Executor) (results []*Result, err error) {
 		return []*Result{&ret}, nil
 	}
 
+	if h.FetchIssue != nil && h.FetchIssue.IsSecretPermissionsError() {
+		ret := Result{
+			Status:   ResultInsufficientPermissions,
+			Endpoint: h.FetchIssue.Path,
+			Message:  "Without this information, this health check is unable to function.",
+		}
+
+		if e.Client.Token() == "" {
+			ret.Message = "No token available so unable to list the endpoint for this mount. " + ret.Message
+		} else {
+			ret.Message = "This token lacks permission to list the endpoint for this mount. " + ret.Message
+		}
+
+		results = append(results, &ret)
+		return
+	}
+
 	ret := Result{
 		Status:   ResultOK,
 		Endpoint: "/{{mount}}/certs",

command/pki_health_check.go

@@ -223,7 +223,15 @@ func (c *PKIHealthCheckCommand) Run(args []string) int {
 
 	// Handle listing, if necessary.
 	if c.flagList {
-		c.UI.Output("Default health check config:")
+		uiFormat := Format(c.UI)
+		if uiFormat == "yaml" {
+			c.UI.Error("YAML output format is not supported by the --list command")
+			return pkiRetUsage
+		}
+
+		if uiFormat != "json" {
+			c.UI.Output("Default health check config:")
+		}
 		config := map[string]map[string]interface{}{}
 		for _, checker := range executor.Checkers {
 			config[checker.Name()] = checker.DefaultConfig()

command/pki_health_check_test.go

@@ -271,6 +271,8 @@ func testPKIHealthCheckCommand(tb testing.TB) (*cli.MockUi, *PKIHealthCheckComma
 }
 
 func execPKIHC(t *testing.T, client *api.Client, ok bool) (int, string, map[string][]map[string]interface{}) {
+	t.Helper()
+
 	stdout := bytes.NewBuffer(nil)
 	stderr := bytes.NewBuffer(nil)
 	runOpts := &RunOptions{
@@ -295,6 +297,8 @@ func execPKIHC(t *testing.T, client *api.Client, ok bool) (int, string, map[stri
 }
 
 func validateExpectedPKIHC(t *testing.T, expected, results map[string][]map[string]interface{}) {
+	t.Helper()
+
 	for test, subtest := range expected {
 		actual, ok := results[test]
 		require.True(t, ok, fmt.Sprintf("expected top-level test %v to be present", test))
@@ -615,7 +619,7 @@ var expectedNoPerm = map[string][]map[string]interface{}{
 	},
 	"root_issued_leaves": {
 		{
-			"status": "ok",
+			"status": "insufficient_permissions",
 		},
 	},
 	"tidy_last_run": {
@@ -625,7 +629,7 @@ var expectedNoPerm = map[string][]map[string]interface{}{
 	},
 	"too_many_certs": {
 		{
-			"status": "ok",
+			"status": "insufficient_permissions",
 		},
 	},
 }