More updates to mutexes and adjust blacklisted roletag default safety buffer

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 defaults to 180 days for roletag blacklist
-		safety_buffer := 259200
+		safety_buffer := 15552000
 		tidyBlacklistConfigEntry, err := b.configTidyRoleTags(req.Storage)
 		if err != nil {
 			return err

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
 	}

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/<cert_name>` 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()
+	// Ensure that we have not
-	defer b.configMutex.Unlock()
 	// If none of the checks fail, save the provided certificate.
 	entry, err := logical.StorageEntryJSON("config/certificate/"+certName, certEntry)
 	if err != nil {

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
 	}

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

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/<role_tag>' 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

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
 	}

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/<instance_id>' 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/<instance_id>' endpoint.",
 			},
 		},