Fix PR comments

This commit is contained in:
Herman Slatman 2021-09-16 23:09:24 +02:00
parent 66464ae302
commit 02cd3b6b3b
No known key found for this signature in database
GPG key ID: F4D8A44EA0A75A4F
5 changed files with 160 additions and 99 deletions

View file

@ -44,13 +44,13 @@ func KeyToID(jwk *jose.JSONWebKey) (string, error) {
} }
type ExternalAccountKey struct { type ExternalAccountKey struct {
ID string `json:"id"` ID string `json:"id"`
ProvisionerName string `json:"provisionerName"` Provisioner string `json:"provisioner"`
Name string `json:"name"` Reference string `json:"reference"`
AccountID string `json:"-"` AccountID string `json:"-"`
KeyBytes []byte `json:"-"` KeyBytes []byte `json:"-"`
CreatedAt time.Time `json:"createdAt"` CreatedAt time.Time `json:"createdAt"`
BoundAt time.Time `json:"boundAt,omitempty"` BoundAt time.Time `json:"boundAt,omitempty"`
} }
func (eak *ExternalAccountKey) AlreadyBound() bool { func (eak *ExternalAccountKey) AlreadyBound() bool {

View file

@ -257,7 +257,7 @@ func (h *Handler) validateExternalAccountBinding(ctx context.Context, nar *NewAc
// about the handler and thus about its dependencies. // about the handler and thus about its dependencies.
eabJSONBytes, err := json.Marshal(nar.ExternalAccountBinding) eabJSONBytes, err := json.Marshal(nar.ExternalAccountBinding)
if err != nil { if err != nil {
return nil, acme.WrapErrorISE(err, "error marshaling externalAccountBinding into JSON") return nil, acme.WrapErrorISE(err, "error marshaling externalAccountBinding into bytes")
} }
eabJWS, err := squarejose.ParseSigned(string(eabJSONBytes)) eabJWS, err := squarejose.ParseSigned(string(eabJSONBytes))

View file

@ -686,11 +686,11 @@ func TestHandler_NewAccount(t *testing.T) {
}, },
MockGetExternalAccountKey: func(ctx context.Context, provisionerName string, keyID string) (*acme.ExternalAccountKey, error) { MockGetExternalAccountKey: func(ctx context.Context, provisionerName string, keyID string) (*acme.ExternalAccountKey, error) {
return &acme.ExternalAccountKey{ return &acme.ExternalAccountKey{
ID: "eakID", ID: "eakID",
ProvisionerName: escProvName, Provisioner: escProvName,
Name: "testeak", Reference: "testeak",
KeyBytes: []byte{1, 3, 3, 7}, KeyBytes: []byte{1, 3, 3, 7},
CreatedAt: time.Now(), CreatedAt: time.Now(),
}, nil }, nil
}, },
MockUpdateExternalAccountKey: func(ctx context.Context, provisionerName string, eak *acme.ExternalAccountKey) error { MockUpdateExternalAccountKey: func(ctx context.Context, provisionerName string, eak *acme.ExternalAccountKey) error {
@ -1059,11 +1059,11 @@ func TestHandler_validateExternalAccountBinding(t *testing.T) {
db: &acme.MockDB{ db: &acme.MockDB{
MockGetExternalAccountKey: func(ctx context.Context, provisionerName string, keyID string) (*acme.ExternalAccountKey, error) { MockGetExternalAccountKey: func(ctx context.Context, provisionerName string, keyID string) (*acme.ExternalAccountKey, error) {
return &acme.ExternalAccountKey{ return &acme.ExternalAccountKey{
ID: "eakID", ID: "eakID",
ProvisionerName: escProvName, Provisioner: escProvName,
Name: "testeak", Reference: "testeak",
KeyBytes: []byte{1, 3, 3, 7}, KeyBytes: []byte{1, 3, 3, 7},
CreatedAt: time.Now(), CreatedAt: time.Now(),
}, nil }, nil
}, },
}, },
@ -1200,12 +1200,12 @@ func TestHandler_validateExternalAccountBinding(t *testing.T) {
db: &acme.MockDB{ db: &acme.MockDB{
MockGetExternalAccountKey: func(ctx context.Context, provisionerName string, keyID string) (*acme.ExternalAccountKey, error) { MockGetExternalAccountKey: func(ctx context.Context, provisionerName string, keyID string) (*acme.ExternalAccountKey, error) {
return &acme.ExternalAccountKey{ return &acme.ExternalAccountKey{
ID: "eakID", ID: "eakID",
ProvisionerName: escProvName, Provisioner: escProvName,
Name: "testeak", Reference: "testeak",
CreatedAt: createdAt, CreatedAt: createdAt,
AccountID: "some-account-id", AccountID: "some-account-id",
BoundAt: boundAt, BoundAt: boundAt,
}, nil }, nil
}, },
}, },
@ -1235,11 +1235,11 @@ func TestHandler_validateExternalAccountBinding(t *testing.T) {
db: &acme.MockDB{ db: &acme.MockDB{
MockGetExternalAccountKey: func(ctx context.Context, provisionerName string, keyID string) (*acme.ExternalAccountKey, error) { MockGetExternalAccountKey: func(ctx context.Context, provisionerName string, keyID string) (*acme.ExternalAccountKey, error) {
return &acme.ExternalAccountKey{ return &acme.ExternalAccountKey{
ID: "eakID", ID: "eakID",
ProvisionerName: escProvName, Provisioner: escProvName,
Name: "testeak", Reference: "testeak",
KeyBytes: []byte{1, 2, 3, 4}, KeyBytes: []byte{1, 2, 3, 4},
CreatedAt: time.Now(), CreatedAt: time.Now(),
}, nil }, nil
}, },
}, },
@ -1271,11 +1271,11 @@ func TestHandler_validateExternalAccountBinding(t *testing.T) {
db: &acme.MockDB{ db: &acme.MockDB{
MockGetExternalAccountKey: func(ctx context.Context, provisionerName string, keyID string) (*acme.ExternalAccountKey, error) { MockGetExternalAccountKey: func(ctx context.Context, provisionerName string, keyID string) (*acme.ExternalAccountKey, error) {
return &acme.ExternalAccountKey{ return &acme.ExternalAccountKey{
ID: "eakID", ID: "eakID",
ProvisionerName: escProvName, Provisioner: escProvName,
Name: "testeak", Reference: "testeak",
KeyBytes: []byte{1, 3, 3, 7}, KeyBytes: []byte{1, 3, 3, 7},
CreatedAt: time.Now(), CreatedAt: time.Now(),
}, nil }, nil
}, },
}, },

View file

@ -28,13 +28,13 @@ func (dba *dbAccount) clone() *dbAccount {
} }
type dbExternalAccountKey struct { type dbExternalAccountKey struct {
ID string `json:"id"` ID string `json:"id"`
ProvisionerName string `json:"provisioner_name"` Provisioner string `json:"provisioner"`
Name string `json:"name"` Reference string `json:"reference"`
AccountID string `json:"accountID,omitempty"` AccountID string `json:"accountID,omitempty"`
KeyBytes []byte `json:"key"` KeyBytes []byte `json:"key"`
CreatedAt time.Time `json:"createdAt"` CreatedAt time.Time `json:"createdAt"`
BoundAt time.Time `json:"boundAt"` BoundAt time.Time `json:"boundAt"`
} }
func (db *DB) getAccountIDByKeyID(ctx context.Context, kid string) (string, error) { func (db *DB) getAccountIDByKeyID(ctx context.Context, kid string) (string, error) {
@ -165,7 +165,7 @@ func (db *DB) UpdateAccount(ctx context.Context, acc *acme.Account) error {
} }
// CreateExternalAccountKey creates a new External Account Binding key with a name // CreateExternalAccountKey creates a new External Account Binding key with a name
func (db *DB) CreateExternalAccountKey(ctx context.Context, provisionerName string, name string) (*acme.ExternalAccountKey, error) { func (db *DB) CreateExternalAccountKey(ctx context.Context, provisionerName string, reference string) (*acme.ExternalAccountKey, error) {
keyID, err := randID() keyID, err := randID()
if err != nil { if err != nil {
return nil, err return nil, err
@ -178,24 +178,24 @@ func (db *DB) CreateExternalAccountKey(ctx context.Context, provisionerName stri
} }
dbeak := &dbExternalAccountKey{ dbeak := &dbExternalAccountKey{
ID: keyID, ID: keyID,
ProvisionerName: provisionerName, Provisioner: provisionerName,
Name: name, Reference: reference,
KeyBytes: random, KeyBytes: random,
CreatedAt: clock.Now(), CreatedAt: clock.Now(),
} }
if err = db.save(ctx, keyID, dbeak, nil, "external_account_key", externalAccountKeyTable); err != nil { if err = db.save(ctx, keyID, dbeak, nil, "external_account_key", externalAccountKeyTable); err != nil {
return nil, err return nil, err
} }
return &acme.ExternalAccountKey{ return &acme.ExternalAccountKey{
ID: dbeak.ID, ID: dbeak.ID,
ProvisionerName: dbeak.ProvisionerName, Provisioner: dbeak.Provisioner,
Name: dbeak.Name, Reference: dbeak.Reference,
AccountID: dbeak.AccountID, AccountID: dbeak.AccountID,
KeyBytes: dbeak.KeyBytes, KeyBytes: dbeak.KeyBytes,
CreatedAt: dbeak.CreatedAt, CreatedAt: dbeak.CreatedAt,
BoundAt: dbeak.BoundAt, BoundAt: dbeak.BoundAt,
}, nil }, nil
} }
@ -206,18 +206,18 @@ func (db *DB) GetExternalAccountKey(ctx context.Context, provisionerName string,
return nil, err return nil, err
} }
if dbeak.ProvisionerName != provisionerName { if dbeak.Provisioner != provisionerName {
return nil, acme.NewError(acme.ErrorUnauthorizedType, "name of provisioner does not match provisioner for which the EAB key was created") return nil, acme.NewError(acme.ErrorUnauthorizedType, "name of provisioner does not match provisioner for which the EAB key was created")
} }
return &acme.ExternalAccountKey{ return &acme.ExternalAccountKey{
ID: dbeak.ID, ID: dbeak.ID,
ProvisionerName: dbeak.ProvisionerName, Provisioner: dbeak.Provisioner,
Name: dbeak.Name, Reference: dbeak.Reference,
AccountID: dbeak.AccountID, AccountID: dbeak.AccountID,
KeyBytes: dbeak.KeyBytes, KeyBytes: dbeak.KeyBytes,
CreatedAt: dbeak.CreatedAt, CreatedAt: dbeak.CreatedAt,
BoundAt: dbeak.BoundAt, BoundAt: dbeak.BoundAt,
}, nil }, nil
} }
@ -240,21 +240,24 @@ func (db *DB) GetExternalAccountKeys(ctx context.Context, provisionerName string
return nil, err return nil, err
} }
keys := make([]*acme.ExternalAccountKey, len(entries)) keys := []*acme.ExternalAccountKey{}
for i, entry := range entries { for _, entry := range entries {
dbeak := new(dbExternalAccountKey) dbeak := new(dbExternalAccountKey)
if err = json.Unmarshal(entry.Value, dbeak); err != nil { if err = json.Unmarshal(entry.Value, dbeak); err != nil {
return nil, errors.Wrapf(err, "error unmarshaling external account key %s into dbExternalAccountKey", string(entry.Key)) return nil, errors.Wrapf(err, "error unmarshaling external account key %s into dbExternalAccountKey", string(entry.Key))
} }
keys[i] = &acme.ExternalAccountKey{ if dbeak.Provisioner != provisionerName {
ID: dbeak.ID, continue
KeyBytes: dbeak.KeyBytes,
ProvisionerName: dbeak.ProvisionerName,
Name: dbeak.Name,
AccountID: dbeak.AccountID,
CreatedAt: dbeak.CreatedAt,
BoundAt: dbeak.BoundAt,
} }
keys = append(keys, &acme.ExternalAccountKey{
ID: dbeak.ID,
KeyBytes: dbeak.KeyBytes,
Provisioner: dbeak.Provisioner,
Reference: dbeak.Reference,
AccountID: dbeak.AccountID,
CreatedAt: dbeak.CreatedAt,
BoundAt: dbeak.BoundAt,
})
} }
return keys, nil return keys, nil
@ -266,18 +269,18 @@ func (db *DB) UpdateExternalAccountKey(ctx context.Context, provisionerName stri
return err return err
} }
if old.ProvisionerName != provisionerName { if old.Provisioner != provisionerName {
return acme.NewError(acme.ErrorUnauthorizedType, "name of provisioner does not match provisioner for which the EAB key was created") return acme.NewError(acme.ErrorUnauthorizedType, "name of provisioner does not match provisioner for which the EAB key was created")
} }
nu := dbExternalAccountKey{ nu := dbExternalAccountKey{
ID: eak.ID, ID: eak.ID,
ProvisionerName: eak.ProvisionerName, Provisioner: eak.Provisioner,
Name: eak.Name, Reference: eak.Reference,
AccountID: eak.AccountID, AccountID: eak.AccountID,
KeyBytes: eak.KeyBytes, KeyBytes: eak.KeyBytes,
CreatedAt: eak.CreatedAt, CreatedAt: eak.CreatedAt,
BoundAt: eak.BoundAt, BoundAt: eak.BoundAt,
} }
return db.save(ctx, nu.ID, nu, old, "external_account_key", externalAccountKeyTable) return db.save(ctx, nu.ID, nu, old, "external_account_key", externalAccountKeyTable)

View file

@ -1,28 +1,30 @@
package api package api
import ( import (
"context"
"net/http" "net/http"
"github.com/go-chi/chi" "github.com/go-chi/chi"
"github.com/smallstep/certificates/api" "github.com/smallstep/certificates/api"
"github.com/smallstep/certificates/authority/admin" "github.com/smallstep/certificates/authority/admin"
"github.com/smallstep/certificates/authority/provisioner"
"go.step.sm/linkedca" "go.step.sm/linkedca"
"google.golang.org/protobuf/types/known/timestamppb" "google.golang.org/protobuf/types/known/timestamppb"
) )
// CreateExternalAccountKeyRequest is the type for POST /admin/acme/eab requests // CreateExternalAccountKeyRequest is the type for POST /admin/acme/eab requests
type CreateExternalAccountKeyRequest struct { type CreateExternalAccountKeyRequest struct {
ProvisionerName string `json:"provisioner"` Provisioner string `json:"provisioner"`
Name string `json:"name"` Reference string `json:"reference"`
} }
// Validate validates a new-admin request body. // Validate validates a new ACME EAB Key request body.
func (r *CreateExternalAccountKeyRequest) Validate() error { func (r *CreateExternalAccountKeyRequest) Validate() error {
if r.ProvisionerName == "" { if r.Provisioner == "" {
return admin.NewError(admin.ErrorBadRequestType, "provisioner name cannot be empty") return admin.NewError(admin.ErrorBadRequestType, "provisioner name cannot be empty")
} }
if r.Name == "" { if r.Reference == "" {
return admin.NewError(admin.ErrorBadRequestType, "name / reference cannot be empty") return admin.NewError(admin.ErrorBadRequestType, "reference cannot be empty")
} }
return nil return nil
} }
@ -33,6 +35,38 @@ type GetExternalAccountKeysResponse struct {
NextCursor string `json:"nextCursor"` NextCursor string `json:"nextCursor"`
} }
// provisionerHasEABEnabled determines if the "requireEAB" setting for an ACME
// provisioner is set to true and thus has EAB enabled.
// TODO: rewrite this into a middleware for the ACME handlers? This probably requires
// ensuring that all the ACME EAB APIs that need the middleware work the same in terms
// of specifying the provisioner; probably a bit of refactoring required.
func (h *Handler) provisionerHasEABEnabled(ctx context.Context, provisionerName string) (bool, error) {
var (
p provisioner.Interface
err error
)
if p, err = h.auth.LoadProvisionerByName(provisionerName); err != nil {
return false, admin.WrapErrorISE(err, "error loading provisioner %s", provisionerName)
}
prov, err := h.db.GetProvisioner(ctx, p.GetID())
if err != nil {
return false, admin.WrapErrorISE(err, "error getting provisioner with ID: %s", p.GetID())
}
details := prov.GetDetails()
if details == nil {
return false, admin.NewErrorISE("error getting details for provisioner with ID: %s", p.GetID())
}
acme := details.GetACME()
if acme == nil {
return false, admin.NewErrorISE("error getting ACME details for provisioner with ID: %s", p.GetID())
}
return acme.GetRequireEab(), nil
}
// CreateExternalAccountKey creates a new External Account Binding key // CreateExternalAccountKey creates a new External Account Binding key
func (h *Handler) CreateExternalAccountKey(w http.ResponseWriter, r *http.Request) { func (h *Handler) CreateExternalAccountKey(w http.ResponseWriter, r *http.Request) {
var body CreateExternalAccountKeyRequest var body CreateExternalAccountKeyRequest
@ -46,17 +80,28 @@ func (h *Handler) CreateExternalAccountKey(w http.ResponseWriter, r *http.Reques
return return
} }
eak, err := h.acmeDB.CreateExternalAccountKey(r.Context(), body.ProvisionerName, body.Name) eabEnabled, err := h.provisionerHasEABEnabled(r.Context(), body.Provisioner)
if err != nil { if err != nil {
api.WriteError(w, admin.WrapErrorISE(err, "error creating external account key %s", body.Name)) api.WriteError(w, err)
return
}
if !eabEnabled {
api.WriteError(w, admin.NewError(admin.ErrorBadRequestType, "ACME EAB not enabled for provisioner %s", body.Provisioner))
return
}
eak, err := h.acmeDB.CreateExternalAccountKey(r.Context(), body.Provisioner, body.Reference)
if err != nil {
api.WriteError(w, admin.WrapErrorISE(err, "error creating external account key %s for provisioner %s", body.Reference, body.Provisioner))
return return
} }
response := &linkedca.EABKey{ response := &linkedca.EABKey{
EabKid: eak.ID, Id: eak.ID,
EabHmacKey: eak.KeyBytes, HmacKey: eak.KeyBytes,
ProvisionerName: eak.ProvisionerName, Provisioner: eak.Provisioner,
Name: eak.Name, Reference: eak.Reference,
} }
api.ProtoJSONStatus(w, response, http.StatusCreated) api.ProtoJSONStatus(w, response, http.StatusCreated)
@ -66,6 +111,8 @@ func (h *Handler) CreateExternalAccountKey(w http.ResponseWriter, r *http.Reques
func (h *Handler) DeleteExternalAccountKey(w http.ResponseWriter, r *http.Request) { func (h *Handler) DeleteExternalAccountKey(w http.ResponseWriter, r *http.Request) {
id := chi.URLParam(r, "id") id := chi.URLParam(r, "id")
// TODO: add provisioner as parameter, so that check can be performed if EAB is enabled or not
if err := h.acmeDB.DeleteExternalAccountKey(r.Context(), id); err != nil { if err := h.acmeDB.DeleteExternalAccountKey(r.Context(), id); err != nil {
api.WriteError(w, admin.WrapErrorISE(err, "error deleting ACME EAB Key %s", id)) api.WriteError(w, admin.WrapErrorISE(err, "error deleting ACME EAB Key %s", id))
return return
@ -78,6 +125,17 @@ func (h *Handler) DeleteExternalAccountKey(w http.ResponseWriter, r *http.Reques
func (h *Handler) GetExternalAccountKeys(w http.ResponseWriter, r *http.Request) { func (h *Handler) GetExternalAccountKeys(w http.ResponseWriter, r *http.Request) {
prov := chi.URLParam(r, "prov") prov := chi.URLParam(r, "prov")
eabEnabled, err := h.provisionerHasEABEnabled(r.Context(), prov)
if err != nil {
api.WriteError(w, err)
return
}
if !eabEnabled {
api.WriteError(w, admin.NewError(admin.ErrorBadRequestType, "ACME EAB not enabled for provisioner %s", prov))
return
}
// TODO: support paging properly? It'll probably leak to the DB layer, as we have to loop through all keys // TODO: support paging properly? It'll probably leak to the DB layer, as we have to loop through all keys
// cursor, limit, err := api.ParseCursor(r) // cursor, limit, err := api.ParseCursor(r)
// if err != nil { // if err != nil {
@ -95,13 +153,13 @@ func (h *Handler) GetExternalAccountKeys(w http.ResponseWriter, r *http.Request)
eaks := make([]*linkedca.EABKey, len(keys)) eaks := make([]*linkedca.EABKey, len(keys))
for i, k := range keys { for i, k := range keys {
eaks[i] = &linkedca.EABKey{ eaks[i] = &linkedca.EABKey{
EabKid: k.ID, Id: k.ID,
EabHmacKey: []byte{}, HmacKey: []byte{},
ProvisionerName: k.ProvisionerName, Provisioner: k.Provisioner,
Name: k.Name, Reference: k.Reference,
Account: k.AccountID, Account: k.AccountID,
CreatedAt: timestamppb.New(k.CreatedAt), CreatedAt: timestamppb.New(k.CreatedAt),
BoundAt: timestamppb.New(k.BoundAt), BoundAt: timestamppb.New(k.BoundAt),
} }
} }