Address various issues related to ACME EAB (#20755)

* Fix various EAB related issues

 - List API wasn't plumbed through properly so it did not work as expected
 - Use random 32 bytes instead of an EC key for EAB key values
 - Update OpenAPI definitions

* Clean up unused EAB keys within tidy

* Move Vault EAB creation path to pki/acme/new-eab

* Update eab vault responses to match up with docs

builtin/logical/pki/acme_jws.go

@@ -259,7 +259,7 @@ func verifyEabPayload(acmeState *acmeState, ac *acmeContext, outer *jwsCtx, expe
 		return nil, fmt.Errorf("%w: failed to verify eab", ErrUnauthorized)
 	}
 
-	verifiedPayload, err := sig.Verify(eabEntry.MacKey)
+	verifiedPayload, err := sig.Verify(eabEntry.PrivateBytes)
 	if err != nil {
 		return nil, err
 	}

builtin/logical/pki/acme_state.go

@@ -600,7 +600,7 @@ func (a *acmeState) LoadEab(sc *storageContext, eabKid string) (*eabType, error)
 		return nil, err
 	}
 	if rawEntry == nil {
-		return nil, fmt.Errorf("no eab found for kid %s", eabKid)
+		return nil, fmt.Errorf("%w: no eab found for kid %s", ErrStorageItemNotFound, eabKid)
 	}
 
 	var eab eabType

builtin/logical/pki/backend.go

@@ -218,7 +218,8 @@ func Backend(conf *logical.BackendConfig) *backend {
 
 			// ACME
 			pathAcmeConfig(&b),
-			pathAcmeEabCreateList(&b),
+			pathAcmeEabCreate(&b),
+			pathAcmeEabList(&b),
 			pathAcmeEabDelete(&b),
 		},
 

builtin/logical/pki/backend_test.go

@@ -6862,6 +6862,7 @@ 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,
 	}

builtin/logical/pki/path_acme_eab.go

@@ -5,12 +5,9 @@ package pki
 
 import (
 	"context"
-	"crypto/ecdsa"
-	"crypto/elliptic"
-	"crypto/x509"
+	"crypto/rand"
 	"encoding/base64"
 	"fmt"
-	"io"
 	"time"
 
 	"github.com/hashicorp/go-uuid"
@@ -23,9 +20,9 @@ import (
  * ACME External Account Bindings, this isn't providing any APIs that an ACME
  * client would use.
  */
-func pathAcmeEabCreateList(b *backend) *framework.Path {
+func pathAcmeEabList(b *backend) *framework.Path {
 	return &framework.Path{
-		Pattern: "acme/eab",
+		Pattern: "acme/eab/?$",
 
 		DisplayAttrs: &framework.DisplayAttributes{
 			OperationPrefix: operationPrefixPKI,
@@ -36,16 +33,35 @@ func pathAcmeEabCreateList(b *backend) *framework.Path {
 		Operations: map[logical.Operation]framework.OperationHandler{
 			logical.ListOperation: &framework.PathOperation{
 				DisplayAttrs: &framework.DisplayAttributes{
-					OperationSuffix: "acme-configuration",
+					OperationVerb:   "list-eab-key",
+					OperationSuffix: "acme",
 				},
 				Callback: b.pathAcmeListEab,
 			},
+		},
+
+		HelpSynopsis:    "list external account bindings to be used for ACME",
+		HelpDescription: `list identifiers that have been generated but yet to be used.`,
+	}
+}
+
+func pathAcmeEabCreate(b *backend) *framework.Path {
+	return &framework.Path{
+		Pattern: "acme/new-eab",
+
+		DisplayAttrs: &framework.DisplayAttributes{
+			OperationPrefix: operationPrefixPKI,
+		},
+
+		Fields: map[string]*framework.FieldSchema{},
+
+		Operations: map[logical.Operation]framework.OperationHandler{
 			logical.UpdateOperation: &framework.PathOperation{
 				Callback:                    b.pathAcmeCreateEab,
 				ForwardPerformanceSecondary: false,
 				ForwardPerformanceStandby:   true,
 				DisplayAttrs: &framework.DisplayAttributes{
-					OperationVerb:   "configure",
+					OperationVerb:   "generate-eab-key",
 					OperationSuffix: "acme",
 				},
 			},
@@ -93,11 +109,11 @@ a warning that it did not exist.`,
 }
 
 type eabType struct {
-	KeyID     string    `json:"-"`
-	KeyType   string    `json:"key-type"`
-	KeyBits   string    `json:"key-bits"`
-	MacKey    []byte    `json:"mac-key"`
-	CreatedOn time.Time `json:"created-on"`
+	KeyID        string    `json:"-"`
+	KeyType      string    `json:"key-type"`
+	KeyBits      int       `json:"key-bits"`
+	PrivateBytes []byte    `json:"private-bytes"`
+	CreatedOn    time.Time `json:"created-on"`
 }
 
 func (b *backend) pathAcmeListEab(ctx context.Context, r *logical.Request, _ *framework.FieldData) (*logical.Response, error) {
@@ -136,17 +152,18 @@ func (b *backend) pathAcmeListEab(ctx context.Context, r *logical.Request, _ *fr
 
 func (b *backend) pathAcmeCreateEab(ctx context.Context, r *logical.Request, _ *framework.FieldData) (*logical.Response, error) {
 	kid := genUuid()
-	macKey, err := generateEabKey(b.GetRandomReader())
+	size := 32
+	bytes, err := uuid.GenerateRandomBytesWithReader(size, rand.Reader)
 	if err != nil {
 		return nil, fmt.Errorf("failed generating eab key: %w", err)
 	}
 
 	eab := &eabType{
-		KeyID:     kid,
-		KeyType:   "ec",
-		KeyBits:   "256",
-		MacKey:    macKey,
-		CreatedOn: time.Now(),
+		KeyID:        kid,
+		KeyType:      "hs",
+		KeyBits:      size * 8,
+		PrivateBytes: bytes,
+		CreatedOn:    time.Now(),
 	}
 
 	sc := b.makeStorageContext(ctx, r.Storage)
@@ -155,15 +172,15 @@ func (b *backend) pathAcmeCreateEab(ctx context.Context, r *logical.Request, _ *
 		return nil, fmt.Errorf("failed saving generated eab: %w", err)
 	}
 
-	encodedKey := base64.RawURLEncoding.EncodeToString(macKey)
+	encodedKey := base64.RawURLEncoding.EncodeToString(eab.PrivateBytes)
 
 	return &logical.Response{
 		Data: map[string]interface{}{
-			"id":          eab.KeyID,
-			"key_type":    eab.KeyType,
-			"key_bits":    eab.KeyBits,
-			"private_key": encodedKey,
-			"created_on":  eab.CreatedOn.Format(time.RFC3339),
+			"id":         eab.KeyID,
+			"key_type":   eab.KeyType,
+			"key_bits":   eab.KeyBits,
+			"key":        encodedKey,
+			"created_on": eab.CreatedOn.Format(time.RFC3339),
 		},
 	}, nil
 }
@@ -188,17 +205,3 @@ func (b *backend) pathAcmeDeleteEab(ctx context.Context, r *logical.Request, d *
 	}
 	return resp, nil
 }
-
-func generateEabKey(random io.Reader) ([]byte, error) {
-	ecKey, err := ecdsa.GenerateKey(elliptic.P256(), random)
-	if err != nil {
-		return nil, err
-	}
-
-	keyBytes, err := x509.MarshalECPrivateKey(ecKey)
-	if err != nil {
-		return nil, err
-	}
-
-	return keyBytes, nil
-}

builtin/logical/pki/path_acme_eab_test.go

@@ -4,7 +4,6 @@
 package pki
 
 import (
-	"crypto/x509"
 	"encoding/base64"
 	"testing"
 	"time"
@@ -21,42 +20,41 @@ func TestACME_EabVaultAPIs(t *testing.T) {
 	var ids []string
 
 	// Generate an EAB
-	resp, err := CBWrite(b, s, "acme/eab", map[string]interface{}{})
+	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", "private_key", "created_on")
-	require.Equal(t, "ec", resp.Data["key_type"])
-	require.Equal(t, "256", resp.Data["key_bits"])
+	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")
 
-	key, err := base64.RawURLEncoding.DecodeString(resp.Data["private_key"].(string))
+	_, err = base64.RawURLEncoding.DecodeString(resp.Data["key"].(string))
 	require.NoError(t, err, "failed base64 decoding private key")
-	_, err = x509.ParseECPrivateKey(key)
 	require.NoError(t, err, "failed parsing private key")
 
 	// Generate another EAB
-	resp, err = CBWrite(b, s, "acme/eab", map[string]interface{}{})
+	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")
+	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, "ec", id0Map["key_type"])
-	require.Equal(t, "256", id0Map["key_bits"])
+	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, "ec", id1Map["key_type"])
-	require.Equal(t, "256", id1Map["key_bits"])
+	require.Equal(t, "hs", id1Map["key_type"])
+	require.Equal(t, 256, id1Map["key_bits"])
 	require.NotEmpty(t, id1Map["created_on"])
 
 	// Delete an EAB
@@ -65,7 +63,7 @@ func TestACME_EabVaultAPIs(t *testing.T) {
 	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")
+	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])

builtin/logical/pki/path_acme_test.go

@@ -13,6 +13,7 @@ import (
 	"crypto/x509"
 	"crypto/x509/pkix"
 	"encoding/base64"
+	"encoding/json"
 	"fmt"
 	"io"
 	"net/http"
@@ -21,7 +22,6 @@ import (
 	"testing"
 	"time"
 
-	"github.com/go-jose/go-jose/v3/json"
 	"github.com/go-test/deep"
 	"github.com/stretchr/testify/require"
 	"golang.org/x/crypto/acme"
@@ -347,13 +347,30 @@ func TestAcmeBasicWorkflowWithEab(t *testing.T) {
 		},
 	}
 
+	// 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)
+
+	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"])
+
 	// 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/acme/eab")
+	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")
 
@@ -763,14 +780,14 @@ 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/eab", map[string]interface{}{})
+	resp, err := client.Logical().WriteWithContext(ctx, "pki/acme/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")
 	kid := resp.Data["id"].(string)
 
-	require.NotEmpty(t, resp.Data["private_key"], "eab key response missing private_key field")
-	base64Key := resp.Data["private_key"].(string)
+	require.NotEmpty(t, resp.Data["key"], "eab key response missing private_key field")
+	base64Key := resp.Data["key"].(string)
 	privateKeyBytes, err := base64.RawURLEncoding.DecodeString(base64Key)
 	require.NoError(t, err, "failed base 64 decoding eab key response")
 

builtin/logical/pki/path_tidy.go

@@ -1520,13 +1520,13 @@ func (b *backend) doTidyAcme(ctx context.Context, req *logical.Request, logger h
 	defer b.acmeAccountLock.Unlock()
 
 	sc := b.makeStorageContext(ctx, req.Storage)
-	list, err := sc.Storage.List(ctx, acmeThumbprintPrefix)
+	thumbprints, err := sc.Storage.List(ctx, acmeThumbprintPrefix)
 	if err != nil {
 		return err
 	}
 
 	b.tidyStatusLock.Lock()
-	b.tidyStatus.acmeAccountsCount = uint(len(list))
+	b.tidyStatus.acmeAccountsCount = uint(len(thumbprints))
 	b.tidyStatusLock.Unlock()
 
 	baseUrl, _, err := getAcmeBaseUrl(sc, req.Path)
@@ -1539,7 +1539,7 @@ func (b *backend) doTidyAcme(ctx context.Context, req *logical.Request, logger h
 		sc:      sc,
 	}
 
-	for _, thumbprint := range list {
+	for _, thumbprint := range thumbprints {
 		err := b.tidyAcmeAccountByThumbprint(b.acmeState, acmeCtx, thumbprint, config.SafetyBuffer, config.AcmeAccountSafetyBuffer)
 		if err != nil {
 			logger.Warn("error tidying account %v: %v", thumbprint, err.Error())
@@ -1559,6 +1559,43 @@ func (b *backend) doTidyAcme(ctx context.Context, req *logical.Request, logger h
 
 	}
 
+	// Clean up any unused EAB
+	eabIds, err := b.acmeState.ListEabIds(sc)
+	if err != nil {
+		return fmt.Errorf("failed listing EAB ids: %w", err)
+	}
+
+	for _, eabId := range eabIds {
+		eab, err := b.acmeState.LoadEab(sc, eabId)
+		if err != nil {
+			if errors.Is(err, ErrStorageItemNotFound) {
+				// We don't need to worry about a consumed EAB
+				continue
+			}
+			return err
+		}
+
+		eabExpiration := eab.CreatedOn.Add(config.AcmeAccountSafetyBuffer)
+		if time.Now().After(eabExpiration) {
+			_, err := b.acmeState.DeleteEab(sc, eabId)
+			if err != nil {
+				return fmt.Errorf("failed to tidy eab %s: %w", eabId, err)
+			}
+		}
+
+		// Check for cancel before continuing.
+		if atomic.CompareAndSwapUint32(b.tidyCancelCAS, 1, 0) {
+			return tidyCancelledError
+		}
+
+		// Check for pause duration to reduce resource consumption.
+		if config.PauseDuration > (0 * time.Second) {
+			b.acmeAccountLock.Unlock() // Correct the Lock
+			time.Sleep(config.PauseDuration)
+			b.acmeAccountLock.Lock()
+		}
+	}
+
 	return nil
 }