From c40570c144a45ccc717f86a2228efac40e10721e Mon Sep 17 00:00:00 2001 From: Steven Clark Date: Wed, 22 Feb 2023 09:01:29 -0500 Subject: [PATCH] Handle permission issue on pki health-check tune checkers (#19276) * Handle permission issue on pki health-check tune checkers - Prior to this fix, if the end-user's Vault token did not have permission to the mount's tune api, we would return as if the tunable params had not been set. - Now check to see if we encountered a permission issue and report that back to the end-user like the other checks do. --- changelog/19276.txt | 3 ++ .../pki_allow_if_modified_since.go | 35 ++++++++++++++----- command/healthcheck/pki_audit_visibility.go | 34 +++++++++++++----- command/healthcheck/pki_tidy_last_run.go | 2 +- command/healthcheck/shared.go | 4 +-- 5 files changed, 59 insertions(+), 19 deletions(-) create mode 100644 changelog/19276.txt diff --git a/changelog/19276.txt b/changelog/19276.txt new file mode 100644 index 000000000..373199478 --- /dev/null +++ b/changelog/19276.txt @@ -0,0 +1,3 @@ +```release-note:bug +cli/pki: Properly report permission issues within health-check mount tune checks +``` diff --git a/command/healthcheck/pki_allow_if_modified_since.go b/command/healthcheck/pki_allow_if_modified_since.go index c9a1b0cbd..1cff1cda5 100644 --- a/command/healthcheck/pki_allow_if_modified_since.go +++ b/command/healthcheck/pki_allow_if_modified_since.go @@ -12,6 +12,7 @@ type AllowIfModifiedSince struct { UnsupportedVersion bool TuneData map[string]interface{} + Fetcher *PathFetch } func NewAllowIfModifiedSinceCheck() Check { @@ -42,15 +43,16 @@ func (h *AllowIfModifiedSince) LoadConfig(config map[string]interface{}) error { } func (h *AllowIfModifiedSince) FetchResources(e *Executor) error { - exit, _, data, err := fetchMountTune(e, func() { + var exit bool + var err error + + exit, h.Fetcher, h.TuneData, err = fetchMountTune(e, func() { h.UnsupportedVersion = true }) - if exit { + + if exit || err != nil { return err } - - h.TuneData = data - return nil } @@ -59,11 +61,28 @@ func (h *AllowIfModifiedSince) Evaluate(e *Executor) (results []*Result, err err ret := Result{ Status: ResultInvalidVersion, Endpoint: "/sys/mounts/{{mount}}/tune", - Message: "This health check requires Vault 1.9+ but an earlier version of Vault Server was contacted, preventing this health check from running.", + Message: "This health check requires Vault 1.12+ but an earlier version of Vault Server was contacted, preventing this health check from running.", } return []*Result{&ret}, nil } + if h.Fetcher.IsSecretPermissionsError() { + ret := Result{ + Status: ResultInsufficientPermissions, + Endpoint: "/sys/mounts/{{mount}}/tune", + Message: "Without this information, this health check is unable to function.", + } + + if e.Client.Token() == "" { + ret.Message = "No token available so unable read the tune endpoint for this mount. " + ret.Message + } else { + ret.Message = "This token lacks permission to read the tune endpoint for this mount. " + ret.Message + } + + results = append(results, &ret) + return + } + req, err := StringList(h.TuneData["passthrough_request_headers"]) if err != nil { return nil, fmt.Errorf("unable to parse value from server for passthrough_request_headers: %w", err) @@ -74,7 +93,7 @@ func (h *AllowIfModifiedSince) Evaluate(e *Executor) (results []*Result, err err return nil, fmt.Errorf("unable to parse value from server for allowed_response_headers: %w", err) } - var foundIMS bool = false + foundIMS := false for _, param := range req { if strings.EqualFold(param, "If-Modified-Since") { foundIMS = true @@ -82,7 +101,7 @@ func (h *AllowIfModifiedSince) Evaluate(e *Executor) (results []*Result, err err } } - var foundLM bool = false + foundLM := false for _, param := range resp { if strings.EqualFold(param, "Last-Modified") { foundLM = true diff --git a/command/healthcheck/pki_audit_visibility.go b/command/healthcheck/pki_audit_visibility.go index 5fb761d60..1984fb97d 100644 --- a/command/healthcheck/pki_audit_visibility.go +++ b/command/healthcheck/pki_audit_visibility.go @@ -58,6 +58,7 @@ type AuditVisibility struct { IgnoredParameters map[string]bool TuneData map[string]interface{} + Fetcher *PathFetch } func NewAuditVisibilityCheck() Check { @@ -100,29 +101,46 @@ func (h *AuditVisibility) LoadConfig(config map[string]interface{}) error { } func (h *AuditVisibility) FetchResources(e *Executor) error { - exit, _, data, err := fetchMountTune(e, func() { + var exit bool + var err error + + exit, h.Fetcher, h.TuneData, err = fetchMountTune(e, func() { h.UnsupportedVersion = true }) - if exit { + + if exit || err != nil { return err } - - h.TuneData = data - return nil } func (h *AuditVisibility) Evaluate(e *Executor) (results []*Result, err error) { if h.UnsupportedVersion { - // Shouldn't happen; /certs has been around forever. ret := Result{ Status: ResultInvalidVersion, - Endpoint: "/{{mount}}/certs", - Message: "This health check requires Vault 1.11+ but an earlier version of Vault Server was contacted, preventing this health check from running.", + Endpoint: "/sys/mounts/{{mount}}/tune", + Message: "This health check requires Vault 1.9+ but an earlier version of Vault Server was contacted, preventing this health check from running.", } return []*Result{&ret}, nil } + if h.Fetcher.IsSecretPermissionsError() { + ret := Result{ + Status: ResultInsufficientPermissions, + Endpoint: "/sys/mounts/{{mount}}/tune", + Message: "Without this information, this health check is unable to function.", + } + + if e.Client.Token() == "" { + ret.Message = "No token available so unable read the tune endpoint for this mount. " + ret.Message + } else { + ret.Message = "This token lacks permission to read the tune endpoint for this mount. " + ret.Message + } + + results = append(results, &ret) + return + } + sourceMap := map[string][]string{ "audit_non_hmac_request_keys": VisibleReqParams, "audit_non_hmac_response_keys": VisibleRespParams, diff --git a/command/healthcheck/pki_tidy_last_run.go b/command/healthcheck/pki_tidy_last_run.go index e07921233..6fed74d33 100644 --- a/command/healthcheck/pki_tidy_last_run.go +++ b/command/healthcheck/pki_tidy_last_run.go @@ -93,7 +93,7 @@ func (h *TidyLastRun) Evaluate(e *Executor) (results []*Result, err error) { ret := Result{ Status: ResultInsufficientPermissions, Endpoint: "/{{mount}}/tidy-status", - Message: "Without this information, this health check is unable tof unction.", + Message: "Without this information, this health check is unable to function.", } if e.Client.Token() == "" { diff --git a/command/healthcheck/shared.go b/command/healthcheck/shared.go index 17f8859ae..9f2b05051 100644 --- a/command/healthcheck/shared.go +++ b/command/healthcheck/shared.go @@ -35,7 +35,7 @@ func StringList(source interface{}) ([]string, error) { func fetchMountTune(e *Executor, versionError func()) (bool, *PathFetch, map[string]interface{}, error) { tuneRet, err := e.FetchIfNotFetched(logical.ReadOperation, "/sys/mounts/{{mount}}/tune") if err != nil { - return true, nil, nil, err + return true, nil, nil, fmt.Errorf("failed to fetch mount tune information: %w", err) } if !tuneRet.IsSecretOK() { @@ -43,7 +43,7 @@ func fetchMountTune(e *Executor, versionError func()) (bool, *PathFetch, map[str versionError() } - return true, nil, nil, nil + return true, tuneRet, nil, nil } var data map[string]interface{} = nil