From c2bc1351c6bedd99efad9757480de4e0db0acf6c Mon Sep 17 00:00:00 2001 From: Herman Slatman Date: Fri, 17 Sep 2021 17:48:09 +0200 Subject: [PATCH] Add provisioner to remove endpoint and clear reference index on delete --- acme/db.go | 8 ++++---- acme/db/nosql/account.go | 17 +++++++++++++---- authority/admin/api/acme.go | 18 ++++++++++++++---- authority/admin/api/handler.go | 2 +- ca/adminClient.go | 6 +++--- 5 files changed, 35 insertions(+), 16 deletions(-) diff --git a/acme/db.go b/acme/db.go index 59a7beb0..aa81f7d3 100644 --- a/acme/db.go +++ b/acme/db.go @@ -23,7 +23,7 @@ type DB interface { GetExternalAccountKey(ctx context.Context, provisionerName string, keyID string) (*ExternalAccountKey, error) GetExternalAccountKeys(ctx context.Context, provisionerName string) ([]*ExternalAccountKey, error) GetExternalAccountKeyByReference(ctx context.Context, provisionerName string, reference string) (*ExternalAccountKey, error) - DeleteExternalAccountKey(ctx context.Context, keyID string) error + DeleteExternalAccountKey(ctx context.Context, provisionerName string, keyID string) error UpdateExternalAccountKey(ctx context.Context, provisionerName string, eak *ExternalAccountKey) error CreateNonce(ctx context.Context) (Nonce, error) @@ -58,7 +58,7 @@ type MockDB struct { MockGetExternalAccountKey func(ctx context.Context, provisionerName string, keyID string) (*ExternalAccountKey, error) MockGetExternalAccountKeys func(ctx context.Context, provisionerName string) ([]*ExternalAccountKey, error) MockGetExternalAccountKeyByReference func(ctx context.Context, provisionerName string, reference string) (*ExternalAccountKey, error) - MockDeleteExternalAccountKey func(ctx context.Context, keyID string) error + MockDeleteExternalAccountKey func(ctx context.Context, provisionerName string, keyID string) error MockUpdateExternalAccountKey func(ctx context.Context, provisionerName string, eak *ExternalAccountKey) error MockCreateNonce func(ctx context.Context) (Nonce, error) @@ -165,9 +165,9 @@ func (m *MockDB) GetExternalAccountKeyByReference(ctx context.Context, provision } // DeleteExternalAccountKey mock -func (m *MockDB) DeleteExternalAccountKey(ctx context.Context, keyID string) error { +func (m *MockDB) DeleteExternalAccountKey(ctx context.Context, provisionerName string, keyID string) error { if m.MockDeleteExternalAccountKey != nil { - return m.MockDeleteExternalAccountKey(ctx, keyID) + return m.MockDeleteExternalAccountKey(ctx, provisionerName, keyID) } else if m.MockError != nil { return m.MockError } diff --git a/acme/db/nosql/account.go b/acme/db/nosql/account.go index b52489bd..1cf94b37 100644 --- a/acme/db/nosql/account.go +++ b/acme/db/nosql/account.go @@ -8,7 +8,6 @@ import ( "github.com/pkg/errors" "github.com/smallstep/certificates/acme" - "github.com/smallstep/nosql" nosqlDB "github.com/smallstep/nosql" "go.step.sm/crypto/jose" ) @@ -238,11 +237,21 @@ func (db *DB) GetExternalAccountKey(ctx context.Context, provisionerName string, }, nil } -func (db *DB) DeleteExternalAccountKey(ctx context.Context, keyID string) error { - _, err := db.db.Get(externalAccountKeyTable, []byte(keyID)) +func (db *DB) DeleteExternalAccountKey(ctx context.Context, provisionerName string, keyID string) error { + dbeak, err := db.getDBExternalAccountKey(ctx, keyID) if err != nil { return errors.Wrapf(err, "error loading ACME EAB Key with Key ID %s", keyID) } + if dbeak.Provisioner != provisionerName { + // TODO: change these ACME error types; they don't make a lot of sense if used in the Admin APIs + return acme.NewError(acme.ErrorUnauthorizedType, "name of provisioner does not match provisioner for which the EAB key was created") + } + if dbeak.Reference != "" { + err = db.db.Del(externalAccountKeysByReferenceTable, []byte(dbeak.Reference)) + if err != nil { + return errors.Wrapf(err, "error deleting ACME EAB Key Reference with Key ID %s and reference %s", keyID, dbeak.Reference) + } + } err = db.db.Del(externalAccountKeyTable, []byte(keyID)) if err != nil { return errors.Wrapf(err, "error deleting ACME EAB Key with Key ID %s", keyID) @@ -286,7 +295,7 @@ func (db *DB) GetExternalAccountKeyByReference(ctx context.Context, provisionerN return nil, nil } k, err := db.db.Get(externalAccountKeysByReferenceTable, []byte(reference)) - if nosql.IsErrNotFound(err) { + if nosqlDB.IsErrNotFound(err) { return nil, errors.Errorf("ACME EAB key for reference %s not found", reference) } else if err != nil { return nil, errors.Wrapf(err, "error loading ACME EAB key for reference %s", reference) diff --git a/authority/admin/api/acme.go b/authority/admin/api/acme.go index de541a23..ceac9320 100644 --- a/authority/admin/api/acme.go +++ b/authority/admin/api/acme.go @@ -120,12 +120,22 @@ func (h *Handler) CreateExternalAccountKey(w http.ResponseWriter, r *http.Reques // DeleteExternalAccountKey deletes an ACME External Account Key. func (h *Handler) DeleteExternalAccountKey(w http.ResponseWriter, r *http.Request) { - id := chi.URLParam(r, "id") + provisioner := chi.URLParam(r, "prov") + keyID := chi.URLParam(r, "id") - // TODO: add provisioner as parameter, so that check can be performed if EAB is enabled or not + eabEnabled, err := h.provisionerHasEABEnabled(r.Context(), provisioner) + if err != nil { + api.WriteError(w, err) + return + } - if err := h.acmeDB.DeleteExternalAccountKey(r.Context(), id); err != nil { - api.WriteError(w, admin.WrapErrorISE(err, "error deleting ACME EAB Key %s", id)) + if !eabEnabled { + api.WriteError(w, admin.NewError(admin.ErrorBadRequestType, "ACME EAB not enabled for provisioner %s", provisioner)) + return + } + + if err := h.acmeDB.DeleteExternalAccountKey(r.Context(), provisioner, keyID); err != nil { + api.WriteError(w, admin.WrapErrorISE(err, "error deleting ACME EAB Key %s", keyID)) return } diff --git a/authority/admin/api/handler.go b/authority/admin/api/handler.go index 2746a3df..95e12d07 100644 --- a/authority/admin/api/handler.go +++ b/authority/admin/api/handler.go @@ -47,5 +47,5 @@ func (h *Handler) Route(r api.Router) { r.MethodFunc("GET", "/acme/eab/{prov}/{ref}", authnz(h.GetExternalAccountKeys)) r.MethodFunc("GET", "/acme/eab/{prov}", authnz(h.GetExternalAccountKeys)) r.MethodFunc("POST", "/acme/eab", authnz(h.CreateExternalAccountKey)) - r.MethodFunc("DELETE", "/acme/eab/{id}", authnz(h.DeleteExternalAccountKey)) + r.MethodFunc("DELETE", "/acme/eab/{prov}/{id}", authnz(h.DeleteExternalAccountKey)) } diff --git a/ca/adminClient.go b/ca/adminClient.go index 8e1202d4..582c99b9 100644 --- a/ca/adminClient.go +++ b/ca/adminClient.go @@ -637,10 +637,10 @@ retry: return eabKey, nil } -// RemoveExternalAccountKey performs the DELETE /admin/acme/eab/{key_id} request to the CA. -func (c *AdminClient) RemoveExternalAccountKey(keyID string) error { +// RemoveExternalAccountKey performs the DELETE /admin/acme/eab/{prov}/{key_id} request to the CA. +func (c *AdminClient) RemoveExternalAccountKey(provisionerName string, keyID string) error { var retried bool - u := c.endpoint.ResolveReference(&url.URL{Path: path.Join(adminURLPrefix, "acme/eab", keyID)}) + u := c.endpoint.ResolveReference(&url.URL{Path: path.Join(adminURLPrefix, "acme/eab", provisionerName, "/", keyID)}) tok, err := c.generateAdminToken(u.Path) if err != nil { return errors.Wrapf(err, "error generating admin token")