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
This commit is contained in:
Steven Clark
2023-05-24 17:17:33 -04:00
committed by GitHub
parent 36d4a25b25
commit 32532c61d1
9 changed files with 124 additions and 64 deletions

View File

@@ -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
}

View File

@@ -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

View File

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

View File

@@ -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,
}

View File

@@ -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
}

View File

@@ -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])

View File

@@ -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")

View File

@@ -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
}

View File

@@ -8,6 +8,7 @@ import (
"context"
"crypto"
"crypto/x509"
"errors"
"fmt"
"sort"
"strings"
@@ -20,6 +21,8 @@ import (
"github.com/hashicorp/vault/sdk/logical"
)
var ErrStorageItemNotFound = errors.New("storage item not found")
const (
storageKeyConfig = "config/keys"
storageIssuerConfig = "config/issuers"