diff --git a/builtin/logical/pki/acme_jws.go b/builtin/logical/pki/acme_jws.go index de54c75d0..3f6ba6d27 100644 --- a/builtin/logical/pki/acme_jws.go +++ b/builtin/logical/pki/acme_jws.go @@ -269,5 +269,10 @@ func verifyEabPayload(acmeState *acmeState, ac *acmeContext, outer *jwsCtx, expe return nil, fmt.Errorf("eab payload does not match outer JWK key: %w", ErrMalformed) } + if eabEntry.AcmeDirectory != ac.acmeDirectory { + // This EAB was not created for this specific ACME directory, reject it + return nil, fmt.Errorf("%w: failed to verify eab", ErrUnauthorized) + } + return eabEntry, nil } diff --git a/builtin/logical/pki/acme_wrappers.go b/builtin/logical/pki/acme_wrappers.go index 9fce2d922..85c51591b 100644 --- a/builtin/logical/pki/acme_wrappers.go +++ b/builtin/logical/pki/acme_wrappers.go @@ -77,7 +77,7 @@ func (b *backend) acmeWrapper(op acmeOperation) framework.OperationFunc { return nil, fmt.Errorf("%w: Can not perform ACME operations until migration has completed", ErrServerInternal) } - acmeBaseUrl, clusterBase, err := getAcmeBaseUrl(sc, r.Path) + acmeBaseUrl, clusterBase, err := getAcmeBaseUrl(sc, r) if err != nil { return nil, err } @@ -87,7 +87,10 @@ func (b *backend) acmeWrapper(op acmeOperation) framework.OperationFunc { return nil, err } - acmeDirectory := getAcmeDirectory(data) + acmeDirectory, err := getAcmeDirectory(r) + if err != nil { + return nil, err + } acmeCtx := &acmeContext{ baseUrl: acmeBaseUrl, @@ -237,19 +240,18 @@ func buildAcmeFrameworkPaths(b *backend, patternFunc func(b *backend, pattern st return patterns } -func getAcmeBaseUrl(sc *storageContext, path string) (*url.URL, *url.URL, error) { +func getAcmeBaseUrl(sc *storageContext, r *logical.Request) (*url.URL, *url.URL, error) { baseUrl, err := getBasePathFromClusterConfig(sc) if err != nil { return nil, nil, err } - directoryPrefix := "" - lastIndex := strings.LastIndex(path, "/acme/") - if lastIndex != -1 { - directoryPrefix = path[0:lastIndex] + directoryPrefix, err := getAcmeDirectory(r) + if err != nil { + return nil, nil, err } - return baseUrl.JoinPath(directoryPrefix, "/acme/"), baseUrl, nil + return baseUrl.JoinPath(directoryPrefix), baseUrl, nil } func getBasePathFromClusterConfig(sc *storageContext) (*url.URL, error) { @@ -291,11 +293,21 @@ func getAcmeIssuer(sc *storageContext, issuerName string) (*issuerEntry, error) return nil, fmt.Errorf("%w: issuer missing proper issuance usage or key", ErrServerInternal) } -func getAcmeDirectory(data *framework.FieldData) string { - requestedIssuer := getRequestedAcmeIssuerFromPath(data) - requestedRole := getRequestedAcmeRoleFromPath(data) +// getAcmeDirectory return the base acme directory path, without a leading '/' and including +// the trailing /acme/ folder which is the root of all our various directories +func getAcmeDirectory(r *logical.Request) (string, error) { + acmePath := r.Path + if !strings.HasPrefix(acmePath, "/") { + acmePath = "/" + acmePath + } - return fmt.Sprintf("issuer-%s::role-%s", requestedIssuer, requestedRole) + lastIndex := strings.LastIndex(acmePath, "/acme/") + if lastIndex == -1 { + return "", fmt.Errorf("%w: unable to determine acme base folder path: %s", ErrServerInternal, acmePath) + } + + // Skip the leading '/' and return our base path with the /acme/ + return strings.TrimLeft(acmePath[0:lastIndex]+"/acme/", "/"), nil } func getAcmeRoleAndIssuer(sc *storageContext, data *framework.FieldData, config *acmeConfigEntry) (*roleEntry, *issuerEntry, error) { diff --git a/builtin/logical/pki/acme_wrappers_test.go b/builtin/logical/pki/acme_wrappers_test.go index 0d575697a..418206649 100644 --- a/builtin/logical/pki/acme_wrappers_test.go +++ b/builtin/logical/pki/acme_wrappers_test.go @@ -6,6 +6,7 @@ package pki import ( "context" "fmt" + "strings" "testing" "github.com/hashicorp/vault/sdk/framework" @@ -92,15 +93,20 @@ func TestACMEIssuerRoleLoading(t *testing.T) { return nil, nil }) + var acmePath string fieldRaw := map[string]interface{}{} - if tt.roleName != "" { - fieldRaw["role"] = tt.roleName - } if tt.issuerName != "" { fieldRaw[issuerRefParam] = tt.issuerName + acmePath = "issuer/" + tt.issuerName + "/" + } + if tt.roleName != "" { + fieldRaw["role"] = tt.roleName + acmePath = acmePath + "roles/" + tt.roleName + "/" } - resp, err := f(context.Background(), &logical.Request{Storage: s}, &framework.FieldData{ + acmePath = strings.TrimLeft(acmePath+"/acme/directory", "/") + + resp, err := f(context.Background(), &logical.Request{Path: acmePath, Storage: s}, &framework.FieldData{ Raw: fieldRaw, Schema: getCsrSignVerbatimSchemaFields(), }) diff --git a/builtin/logical/pki/backend.go b/builtin/logical/pki/backend.go index 6966bd737..8caf2f6a2 100644 --- a/builtin/logical/pki/backend.go +++ b/builtin/logical/pki/backend.go @@ -218,7 +218,6 @@ func Backend(conf *logical.BackendConfig) *backend { // ACME pathAcmeConfig(&b), - pathAcmeEabCreate(&b), pathAcmeEabList(&b), pathAcmeEabDelete(&b), }, @@ -248,6 +247,7 @@ func Backend(conf *logical.BackendConfig) *backend { acmePaths = append(acmePaths, pathAcmeChallenge(&b)...) acmePaths = append(acmePaths, pathAcmeAuthorization(&b)...) acmePaths = append(acmePaths, pathAcmeRevoke(&b)...) + acmePaths = append(acmePaths, pathAcmeNewEab(&b)...) // auth'd API that lives underneath the various /acme paths for _, acmePath := range acmePaths { b.Backend.Paths = append(b.Backend.Paths, acmePath) @@ -268,6 +268,7 @@ func Backend(conf *logical.BackendConfig) *backend { b.PathsSpecial.Unauthenticated = append(b.PathsSpecial.Unauthenticated, acmePrefix+"acme/order/+") b.PathsSpecial.Unauthenticated = append(b.PathsSpecial.Unauthenticated, acmePrefix+"acme/order/+/finalize") b.PathsSpecial.Unauthenticated = append(b.PathsSpecial.Unauthenticated, acmePrefix+"acme/order/+/cert") + // We specifically do NOT add acme/new-eab to this as it should be auth'd } if constants.IsEnterprise { diff --git a/builtin/logical/pki/backend_test.go b/builtin/logical/pki/backend_test.go index 9f2e84fb9..6e7791993 100644 --- a/builtin/logical/pki/backend_test.go +++ b/builtin/logical/pki/backend_test.go @@ -6862,9 +6862,8 @@ func TestProperAuthing(t *testing.T) { "unified-crl/delta/pem": shouldBeUnauthedReadList, "unified-ocsp": shouldBeUnauthedWriteOnly, "unified-ocsp/dGVzdAo=": shouldBeUnauthedReadList, - "acme/new-eab": shouldBeAuthed, - "acme/eab": shouldBeAuthed, - "acme/eab/" + eabKid: shouldBeAuthed, + "eab": shouldBeAuthed, + "eab/" + eabKid: shouldBeAuthed, } // Add ACME based paths to the test suite @@ -6881,6 +6880,9 @@ func TestProperAuthing(t *testing.T) { paths[acmePrefix+"acme/order/13b80844-e60d-42d2-b7e9-152a8e834b90"] = shouldBeUnauthedWriteOnly paths[acmePrefix+"acme/order/13b80844-e60d-42d2-b7e9-152a8e834b90/finalize"] = shouldBeUnauthedWriteOnly paths[acmePrefix+"acme/order/13b80844-e60d-42d2-b7e9-152a8e834b90/cert"] = shouldBeUnauthedWriteOnly + + // Make sure this new-eab path is auth'd + paths[acmePrefix+"acme/new-eab"] = shouldBeAuthed } for path, checkerType := range paths { @@ -6938,7 +6940,7 @@ func TestProperAuthing(t *testing.T) { if strings.Contains(raw_path, "acme/") && strings.Contains(raw_path, "{order_id}") { raw_path = strings.ReplaceAll(raw_path, "{order_id}", "13b80844-e60d-42d2-b7e9-152a8e834b90") } - if strings.Contains(raw_path, "acme/eab") && strings.Contains(raw_path, "{key_id}") { + if strings.Contains(raw_path, "eab") && strings.Contains(raw_path, "{key_id}") { raw_path = strings.ReplaceAll(raw_path, "{key_id}", eabKid) } diff --git a/builtin/logical/pki/path_acme_eab.go b/builtin/logical/pki/path_acme_eab.go index 35fcde2f3..27fae7531 100644 --- a/builtin/logical/pki/path_acme_eab.go +++ b/builtin/logical/pki/path_acme_eab.go @@ -22,7 +22,7 @@ import ( */ func pathAcmeEabList(b *backend) *framework.Path { return &framework.Path{ - Pattern: "acme/eab/?$", + Pattern: "eab/?$", DisplayAttrs: &framework.DisplayAttributes{ OperationPrefix: operationPrefixPKI, @@ -45,16 +45,17 @@ func pathAcmeEabList(b *backend) *framework.Path { } } -func pathAcmeEabCreate(b *backend) *framework.Path { +func pathAcmeNewEab(b *backend) []*framework.Path { + return buildAcmeFrameworkPaths(b, patternAcmeNewEab, "/new-eab") +} + +func patternAcmeNewEab(b *backend, pattern string) *framework.Path { + fields := map[string]*framework.FieldSchema{} + addFieldsForACMEPath(fields, pattern) + return &framework.Path{ - Pattern: "acme/new-eab", - - DisplayAttrs: &framework.DisplayAttributes{ - OperationPrefix: operationPrefixPKI, - }, - - Fields: map[string]*framework.FieldSchema{}, - + Pattern: pattern, + Fields: fields, Operations: map[logical.Operation]framework.OperationHandler{ logical.UpdateOperation: &framework.PathOperation{ Callback: b.pathAcmeCreateEab, @@ -67,15 +68,18 @@ func pathAcmeEabCreate(b *backend) *framework.Path { }, }, - HelpSynopsis: "Generate or list external account bindings to be used for ACME", - HelpDescription: `Generate single use id/key pairs to be used for ACME EAB or list -identifiers that have been generated but yet to be used.`, + DisplayAttrs: &framework.DisplayAttributes{ + OperationPrefix: operationPrefixPKI, + }, + + HelpSynopsis: "Generate external account bindings to be used for ACME", + HelpDescription: `Generate single use id/key pairs to be used for ACME EAB.`, } } func pathAcmeEabDelete(b *backend) *framework.Path { return &framework.Path{ - Pattern: "acme/eab/" + uuidNameRegex("key_id"), + Pattern: "eab/" + uuidNameRegex("key_id"), DisplayAttrs: &framework.DisplayAttributes{ OperationPrefix: operationPrefixPKI, @@ -109,11 +113,12 @@ a warning that it did not exist.`, } type eabType struct { - KeyID string `json:"-"` - KeyType string `json:"key-type"` - KeyBits int `json:"key-bits"` - PrivateBytes []byte `json:"private-bytes"` - CreatedOn time.Time `json:"created-on"` + KeyID string `json:"-"` + KeyType string `json:"key-type"` + KeyBits int `json:"key-bits"` + PrivateBytes []byte `json:"private-bytes"` + AcmeDirectory string `json:"acme-directory"` + CreatedOn time.Time `json:"created-on"` } func (b *backend) pathAcmeListEab(ctx context.Context, r *logical.Request, _ *framework.FieldData) (*logical.Response, error) { @@ -137,9 +142,10 @@ func (b *backend) pathAcmeListEab(ctx context.Context, r *logical.Request, _ *fr keyIds = append(keyIds, eab.KeyID) keyInfos[eab.KeyID] = map[string]interface{}{ - "key_type": eab.KeyType, - "key_bits": eab.KeyBits, - "created_on": eab.CreatedOn.Format(time.RFC3339), + "key_type": eab.KeyType, + "key_bits": eab.KeyBits, + "acme_directory": eab.AcmeDirectory, + "created_on": eab.CreatedOn.Format(time.RFC3339), } } @@ -150,7 +156,7 @@ func (b *backend) pathAcmeListEab(ctx context.Context, r *logical.Request, _ *fr return resp, nil } -func (b *backend) pathAcmeCreateEab(ctx context.Context, r *logical.Request, _ *framework.FieldData) (*logical.Response, error) { +func (b *backend) pathAcmeCreateEab(ctx context.Context, r *logical.Request, data *framework.FieldData) (*logical.Response, error) { kid := genUuid() size := 32 bytes, err := uuid.GenerateRandomBytesWithReader(size, rand.Reader) @@ -158,12 +164,18 @@ func (b *backend) pathAcmeCreateEab(ctx context.Context, r *logical.Request, _ * return nil, fmt.Errorf("failed generating eab key: %w", err) } + acmeDirectory, err := getAcmeDirectory(r) + if err != nil { + return nil, err + } + eab := &eabType{ - KeyID: kid, - KeyType: "hs", - KeyBits: size * 8, - PrivateBytes: bytes, - CreatedOn: time.Now(), + KeyID: kid, + KeyType: "hs", + KeyBits: size * 8, + PrivateBytes: bytes, + AcmeDirectory: acmeDirectory, + CreatedOn: time.Now(), } sc := b.makeStorageContext(ctx, r.Storage) @@ -176,11 +188,12 @@ func (b *backend) pathAcmeCreateEab(ctx context.Context, r *logical.Request, _ * return &logical.Response{ Data: map[string]interface{}{ - "id": eab.KeyID, - "key_type": eab.KeyType, - "key_bits": eab.KeyBits, - "key": encodedKey, - "created_on": eab.CreatedOn.Format(time.RFC3339), + "id": eab.KeyID, + "key_type": eab.KeyType, + "key_bits": eab.KeyBits, + "key": encodedKey, + "acme_directory": eab.AcmeDirectory, + "created_on": eab.CreatedOn.Format(time.RFC3339), }, }, nil } diff --git a/builtin/logical/pki/path_acme_eab_test.go b/builtin/logical/pki/path_acme_eab_test.go deleted file mode 100644 index f722e7b33..000000000 --- a/builtin/logical/pki/path_acme_eab_test.go +++ /dev/null @@ -1,75 +0,0 @@ -// Copyright (c) HashiCorp, Inc. -// SPDX-License-Identifier: MPL-2.0 - -package pki - -import ( - "encoding/base64" - "testing" - "time" - - "github.com/hashicorp/go-uuid" - "github.com/stretchr/testify/require" -) - -// TestACME_EabVaultAPIs verify the various Vault auth'd APIs for EAB work as expected, -// with creation, listing and deletions. -func TestACME_EabVaultAPIs(t *testing.T) { - b, s := CreateBackendWithStorage(t) - - var ids []string - - // Generate an EAB - resp, err := CBWrite(b, s, "acme/new-eab", map[string]interface{}{}) - requireSuccessNonNilResponse(t, resp, err, "Failed generating eab") - requireFieldsSetInResp(t, resp, "id", "key_type", "key_bits", "key", "created_on") - require.Equal(t, "hs", resp.Data["key_type"]) - require.Equal(t, 256, resp.Data["key_bits"]) - ids = append(ids, resp.Data["id"].(string)) - _, err = uuid.ParseUUID(resp.Data["id"].(string)) - require.NoError(t, err, "failed parsing id as a uuid") - - _, err = base64.RawURLEncoding.DecodeString(resp.Data["key"].(string)) - require.NoError(t, err, "failed base64 decoding private key") - require.NoError(t, err, "failed parsing private key") - - // Generate another EAB - resp, err = CBWrite(b, s, "acme/new-eab", map[string]interface{}{}) - requireSuccessNonNilResponse(t, resp, err, "Failed generating eab") - ids = append(ids, resp.Data["id"].(string)) - - // List our EABs - resp, err = CBList(b, s, "acme/eab/") - requireSuccessNonNilResponse(t, resp, err, "failed list") - - require.ElementsMatch(t, ids, resp.Data["keys"]) - keyInfo := resp.Data["key_info"].(map[string]interface{}) - id0Map := keyInfo[ids[0]].(map[string]interface{}) - require.Equal(t, "hs", id0Map["key_type"]) - require.Equal(t, 256, id0Map["key_bits"]) - require.NotEmpty(t, id0Map["created_on"]) - _, err = time.Parse(time.RFC3339, id0Map["created_on"].(string)) - require.NoError(t, err, "failed to parse created_on date: %s", id0Map["created_on"]) - - id1Map := keyInfo[ids[1]].(map[string]interface{}) - - require.Equal(t, "hs", id1Map["key_type"]) - require.Equal(t, 256, id1Map["key_bits"]) - require.NotEmpty(t, id1Map["created_on"]) - - // Delete an EAB - resp, err = CBDelete(b, s, "acme/eab/"+ids[0]) - requireSuccessNonNilResponse(t, resp, err, "failed deleting eab identifier") - require.Len(t, resp.Warnings, 0, "no warnings should have been set on delete") - - // Make sure it's really gone - resp, err = CBList(b, s, "acme/eab/") - requireSuccessNonNilResponse(t, resp, err, "failed list post delete") - require.Len(t, resp.Data["keys"], 1) - require.Contains(t, resp.Data["keys"], ids[1]) - - // Delete the same EAB again, we should just get a warning but still success. - resp, err = CBDelete(b, s, "acme/eab/"+ids[0]) - requireSuccessNonNilResponse(t, resp, err, "failed deleting eab identifier") - require.Len(t, resp.Warnings, 1, "expected a warning to be set on repeated delete call") -} diff --git a/builtin/logical/pki/path_acme_test.go b/builtin/logical/pki/path_acme_test.go index b1882ad25..cb26bfa5c 100644 --- a/builtin/logical/pki/path_acme_test.go +++ b/builtin/logical/pki/path_acme_test.go @@ -45,16 +45,16 @@ func TestAcmeBasicWorkflow(t *testing.T) { name string prefixUrl string }{ - {"root", ""}, - {"role", "/roles/test-role"}, - {"issuer", "/issuer/int-ca"}, - {"issuer_role", "/issuer/int-ca/roles/test-role"}, + {"root", "acme/"}, + {"role", "roles/test-role/acme/"}, + {"issuer", "issuer/int-ca/acme/"}, + {"issuer_role", "issuer/int-ca/roles/test-role/acme/"}, } testCtx := context.Background() for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - baseAcmeURL := "/v1/pki" + tc.prefixUrl + "/acme/" + baseAcmeURL := "/v1/pki/" + tc.prefixUrl accountKey, err := rsa.GenerateKey(rand.Reader, 2048) require.NoError(t, err, "failed creating rsa key") @@ -329,76 +329,110 @@ func TestAcmeBasicWorkflowWithEab(t *testing.T) { }) require.NoError(t, err) - baseAcmeURL := "/v1/pki/acme/" - accountKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) - require.NoError(t, err, "failed creating ec key") - - acmeClient := getAcmeClientForCluster(t, cluster, baseAcmeURL, accountKey) - - t.Logf("Testing discover on %s", baseAcmeURL) - discovery, err := acmeClient.Discover(testCtx) - require.NoError(t, err, "failed acme discovery call") - require.True(t, discovery.ExternalAccountRequired, "bad value for external account required in directory") - - // Create new account without EAB, should fail - t.Logf("Testing register on %s", baseAcmeURL) - _, err = acmeClient.Register(testCtx, &acme.Account{}, func(tosURL string) bool { return true }) - require.ErrorContains(t, err, "urn:ietf:params:acme:error:externalAccountRequired", - "expected failure creating an account without eab") - - kid, eabKeyBytes := getEABKey(t, client) - acct := &acme.Account{ - ExternalAccountBinding: &acme.ExternalAccountBinding{ - KID: kid, - Key: eabKeyBytes, - }, + cases := []struct { + name string + prefixUrl string + }{ + {"root", "acme/"}, + {"role", "roles/test-role/acme/"}, + {"issuer", "issuer/int-ca/acme/"}, + {"issuer_role", "issuer/int-ca/roles/test-role/acme/"}, } - // Make sure we can list our key - resp, err := client.Logical().ListWithContext(context.Background(), "pki/acme/eab") - require.NoError(t, err, "failed to list eab tokens") - require.NotNil(t, resp, "list response for eab tokens should not be nil") - require.Contains(t, resp.Data, "keys") - require.Contains(t, resp.Data, "key_info") - require.Len(t, resp.Data["keys"], 1) - require.Contains(t, resp.Data["keys"], kid) + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + baseAcmeURL := "/v1/pki/" + tc.prefixUrl + accountKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err, "failed creating ec key") - keyInfo := resp.Data["key_info"].(map[string]interface{}) - require.Contains(t, keyInfo, kid) + acmeClient := getAcmeClientForCluster(t, cluster, baseAcmeURL, accountKey) - infoForKid := keyInfo[kid].(map[string]interface{}) - keyBits := infoForKid["key_bits"].(json.Number) - require.Equal(t, "256", keyBits.String()) - require.Equal(t, "hs", infoForKid["key_type"]) + t.Logf("Testing discover on %s", baseAcmeURL) + discovery, err := acmeClient.Discover(testCtx) + require.NoError(t, err, "failed acme discovery call") + require.True(t, discovery.ExternalAccountRequired, "bad value for external account required in directory") - // Create new account with EAB - t.Logf("Testing register on %s", baseAcmeURL) - _, err = acmeClient.Register(testCtx, acct, func(tosURL string) bool { return true }) - require.NoError(t, err, "failed registering new account with eab") + // Create new account without EAB, should fail + t.Logf("Testing register on %s", baseAcmeURL) + _, err = acmeClient.Register(testCtx, &acme.Account{}, func(tosURL string) bool { return true }) + require.ErrorContains(t, err, "urn:ietf:params:acme:error:externalAccountRequired", + "expected failure creating an account without eab") - // Make sure our EAB is no longer available - resp, err = client.Logical().ListWithContext(context.Background(), "pki/acme/eab") - require.NoError(t, err, "failed to list eab tokens") - require.Nil(t, resp, "list response for eab tokens should have been nil due to empty list") + // Test fetch, list, delete workflow + kid, _ := getEABKey(t, client, tc.prefixUrl) + resp, err := client.Logical().ListWithContext(testCtx, "pki/eab") + require.NoError(t, err, "failed to list eab tokens") + require.NotNil(t, resp, "list response for eab tokens should not be nil") + require.Contains(t, resp.Data, "keys") + require.Contains(t, resp.Data, "key_info") + require.Len(t, resp.Data["keys"], 1) + require.Contains(t, resp.Data["keys"], kid) - // Attempt to create another account with the same EAB as before -- should fail - accountKey2, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) - require.NoError(t, err, "failed creating ec key") + _, err = client.Logical().DeleteWithContext(testCtx, "pki/eab/"+kid) + require.NoError(t, err, "failed to delete eab") - acmeClient2 := getAcmeClientForCluster(t, cluster, baseAcmeURL, accountKey2) - acct2 := &acme.Account{ - ExternalAccountBinding: &acme.ExternalAccountBinding{ - KID: kid, - Key: eabKeyBytes, - }, + // List eabs should return zero results + resp, err = client.Logical().ListWithContext(testCtx, "pki/eab") + require.NoError(t, err, "failed to list eab tokens") + require.Nil(t, resp, "list response for eab tokens should have been nil") + + // fetch a new EAB + kid, eabKeyBytes := getEABKey(t, client, tc.prefixUrl) + acct := &acme.Account{ + ExternalAccountBinding: &acme.ExternalAccountBinding{ + KID: kid, + Key: eabKeyBytes, + }, + } + + // Make sure we can list our key + resp, err = client.Logical().ListWithContext(testCtx, "pki/eab") + require.NoError(t, err, "failed to list eab tokens") + require.NotNil(t, resp, "list response for eab tokens should not be nil") + require.Contains(t, resp.Data, "keys") + require.Contains(t, resp.Data, "key_info") + require.Len(t, resp.Data["keys"], 1) + require.Contains(t, resp.Data["keys"], kid) + + keyInfo := resp.Data["key_info"].(map[string]interface{}) + require.Contains(t, keyInfo, kid) + + infoForKid := keyInfo[kid].(map[string]interface{}) + keyBits := infoForKid["key_bits"].(json.Number) + require.Equal(t, "256", keyBits.String()) + require.Equal(t, "hs", infoForKid["key_type"]) + require.Equal(t, tc.prefixUrl, infoForKid["acme_directory"]) + + // Create new account with EAB + t.Logf("Testing register on %s", baseAcmeURL) + _, err = acmeClient.Register(testCtx, acct, func(tosURL string) bool { return true }) + require.NoError(t, err, "failed registering new account with eab") + + // Make sure our EAB is no longer available + resp, err = client.Logical().ListWithContext(context.Background(), "pki/eab") + require.NoError(t, err, "failed to list eab tokens") + require.Nil(t, resp, "list response for eab tokens should have been nil due to empty list") + + // Attempt to create another account with the same EAB as before -- should fail + accountKey2, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err, "failed creating ec key") + + acmeClient2 := getAcmeClientForCluster(t, cluster, baseAcmeURL, accountKey2) + acct2 := &acme.Account{ + ExternalAccountBinding: &acme.ExternalAccountBinding{ + KID: kid, + Key: eabKeyBytes, + }, + } + + _, err = acmeClient2.Register(testCtx, acct2, func(tosURL string) bool { return true }) + require.ErrorContains(t, err, "urn:ietf:params:acme:error:unauthorized", "should fail due to EAB re-use") + + // We can lookup/find an existing account without EAB if we have the account key + _, err = acmeClient.GetReg(testCtx /* unused url */, "") + require.NoError(t, err, "expected to lookup existing account without eab") + }) } - - _, err = acmeClient2.Register(testCtx, acct2, func(tosURL string) bool { return true }) - require.ErrorContains(t, err, "urn:ietf:params:acme:error:unauthorized", "should fail due to EAB re-use") - - // We can lookup/find an existing account without EAB if we have the account key - _, err = acmeClient.GetReg(testCtx /* unused url */, "") - require.NoError(t, err, "expected to lookup existing account without eab") } // TestAcmeNonce a basic test that will validate we get back a nonce with the proper status codes @@ -540,6 +574,41 @@ func TestAcmeAccountsCrossingDirectoryPath(t *testing.T) { // swallows the error we are sending back to a no account error } +// TestAcmeEabCrossingDirectoryPath make sure that if an account attempts to use a different ACME +// directory path that an EAB was created within we get an error. +func TestAcmeEabCrossingDirectoryPath(t *testing.T) { + t.Parallel() + cluster, client, _ := setupAcmeBackend(t) + defer cluster.Cleanup() + + // Enable EAB + _, err := client.Logical().WriteWithContext(context.Background(), "pki/config/acme", map[string]interface{}{ + "enabled": true, + "eab_policy": "always-required", + }) + require.NoError(t, err) + + baseAcmeURL := "/v1/pki/acme/" + accountKey, err := rsa.GenerateKey(rand.Reader, 2048) + require.NoError(t, err, "failed creating rsa key") + + testCtx := context.Background() + acmeClient := getAcmeClientForCluster(t, cluster, baseAcmeURL, accountKey) + + // fetch a new EAB + kid, eabKeyBytes := getEABKey(t, client, "roles/test-role/acme/") + acct := &acme.Account{ + ExternalAccountBinding: &acme.ExternalAccountBinding{ + KID: kid, + Key: eabKeyBytes, + }, + } + + // Create new account + _, err = acmeClient.Register(testCtx, acct, func(tosURL string) bool { return true }) + require.ErrorContains(t, err, "failed to verify eab", "should have failed as EAB is for a different directory") +} + // TestAcmeDisabledWithEnvVar verifies if VAULT_DISABLE_PUBLIC_ACME is set that we completely // disable the ACME service func TestAcmeDisabledWithEnvVar(t *testing.T) { @@ -1010,8 +1079,8 @@ func getAcmeClientForCluster(t *testing.T, cluster *vault.TestCluster, baseUrl s } } -func getEABKey(t *testing.T, client *api.Client) (string, []byte) { - resp, err := client.Logical().WriteWithContext(ctx, "pki/acme/new-eab", map[string]interface{}{}) +func getEABKey(t *testing.T, client *api.Client, baseUrl string) (string, []byte) { + resp, err := client.Logical().WriteWithContext(ctx, path.Join("pki/", baseUrl, "/new-eab"), map[string]interface{}{}) require.NoError(t, err, "failed getting eab key") require.NotNil(t, resp, "eab key returned nil response") require.NotEmpty(t, resp.Data["id"], "eab key response missing id field") @@ -1022,5 +1091,12 @@ func getEABKey(t *testing.T, client *api.Client) (string, []byte) { privateKeyBytes, err := base64.RawURLEncoding.DecodeString(base64Key) require.NoError(t, err, "failed base 64 decoding eab key response") + require.Equal(t, "hs", resp.Data["key_type"], "eab key_type field mis-match") + require.Equal(t, json.Number("256"), resp.Data["key_bits"], "eab key_bits field mis-match") + require.Equal(t, baseUrl, resp.Data["acme_directory"], "eab acme_directory field mis-match") + require.NotEmpty(t, resp.Data["created_on"], "empty created_on field") + _, err = time.Parse(time.RFC3339, resp.Data["created_on"].(string)) + require.NoError(t, err, "failed parsing eab created_on field") + return kid, privateKeyBytes } diff --git a/builtin/logical/pki/path_tidy.go b/builtin/logical/pki/path_tidy.go index 727ab7dab..61ea64de5 100644 --- a/builtin/logical/pki/path_tidy.go +++ b/builtin/logical/pki/path_tidy.go @@ -1529,7 +1529,7 @@ func (b *backend) doTidyAcme(ctx context.Context, req *logical.Request, logger h b.tidyStatus.acmeAccountsCount = uint(len(thumbprints)) b.tidyStatusLock.Unlock() - baseUrl, _, err := getAcmeBaseUrl(sc, req.Path) + baseUrl, _, err := getAcmeBaseUrl(sc, req) if err != nil { return err } diff --git a/builtin/logical/pkiext/pkiext_binary/acme_test.go b/builtin/logical/pkiext/pkiext_binary/acme_test.go index 0f4b8f9a3..4e43852cf 100644 --- a/builtin/logical/pkiext/pkiext_binary/acme_test.go +++ b/builtin/logical/pkiext/pkiext_binary/acme_test.go @@ -36,6 +36,7 @@ func Test_ACME(t *testing.T) { tc := map[string]func(t *testing.T, cluster *VaultPkiCluster){ "certbot": SubtestACMECertbot, + "certbot eab": SubtestACMECertbotEab, "acme ip sans": SubtestACMEIPAndDNS, "acme wildcard": SubtestACMEWildcardDNS, "acme prevents ica": SubtestACMEPreventsICADNS, @@ -153,6 +154,111 @@ func SubtestACMECertbot(t *testing.T, cluster *VaultPkiCluster) { require.NotEqual(t, 0, retcode, "expected non-zero retcode double revoke command result") } +func SubtestACMECertbotEab(t *testing.T, cluster *VaultPkiCluster) { + mountName := "pki-certbot-eab" + pki, err := cluster.CreateAcmeMount(mountName) + require.NoError(t, err, "failed setting up acme mount") + + err = pki.UpdateAcmeConfig(true, map[string]interface{}{ + "eab_policy": "new-account-required", + }) + require.NoError(t, err) + + eabId, base64EabKey, err := pki.GetEabKey("acme/") + + directory := "https://" + pki.GetActiveContainerIP() + ":8200/v1/" + mountName + "/acme/directory" + vaultNetwork := pki.GetContainerNetworkName() + + logConsumer, logStdout, logStderr := getDockerLog(t) + + t.Logf("creating on network: %v", vaultNetwork) + runner, err := hDocker.NewServiceRunner(hDocker.RunOptions{ + ImageRepo: "docker.mirror.hashicorp.services/certbot/certbot", + ImageTag: "latest", + ContainerName: "vault_pki_certbot_eab_test", + NetworkName: vaultNetwork, + Entrypoint: []string{"sleep", "45"}, + LogConsumer: logConsumer, + LogStdout: logStdout, + LogStderr: logStderr, + }) + require.NoError(t, err, "failed creating service runner") + + ctx := context.Background() + result, err := runner.Start(ctx, true, false) + require.NoError(t, err, "could not start container") + require.NotNil(t, result, "could not start container") + + defer runner.Stop(context.Background(), result.Container.ID) + + networks, err := runner.GetNetworkAndAddresses(result.Container.ID) + require.NoError(t, err, "could not read container's IP address") + require.Contains(t, networks, vaultNetwork, "expected to contain vault network") + + ipAddr := networks[vaultNetwork] + hostname := "certbot-eab-acme-client.dadgarcorp.com" + + err = pki.AddHostname(hostname, ipAddr) + require.NoError(t, err, "failed to update vault host files") + + certbotCmd := []string{ + "certbot", + "certonly", + "--no-eff-email", + "--email", "certbot.client@dadgarcorp.com", + "--eab-kid", eabId, + "--eab-hmac-key", base64EabKey, + "--agree-tos", + "--no-verify-ssl", + "--standalone", + "--non-interactive", + "--server", directory, + "-d", hostname, + } + logCatCmd := []string{"cat", "/var/log/letsencrypt/letsencrypt.log"} + + stdout, stderr, retcode, err := runner.RunCmdWithOutput(ctx, result.Container.ID, certbotCmd) + t.Logf("Certbot Issue Command: %v\nstdout: %v\nstderr: %v\n", certbotCmd, string(stdout), string(stderr)) + if err != nil || retcode != 0 { + logsStdout, logsStderr, _, _ := runner.RunCmdWithOutput(ctx, result.Container.ID, logCatCmd) + t.Logf("Certbot logs\nstdout: %v\nstderr: %v\n", string(logsStdout), string(logsStderr)) + } + require.NoError(t, err, "got error running issue command") + require.Equal(t, 0, retcode, "expected zero retcode issue command result") + + certbotRevokeCmd := []string{ + "certbot", + "revoke", + "--no-eff-email", + "--email", "certbot.client@dadgarcorp.com", + "--agree-tos", + "--no-verify-ssl", + "--non-interactive", + "--no-delete-after-revoke", + "--cert-name", hostname, + } + + stdout, stderr, retcode, err = runner.RunCmdWithOutput(ctx, result.Container.ID, certbotRevokeCmd) + t.Logf("Certbot Revoke Command: %v\nstdout: %v\nstderr: %v\n", certbotRevokeCmd, string(stdout), string(stderr)) + if err != nil || retcode != 0 { + logsStdout, logsStderr, _, _ := runner.RunCmdWithOutput(ctx, result.Container.ID, logCatCmd) + t.Logf("Certbot logs\nstdout: %v\nstderr: %v\n", string(logsStdout), string(logsStderr)) + } + require.NoError(t, err, "got error running revoke command") + require.Equal(t, 0, retcode, "expected zero retcode revoke command result") + + // Revoking twice should fail. + stdout, stderr, retcode, err = runner.RunCmdWithOutput(ctx, result.Container.ID, certbotRevokeCmd) + t.Logf("Certbot Double Revoke Command: %v\nstdout: %v\nstderr: %v\n", certbotRevokeCmd, string(stdout), string(stderr)) + if err != nil || retcode == 0 { + logsStdout, logsStderr, _, _ := runner.RunCmdWithOutput(ctx, result.Container.ID, logCatCmd) + t.Logf("Certbot logs\nstdout: %v\nstderr: %v\n", string(logsStdout), string(logsStderr)) + } + + require.NoError(t, err, "got error running double revoke command") + require.NotEqual(t, 0, retcode, "expected non-zero retcode double revoke command result") +} + func SubtestACMEIPAndDNS(t *testing.T, cluster *VaultPkiCluster) { pki, err := cluster.CreateAcmeMount("pki-ip-dns-sans") require.NoError(t, err, "failed setting up acme mount") diff --git a/builtin/logical/pkiext/pkiext_binary/pki_mount.go b/builtin/logical/pkiext/pkiext_binary/pki_mount.go index 4c2ac7a80..f949cb6ae 100644 --- a/builtin/logical/pkiext/pkiext_binary/pki_mount.go +++ b/builtin/logical/pkiext/pkiext_binary/pki_mount.go @@ -5,7 +5,9 @@ package pkiext_binary import ( "context" + "encoding/base64" "fmt" + "path" "github.com/hashicorp/vault/api" ) @@ -114,6 +116,23 @@ func (vpm *VaultPkiMount) UpdateRole(roleName string, config map[string]interfac return err } +func (vpm *VaultPkiMount) GetEabKey(acmeDirectory string) (string, string, error) { + eabPath := path.Join(vpm.mount, acmeDirectory, "/new-eab") + resp, err := vpm.GetActiveNode().Logical().WriteWithContext(context.Background(), eabPath, map[string]interface{}{}) + if err != nil { + return "", "", fmt.Errorf("failed fetching eab from %s: %w", eabPath, err) + } + eabId := resp.Data["id"].(string) + base64EabKey := resp.Data["key"].(string) + // just make sure we get something valid back from the server, we still want to pass back the base64 version + // to the caller... + _, err = base64.RawURLEncoding.DecodeString(base64EabKey) + if err != nil { + return "", "", fmt.Errorf("failed decoding key response field: %s: %w", base64EabKey, err) + } + return eabId, base64EabKey, nil +} + func mergeWithDefaults(config map[string]interface{}, defaults map[string]interface{}) map[string]interface{} { myConfig := config if myConfig == nil {