diff --git a/builtin/logical/pki/path_acme_directory.go b/builtin/logical/pki/path_acme_directory.go index bd9f2e2106..a8329a0258 100644 --- a/builtin/logical/pki/path_acme_directory.go +++ b/builtin/logical/pki/path_acme_directory.go @@ -106,7 +106,7 @@ func getAcmeBaseUrl(sc *storageContext, path string) (*url.URL, error) { directoryPrefix = path[0:lastIndex] } - return baseUrl.JoinPath(directoryPrefix), nil + return baseUrl.JoinPath(directoryPrefix, "/acme/"), nil } func acmeErrorWrapper(op framework.OperationFunc) framework.OperationFunc { @@ -122,11 +122,11 @@ func acmeErrorWrapper(op framework.OperationFunc) framework.OperationFunc { func (b *backend) acmeDirectoryHandler(acmeCtx acmeContext, r *logical.Request, _ *framework.FieldData) (*logical.Response, error) { rawBody, err := json.Marshal(map[string]interface{}{ - "newNonce": acmeCtx.baseUrl.JoinPath("/acme/new-nonce").String(), - "newAccount": acmeCtx.baseUrl.JoinPath("/acme/new-account").String(), - "newOrder": acmeCtx.baseUrl.JoinPath("/acme/new-order").String(), - "revokeCert": acmeCtx.baseUrl.JoinPath("/acme/revoke-cert").String(), - "keyChange": acmeCtx.baseUrl.JoinPath("/acme/key-change").String(), + "newNonce": acmeCtx.baseUrl.JoinPath("new-nonce").String(), + "newAccount": acmeCtx.baseUrl.JoinPath("new-account").String(), + "newOrder": acmeCtx.baseUrl.JoinPath("new-order").String(), + "revokeCert": acmeCtx.baseUrl.JoinPath("revoke-cert").String(), + "keyChange": acmeCtx.baseUrl.JoinPath("key-change").String(), "meta": map[string]interface{}{ "externalAccountRequired": false, }, diff --git a/builtin/logical/pki/path_acme_new_account.go b/builtin/logical/pki/path_acme_new_account.go index 46b49ab397..cc922f3670 100644 --- a/builtin/logical/pki/path_acme_new_account.go +++ b/builtin/logical/pki/path_acme_new_account.go @@ -169,7 +169,7 @@ func (b *backend) acmeNewAccountSearchHandler(acmeCtx acmeContext, r *logical.Re return nil, fmt.Errorf("error loading account: %w", err) } - location := acmeCtx.baseUrl.String() + "/acme/account/" + userCtx.Kid + location := acmeCtx.baseUrl.String() + "account/" + userCtx.Kid return formatAccountResponse(location, account["status"].(string), account["contact"].([]string)), nil } @@ -201,6 +201,6 @@ func (b *backend) acmeNewAccountCreateHandler(acmeCtx acmeContext, r *logical.Re return nil, fmt.Errorf("failed to create account: %w", err) } - location := acmeCtx.baseUrl.String() + "/acme/account/" + userCtx.Kid + location := acmeCtx.baseUrl.String() + "account/" + userCtx.Kid return formatAccountResponse(location, account["status"].(string), account["contact"].([]string)), nil } diff --git a/builtin/logical/pki/path_acme_nonce.go b/builtin/logical/pki/path_acme_nonce.go index 35dba21bbd..4a6d227ea7 100644 --- a/builtin/logical/pki/path_acme_nonce.go +++ b/builtin/logical/pki/path_acme_nonce.go @@ -71,11 +71,14 @@ func (b *backend) acmeNonceHandler(ctx acmeContext, r *logical.Request, _ *frame }, Data: map[string]interface{}{ logical.HTTPStatusCode: httpStatus, + // Get around Vault limitation of requiring a body set if the status is not http.StatusNoContent + // for our HEAD request responses. + logical.HTTPContentType: "", }, }, nil } func genAcmeLinkHeader(ctx acmeContext) []string { - path := fmt.Sprintf("<%s>;rel=\"index\"", ctx.baseUrl.JoinPath("/acme/directory").String()) + path := fmt.Sprintf("<%s>;rel=\"index\"", ctx.baseUrl.JoinPath("directory").String()) return []string{path} } diff --git a/builtin/logical/pki/path_acme_test.go b/builtin/logical/pki/path_acme_test.go index 18f2bccf22..e2e878ed4e 100644 --- a/builtin/logical/pki/path_acme_test.go +++ b/builtin/logical/pki/path_acme_test.go @@ -1,10 +1,16 @@ package pki import ( + "context" "fmt" + "io" "net/http" "testing" + "github.com/hashicorp/vault/api" + vaulthttp "github.com/hashicorp/vault/http" + "github.com/hashicorp/vault/vault" + "github.com/hashicorp/vault/sdk/logical" "github.com/stretchr/testify/require" @@ -15,28 +21,29 @@ import ( // are available and produce the correct responses. func TestAcmeDirectory(t *testing.T) { t.Parallel() - b, s, pathConfig := setupAcmeBackend(t) + cluster, client, pathConfig := setupAcmeBackend(t) + defer cluster.Cleanup() cases := []struct { name string prefixUrl string directoryUrl string }{ - {"root", "", "acme/directory"}, - {"role", "/roles/test-role", "roles/test-role/acme/directory"}, - {"issuer", "/issuer/default", "issuer/default/acme/directory"}, - {"issuer_role", "/issuer/default/roles/test-role", "issuer/default/roles/test-role/acme/directory"}, - {"issuer_role_acme", "/issuer/acme/roles/acme", "issuer/acme/roles/acme/acme/directory"}, + {"root", "", "pki/acme/directory"}, + {"role", "/roles/test-role", "pki/roles/test-role/acme/directory"}, + {"issuer", "/issuer/default", "pki/issuer/default/acme/directory"}, + {"issuer_role", "/issuer/default/roles/test-role", "pki/issuer/default/roles/test-role/acme/directory"}, + {"issuer_role_acme", "/issuer/acme/roles/acme", "pki/issuer/acme/roles/acme/acme/directory"}, } + testCtx := context.Background() for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - dirResp, err := CBRead(b, s, tc.directoryUrl) + dirResp, err := client.Logical().ReadRawWithContext(testCtx, tc.directoryUrl) require.NoError(t, err, "failed reading ACME directory configuration") - require.Contains(t, dirResp.Data, "http_content_type", "missing Content-Type header") - require.Contains(t, dirResp.Data["http_content_type"], "application/json", - "missing appropriate content type in header") + require.Equal(t, 200, dirResp.StatusCode) + require.Equal(t, "application/json", dirResp.Header.Get("Content-Type")) requiredUrls := map[string]string{ "newNonce": pathConfig + tc.prefixUrl + "/acme/new-nonce", @@ -46,7 +53,10 @@ func TestAcmeDirectory(t *testing.T) { "keyChange": pathConfig + tc.prefixUrl + "/acme/key-change", } - rawBodyBytes := dirResp.Data["http_raw_body"].([]byte) + rawBodyBytes, err := io.ReadAll(dirResp.Body) + require.NoError(t, err, "failed reading from directory response body") + _ = dirResp.Body.Close() + respType := map[string]interface{}{} err = json.Unmarshal(rawBodyBytes, &respType) require.NoError(t, err, "failed unmarshalling ACME directory response body") @@ -59,60 +69,60 @@ func TestAcmeDirectory(t *testing.T) { } } +// TestAcmeNonce a basic test that will validate we get back a nonce with the proper status codes +// based on the func TestAcmeNonce(t *testing.T) { t.Parallel() - b, s, pathConfig := setupAcmeBackend(t) + cluster, client, pathConfig := setupAcmeBackend(t) + defer cluster.Cleanup() cases := []struct { name string prefixUrl string directoryUrl string }{ - {"root", "", "acme/new-nonce"}, - {"role", "/roles/test-role", "roles/test-role/acme/new-nonce"}, - {"issuer", "/issuer/default", "issuer/default/acme/new-nonce"}, - {"issuer_role", "/issuer/default/roles/test-role", "issuer/default/roles/test-role/acme/new-nonce"}, + {"root", "", "pki/acme/new-nonce"}, + {"role", "/roles/test-role", "pki/roles/test-role/acme/new-nonce"}, + {"issuer", "/issuer/default", "pki/issuer/default/acme/new-nonce"}, + {"issuer_role", "/issuer/default/roles/test-role", "pki/issuer/default/roles/test-role/acme/new-nonce"}, } + for _, tc := range cases { for _, httpOp := range []string{"get", "header"} { t.Run(fmt.Sprintf("%s-%s", tc.name, httpOp), func(t *testing.T) { - var resp *logical.Response - var err error + var req *api.Request switch httpOp { case "get": - resp, err = CBRead(b, s, tc.directoryUrl) + req = client.NewRequest(http.MethodGet, "/v1/"+tc.directoryUrl) case "header": - resp, err = CBHeader(b, s, tc.directoryUrl) + req = client.NewRequest(http.MethodHead, "/v1/"+tc.directoryUrl) } - require.NoError(t, err, "failed %s op for new-nouce", httpOp) + res, err := client.RawRequestWithContext(ctx, req) + require.NoError(t, err, "failed sending raw request") + _ = res.Body.Close() // Proper Status Code switch httpOp { case "get": - require.Equal(t, http.StatusNoContent, resp.Data["http_status_code"]) + require.Equal(t, http.StatusNoContent, res.StatusCode) case "header": - require.Equal(t, http.StatusOK, resp.Data["http_status_code"]) + require.Equal(t, http.StatusOK, res.StatusCode) } + // Make sure we don't have a Content-Type header. + require.Equal(t, "", res.Header.Get("Content-Type")) + // Make sure we return the Cache-Control header - require.Contains(t, resp.Headers, "Cache-Control", "missing Cache-Control header") - require.Contains(t, resp.Headers["Cache-Control"], "no-store", + require.Contains(t, res.Header.Get("Cache-Control"), "no-store", "missing Cache-Control header with no-store header value") - require.Len(t, resp.Headers["Cache-Control"], 1, - "Cache-Control header should have only a single header") // Test for our nonce header value - require.Contains(t, resp.Headers, "Replay-Nonce", "missing Replay-Nonce header") - require.NotEmpty(t, resp.Headers["Replay-Nonce"], "missing Replay-Nonce header with an actual value") - require.Len(t, resp.Headers["Replay-Nonce"], 1, - "Replay-Nonce header should have only a single header") + require.NotEmpty(t, res.Header.Get("Replay-Nonce"), "missing Replay-Nonce header with an actual value") // Test Link header value - require.Contains(t, resp.Headers, "Link", "missing Link header") expectedLinkHeader := fmt.Sprintf("<%s>;rel=\"index\"", pathConfig+tc.prefixUrl+"/acme/directory") - require.Contains(t, resp.Headers["Link"], expectedLinkHeader, + require.Contains(t, res.Header.Get("Link"), expectedLinkHeader, "different value for link header than expected") - require.Len(t, resp.Headers["Link"], 1, "Link header should have only a single header") }) } } @@ -121,31 +131,33 @@ func TestAcmeNonce(t *testing.T) { // TestAcmeClusterPathNotConfigured basic testing of the ACME error handler. func TestAcmeClusterPathNotConfigured(t *testing.T) { t.Parallel() - b, s := CreateBackendWithStorage(t) + cluster, client := setupTestPkiCluster(t) + defer cluster.Cleanup() // Do not fill in the path option within the local cluster configuration cases := []struct { name string directoryUrl string }{ - {"root", "acme/directory"}, - {"role", "roles/test-role/acme/directory"}, - {"issuer", "issuer/default/acme/directory"}, - {"issuer_role", "issuer/default/roles/test-role/acme/directory"}, + {"root", "pki/acme/directory"}, + {"role", "pki/roles/test-role/acme/directory"}, + {"issuer", "pki/issuer/default/acme/directory"}, + {"issuer_role", "pki/issuer/default/roles/test-role/acme/directory"}, } + testCtx := context.Background() + for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - dirResp, err := CBRead(b, s, tc.directoryUrl) - require.NoError(t, err, "failed reading ACME directory configuration") + dirResp, err := client.Logical().ReadRawWithContext(testCtx, tc.directoryUrl) + require.Error(t, err, "expected failure reading ACME directory configuration got none") - require.Contains(t, dirResp.Data, "http_content_type", "missing Content-Type header") - require.Contains(t, dirResp.Data["http_content_type"], "application/problem+json", - "missing appropriate content type in header") + require.Equal(t, "application/problem+json", dirResp.Header.Get("Content-Type")) + require.Equal(t, http.StatusInternalServerError, dirResp.StatusCode) - require.Equal(t, http.StatusInternalServerError, dirResp.Data["http_status_code"]) + rawBodyBytes, err := io.ReadAll(dirResp.Body) + require.NoError(t, err, "failed reading from directory response body") + _ = dirResp.Body.Close() - require.Contains(t, dirResp.Data, "http_raw_body", "missing http_raw_body from data") - rawBodyBytes := dirResp.Data["http_raw_body"].([]byte) respType := map[string]interface{}{} err = json.Unmarshal(rawBodyBytes, &respType) require.NoError(t, err, "failed unmarshalling ACME directory response body") @@ -156,16 +168,38 @@ func TestAcmeClusterPathNotConfigured(t *testing.T) { } } -func setupAcmeBackend(t *testing.T) (*backend, logical.Storage, string) { - b, s := CreateBackendWithStorage(t) +func setupAcmeBackend(t *testing.T) (*vault.TestCluster, *api.Client, string) { + cluster, client := setupTestPkiCluster(t) // Setting templated AIAs should succeed. pathConfig := "https://localhost:8200/v1/pki" - _, err := CBWrite(b, s, "config/cluster", map[string]interface{}{ + _, err := client.Logical().WriteWithContext(context.Background(), "pki/config/cluster", map[string]interface{}{ "path": pathConfig, "aia_path": "http://localhost:8200/cdn/pki", }) require.NoError(t, err) - return b, s, pathConfig + + // Allow certain headers to pass through for ACME support + _, err = client.Logical().WriteWithContext(context.Background(), "sys/mounts/pki/tune", map[string]interface{}{ + "allowed_response_headers": []string{"Last-Modified", "Replay-Nonce", "Link"}, + }) + require.NoError(t, err, "failed tuning mount response headers") + + return cluster, client, pathConfig +} + +func setupTestPkiCluster(t *testing.T) (*vault.TestCluster, *api.Client) { + coreConfig := &vault.CoreConfig{ + LogicalBackends: map[string]logical.Factory{ + "pki": Factory, + }, + } + cluster := vault.NewTestCluster(t, coreConfig, &vault.TestClusterOptions{ + HandlerFunc: vaulthttp.Handler, + }) + cluster.Start() + client := cluster.Cores[0].Client + mountPKIEndpoint(t, client, "pki") + return cluster, client }