From 0bd356f6cd9bd82c108535439a7f83796c89030e Mon Sep 17 00:00:00 2001 From: Steven Clark Date: Mon, 5 Jun 2023 10:38:03 -0400 Subject: [PATCH] Generate ACME EAB tokens that do not start with '-' (#20945) * Generate ACME EAB tokens that do not start with - - To avoid people having issues copying EAB tokens and using them on command lines when they start with - from the base64 encoded values, append a prefix. - Remove the key_bits data from the eab api, not really useful and now technically wrong - Fix up some issues with tests not running in parallel. - Update docs to reflect new EAB apis. * Add ACME directory to the various EAB output APIs * Update EAB token prefix to be divisable by 3 - Our decoded prefix was not divisable by 3, which meant the last character might be tweaked by the rest of the input --- builtin/logical/pki/path_acme_eab.go | 26 ++++++++++++++++++------- builtin/logical/pki/path_acme_test.go | 11 ++++++----- website/content/api-docs/secret/pki.mdx | 19 +++++++++++------- 3 files changed, 37 insertions(+), 19 deletions(-) diff --git a/builtin/logical/pki/path_acme_eab.go b/builtin/logical/pki/path_acme_eab.go index 27fae753100d..d575d6c453d1 100644 --- a/builtin/logical/pki/path_acme_eab.go +++ b/builtin/logical/pki/path_acme_eab.go @@ -8,6 +8,7 @@ import ( "crypto/rand" "encoding/base64" "fmt" + "path" "time" "github.com/hashicorp/go-uuid" @@ -15,6 +16,21 @@ import ( "github.com/hashicorp/vault/sdk/logical" ) +var decodedTokenPrefix = mustBase64Decode("vault-eab-0-") + +func mustBase64Decode(s string) []byte { + bytes, err := base64.RawURLEncoding.DecodeString(s) + if err != nil { + panic(fmt.Sprintf("Token prefix value: %s failed decoding: %v", s, err)) + } + + // Should be dividable by 3 otherwise our prefix will not be properly honored. + if len(bytes)%3 != 0 { + panic(fmt.Sprintf("Token prefix value: %s is not dividable by 3, will not prefix properly", s)) + } + return bytes +} + /* * This file unlike the other path_acme_xxx.go are VAULT APIs to manage the * ACME External Account Bindings, this isn't providing any APIs that an ACME @@ -115,7 +131,6 @@ 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"` AcmeDirectory string `json:"acme-directory"` CreatedOn time.Time `json:"created-on"` @@ -143,8 +158,7 @@ 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, - "acme_directory": eab.AcmeDirectory, + "acme_directory": path.Join(eab.AcmeDirectory, "directory"), "created_on": eab.CreatedOn.Format(time.RFC3339), } } @@ -172,8 +186,7 @@ func (b *backend) pathAcmeCreateEab(ctx context.Context, r *logical.Request, dat eab := &eabType{ KeyID: kid, KeyType: "hs", - KeyBits: size * 8, - PrivateBytes: bytes, + PrivateBytes: append(decodedTokenPrefix, bytes...), // we do this to avoid generating tokens that start with - AcmeDirectory: acmeDirectory, CreatedOn: time.Now(), } @@ -190,9 +203,8 @@ func (b *backend) pathAcmeCreateEab(ctx context.Context, r *logical.Request, dat Data: map[string]interface{}{ "id": eab.KeyID, "key_type": eab.KeyType, - "key_bits": eab.KeyBits, "key": encodedKey, - "acme_directory": eab.AcmeDirectory, + "acme_directory": path.Join(eab.AcmeDirectory, "directory"), "created_on": eab.CreatedOn.Format(time.RFC3339), }, }, nil diff --git a/builtin/logical/pki/path_acme_test.go b/builtin/logical/pki/path_acme_test.go index be60c3680fb4..ebc7d253eceb 100644 --- a/builtin/logical/pki/path_acme_test.go +++ b/builtin/logical/pki/path_acme_test.go @@ -375,10 +375,8 @@ func TestAcmeBasicWorkflowWithEab(t *testing.T) { 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"]) + require.Equal(t, tc.prefixUrl+"directory", infoForKid["acme_directory"]) // Create new account with EAB t.Logf("Testing register on %s", baseAcmeURL) @@ -651,6 +649,8 @@ func TestAcmeConfigChecksPublicAcmeEnv(t *testing.T) { // CSR's selected TTL value in ACME and the issuer's leaf_not_after_behavior setting is set to Err, // we will override the configured behavior and truncate to the issuer's NotAfter func TestAcmeTruncatesToIssuerExpiry(t *testing.T) { + t.Parallel() + cluster, client, _ := setupAcmeBackend(t) defer cluster.Cleanup() @@ -1048,6 +1048,7 @@ func testAcmeCertSignedByCa(t *testing.T, client *api.Client, derCerts [][]byte, // TestAcmeValidationError make sure that we properly return errors on validation errors. func TestAcmeValidationError(t *testing.T) { + t.Parallel() cluster, _, _ := setupAcmeBackend(t) defer cluster.Cleanup() @@ -1198,12 +1199,12 @@ func getEABKey(t *testing.T, client *api.Client, baseUrl string) (string, []byte require.NotEmpty(t, resp.Data["key"], "eab key response missing private_key field") base64Key := resp.Data["key"].(string) + require.True(t, strings.HasPrefix(base64Key, "vault-eab-0-"), "%s should have had a prefix of vault-eab-0-", base64Key) 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.Equal(t, baseUrl+"directory", 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") diff --git a/website/content/api-docs/secret/pki.mdx b/website/content/api-docs/secret/pki.mdx index 81e23213b191..885c0e480c56 100644 --- a/website/content/api-docs/secret/pki.mdx +++ b/website/content/api-docs/secret/pki.mdx @@ -224,11 +224,16 @@ This endpoint returns a new ACME binding token. The `id` response field can be used as the key identifier and the `key` response field be used as the EAB HMAC key in the ACME Client. -Each call to this endpoint will generate and return a new EAB binding token. +Each call to this endpoint will generate and return a new EAB binding token +that is linked to the specific ACME directory it resides under. EAB tokens +are not usable across different ACME directories. -| Method | Path | -| :----- | :------------------ | -| `POST` | `/pki/acme/new-eab` | +| Method | Path | +|:-------|:---------------------------------------------------| +| `POST` | `/pki/acme/new-eab` | +| `POST` | `/pki/issuer/:issuer_ref/acme/new-eab` | +| `POST` | `/pki/roles/:role/acme/new-eab` | +| `POST` | `/pki/issuer/:issuer_ref/roles/:role/acme/new-eab` | #### Parameters @@ -250,8 +255,8 @@ $ curl \ "data": { "created_on": "2023-05-24T14:33:00-04:00", "id": "bc8088d9-3816-5177-ae8e-d8393265f7dd", - "key_bits": "256", "key_type": "hs", + "acme_directory": "acme/directory", "key": "MHcCAQE... additional data elided ...", } } @@ -283,8 +288,8 @@ $ curl \ "key_info": { "bc8088d9-3816-5177-ae8e-d8393265f7dd": { "created_on": "2023-05-24T14:33:00-04:00", - "key_bits": "256", - "key_type": "hs" + "key_type": "hs", + "acme_directory": "acme/directory" } }, "keys": [