Merge pull request #1897 from hashicorp/secret-id-accessor-locks

Safely manipulate secret id accessors
This commit is contained in:
Vishal Nayak 2016-09-19 11:37:38 -04:00 committed by GitHub
commit 97dc0e9f64
3 changed files with 83 additions and 30 deletions

View file

@ -20,22 +20,28 @@ type backend struct {
// Guard to clean-up the expired SecretID entries
tidySecretIDCASGuard uint32
// Map of locks to make changes to role entries. This will be initiated
// to a predefined number of locks when the backend is created, and
// will be indexed based on salted role names.
// Map of locks to make changes to role entries. These will be
// initialized to a predefined number of locks when the backend is
// created, and will be indexed based on salted role names.
roleLocksMap map[string]*sync.RWMutex
// Map of locks to make changes to the storage entries of RoleIDs
// generated. This will be initiated to a predefined number of locks
// generated. These will be initialized to a predefined number of locks
// when the backend is created, and will be indexed based on the salted
// RoleIDs.
roleIDLocksMap map[string]*sync.RWMutex
// Map of locks to make changes to the storage entries of SecretIDs
// generated. This will be initiated to a predefined number of locks
// generated. These will be initialized to a predefined number of locks
// when the backend is created, and will be indexed based on the HMAC-ed
// SecretIDs.
secretIDLocksMap map[string]*sync.RWMutex
// Map of locks to make changes to the storage entries of
// SecretIDAccessors generated. These will be initialized to a
// predefined number of locks when the backend is created, and will be
// indexed based on the SecretIDAccessors itself.
secretIDAccessorLocksMap map[string]*sync.RWMutex
}
func Factory(conf *logical.BackendConfig) (logical.Backend, error) {
@ -63,11 +69,14 @@ func Backend(conf *logical.BackendConfig) (*backend, error) {
// Create the map of locks to modify the registered roles
roleLocksMap: make(map[string]*sync.RWMutex, 257),
// Create the map of locks to modify the generated RoleIDs.
// Create the map of locks to modify the generated RoleIDs
roleIDLocksMap: make(map[string]*sync.RWMutex, 257),
// Create the map of locks to modify the generated SecretIDs.
// Create the map of locks to modify the generated SecretIDs
secretIDLocksMap: make(map[string]*sync.RWMutex, 257),
// Create the map of locks to modify the generated SecretIDAccessors
secretIDAccessorLocksMap: make(map[string]*sync.RWMutex, 257),
}
// Create 256 locks each for managing RoleID and SecretIDs. This will avoid
@ -86,12 +95,17 @@ func Backend(conf *logical.BackendConfig) (*backend, error) {
return nil, fmt.Errorf("failed to create secret ID locks: %v", err)
}
if err = locksutil.CreateLocks(b.secretIDAccessorLocksMap, 256); err != nil {
return nil, fmt.Errorf("failed to create secret ID accessor locks: %v", err)
}
// Have an extra lock to use in case the indexing does not result in a lock.
// This happens if the indexing value is not beginning with hex characters.
// These locks can be used for listing purposes as well.
b.roleLocksMap["custom"] = &sync.RWMutex{}
b.secretIDLocksMap["custom"] = &sync.RWMutex{}
b.roleIDLocksMap["custom"] = &sync.RWMutex{}
b.secretIDLocksMap["custom"] = &sync.RWMutex{}
b.secretIDAccessorLocksMap["custom"] = &sync.RWMutex{}
// Attach the paths and secrets that are to be handled by the backend
b.Backend = &framework.Backend{

View file

@ -920,11 +920,9 @@ func (b *backend) pathRoleSecretIDSecretIDDelete(req *logical.Request, data *fra
return nil, err
}
accessorEntryIndex := "accessor/" + b.salt.SaltID(result.SecretIDAccessor)
// Delete the accessor of the SecretID first
if err := req.Storage.Delete(accessorEntryIndex); err != nil {
return nil, fmt.Errorf("failed to delete accessor storage entry: %s", err)
if err := b.deleteSecretIDAccessorEntry(req.Storage, result.SecretIDAccessor); err != nil {
return nil, err
}
// Delete the storage entry that corresponds to the SecretID
@ -1014,15 +1012,14 @@ func (b *backend) pathRoleSecretIDAccessorDelete(req *logical.Request, data *fra
}
entryIndex := fmt.Sprintf("secret_id/%s/%s", roleNameHMAC, accessorEntry.SecretIDHMAC)
accessorEntryIndex := "accessor/" + b.salt.SaltID(secretIDAccessor)
lock := b.secretIDLock(accessorEntry.SecretIDHMAC)
lock.Lock()
defer lock.Unlock()
// Delete the accessor of the SecretID first
if err := req.Storage.Delete(accessorEntryIndex); err != nil {
return nil, fmt.Errorf("failed to delete accessor storage entry: %s", err)
if err := b.deleteSecretIDAccessorEntry(req.Storage, secretIDAccessor); err != nil {
return nil, err
}
// Delete the storage entry that corresponds to the SecretID

View file

@ -222,9 +222,9 @@ func (b *backend) validateBindSecretID(s logical.Storage, roleName, secretID, hm
// the storage but do not fail the validation request. Subsequest
// requests to use the same SecretID will fail.
if result.SecretIDNumUses == 1 {
accessorEntryIndex := "accessor/" + b.salt.SaltID(result.SecretIDAccessor)
if err := s.Delete(accessorEntryIndex); err != nil {
return false, nil, fmt.Errorf("failed to delete accessor storage entry: %s", err)
// Delete the secret IDs accessor first
if err := b.deleteSecretIDAccessorEntry(s, result.SecretIDAccessor); err != nil {
return false, nil, err
}
if err := s.Delete(entryIndex); err != nil {
return false, nil, fmt.Errorf("failed to delete SecretID: %s", err)
@ -243,8 +243,8 @@ func (b *backend) validateBindSecretID(s logical.Storage, roleName, secretID, hm
return true, result.Metadata, nil
}
// Creates a SHA256 HMAC of the given 'value' using the given 'key'
// and returns a hex encoded string.
// Creates a SHA256 HMAC of the given 'value' using the given 'key' and returns
// a hex encoded string.
func createHMAC(key, value string) (string, error) {
if key == "" {
return "", fmt.Errorf("invalid HMAC key")
@ -254,10 +254,9 @@ func createHMAC(key, value string) (string, error) {
return hex.EncodeToString(hm.Sum(nil)), nil
}
// secretIDLock is used to get a lock from the pre-initialized map
// of locks. Map is indexed based on the first 2 characters of the
// secretIDHMAC. If the input is not hex encoded or if empty, a
// "custom" lock will be returned.
// secretIDLock is used to get a lock from the pre-initialized map of locks.
// Map is indexed based on the first 2 characters of the secretIDHMAC. If the
// input is not hex encoded or if empty, a "custom" lock will be returned.
func (b *backend) secretIDLock(secretIDHMAC string) *sync.RWMutex {
var lock *sync.RWMutex
var ok bool
@ -265,12 +264,31 @@ func (b *backend) secretIDLock(secretIDHMAC string) *sync.RWMutex {
lock, ok = b.secretIDLocksMap[secretIDHMAC[0:2]]
}
if !ok || lock == nil {
// Fall back for custom SecretIDs
// Fall back for custom lock to make sure that this method
// never returns nil
lock = b.secretIDLocksMap["custom"]
}
return lock
}
// secretIDAccessorLock is used to get a lock from the pre-initialized map
// of locks. Map is indexed based on the first 2 characters of the
// secretIDAccessor. If the input is not hex encoded or if empty, a "custom"
// lock will be returned.
func (b *backend) secretIDAccessorLock(secretIDAccessor string) *sync.RWMutex {
var lock *sync.RWMutex
var ok bool
if len(secretIDAccessor) >= 2 {
lock, ok = b.secretIDAccessorLocksMap[secretIDAccessor[0:2]]
}
if !ok || lock == nil {
// Fall back for custom lock to make sure that this method
// never returns nil
lock = b.secretIDAccessorLocksMap["custom"]
}
return lock
}
// registerSecretIDEntry creates a new storage entry for the given SecretID.
func (b *backend) registerSecretIDEntry(s logical.Storage, roleName, secretID, hmacKey string, secretEntry *secretIDStorageEntry) (*secretIDStorageEntry, error) {
secretIDHMAC, err := createHMAC(hmacKey, secretID)
@ -331,7 +349,7 @@ func (b *backend) registerSecretIDEntry(s logical.Storage, roleName, secretID, h
}
// Before storing the SecretID, store its accessor.
if err := b.createAccessor(s, secretEntry, secretIDHMAC); err != nil {
if err := b.createSecretIDAccessorEntry(s, secretEntry, secretIDHMAC); err != nil {
return nil, err
}
@ -345,8 +363,7 @@ func (b *backend) registerSecretIDEntry(s logical.Storage, roleName, secretID, h
}
// secretIDAccessorEntry is used to read the storage entry that maps an
// accessor to a secret_id. This method should be called when the lock
// for the corresponding SecretID is held.
// accessor to a secret_id.
func (b *backend) secretIDAccessorEntry(s logical.Storage, secretIDAccessor string) (*secretIDAccessorStorageEntry, error) {
if secretIDAccessor == "" {
return nil, fmt.Errorf("missing secretIDAccessor")
@ -357,6 +374,10 @@ func (b *backend) secretIDAccessorEntry(s logical.Storage, secretIDAccessor stri
// Create index entry, mapping the accessor to the token ID
entryIndex := "accessor/" + b.salt.SaltID(secretIDAccessor)
accessorLock := b.secretIDAccessorLock(secretIDAccessor)
accessorLock.RLock()
defer accessorLock.RUnlock()
if entry, err := s.Get(entryIndex); err != nil {
return nil, err
} else if entry == nil {
@ -368,10 +389,10 @@ func (b *backend) secretIDAccessorEntry(s logical.Storage, secretIDAccessor stri
return &result, nil
}
// createAccessor creates an identifier for the SecretID. A storage index,
// createSecretIDAccessorEntry creates an identifier for the SecretID. A storage index,
// mapping the accessor to the SecretID is also created. This method should
// be called when the lock for the corresponding SecretID is held.
func (b *backend) createAccessor(s logical.Storage, entry *secretIDStorageEntry, secretIDHMAC string) error {
func (b *backend) createSecretIDAccessorEntry(s logical.Storage, entry *secretIDStorageEntry, secretIDHMAC string) error {
// Create a random accessor
accessorUUID, err := uuid.GenerateUUID()
if err != nil {
@ -381,6 +402,11 @@ func (b *backend) createAccessor(s logical.Storage, entry *secretIDStorageEntry,
// Create index entry, mapping the accessor to the token ID
entryIndex := "accessor/" + b.salt.SaltID(entry.SecretIDAccessor)
accessorLock := b.secretIDAccessorLock(accessorUUID)
accessorLock.Lock()
defer accessorLock.Unlock()
if entry, err := logical.StorageEntryJSON(entryIndex, &secretIDAccessorStorageEntry{
SecretIDHMAC: secretIDHMAC,
}); err != nil {
@ -392,6 +418,22 @@ func (b *backend) createAccessor(s logical.Storage, entry *secretIDStorageEntry,
return nil
}
// deleteSecretIDAccessorEntry deletes the storage index mapping the accessor to a SecretID.
func (b *backend) deleteSecretIDAccessorEntry(s logical.Storage, secretIDAccessor string) error {
accessorEntryIndex := "accessor/" + b.salt.SaltID(secretIDAccessor)
accessorLock := b.secretIDAccessorLock(secretIDAccessor)
accessorLock.Lock()
defer accessorLock.Unlock()
// Delete the accessor of the SecretID first
if err := s.Delete(accessorEntryIndex); err != nil {
return fmt.Errorf("failed to delete accessor storage entry: %s", err)
}
return nil
}
// flushRoleSecrets deletes all the SecretIDs that belong to the given
// RoleID.
func (b *backend) flushRoleSecrets(s logical.Storage, roleName, hmacKey string) error {