Skip to content

Commit

Permalink
Add an idle timeout for the server (#4760)
Browse files Browse the repository at this point in the history 
* Add an idle timeout for the server

Because tidy operations can be long-running, this also changes all tidy
operations to behave the same operationally (kick off the process, get a
warning back, log errors to server log) and makes them all run in a
goroutine.

This could mean a sort of hard stop if Vault gets sealed because the
function won't have the read lock. This should generally be okay
(running tidy again should pick back up where it left off), but future
work could use cleanup funcs to trigger the functions to stop.

* Fix up tidy test

* Add deadline to cluster connections and an idle timeout to the cluster server, plus add readheader/read timeout to api server
  • Loading branch information
@jefferai
jefferai authored Jun 16, 2018
1 parent 43e218e commit f493d24
Show file tree
Hide file tree
Showing 18 changed files with 1,184 additions and 1,195 deletions.
208 changes: 110 additions & 98 deletions builtin/credential/approle/path_tidy_user_id.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,141 +26,153 @@ func pathTidySecretID(b *backend) *framework.Path {
}

// tidySecretID is used to delete entries in the whitelist that are expired.
func (b *backend) tidySecretID(ctx context.Context, s logical.Storage) error {
grabbed := atomic.CompareAndSwapUint32(b.tidySecretIDCASGuard, 0, 1)
if grabbed {
defer atomic.StoreUint32(b.tidySecretIDCASGuard, 0)
} else {
return fmt.Errorf("SecretID tidy operation already running")
func (b *backend) tidySecretID(ctx context.Context, s logical.Storage) (*logical.Response, error) {
if !atomic.CompareAndSwapUint32(b.tidySecretIDCASGuard, 0, 1) {
resp := &logical.Response{}
resp.AddWarning("Tidy operation already in progress.")
return resp, nil
}

var result error
go func() {
defer atomic.StoreUint32(b.tidySecretIDCASGuard, 0)

tidyFunc := func(secretIDPrefixToUse, accessorIDPrefixToUse string) error {
roleNameHMACs, err := s.List(ctx, secretIDPrefixToUse)
if err != nil {
return err
}
var result error

// List all the accessors and add them all to a map
accessorHashes, err := s.List(ctx, accessorIDPrefixToUse)
if err != nil {
return err
}
accessorMap := make(map[string]bool, len(accessorHashes))
for _, accessorHash := range accessorHashes {
accessorMap[accessorHash] = true
}
// Don't cancel when the original client request goes away
ctx = context.Background()

secretIDCleanupFunc := func(secretIDHMAC, roleNameHMAC, secretIDPrefixToUse string) error {
lock := b.secretIDLock(secretIDHMAC)
lock.Lock()
defer lock.Unlock()
logger := b.Logger().Named("tidy")

entryIndex := fmt.Sprintf("%s%s%s", secretIDPrefixToUse, roleNameHMAC, secretIDHMAC)
secretIDEntry, err := s.Get(ctx, entryIndex)
tidyFunc := func(secretIDPrefixToUse, accessorIDPrefixToUse string) error {
roleNameHMACs, err := s.List(ctx, secretIDPrefixToUse)
if err != nil {
return errwrap.Wrapf(fmt.Sprintf("error fetching SecretID %q: {{err}}", secretIDHMAC), err)
return err
}

if secretIDEntry == nil {
result = multierror.Append(result, fmt.Errorf("entry for SecretID %q is nil", secretIDHMAC))
return nil
// List all the accessors and add them all to a map
accessorHashes, err := s.List(ctx, accessorIDPrefixToUse)
if err != nil {
return err
}

if secretIDEntry.Value == nil || len(secretIDEntry.Value) == 0 {
return fmt.Errorf("found entry for SecretID %q but actual SecretID is empty", secretIDHMAC)
accessorMap := make(map[string]bool, len(accessorHashes))
for _, accessorHash := range accessorHashes {
accessorMap[accessorHash] = true
}

var result secretIDStorageEntry
if err := secretIDEntry.DecodeJSON(&result); err != nil {
return err
}
secretIDCleanupFunc := func(secretIDHMAC, roleNameHMAC, secretIDPrefixToUse string) error {
lock := b.secretIDLock(secretIDHMAC)
lock.Lock()
defer lock.Unlock()

// If a secret ID entry does not have a corresponding accessor
// entry, revoke the secret ID immediately
accessorEntry, err := b.secretIDAccessorEntry(ctx, s, result.SecretIDAccessor, secretIDPrefixToUse)
if err != nil {
return errwrap.Wrapf("failed to read secret ID accessor entry: {{err}}", err)
}
if accessorEntry == nil {
if err := s.Delete(ctx, entryIndex); err != nil {
return errwrap.Wrapf(fmt.Sprintf("error deleting secret ID %q from storage: {{err}}", secretIDHMAC), err)
entryIndex := fmt.Sprintf("%s%s%s", secretIDPrefixToUse, roleNameHMAC, secretIDHMAC)
secretIDEntry, err := s.Get(ctx, entryIndex)
if err != nil {
return errwrap.Wrapf(fmt.Sprintf("error fetching SecretID %q: {{err}}", secretIDHMAC), err)
}

if secretIDEntry == nil {
result = multierror.Append(result, fmt.Errorf("entry for SecretID %q is nil", secretIDHMAC))
return nil
}

if secretIDEntry.Value == nil || len(secretIDEntry.Value) == 0 {
return fmt.Errorf("found entry for SecretID %q but actual SecretID is empty", secretIDHMAC)
}

var result secretIDStorageEntry
if err := secretIDEntry.DecodeJSON(&result); err != nil {
return err
}
return nil
}

// ExpirationTime not being set indicates non-expiring SecretIDs
if !result.ExpirationTime.IsZero() && time.Now().After(result.ExpirationTime) {
// Clean up the accessor of the secret ID first
err = b.deleteSecretIDAccessorEntry(ctx, s, result.SecretIDAccessor, secretIDPrefixToUse)
// If a secret ID entry does not have a corresponding accessor
// entry, revoke the secret ID immediately
accessorEntry, err := b.secretIDAccessorEntry(ctx, s, result.SecretIDAccessor, secretIDPrefixToUse)
if err != nil {
return errwrap.Wrapf("failed to delete secret ID accessor entry: {{err}}", err)
return errwrap.Wrapf("failed to read secret ID accessor entry: {{err}}", err)
}
if accessorEntry == nil {
if err := s.Delete(ctx, entryIndex); err != nil {
return errwrap.Wrapf(fmt.Sprintf("error deleting secret ID %q from storage: {{err}}", secretIDHMAC), err)
}
return nil
}

// ExpirationTime not being set indicates non-expiring SecretIDs
if !result.ExpirationTime.IsZero() && time.Now().After(result.ExpirationTime) {
// Clean up the accessor of the secret ID first
err = b.deleteSecretIDAccessorEntry(ctx, s, result.SecretIDAccessor, secretIDPrefixToUse)
if err != nil {
return errwrap.Wrapf("failed to delete secret ID accessor entry: {{err}}", err)
}

if err := s.Delete(ctx, entryIndex); err != nil {
return errwrap.Wrapf(fmt.Sprintf("error deleting SecretID %q from storage: {{err}}", secretIDHMAC), err)
}

return nil
}

if err := s.Delete(ctx, entryIndex); err != nil {
return errwrap.Wrapf(fmt.Sprintf("error deleting SecretID %q from storage: {{err}}", secretIDHMAC), err)
// At this point, the secret ID is not expired and is valid. Delete
// the corresponding accessor from the accessorMap. This will leave
// only the dangling accessors in the map which can then be cleaned
// up later.
salt, err := b.Salt(ctx)
if err != nil {
return err
}
delete(accessorMap, salt.SaltID(result.SecretIDAccessor))

return nil
}

// At this point, the secret ID is not expired and is valid. Delete
// the corresponding accessor from the accessorMap. This will leave
// only the dangling accessors in the map which can then be cleaned
// up later.
salt, err := b.Salt(ctx)
if err != nil {
return err
for _, roleNameHMAC := range roleNameHMACs {
secretIDHMACs, err := s.List(ctx, fmt.Sprintf("%s%s", secretIDPrefixToUse, roleNameHMAC))
if err != nil {
return err
}
for _, secretIDHMAC := range secretIDHMACs {
err = secretIDCleanupFunc(secretIDHMAC, roleNameHMAC, secretIDPrefixToUse)
if err != nil {
return err
}
}
}
delete(accessorMap, salt.SaltID(result.SecretIDAccessor))

return nil
}

for _, roleNameHMAC := range roleNameHMACs {
secretIDHMACs, err := s.List(ctx, fmt.Sprintf("%s%s", secretIDPrefixToUse, roleNameHMAC))
if err != nil {
return err
}
for _, secretIDHMAC := range secretIDHMACs {
err = secretIDCleanupFunc(secretIDHMAC, roleNameHMAC, secretIDPrefixToUse)
// Accessor indexes were not getting cleaned up until 0.9.3. This is a fix
// to clean up the dangling accessor entries.
for accessorHash, _ := range accessorMap {
// Ideally, locking should be performed here. But for that, accessors
// are required in plaintext, which are not available. Hence performing
// a racy cleanup.
err = s.Delete(ctx, secretIDAccessorPrefix+accessorHash)
if err != nil {
return err
}
}
}

// Accessor indexes were not getting cleaned up until 0.9.3. This is a fix
// to clean up the dangling accessor entries.
for accessorHash, _ := range accessorMap {
// Ideally, locking should be performed here. But for that, accessors
// are required in plaintext, which are not available. Hence performing
// a racy cleanup.
err = s.Delete(ctx, secretIDAccessorPrefix+accessorHash)
if err != nil {
return err
}
return nil
}

return nil
}

err := tidyFunc(secretIDPrefix, secretIDAccessorPrefix)
if err != nil {
return err
}
err = tidyFunc(secretIDLocalPrefix, secretIDAccessorLocalPrefix)
if err != nil {
return err
}
err := tidyFunc(secretIDPrefix, secretIDAccessorPrefix)
if err != nil {
logger.Error("error tidying global secret IDs", "error", err)
return
}
err = tidyFunc(secretIDLocalPrefix, secretIDAccessorLocalPrefix)
if err != nil {
logger.Error("error tidying local secret IDs", "error", err)
return
}
}()

return result
resp := &logical.Response{}
resp.AddWarning("Tidy operation successfully started. Any information from the operation will be printed to Vault's server logs.")
return resp, nil
}

// pathTidySecretIDUpdate is used to delete the expired SecretID entries
func (b *backend) pathTidySecretIDUpdate(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) {
return nil, b.tidySecretID(ctx, req.Storage)
return b.tidySecretID(ctx, req.Storage)
}

const pathTidySecretIDSyn = "Trigger the clean-up of expired SecretID entries."
Expand Down
6 changes: 5 additions & 1 deletion builtin/credential/approle/path_tidy_user_id_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package approle
import (
"context"
"testing"
"time"

"github.com/hashicorp/vault/logical"
)
Expand Down Expand Up @@ -64,11 +65,14 @@ func TestAppRole_TidyDanglingAccessors(t *testing.T) {
t.Fatalf("bad: len(accessorHashes); expect 3, got %d", len(accessorHashes))
}

err = b.tidySecretID(context.Background(), storage)
_, err = b.tidySecretID(context.Background(), storage)
if err != nil {
t.Fatal(err)
}

// It runs async so we give it a bit of time to run
time.Sleep(10 * time.Second)

accessorHashes, err = storage.List(context.Background(), "accessor/")
if err != nil {
t.Fatal(err)
Expand Down
83 changes: 51 additions & 32 deletions builtin/credential/aws/path_tidy_identity_whitelist.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,53 +33,72 @@ expiration, before it is removed from the backend storage.`,
}

// tidyWhitelistIdentity is used to delete entries in the whitelist that are expired.
func (b *backend) tidyWhitelistIdentity(ctx context.Context, s logical.Storage, safety_buffer int) error {
grabbed := atomic.CompareAndSwapUint32(b.tidyWhitelistCASGuard, 0, 1)
if grabbed {
defer atomic.StoreUint32(b.tidyWhitelistCASGuard, 0)
} else {
return fmt.Errorf("identity whitelist tidy operation already running")
func (b *backend) tidyWhitelistIdentity(ctx context.Context, s logical.Storage, safety_buffer int) (*logical.Response, error) {
if !atomic.CompareAndSwapUint32(b.tidyWhitelistCASGuard, 0, 1) {
resp := &logical.Response{}
resp.AddWarning("Tidy operation already in progress.")
return resp, nil
}

bufferDuration := time.Duration(safety_buffer) * time.Second
go func() {
defer atomic.StoreUint32(b.tidyWhitelistCASGuard, 0)

identities, err := s.List(ctx, "whitelist/identity/")
if err != nil {
return err
}
// Don't cancel when the original client request goes away
ctx = context.Background()

for _, instanceID := range identities {
identityEntry, err := s.Get(ctx, "whitelist/identity/"+instanceID)
if err != nil {
return errwrap.Wrapf(fmt.Sprintf("error fetching identity of instanceID %q: {{err}}", instanceID), err)
}
logger := b.Logger().Named("wltidy")

if identityEntry == nil {
return fmt.Errorf("identity entry for instanceID %q is nil", instanceID)
}
bufferDuration := time.Duration(safety_buffer) * time.Second

if identityEntry.Value == nil || len(identityEntry.Value) == 0 {
return fmt.Errorf("found identity entry for instanceID %q but actual identity is empty", instanceID)
}
doTidy := func() error {
identities, err := s.List(ctx, "whitelist/identity/")
if err != nil {
return err
}

for _, instanceID := range identities {
identityEntry, err := s.Get(ctx, "whitelist/identity/"+instanceID)
if err != nil {
return errwrap.Wrapf(fmt.Sprintf("error fetching identity of instanceID %q: {{err}}", instanceID), err)
}

if identityEntry == nil {
return fmt.Errorf("identity entry for instanceID %q is nil", instanceID)
}

if identityEntry.Value == nil || len(identityEntry.Value) == 0 {
return fmt.Errorf("found identity entry for instanceID %q but actual identity is empty", instanceID)
}

var result whitelistIdentity
if err := identityEntry.DecodeJSON(&result); err != nil {
return err
}

if time.Now().After(result.ExpirationTime.Add(bufferDuration)) {
if err := s.Delete(ctx, "whitelist/identity"+instanceID); err != nil {
return errwrap.Wrapf(fmt.Sprintf("error deleting identity of instanceID %q from storage: {{err}}", instanceID), err)
}
}
}

var result whitelistIdentity
if err := identityEntry.DecodeJSON(&result); err != nil {
return err
return nil
}

if time.Now().After(result.ExpirationTime.Add(bufferDuration)) {
if err := s.Delete(ctx, "whitelist/identity"+instanceID); err != nil {
return errwrap.Wrapf(fmt.Sprintf("error deleting identity of instanceID %q from storage: {{err}}", instanceID), err)
}
if err := doTidy(); err != nil {
logger.Error("error running whitelist tidy", "error", err)
return
}
}
}()

return nil
resp := &logical.Response{}
resp.AddWarning("Tidy operation successfully started. Any information from the operation will be printed to Vault's server logs.")
return resp, nil
}

// pathTidyIdentityWhitelistUpdate is used to delete entries in the whitelist that are expired.
func (b *backend) pathTidyIdentityWhitelistUpdate(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) {
return nil, b.tidyWhitelistIdentity(ctx, req.Storage, data.Get("safety_buffer").(int))
return b.tidyWhitelistIdentity(ctx, req.Storage, data.Get("safety_buffer").(int))
}

const pathTidyIdentityWhitelistSyn = `
Expand Down
Loading

0 comments on commit f493d24

Please sign in to comment.