diff --git a/acme/account.go b/acme/account.go index 0113561f..330d2839 100644 --- a/acme/account.go +++ b/acme/account.go @@ -52,8 +52,12 @@ type ExternalAccountKey struct { BoundAt time.Time `json:"boundAt,omitempty"` } +func (eak *ExternalAccountKey) AlreadyBound() bool { + return !eak.BoundAt.IsZero() +} + func (eak *ExternalAccountKey) BindTo(account *Account) { eak.AccountID = account.ID eak.BoundAt = time.Now() - eak.KeyBytes = []byte{} // TODO: ensure that single use keys are OK + eak.KeyBytes = []byte{} // clearing the key bytes; can only be used once } diff --git a/acme/api/account.go b/acme/api/account.go index 9a48574a..8e9e9b11 100644 --- a/acme/api/account.go +++ b/acme/api/account.go @@ -8,7 +8,6 @@ import ( "github.com/go-chi/chi" "github.com/smallstep/certificates/acme" "github.com/smallstep/certificates/api" - "github.com/smallstep/certificates/authority/provisioner" "github.com/smallstep/certificates/logging" squarejose "gopkg.in/square/go-jose.v2" @@ -125,7 +124,7 @@ func (h *Handler) NewAccount(w http.ResponseWriter, r *http.Request) { api.WriteError(w, acme.WrapErrorISE(err, "error creating account")) return } - if eak != nil { // means that we have a (valid) External Account Binding key that should be used + if eak != nil { // means that we have a (valid) External Account Binding key that should be bound, updated and sent in the response eak.BindTo(acc) if err := h.db.UpdateExternalAccountKey(ctx, eak); err != nil { api.WriteError(w, acme.WrapErrorISE(err, "error updating external account binding key")) @@ -134,7 +133,7 @@ func (h *Handler) NewAccount(w http.ResponseWriter, r *http.Request) { acc.ExternalAccountBinding = nar.ExternalAccountBinding } } else { - // Account exists // + // Account exists httpStatus = http.StatusOK } @@ -227,19 +226,12 @@ func (h *Handler) GetOrdersByAccountID(w http.ResponseWriter, r *http.Request) { // validateExternalAccountBinding validates the externalAccountBinding property in a call to new-account func (h *Handler) validateExternalAccountBinding(ctx context.Context, nar *NewAccountRequest) (*acme.ExternalAccountKey, error) { - - prov, err := provisionerFromContext(ctx) + acmeProv, err := acmeProvisionerFromContext(ctx) if err != nil { - return nil, err + return nil, acme.WrapErrorISE(err, "could not load ACME provisioner from context") } - acmeProv, ok := prov.(*provisioner.ACME) // TODO: rewrite into providing configuration via function on acme.Provisioner - if !ok || acmeProv == nil { - return nil, acme.NewErrorISE("provisioner in context is not an ACME provisioner") - } - - shouldSkipAccountBindingValidation := !acmeProv.RequireEAB - if shouldSkipAccountBindingValidation { + if !acmeProv.RequireEAB { return nil, nil } @@ -249,7 +241,7 @@ func (h *Handler) validateExternalAccountBinding(ctx context.Context, nar *NewAc eabJSONBytes, err := json.Marshal(nar.ExternalAccountBinding) if err != nil { - return nil, acme.WrapErrorISE(err, "error marshaling externalAccountBinding") + return nil, acme.WrapErrorISE(err, "error marshaling externalAccountBinding into JSON") } eabJWS, err := squarejose.ParseSigned(string(eabJSONBytes)) @@ -266,8 +258,8 @@ func (h *Handler) validateExternalAccountBinding(ctx context.Context, nar *NewAc return nil, acme.WrapErrorISE(err, "error retrieving external account key") } - if !externalAccountKey.BoundAt.IsZero() { // TODO: ensure that single use keys are OK - return nil, acme.NewError(acme.ErrorUnauthorizedType, "external account binding key with id '%s' was already used", keyID) + if externalAccountKey.AlreadyBound() { + return nil, acme.NewError(acme.ErrorUnauthorizedType, "external account binding key with id '%s' was already bound to account '%s' on %s", keyID, externalAccountKey.AccountID, externalAccountKey.BoundAt) } payload, err := eabJWS.Verify(externalAccountKey.KeyBytes) diff --git a/acme/api/handler.go b/acme/api/handler.go index 4519bd38..e740cc1e 100644 --- a/acme/api/handler.go +++ b/acme/api/handler.go @@ -153,18 +153,12 @@ func (d *Directory) ToLog() (interface{}, error) { // for client configuration. func (h *Handler) GetDirectory(w http.ResponseWriter, r *http.Request) { ctx := r.Context() - prov, err := provisionerFromContext(ctx) + acmeProv, err := acmeProvisionerFromContext(ctx) if err != nil { api.WriteError(w, err) return } - acmeProv, ok := prov.(*provisioner.ACME) // TODO: rewrite into providing configuration via function on acme.Provisioner - if !ok || acmeProv == nil { - api.WriteError(w, acme.NewErrorISE("provisioner in context is not an ACME provisioner")) - return - } - api.JSON(w, &Directory{ NewNonce: h.linker.GetLink(ctx, NewNonceLinkType), NewAccount: h.linker.GetLink(ctx, NewAccountLinkType), diff --git a/acme/api/middleware.go b/acme/api/middleware.go index b2244dd7..0fb38d1e 100644 --- a/acme/api/middleware.go +++ b/acme/api/middleware.go @@ -471,6 +471,20 @@ func provisionerFromContext(ctx context.Context) (acme.Provisioner, error) { return pval, nil } +// acmeProvisionerFromContext searches the context for an ACME provisioner. Returns +// pointer to an ACME provisioner or an error. +func acmeProvisionerFromContext(ctx context.Context) (*provisioner.ACME, error) { + prov, err := provisionerFromContext(ctx) + if err != nil { + return nil, err + } + acmeProv, ok := prov.(*provisioner.ACME) + if !ok || acmeProv == nil { + return nil, acme.NewErrorISE("provisioner in context is not an ACME provisioner") + } + return acmeProv, nil +} + // payloadFromContext searches the context for a payload. Returns the payload // or an error. func payloadFromContext(ctx context.Context) (*payloadInfo, error) { diff --git a/authority/admin/api/eak.go b/authority/admin/api/eak.go index 6f785f7a..8ea83309 100644 --- a/authority/admin/api/eak.go +++ b/authority/admin/api/eak.go @@ -29,7 +29,7 @@ func (h *Handler) CreateExternalAccountKey(w http.ResponseWriter, r *http.Reques // TODO: Validate input - eak, err := h.db.CreateExternalAccountKey(r.Context(), body.Name) + eak, err := h.acmeDB.CreateExternalAccountKey(r.Context(), body.Name) if err != nil { api.WriteError(w, admin.WrapErrorISE(err, "error creating external account key %s", body.Name)) return diff --git a/authority/admin/api/handler.go b/authority/admin/api/handler.go index ba72581d..b20e29ab 100644 --- a/authority/admin/api/handler.go +++ b/authority/admin/api/handler.go @@ -1,6 +1,7 @@ package api import ( + "github.com/smallstep/certificates/acme" "github.com/smallstep/certificates/api" "github.com/smallstep/certificates/authority" "github.com/smallstep/certificates/authority/admin" @@ -8,15 +9,18 @@ import ( // Handler is the ACME API request handler. type Handler struct { - db admin.DB - auth *authority.Authority + db admin.DB + auth *authority.Authority + acmeDB acme.DB } // NewHandler returns a new Authority Config Handler. -func NewHandler(auth *authority.Authority) api.RouterHandler { - h := &Handler{db: auth.GetAdminDatabase(), auth: auth} - - return h +func NewHandler(auth *authority.Authority, adminDB admin.DB, acmeDB acme.DB) api.RouterHandler { + return &Handler{ + db: adminDB, + auth: auth, + acmeDB: acmeDB, + } } // Route traffic and implement the Router interface. diff --git a/authority/admin/db.go b/authority/admin/db.go index 14520207..8a6339d9 100644 --- a/authority/admin/db.go +++ b/authority/admin/db.go @@ -7,8 +7,6 @@ import ( "github.com/pkg/errors" "go.step.sm/linkedca" - - "github.com/smallstep/certificates/authority/admin/eak" ) const ( @@ -56,7 +54,7 @@ func UnmarshalProvisionerDetails(typ linkedca.Provisioner_Type, data []byte) (*l return &linkedca.ProvisionerDetails{Data: v.Data}, nil } -// DB is the DB interface expected by the step-ca ACME API. +// DB is the DB interface expected by the step-ca Admin API. type DB interface { CreateProvisioner(ctx context.Context, prov *linkedca.Provisioner) error GetProvisioner(ctx context.Context, id string) (*linkedca.Provisioner, error) @@ -69,8 +67,6 @@ type DB interface { GetAdmins(ctx context.Context) ([]*linkedca.Admin, error) UpdateAdmin(ctx context.Context, admin *linkedca.Admin) error DeleteAdmin(ctx context.Context, id string) error - - CreateExternalAccountKey(ctx context.Context, name string) (*eak.ExternalAccountKey, error) } // MockDB is an implementation of the DB interface that should only be used as @@ -88,8 +84,6 @@ type MockDB struct { MockUpdateAdmin func(ctx context.Context, adm *linkedca.Admin) error MockDeleteAdmin func(ctx context.Context, id string) error - MockCreateExternalAccountKey func(ctx context.Context, name string) (*eak.ExternalAccountKey, error) - MockError error MockRet1 interface{} } @@ -183,10 +177,3 @@ func (m *MockDB) DeleteAdmin(ctx context.Context, id string) error { } return m.MockError } - -func (m *MockDB) CreateExternalAccountKey(ctx context.Context, name string) (*eak.ExternalAccountKey, error) { - if m.MockCreateExternalAccountKey != nil { - return m.MockCreateExternalAccountKey(ctx, name) - } - return m.MockRet1.(*eak.ExternalAccountKey), m.MockError -} diff --git a/authority/admin/db/nosql/admin_test.go b/authority/admin/db/nosql/admin_test.go index 092d72db..013cb489 100644 --- a/authority/admin/db/nosql/admin_test.go +++ b/authority/admin/db/nosql/admin_test.go @@ -11,7 +11,6 @@ import ( "github.com/smallstep/certificates/authority/admin" "github.com/smallstep/certificates/db" "github.com/smallstep/nosql" - "github.com/smallstep/nosql/database" nosqldb "github.com/smallstep/nosql/database" "go.step.sm/linkedca" "google.golang.org/protobuf/types/known/timestamppb" @@ -996,7 +995,7 @@ func TestDB_GetAdmins(t *testing.T) { "fail/db.List-error": func(t *testing.T) test { return test{ db: &db.MockNoSQLDB{ - MList: func(bucket []byte) ([]*database.Entry, error) { + MList: func(bucket []byte) ([]*nosqldb.Entry, error) { assert.Equals(t, bucket, adminsTable) return nil, errors.New("force") @@ -1006,14 +1005,14 @@ func TestDB_GetAdmins(t *testing.T) { } }, "fail/unmarshal-error": func(t *testing.T) test { - ret := []*database.Entry{ + ret := []*nosqldb.Entry{ {Bucket: adminsTable, Key: []byte("foo"), Value: foob}, {Bucket: adminsTable, Key: []byte("bar"), Value: barb}, {Bucket: adminsTable, Key: []byte("zap"), Value: []byte("zap")}, } return test{ db: &db.MockNoSQLDB{ - MList: func(bucket []byte) ([]*database.Entry, error) { + MList: func(bucket []byte) ([]*nosqldb.Entry, error) { assert.Equals(t, bucket, adminsTable) return ret, nil @@ -1023,10 +1022,10 @@ func TestDB_GetAdmins(t *testing.T) { } }, "ok/none": func(t *testing.T) test { - ret := []*database.Entry{} + ret := []*nosqldb.Entry{} return test{ db: &db.MockNoSQLDB{ - MList: func(bucket []byte) ([]*database.Entry, error) { + MList: func(bucket []byte) ([]*nosqldb.Entry, error) { assert.Equals(t, bucket, adminsTable) return ret, nil @@ -1038,13 +1037,13 @@ func TestDB_GetAdmins(t *testing.T) { } }, "ok/only-invalid": func(t *testing.T) test { - ret := []*database.Entry{ + ret := []*nosqldb.Entry{ {Bucket: adminsTable, Key: []byte("bar"), Value: barb}, {Bucket: adminsTable, Key: []byte("baz"), Value: bazb}, } return test{ db: &db.MockNoSQLDB{ - MList: func(bucket []byte) ([]*database.Entry, error) { + MList: func(bucket []byte) ([]*nosqldb.Entry, error) { assert.Equals(t, bucket, adminsTable) return ret, nil @@ -1056,7 +1055,7 @@ func TestDB_GetAdmins(t *testing.T) { } }, "ok": func(t *testing.T) test { - ret := []*database.Entry{ + ret := []*nosqldb.Entry{ {Bucket: adminsTable, Key: []byte("foo"), Value: foob}, {Bucket: adminsTable, Key: []byte("bar"), Value: barb}, {Bucket: adminsTable, Key: []byte("baz"), Value: bazb}, @@ -1064,7 +1063,7 @@ func TestDB_GetAdmins(t *testing.T) { } return test{ db: &db.MockNoSQLDB{ - MList: func(bucket []byte) ([]*database.Entry, error) { + MList: func(bucket []byte) ([]*nosqldb.Entry, error) { assert.Equals(t, bucket, adminsTable) return ret, nil diff --git a/authority/admin/db/nosql/eak.go b/authority/admin/db/nosql/eak.go deleted file mode 100644 index a3a3d96d..00000000 --- a/authority/admin/db/nosql/eak.go +++ /dev/null @@ -1,51 +0,0 @@ -package nosql - -import ( - "context" - "crypto/rand" - "time" - - "github.com/smallstep/certificates/authority/admin/eak" -) - -type dbExternalAccountKey struct { - ID string `json:"id"` - Name string `json:"name"` - AccountID string `json:"accountID,omitempty"` - KeyBytes []byte `json:"key,omitempty"` - CreatedAt time.Time `json:"createdAt"` - BoundAt time.Time `json:"boundAt"` -} - -// CreateExternalAccountKey creates a new External Account Binding key -func (db *DB) CreateExternalAccountKey(ctx context.Context, name string) (*eak.ExternalAccountKey, error) { - keyID, err := randID() - if err != nil { - return nil, err - } - - random := make([]byte, 32) - _, err = rand.Read(random) - if err != nil { - return nil, err - } - - dbeak := &dbExternalAccountKey{ - ID: keyID, - Name: name, - KeyBytes: random, - CreatedAt: clock.Now(), - } - - if err = db.save(ctx, keyID, dbeak, nil, "external_account_key", externalAccountKeyTable); err != nil { - return nil, err - } - return &eak.ExternalAccountKey{ - ID: dbeak.ID, - Name: dbeak.Name, - AccountID: dbeak.AccountID, - KeyBytes: dbeak.KeyBytes, - CreatedAt: dbeak.CreatedAt, - BoundAt: dbeak.BoundAt, - }, nil -} diff --git a/authority/admin/db/nosql/nosql.go b/authority/admin/db/nosql/nosql.go index 8019cbdf..18599b02 100644 --- a/authority/admin/db/nosql/nosql.go +++ b/authority/admin/db/nosql/nosql.go @@ -11,9 +11,8 @@ import ( ) var ( - adminsTable = []byte("admins") - provisionersTable = []byte("provisioners") - externalAccountKeyTable = []byte("acme_external_account_keys") + adminsTable = []byte("admins") + provisionersTable = []byte("provisioners") ) // DB is a struct that implements the AdminDB interface. @@ -24,7 +23,7 @@ type DB struct { // New configures and returns a new Authority DB backend implemented using a nosql DB. func New(db nosqlDB.DB, authorityID string) (*DB, error) { - tables := [][]byte{adminsTable, provisionersTable, externalAccountKeyTable} + tables := [][]byte{adminsTable, provisionersTable} for _, b := range tables { if err := db.CreateTable(b); err != nil { return nil, errors.Wrapf(err, "error creating table %s", diff --git a/authority/admin/db/nosql/provisioner_test.go b/authority/admin/db/nosql/provisioner_test.go index 95811f26..9759f5d8 100644 --- a/authority/admin/db/nosql/provisioner_test.go +++ b/authority/admin/db/nosql/provisioner_test.go @@ -11,7 +11,6 @@ import ( "github.com/smallstep/certificates/authority/admin" "github.com/smallstep/certificates/db" "github.com/smallstep/nosql" - "github.com/smallstep/nosql/database" nosqldb "github.com/smallstep/nosql/database" "go.step.sm/linkedca" ) @@ -746,7 +745,7 @@ func TestDB_GetProvisioners(t *testing.T) { "fail/db.List-error": func(t *testing.T) test { return test{ db: &db.MockNoSQLDB{ - MList: func(bucket []byte) ([]*database.Entry, error) { + MList: func(bucket []byte) ([]*nosqldb.Entry, error) { assert.Equals(t, bucket, provisionersTable) return nil, errors.New("force") @@ -756,14 +755,14 @@ func TestDB_GetProvisioners(t *testing.T) { } }, "fail/unmarshal-error": func(t *testing.T) test { - ret := []*database.Entry{ + ret := []*nosqldb.Entry{ {Bucket: provisionersTable, Key: []byte("foo"), Value: foob}, {Bucket: provisionersTable, Key: []byte("bar"), Value: barb}, {Bucket: provisionersTable, Key: []byte("zap"), Value: []byte("zap")}, } return test{ db: &db.MockNoSQLDB{ - MList: func(bucket []byte) ([]*database.Entry, error) { + MList: func(bucket []byte) ([]*nosqldb.Entry, error) { assert.Equals(t, bucket, provisionersTable) return ret, nil @@ -773,10 +772,10 @@ func TestDB_GetProvisioners(t *testing.T) { } }, "ok/none": func(t *testing.T) test { - ret := []*database.Entry{} + ret := []*nosqldb.Entry{} return test{ db: &db.MockNoSQLDB{ - MList: func(bucket []byte) ([]*database.Entry, error) { + MList: func(bucket []byte) ([]*nosqldb.Entry, error) { assert.Equals(t, bucket, provisionersTable) return ret, nil @@ -788,13 +787,13 @@ func TestDB_GetProvisioners(t *testing.T) { } }, "ok/only-invalid": func(t *testing.T) test { - ret := []*database.Entry{ + ret := []*nosqldb.Entry{ {Bucket: provisionersTable, Key: []byte("bar"), Value: barb}, {Bucket: provisionersTable, Key: []byte("baz"), Value: bazb}, } return test{ db: &db.MockNoSQLDB{ - MList: func(bucket []byte) ([]*database.Entry, error) { + MList: func(bucket []byte) ([]*nosqldb.Entry, error) { assert.Equals(t, bucket, provisionersTable) return ret, nil @@ -806,7 +805,7 @@ func TestDB_GetProvisioners(t *testing.T) { } }, "ok": func(t *testing.T) test { - ret := []*database.Entry{ + ret := []*nosqldb.Entry{ {Bucket: provisionersTable, Key: []byte("foo"), Value: foob}, {Bucket: provisionersTable, Key: []byte("bar"), Value: barb}, {Bucket: provisionersTable, Key: []byte("baz"), Value: bazb}, @@ -814,7 +813,7 @@ func TestDB_GetProvisioners(t *testing.T) { } return test{ db: &db.MockNoSQLDB{ - MList: func(bucket []byte) ([]*database.Entry, error) { + MList: func(bucket []byte) ([]*nosqldb.Entry, error) { assert.Equals(t, bucket, provisionersTable) return ret, nil diff --git a/ca/ca.go b/ca/ca.go index 4551286b..059101dc 100644 --- a/ca/ca.go +++ b/ca/ca.go @@ -182,7 +182,7 @@ func (ca *CA) Init(config *config.Config) (*CA, error) { if config.AuthorityConfig.EnableAdmin { adminDB := auth.GetAdminDatabase() if adminDB != nil { - adminHandler := adminAPI.NewHandler(auth) + adminHandler := adminAPI.NewHandler(auth, adminDB, acmeDB) mux.Route("/admin", func(r chi.Router) { adminHandler.Route(r) })