Add ACME health checks to pki health-check CLI (#20619)

* Add ACME health checks to pki health-check CLI

 - Verify we have the required header values listed within allowed_response_headers: 'Replay-Nonce', 'Link', 'Location'
 - Make sure the local cluster config path variable contains an URL with an https scheme

* Split ACME health checks into two separate verifications

 - Promote ACME usage through the enable_acme_issuance check, if ACME is disabled currently
 - If ACME is enabled verify that we have a valid
    'path' field within local cluster configuration as well as the proper response headers allowed.
 - Factor out response header verifications into a separate check mainly to work around possible permission issues.

* Only recommend enabling ACME on mounts with intermediate issuers

* Attempt to connect to the ACME directory based on the cluster path variable

 - Final health check is to attempt to connect to the ACME directory based on the cluster local 'path' value. Only if we successfully connect do we say ACME is healthy.

* Fix broken unit test
This commit is contained in:
Steven Clark 2023-05-23 10:37:31 -04:00 committed by GitHub
parent 5eb03f785e
commit c8837f2010
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 451 additions and 1 deletions

View File

@ -0,0 +1,155 @@
// Copyright (c) HashiCorp, Inc.
// SPDX-License-Identifier: MPL-2.0
package healthcheck
import (
"fmt"
"strings"
"github.com/hashicorp/go-secure-stdlib/parseutil"
"github.com/hashicorp/go-secure-stdlib/strutil"
"github.com/hashicorp/vault/sdk/logical"
)
type AllowAcmeHeaders struct {
Enabled bool
UnsupportedVersion bool
TuneFetcher *PathFetch
TuneData map[string]interface{}
AcmeConfigFetcher *PathFetch
}
func NewAllowAcmeHeaders() Check {
return &AllowAcmeHeaders{}
}
func (h *AllowAcmeHeaders) Name() string {
return "allow_acme_headers"
}
func (h *AllowAcmeHeaders) IsEnabled() bool {
return h.Enabled
}
func (h *AllowAcmeHeaders) DefaultConfig() map[string]interface{} {
return map[string]interface{}{}
}
func (h *AllowAcmeHeaders) LoadConfig(config map[string]interface{}) error {
enabled, err := parseutil.ParseBool(config["enabled"])
if err != nil {
return fmt.Errorf("error parsing %v.enabled: %w", h.Name(), err)
}
h.Enabled = enabled
return nil
}
func (h *AllowAcmeHeaders) FetchResources(e *Executor) error {
var err error
h.AcmeConfigFetcher, err = e.FetchIfNotFetched(logical.ReadOperation, "/{{mount}}/config/acme")
if err != nil {
return err
}
if h.AcmeConfigFetcher.IsUnsupportedPathError() {
h.UnsupportedVersion = true
}
_, h.TuneFetcher, h.TuneData, err = fetchMountTune(e, func() {
h.UnsupportedVersion = true
})
if err != nil {
return err
}
return nil
}
func (h *AllowAcmeHeaders) Evaluate(e *Executor) ([]*Result, error) {
if h.UnsupportedVersion {
ret := Result{
Status: ResultInvalidVersion,
Endpoint: h.AcmeConfigFetcher.Path,
Message: "This health check requires Vault 1.14+ but an earlier version of Vault Server was contacted, preventing this health check from running.",
}
return []*Result{&ret}, nil
}
if h.AcmeConfigFetcher.IsSecretPermissionsError() {
msg := "Without read access to ACME configuration, this health check is unable to function."
return craftInsufficientPermissionResult(e, h.AcmeConfigFetcher.Path, msg), nil
}
acmeEnabled, err := isAcmeEnabled(h.AcmeConfigFetcher)
if err != nil {
return nil, err
}
if !acmeEnabled {
ret := Result{
Status: ResultNotApplicable,
Endpoint: h.AcmeConfigFetcher.Path,
Message: "ACME is not enabled, no additional response headers required.",
}
return []*Result{&ret}, nil
}
if h.TuneFetcher.IsSecretPermissionsError() {
msg := "Without access to mount tune information, this health check is unable to function."
return craftInsufficientPermissionResult(e, h.TuneFetcher.Path, msg), nil
}
resp, err := StringList(h.TuneData["allowed_response_headers"])
if err != nil {
return nil, fmt.Errorf("unable to parse value from server for allowed_response_headers: %w", err)
}
requiredResponseHeaders := []string{"Replay-Nonce", "Link", "Location"}
foundResponseHeaders := []string{}
for _, param := range resp {
for _, reqHeader := range requiredResponseHeaders {
if strings.EqualFold(param, reqHeader) {
foundResponseHeaders = append(foundResponseHeaders, reqHeader)
break
}
}
}
foundAllHeaders := strutil.EquivalentSlices(requiredResponseHeaders, foundResponseHeaders)
if !foundAllHeaders {
ret := Result{
Status: ResultWarning,
Endpoint: "/sys/mounts/{{mount}}/tune",
Message: "Mount hasn't enabled 'Replay-Nonce', 'Link', 'Location' response headers, these are required for ACME to function.",
}
return []*Result{&ret}, nil
}
ret := Result{
Status: ResultOK,
Endpoint: "/sys/mounts/{{mount}}/tune",
Message: "Mount has enabled 'Replay-Nonce', 'Link', 'Location' response headers.",
}
return []*Result{&ret}, nil
}
func craftInsufficientPermissionResult(e *Executor, path, errorMsg string) []*Result {
ret := Result{
Status: ResultInsufficientPermissions,
Endpoint: path,
Message: errorMsg,
}
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
}
return []*Result{&ret}
}

View File

@ -0,0 +1,237 @@
// Copyright (c) HashiCorp, Inc.
// SPDX-License-Identifier: MPL-2.0
package healthcheck
import (
"bytes"
"context"
"crypto/tls"
"fmt"
"net/http"
"net/url"
"github.com/hashicorp/go-secure-stdlib/parseutil"
"github.com/hashicorp/vault/sdk/logical"
"golang.org/x/crypto/acme"
)
type EnableAcmeIssuance struct {
Enabled bool
UnsupportedVersion bool
AcmeConfigFetcher *PathFetch
ClusterConfigFetcher *PathFetch
TotalIssuers int
RootIssuers int
}
func NewEnableAcmeIssuance() Check {
return &EnableAcmeIssuance{}
}
func (h *EnableAcmeIssuance) Name() string {
return "enable_acme_issuance"
}
func (h *EnableAcmeIssuance) IsEnabled() bool {
return h.Enabled
}
func (h *EnableAcmeIssuance) DefaultConfig() map[string]interface{} {
return map[string]interface{}{}
}
func (h *EnableAcmeIssuance) LoadConfig(config map[string]interface{}) error {
enabled, err := parseutil.ParseBool(config["enabled"])
if err != nil {
return fmt.Errorf("error parsing %v.enabled: %w", h.Name(), err)
}
h.Enabled = enabled
return nil
}
func (h *EnableAcmeIssuance) FetchResources(e *Executor) error {
var err error
h.AcmeConfigFetcher, err = e.FetchIfNotFetched(logical.ReadOperation, "/{{mount}}/config/acme")
if err != nil {
return err
}
if h.AcmeConfigFetcher.IsUnsupportedPathError() {
h.UnsupportedVersion = true
}
h.ClusterConfigFetcher, err = e.FetchIfNotFetched(logical.ReadOperation, "/{{mount}}/config/cluster")
if err != nil {
return err
}
if h.ClusterConfigFetcher.IsUnsupportedPathError() {
h.UnsupportedVersion = true
}
h.TotalIssuers, h.RootIssuers, err = doesMountContainOnlyRootIssuers(e)
return nil
}
func doesMountContainOnlyRootIssuers(e *Executor) (int, int, error) {
exit, _, issuers, err := pkiFetchIssuersList(e, func() {})
if exit || err != nil {
return 0, 0, err
}
totalIssuers := 0
rootIssuers := 0
for _, issuer := range issuers {
skip, _, cert, err := pkiFetchIssuer(e, issuer, func() {})
if skip || err != nil {
if err != nil {
return 0, 0, err
}
continue
}
totalIssuers++
if !bytes.Equal(cert.RawSubject, cert.RawIssuer) {
continue
}
if err := cert.CheckSignatureFrom(cert); err != nil {
continue
}
rootIssuers++
}
return totalIssuers, rootIssuers, nil
}
func isAcmeEnabled(fetcher *PathFetch) (bool, error) {
isEnabledRaw, ok := fetcher.Secret.Data["enabled"]
if !ok {
return false, fmt.Errorf("enabled configuration field missing from acme config")
}
parseBool, err := parseutil.ParseBool(isEnabledRaw)
if err != nil {
return false, fmt.Errorf("failed parsing 'enabled' field from ACME config: %w", err)
}
return parseBool, nil
}
func verifyLocalPathUrl(h *EnableAcmeIssuance) error {
localPathRaw, ok := h.ClusterConfigFetcher.Secret.Data["path"]
if !ok {
return fmt.Errorf("'path' field missing from config")
}
localPath, err := parseutil.ParseString(localPathRaw)
if err != nil {
return fmt.Errorf("failed converting 'path' field from local config: %w", err)
}
if localPath == "" {
return fmt.Errorf("'path' field not configured within /{{mount}}/config/cluster")
}
parsedUrl, err := url.Parse(localPath)
if err != nil {
return fmt.Errorf("failed to parse URL from path config: %v: %w", localPathRaw, err)
}
if parsedUrl.Scheme != "https" {
return fmt.Errorf("the configured 'path' field in /{{mount}}/config/cluster was not using an https scheme")
}
// Avoid issues with SSL certificates for this check, we just want to validate that we would
// hit an ACME server with the path they specified in configuration
tr := &http.Transport{
TLSClientConfig: &tls.Config{InsecureSkipVerify: true},
}
client := &http.Client{Transport: tr}
acmeDirectoryUrl := parsedUrl.JoinPath("/acme/", "directory")
acmeClient := acme.Client{HTTPClient: client, DirectoryURL: acmeDirectoryUrl.String()}
_, err = acmeClient.Discover(context.Background())
if err != nil {
return fmt.Errorf("using configured 'path' field ('%s') in /{{mount}}/config/cluster failed to reach the ACME"+
" directory: %s: %w", parsedUrl.String(), acmeDirectoryUrl.String(), err)
}
return nil
}
func (h *EnableAcmeIssuance) Evaluate(e *Executor) (results []*Result, err error) {
if h.UnsupportedVersion {
ret := Result{
Status: ResultInvalidVersion,
Endpoint: h.AcmeConfigFetcher.Path,
Message: "This health check requires Vault 1.14+ but an earlier version of Vault Server was contacted, preventing this health check from running.",
}
return []*Result{&ret}, nil
}
if h.AcmeConfigFetcher.IsSecretPermissionsError() {
msg := "Without this information, this health check is unable to function."
return craftInsufficientPermissionResult(e, h.AcmeConfigFetcher.Path, msg), nil
}
acmeEnabled, err := isAcmeEnabled(h.AcmeConfigFetcher)
if err != nil {
return nil, err
}
if !acmeEnabled {
if h.TotalIssuers == 0 {
ret := Result{
Status: ResultNotApplicable,
Endpoint: h.AcmeConfigFetcher.Path,
Message: "No issuers in mount, ACME is not required.",
}
return []*Result{&ret}, nil
}
if h.TotalIssuers == h.RootIssuers {
ret := Result{
Status: ResultNotApplicable,
Endpoint: h.AcmeConfigFetcher.Path,
Message: "Mount contains only root issuers, ACME is not required.",
}
return []*Result{&ret}, nil
}
ret := Result{
Status: ResultInformational,
Endpoint: h.AcmeConfigFetcher.Path,
Message: "Consider enabling ACME support to support a self-rotating PKI infrastructure.",
}
return []*Result{&ret}, nil
}
if h.ClusterConfigFetcher.IsSecretPermissionsError() {
msg := "Without this information, this health check is unable to function."
return craftInsufficientPermissionResult(e, h.ClusterConfigFetcher.Path, msg), nil
}
localPathIssue := verifyLocalPathUrl(h)
if localPathIssue != nil {
ret := Result{
Status: ResultWarning,
Endpoint: h.ClusterConfigFetcher.Path,
Message: "ACME enabled in config but not functional: " + localPathIssue.Error(),
}
return []*Result{&ret}, nil
}
ret := Result{
Status: ResultOK,
Endpoint: h.ClusterConfigFetcher.Path,
Message: "ACME enabled and successfully connected to the ACME directory.",
}
return []*Result{&ret}, nil
}

View File

@ -220,6 +220,8 @@ func (c *PKIHealthCheckCommand) Run(args []string) int {
executor.AddCheck(healthcheck.NewEnableAutoTidyCheck())
executor.AddCheck(healthcheck.NewTidyLastRunCheck())
executor.AddCheck(healthcheck.NewTooManyCertsCheck())
executor.AddCheck(healthcheck.NewEnableAcmeIssuance())
executor.AddCheck(healthcheck.NewAllowAcmeHeaders())
if c.flagDefaultDisabled {
executor.DefaultEnabled = false
}

View File

@ -7,6 +7,7 @@ import (
"bytes"
"encoding/json"
"fmt"
"net/url"
"strings"
"testing"
"time"
@ -30,7 +31,7 @@ func TestPKIHC_AllGood(t *testing.T) {
AuditNonHMACRequestKeys: healthcheck.VisibleReqParams,
AuditNonHMACResponseKeys: healthcheck.VisibleRespParams,
PassthroughRequestHeaders: []string{"If-Modified-Since"},
AllowedResponseHeaders: []string{"Last-Modified"},
AllowedResponseHeaders: []string{"Last-Modified", "Replay-Nonce", "Link", "Location"},
MaxLeaseTTL: "36500d",
},
}); err != nil {
@ -69,6 +70,21 @@ func TestPKIHC_AllGood(t *testing.T) {
t.Fatalf("failed to run tidy: %v", err)
}
path, err := url.Parse(client.Address())
require.NoError(t, err, "failed parsing client address")
if _, err := client.Logical().Write("pki/config/cluster", map[string]interface{}{
"path": path.JoinPath("/v1/", "pki/").String(),
}); err != nil {
t.Fatalf("failed to update local cluster: %v", err)
}
if _, err := client.Logical().Write("pki/config/acme", map[string]interface{}{
"enabled": "true",
}); err != nil {
t.Fatalf("failed to update acme config: %v", err)
}
_, _, results := execPKIHC(t, client, true)
validateExpectedPKIHC(t, expectedAllGood, results)
@ -345,6 +361,11 @@ var expectedAllGood = map[string][]map[string]interface{}{
"status": "ok",
},
},
"allow_acme_headers": {
{
"status": "ok",
},
},
"allow_if_modified_since": {
{
"status": "ok",
@ -355,6 +376,11 @@ var expectedAllGood = map[string][]map[string]interface{}{
"status": "ok",
},
},
"enable_acme_issuance": {
{
"status": "ok",
},
},
"enable_auto_tidy": {
{
"status": "ok",
@ -406,6 +432,11 @@ var expectedAllBad = map[string][]map[string]interface{}{
"status": "critical",
},
},
"allow_acme_headers": {
{
"status": "not_applicable",
},
},
"allow_if_modified_since": {
{
"status": "informational",
@ -503,6 +534,11 @@ var expectedAllBad = map[string][]map[string]interface{}{
"status": "informational",
},
},
"enable_acme_issuance": {
{
"status": "not_applicable",
},
},
"enable_auto_tidy": {
{
"status": "informational",
@ -554,8 +590,18 @@ var expectedEmptyWithIssuer = map[string][]map[string]interface{}{
"status": "ok",
},
},
"allow_acme_headers": {
{
"status": "not_applicable",
},
},
"allow_if_modified_since": nil,
"audit_visibility": nil,
"enable_acme_issuance": {
{
"status": "not_applicable",
},
},
"enable_auto_tidy": {
{
"status": "informational",
@ -598,8 +644,18 @@ var expectedNoPerm = map[string][]map[string]interface{}{
"status": "critical",
},
},
"allow_acme_headers": {
{
"status": "insufficient_permissions",
},
},
"allow_if_modified_since": nil,
"audit_visibility": nil,
"enable_acme_issuance": {
{
"status": "insufficient_permissions",
},
},
"enable_auto_tidy": {
{
"status": "insufficient_permissions",