diff --git a/builtin/credential/aws/backend.go b/builtin/credential/aws/backend.go index 67985fd8ac..9f26257909 100644 --- a/builtin/credential/aws/backend.go +++ b/builtin/credential/aws/backend.go @@ -25,6 +25,9 @@ type backend struct { // Lock to make changes to any of the backend's configuration endpoints. configMutex sync.RWMutex + // Lock to make changes to the blacklist entries + blacklistMutex sync.RWMutex + // Duration after which the periodic function of the backend needs to // tidy the blacklist and whitelist entries. tidyCooldownPeriod time.Duration @@ -101,8 +104,8 @@ func (b *backend) periodicFunc(req *logical.Request) error { // Run the tidy operations for the first time. Then run it when current // time matches the nextTidyTime. if b.nextTidyTime.IsZero() || !time.Now().UTC().Before(b.nextTidyTime) { - // safety_buffer defaults to 72h - safety_buffer := 259200 + // safety_buffer defaults to 180 days for roletag blacklist + safety_buffer := 15552000 tidyBlacklistConfigEntry, err := b.configTidyRoleTags(req.Storage) if err != nil { return err diff --git a/builtin/credential/aws/path_blacklist_roletag.go b/builtin/credential/aws/path_blacklist_roletag.go index 5507282d7d..775854eb43 100644 --- a/builtin/credential/aws/path_blacklist_roletag.go +++ b/builtin/credential/aws/path_blacklist_roletag.go @@ -48,6 +48,9 @@ func pathListBlacklistRoleTags(b *backend) *framework.Path { // Lists all the blacklisted role tags. func (b *backend) pathBlacklistRoleTagsList( req *logical.Request, data *framework.FieldData) (*logical.Response, error) { + b.blacklistMutex.RLock() + defer b.blacklistMutex.RUnlock() + tags, err := req.Storage.List("blacklist/roletag/") if err != nil { return nil, err @@ -69,7 +72,14 @@ func (b *backend) pathBlacklistRoleTagsList( // Fetch an entry from the role tag blacklist for a given tag. // This method takes a role tag in its original form and not a base64 encoded form. -func blacklistRoleTagEntry(s logical.Storage, tag string) (*roleTagBlacklistEntry, error) { +func (b *backend) blacklistRoleTagEntry(s logical.Storage, tag string) (*roleTagBlacklistEntry, error) { + b.blacklistMutex.RLock() + defer b.blacklistMutex.RUnlock() + + return b.blacklistRoleTagEntryInternal(s, tag) +} + +func (b *backend) blacklistRoleTagEntryInternal(s logical.Storage, tag string) (*roleTagBlacklistEntry, error) { entry, err := s.Get("blacklist/roletag/" + base64.StdEncoding.EncodeToString([]byte(tag))) if err != nil { return nil, err @@ -88,6 +98,9 @@ func blacklistRoleTagEntry(s logical.Storage, tag string) (*roleTagBlacklistEntr // Deletes an entry from the role tag blacklist for a given tag. func (b *backend) pathBlacklistRoleTagDelete( req *logical.Request, data *framework.FieldData) (*logical.Response, error) { + b.blacklistMutex.Lock() + defer b.blacklistMutex.Unlock() + tag := data.Get("role_tag").(string) if tag == "" { return logical.ErrorResponse("missing role_tag"), nil @@ -106,7 +119,7 @@ func (b *backend) pathBlacklistRoleTagRead( return logical.ErrorResponse("missing role_tag"), nil } - entry, err := blacklistRoleTagEntry(req.Storage, tag) + entry, err := b.blacklistRoleTagEntry(req.Storage, tag) if err != nil { return nil, err } @@ -161,8 +174,11 @@ func (b *backend) pathBlacklistRoleTagUpdate( return logical.ErrorResponse("role entry not found"), nil } + b.blacklistMutex.Lock() + defer b.blacklistMutex.Unlock() + // Check if the role tag is already blacklisted. If yes, update it. - blEntry, err := blacklistRoleTagEntry(req.Storage, tag) + blEntry, err := b.blacklistRoleTagEntryInternal(req.Storage, tag) if err != nil { return nil, err } diff --git a/builtin/credential/aws/path_config_certificate.go b/builtin/credential/aws/path_config_certificate.go index 2460897469..9adaa50c04 100644 --- a/builtin/credential/aws/path_config_certificate.go +++ b/builtin/credential/aws/path_config_certificate.go @@ -95,6 +95,7 @@ func (b *backend) pathConfigCertificateExistenceCheck(req *logical.Request, data if certName == "" { return false, fmt.Errorf("missing cert_name") } + entry, err := b.awsPublicCertificateEntry(req.Storage, certName) if err != nil { return false, err @@ -138,6 +139,11 @@ func decodePEMAndParseCertificate(certificate string) (*x509.Certificate, error) // certificates, that were registered using `config/certificate/` endpoint. // This method will also append default certificate in the backend, to the slice. func (b *backend) awsPublicCertificates(s logical.Storage) ([]*x509.Certificate, error) { + // Lock at beginning and use internal method so that we are consistent as + // we iterate through + b.configMutex.RLock() + defer b.configMutex.RUnlock() + var certs []*x509.Certificate // Append the generic certificate provided in the AWS EC2 instance metadata documentation. @@ -155,7 +161,7 @@ func (b *backend) awsPublicCertificates(s logical.Storage) ([]*x509.Certificate, // Iterate through each certificate, parse and append it to a slice. for _, cert := range registeredCerts { - certEntry, err := b.awsPublicCertificateEntry(s, cert) + certEntry, err := b.awsPublicCertificateEntryInternal(s, cert) if err != nil { return nil, err } @@ -178,6 +184,11 @@ func (b *backend) awsPublicCertificateEntry(s logical.Storage, certName string) b.configMutex.RLock() defer b.configMutex.RUnlock() + return b.awsPublicCertificateEntryInternal(s, certName) +} + +// Internal version of the above that does no locking +func (b *backend) awsPublicCertificateEntryInternal(s logical.Storage, certName string) (*awsPublicCert, error) { entry, err := s.Get("config/certificate/" + certName) if err != nil { return nil, err @@ -238,8 +249,11 @@ func (b *backend) pathConfigCertificateCreateUpdate( return logical.ErrorResponse("missing cert_name"), nil } + b.configMutex.Lock() + defer b.configMutex.Unlock() + // Check if there is already a certificate entry registered. - certEntry, err := b.awsPublicCertificateEntry(req.Storage, certName) + certEntry, err := b.awsPublicCertificateEntryInternal(req.Storage, certName) if err != nil { return nil, err } @@ -275,8 +289,7 @@ func (b *backend) pathConfigCertificateCreateUpdate( return logical.ErrorResponse("invalid certificate; failed to decode and parse certificate"), nil } - b.configMutex.Lock() - defer b.configMutex.Unlock() + // Ensure that we have not // If none of the checks fail, save the provided certificate. entry, err := logical.StorageEntryJSON("config/certificate/"+certName, certEntry) if err != nil { diff --git a/builtin/credential/aws/path_config_client.go b/builtin/credential/aws/path_config_client.go index b73a912fb3..6fe624eb44 100644 --- a/builtin/credential/aws/path_config_client.go +++ b/builtin/credential/aws/path_config_client.go @@ -47,8 +47,6 @@ func pathConfigClient(b *backend) *framework.Path { // Returning 'true' forces an UpdateOperation, CreateOperation otherwise. func (b *backend) pathConfigClientExistenceCheck( req *logical.Request, data *framework.FieldData) (bool, error) { - b.configMutex.RLock() - defer b.configMutex.RUnlock() entry, err := b.clientConfigEntry(req.Storage) if err != nil { @@ -59,6 +57,14 @@ func (b *backend) pathConfigClientExistenceCheck( // Fetch the client configuration required to access the AWS API. func (b *backend) clientConfigEntry(s logical.Storage) (*clientConfig, error) { + b.configMutex.RLock() + defer b.configMutex.RUnlock() + + return b.clientConfigEntry(s) +} + +// Internal version that does no locking +func (b *backend) clientConfigEntryInternal(s logical.Storage) (*clientConfig, error) { entry, err := s.Get("config/client") if err != nil { return nil, err @@ -76,8 +82,6 @@ func (b *backend) clientConfigEntry(s logical.Storage) (*clientConfig, error) { func (b *backend) pathConfigClientRead( req *logical.Request, data *framework.FieldData) (*logical.Response, error) { - b.configMutex.RLock() - defer b.configMutex.RUnlock() clientConfig, err := b.clientConfigEntry(req.Storage) if err != nil { return nil, err @@ -114,7 +118,7 @@ func (b *backend) pathConfigClientCreateUpdate( b.configMutex.Lock() defer b.configMutex.Unlock() - configEntry, err := b.clientConfigEntry(req.Storage) + configEntry, err := b.clientConfigEntryInternal(req.Storage) if err != nil { return nil, err } diff --git a/builtin/credential/aws/path_config_tidy_identities.go b/builtin/credential/aws/path_config_tidy_identities.go index bd59ddca30..86076d50c6 100644 --- a/builtin/credential/aws/path_config_tidy_identities.go +++ b/builtin/credential/aws/path_config_tidy_identities.go @@ -44,9 +44,6 @@ expiration, before it is removed from the backend storage.`, } func (b *backend) pathConfigTidyIdentitiesExistenceCheck(req *logical.Request, data *framework.FieldData) (bool, error) { - b.configMutex.RLock() - defer b.configMutex.RUnlock() - entry, err := b.configTidyIdentities(req.Storage) if err != nil { return false, err @@ -55,6 +52,13 @@ func (b *backend) pathConfigTidyIdentitiesExistenceCheck(req *logical.Request, d } func (b *backend) configTidyIdentities(s logical.Storage) (*tidyWhitelistIdentityConfig, error) { + b.configMutex.RLock() + defer b.configMutex.RUnlock() + + return b.configTidyIdentitiesInternal(s) +} + +func (b *backend) configTidyIdentitiesInternal(s logical.Storage) (*tidyWhitelistIdentityConfig, error) { entry, err := s.Get(identityWhitelistConfigPath) if err != nil { return nil, err @@ -74,7 +78,7 @@ func (b *backend) pathConfigTidyIdentitiesCreateUpdate(req *logical.Request, dat b.configMutex.Lock() defer b.configMutex.Unlock() - configEntry, err := b.configTidyIdentities(req.Storage) + configEntry, err := b.configTidyIdentitiesInternal(req.Storage) if err != nil { return nil, err } @@ -109,9 +113,6 @@ func (b *backend) pathConfigTidyIdentitiesCreateUpdate(req *logical.Request, dat } func (b *backend) pathConfigTidyIdentitiesRead(req *logical.Request, data *framework.FieldData) (*logical.Response, error) { - b.configMutex.RLock() - defer b.configMutex.RUnlock() - clientConfig, err := b.configTidyIdentities(req.Storage) if err != nil { return nil, err diff --git a/builtin/credential/aws/path_config_tidy_roletags.go b/builtin/credential/aws/path_config_tidy_roletags.go index 6ea693c884..bc02551409 100644 --- a/builtin/credential/aws/path_config_tidy_roletags.go +++ b/builtin/credential/aws/path_config_tidy_roletags.go @@ -18,15 +18,16 @@ func pathConfigTidyRoleTags(b *backend) *framework.Path { Fields: map[string]*framework.FieldSchema{ "safety_buffer": &framework.FieldSchema{ Type: framework.TypeDurationSecond, - Default: 259200, //72h + Default: 15552000, //180d Description: `The amount of extra time that must have passed beyond the roletag -expiration, before it is removed from the backend storage.`, +expiration, before it is removed from the backend storage. +Defaults to 4320h (180 days).`, }, "disable_periodic_tidy": &framework.FieldSchema{ Type: framework.TypeBool, Default: false, - Description: "If set to 'true', disables the periodic tidying of the 'blacklist/roletag/' entries.", + Description: "If set to 'true', disables the periodic tidying of blacklisted entries.", }, }, @@ -45,9 +46,6 @@ expiration, before it is removed from the backend storage.`, } func (b *backend) pathConfigTidyRoleTagsExistenceCheck(req *logical.Request, data *framework.FieldData) (bool, error) { - b.configMutex.RLock() - defer b.configMutex.RUnlock() - entry, err := b.configTidyRoleTags(req.Storage) if err != nil { return false, err @@ -56,6 +54,13 @@ func (b *backend) pathConfigTidyRoleTagsExistenceCheck(req *logical.Request, dat } func (b *backend) configTidyRoleTags(s logical.Storage) (*tidyBlacklistRoleTagConfig, error) { + b.configMutex.RLock() + defer b.configMutex.RUnlock() + + return b.configTidyRoleTagsInternal(s) +} + +func (b *backend) configTidyRoleTagsInternal(s logical.Storage) (*tidyBlacklistRoleTagConfig, error) { entry, err := s.Get(roletagBlacklistConfigPath) if err != nil { return nil, err @@ -76,7 +81,7 @@ func (b *backend) pathConfigTidyRoleTagsCreateUpdate(req *logical.Request, data b.configMutex.Lock() defer b.configMutex.Unlock() - configEntry, err := b.configTidyRoleTags(req.Storage) + configEntry, err := b.configTidyRoleTagsInternal(req.Storage) if err != nil { return nil, err } @@ -109,9 +114,6 @@ func (b *backend) pathConfigTidyRoleTagsCreateUpdate(req *logical.Request, data } func (b *backend) pathConfigTidyRoleTagsRead(req *logical.Request, data *framework.FieldData) (*logical.Response, error) { - b.configMutex.RLock() - defer b.configMutex.RUnlock() - clientConfig, err := b.configTidyRoleTags(req.Storage) if err != nil { return nil, err diff --git a/builtin/credential/aws/path_login.go b/builtin/credential/aws/path_login.go index d13d2eedaf..42ff38e851 100644 --- a/builtin/credential/aws/path_login.go +++ b/builtin/credential/aws/path_login.go @@ -419,7 +419,7 @@ func (b *backend) handleRoleTagLogin(s logical.Storage, identityDoc *identityDoc } // Check if the role tag is blacklisted. - blacklistEntry, err := blacklistRoleTagEntry(s, rTagValue) + blacklistEntry, err := b.blacklistRoleTagEntry(s, rTagValue) if err != nil { return nil, err } diff --git a/builtin/credential/aws/path_role_tag.go b/builtin/credential/aws/path_role_tag.go index 2c71d83786..814d8e41ab 100644 --- a/builtin/credential/aws/path_role_tag.go +++ b/builtin/credential/aws/path_role_tag.go @@ -30,18 +30,18 @@ func pathRoleTag(b *backend) *framework.Path { "instance_id": &framework.FieldSchema{ Type: framework.TypeString, Description: `Instance ID for which this tag is intended for. -This is an optional field, but if set, the created tag can only be used by the instance with the given ID.`, +If set, the created tag can only be used by the instance with the given ID.`, }, "policies": &framework.FieldSchema{ Type: framework.TypeString, - Description: "Policies to be associated with the tag.", + Description: "Policies to be associated with the tag. If set, must be a subset of the role's policies.", }, "max_ttl": &framework.FieldSchema{ Type: framework.TypeDurationSecond, Default: 0, - Description: "The maximum allowed lease duration.", + Description: "If set, specifies the maximum allowed token lifetime.", }, "allow_instance_migration": &framework.FieldSchema{ @@ -53,7 +53,7 @@ This is an optional field, but if set, the created tag can only be used by the i "disallow_reauthentication": &framework.FieldSchema{ Type: framework.TypeBool, Default: false, - Description: "If set, only allows a single token to be granted per instance ID. In order to perform a fresh login, the entry in whitelist for the instance ID needs to be cleared using 'auth/aws/whitelist/identity/' endpoint.", + Description: "If set, only allows a single token to be granted per instance ID. In order to perform a fresh login, the entry in whitelist for the instance ID needs to be cleared using the 'auth/aws/identity-whitelist/' endpoint.", }, },