Add RW locks to prevent concurrent updates to the DB

Although this may slow certain API calls down and may not be, strictly
necessary, I think it's best to put all the ACME EAB operations behind
RW locks to prevent concurrent updates to the DB and guarantee
consistent result sets.

acme/db/nosql/account.go

@@ -3,7 +3,6 @@ package nosql
 import (
 	"context"
 	"encoding/json"
-	"sync"
 	"time"
 
 	"github.com/pkg/errors"
@@ -12,9 +11,6 @@ import (
 	"go.step.sm/crypto/jose"
 )
 
-// Mutex for locking referencesByProvisioner index operations.
-var referencesByProvisionerIndexMux sync.Mutex
-
 // dbAccount represents an ACME account.
 type dbAccount struct {
 	ID            string           `json:"id"`
@@ -30,21 +26,6 @@ func (dba *dbAccount) clone() *dbAccount {
 	return &nu
 }
 
-type dbExternalAccountKey struct {
-	ID            string    `json:"id"`
-	ProvisionerID string    `json:"provisionerID"`
-	Reference     string    `json:"reference"`
-	AccountID     string    `json:"accountID,omitempty"`
-	KeyBytes      []byte    `json:"key"`
-	CreatedAt     time.Time `json:"createdAt"`
-	BoundAt       time.Time `json:"boundAt"`
-}
-
-type dbExternalAccountKeyReference struct {
-	Reference            string `json:"reference"`
-	ExternalAccountKeyID string `json:"externalAccountKeyID"`
-}
-
 func (db *DB) getAccountIDByKeyID(ctx context.Context, kid string) (string, error) {
 	id, err := db.db.Get(accountByKeyIDTable, []byte(kid))
 	if err != nil {
@@ -73,24 +54,6 @@ func (db *DB) getDBAccount(ctx context.Context, id string) (*dbAccount, error) {
 	return dbacc, nil
 }
 
-// getDBExternalAccountKey retrieves and unmarshals dbExternalAccountKey.
-func (db *DB) getDBExternalAccountKey(ctx context.Context, id string) (*dbExternalAccountKey, error) {
-	data, err := db.db.Get(externalAccountKeyTable, []byte(id))
-	if err != nil {
-		if nosqlDB.IsErrNotFound(err) {
-			return nil, acme.ErrNotFound
-		}
-		return nil, errors.Wrapf(err, "error loading external account key %s", id)
-	}
-
-	dbeak := new(dbExternalAccountKey)
-	if err = json.Unmarshal(data, dbeak); err != nil {
-		return nil, errors.Wrapf(err, "error unmarshaling external account key %s into dbExternalAccountKey", id)
-	}
-
-	return dbeak, nil
-}
-
 // GetAccount retrieves an ACME account by ID.
 func (db *DB) GetAccount(ctx context.Context, id string) (*acme.Account, error) {
 	dbacc, err := db.getDBAccount(ctx, id)

acme/db/nosql/eab.go

@@ -4,14 +4,59 @@ import (
 	"context"
 	"crypto/rand"
 	"encoding/json"
+	"sync"
+	"time"
 
 	"github.com/pkg/errors"
 	"github.com/smallstep/certificates/acme"
 	nosqlDB "github.com/smallstep/nosql"
 )
 
+// externalAccountKeyMutex for read/write locking of EAK operations.
+var externalAccountKeyMutex sync.RWMutex
+
+// referencesByProvisionerIndexMutex for locking referencesByProvisioner index operations.
+var referencesByProvisionerIndexMutex sync.Mutex
+
+type dbExternalAccountKey struct {
+	ID            string    `json:"id"`
+	ProvisionerID string    `json:"provisionerID"`
+	Reference     string    `json:"reference"`
+	AccountID     string    `json:"accountID,omitempty"`
+	KeyBytes      []byte    `json:"key"`
+	CreatedAt     time.Time `json:"createdAt"`
+	BoundAt       time.Time `json:"boundAt"`
+}
+
+type dbExternalAccountKeyReference struct {
+	Reference            string `json:"reference"`
+	ExternalAccountKeyID string `json:"externalAccountKeyID"`
+}
+
+// getDBExternalAccountKey retrieves and unmarshals dbExternalAccountKey.
+func (db *DB) getDBExternalAccountKey(ctx context.Context, id string) (*dbExternalAccountKey, error) {
+	data, err := db.db.Get(externalAccountKeyTable, []byte(id))
+	if err != nil {
+		if nosqlDB.IsErrNotFound(err) {
+			return nil, acme.ErrNotFound
+		}
+		return nil, errors.Wrapf(err, "error loading external account key %s", id)
+	}
+
+	dbeak := new(dbExternalAccountKey)
+	if err = json.Unmarshal(data, dbeak); err != nil {
+		return nil, errors.Wrapf(err, "error unmarshaling external account key %s into dbExternalAccountKey", id)
+	}
+
+	return dbeak, nil
+}
+
 // CreateExternalAccountKey creates a new External Account Binding key with a name
 func (db *DB) CreateExternalAccountKey(ctx context.Context, provisionerID, reference string) (*acme.ExternalAccountKey, error) {
+
+	externalAccountKeyMutex.Lock()
+	defer externalAccountKeyMutex.Unlock()
+
 	keyID, err := randID()
 	if err != nil {
 		return nil, err
@@ -62,6 +107,9 @@ func (db *DB) CreateExternalAccountKey(ctx context.Context, provisionerID, refer
 
 // GetExternalAccountKey retrieves an External Account Binding key by KeyID
 func (db *DB) GetExternalAccountKey(ctx context.Context, provisionerID, keyID string) (*acme.ExternalAccountKey, error) {
+	externalAccountKeyMutex.RLock()
+	defer externalAccountKeyMutex.RUnlock()
+
 	dbeak, err := db.getDBExternalAccountKey(ctx, keyID)
 	if err != nil {
 		return nil, err
@@ -83,6 +131,9 @@ func (db *DB) GetExternalAccountKey(ctx context.Context, provisionerID, keyID st
 }
 
 func (db *DB) DeleteExternalAccountKey(ctx context.Context, provisionerID, keyID string) error {
+	externalAccountKeyMutex.Lock()
+	defer externalAccountKeyMutex.Unlock()
+
 	dbeak, err := db.getDBExternalAccountKey(ctx, keyID)
 	if err != nil {
 		return errors.Wrapf(err, "error loading ACME EAB Key with Key ID %s", keyID)
@@ -109,8 +160,8 @@ func (db *DB) DeleteExternalAccountKey(ctx context.Context, provisionerID, keyID
 
 // GetExternalAccountKeys retrieves all External Account Binding keys for a provisioner
 func (db *DB) GetExternalAccountKeys(ctx context.Context, provisionerID string) ([]*acme.ExternalAccountKey, error) {
-
+	externalAccountKeyMutex.RLock()
-	// TODO: mutex?
+	defer externalAccountKeyMutex.RUnlock()
 
 	var eakIDs []string
 	r, err := db.db.Get(externalAccountKeyIDsByProvisionerIDTable, []byte(provisionerID))
@@ -152,6 +203,9 @@ func (db *DB) GetExternalAccountKeys(ctx context.Context, provisionerID string)
 
 // GetExternalAccountKeyByReference retrieves an External Account Binding key with unique reference
 func (db *DB) GetExternalAccountKeyByReference(ctx context.Context, provisionerID, reference string) (*acme.ExternalAccountKey, error) {
+	externalAccountKeyMutex.RLock()
+	defer externalAccountKeyMutex.RUnlock()
+
 	if reference == "" {
 		return nil, nil
 	}
@@ -171,6 +225,9 @@ func (db *DB) GetExternalAccountKeyByReference(ctx context.Context, provisionerI
 }
 
 func (db *DB) UpdateExternalAccountKey(ctx context.Context, provisionerID string, eak *acme.ExternalAccountKey) error {
+	externalAccountKeyMutex.Lock()
+	defer externalAccountKeyMutex.Unlock()
+
 	old, err := db.getDBExternalAccountKey(ctx, eak.ID)
 	if err != nil {
 		return err
@@ -202,8 +259,8 @@ func (db *DB) UpdateExternalAccountKey(ctx context.Context, provisionerID string
 }
 
 func (db *DB) addEAKID(ctx context.Context, provisionerID, eakID string) error {
-	referencesByProvisionerIndexMux.Lock()
+	referencesByProvisionerIndexMutex.Lock()
-	defer referencesByProvisionerIndexMux.Unlock()
+	defer referencesByProvisionerIndexMutex.Unlock()
 
 	if eakID == "" {
 		return errors.Errorf("can't add empty eakID for provisioner %s", provisionerID)
@@ -245,8 +302,8 @@ func (db *DB) addEAKID(ctx context.Context, provisionerID, eakID string) error {
 }
 
 func (db *DB) deleteEAKID(ctx context.Context, provisionerID, eakID string) error {
-	referencesByProvisionerIndexMux.Lock()
+	referencesByProvisionerIndexMutex.Lock()
-	defer referencesByProvisionerIndexMux.Unlock()
+	defer referencesByProvisionerIndexMutex.Unlock()
 
 	var eakIDs []string
 	b, err := db.db.Get(externalAccountKeyIDsByProvisionerIDTable, []byte(provisionerID))