From 24ae32f10d4009e6a60c75889761b91711114ff2 Mon Sep 17 00:00:00 2001 From: Oren Shomron Date: Sat, 14 May 2016 19:56:49 -0400 Subject: [PATCH 1/7] Support listing ldap group to policy mappings (Fixes #1270) --- builtin/credential/ldap/backend.go | 2 ++ builtin/credential/ldap/backend_test.go | 39 +++++++++++++++++++++++++ builtin/credential/ldap/path_groups.go | 22 ++++++++++++++ builtin/credential/ldap/path_users.go | 24 ++++++++++++++- 4 files changed, 86 insertions(+), 1 deletion(-) diff --git a/builtin/credential/ldap/backend.go b/builtin/credential/ldap/backend.go index ad29bd5a5b..70e1d62da0 100644 --- a/builtin/credential/ldap/backend.go +++ b/builtin/credential/ldap/backend.go @@ -35,7 +35,9 @@ func Backend() *framework.Backend { Paths: append([]*framework.Path{ pathConfig(&b), pathGroups(&b), + pathGroupsList(&b), pathUsers(&b), + pathUsersList(&b), }, mfa.MFAPaths(b.Backend, pathLogin(&b))..., ), diff --git a/builtin/credential/ldap/backend_test.go b/builtin/credential/ldap/backend_test.go index c8e21be70e..5323c8eea8 100644 --- a/builtin/credential/ldap/backend_test.go +++ b/builtin/credential/ldap/backend_test.go @@ -2,6 +2,7 @@ package ldap import ( "fmt" + "reflect" "testing" "time" @@ -38,6 +39,8 @@ func TestBackend_basic(t *testing.T) { testAccStepGroup(t, "engineers", "bar"), testAccStepUser(t, "tesla", "engineers"), testAccStepLogin(t, "tesla", "password"), + testAccStepGroupList(t, []string{"engineers", "scientists"}), + testAccStepUserList(t, []string{"tesla"}), }, }) } @@ -321,3 +324,39 @@ func TestLDAPEscape(t *testing.T) { } } } + +func testAccStepGroupList(t *testing.T, groups []string) logicaltest.TestStep { + return logicaltest.TestStep{ + Operation: logical.ListOperation, + Path: "groups", + Check: func(resp *logical.Response) error { + if resp.IsError() { + return fmt.Errorf("Got error response: %#v", *resp) + } + + exp := groups + if !reflect.DeepEqual(exp, resp.Data["keys"].([]string)) { + return fmt.Errorf("expected:\n%#v\ngot:\n%#v\n", exp, resp.Data["keys"]) + } + return nil + }, + } +} + +func testAccStepUserList(t *testing.T, users []string) logicaltest.TestStep { + return logicaltest.TestStep{ + Operation: logical.ListOperation, + Path: "users", + Check: func(resp *logical.Response) error { + if resp.IsError() { + return fmt.Errorf("Got error response: %#v", *resp) + } + + exp := users + if !reflect.DeepEqual(exp, resp.Data["keys"].([]string)) { + return fmt.Errorf("expected:\n%#v\ngot:\n%#v\n", exp, resp.Data["keys"]) + } + return nil + }, + } +} diff --git a/builtin/credential/ldap/path_groups.go b/builtin/credential/ldap/path_groups.go index c13ead699b..998fdc41b6 100644 --- a/builtin/credential/ldap/path_groups.go +++ b/builtin/credential/ldap/path_groups.go @@ -8,6 +8,19 @@ import ( "github.com/hashicorp/vault/logical/framework" ) +func pathGroupsList(b *backend) *framework.Path { + return &framework.Path{ + Pattern: "groups/?$", + + Callbacks: map[logical.Operation]framework.OperationFunc{ + logical.ListOperation: b.pathGroupList, + }, + + HelpSynopsis: pathGroupHelpSyn, + HelpDescription: pathGroupHelpDesc, + } +} + func pathGroups(b *backend) *framework.Path { return &framework.Path{ Pattern: `groups/(?P.+)`, @@ -94,6 +107,15 @@ func (b *backend) pathGroupWrite( return nil, nil } +func (b *backend) pathGroupList( + req *logical.Request, d *framework.FieldData) (*logical.Response, error) { + groups, err := req.Storage.List("group/") + if err != nil { + return nil, err + } + return logical.ListResponse(groups), nil +} + type GroupEntry struct { Policies []string } diff --git a/builtin/credential/ldap/path_users.go b/builtin/credential/ldap/path_users.go index 7f03a8becd..601405a486 100644 --- a/builtin/credential/ldap/path_users.go +++ b/builtin/credential/ldap/path_users.go @@ -7,6 +7,19 @@ import ( "github.com/hashicorp/vault/logical/framework" ) +func pathUsersList(b *backend) *framework.Path { + return &framework.Path{ + Pattern: "users/?$", + + Callbacks: map[logical.Operation]framework.OperationFunc{ + logical.ListOperation: b.pathUserList, + }, + + HelpSynopsis: pathUserHelpSyn, + HelpDescription: pathUserHelpDesc, + } +} + func pathUsers(b *backend) *framework.Path { return &framework.Path{ Pattern: `users/(?P.+)`, @@ -25,7 +38,7 @@ func pathUsers(b *backend) *framework.Path { Callbacks: map[logical.Operation]framework.OperationFunc{ logical.DeleteOperation: b.pathUserDelete, logical.ReadOperation: b.pathUserRead, - logical.UpdateOperation: b.pathUserWrite, + logical.UpdateOperation: b.pathUserWrite, }, HelpSynopsis: pathUserHelpSyn, @@ -99,6 +112,15 @@ func (b *backend) pathUserWrite( return nil, nil } +func (b *backend) pathUserList( + req *logical.Request, d *framework.FieldData) (*logical.Response, error) { + users, err := req.Storage.List("user/") + if err != nil { + return nil, err + } + return logical.ListResponse(users), nil +} + type UserEntry struct { Groups []string } From 99e1c2680aaf54bff6fef64cd6dfadf20540d298 Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Mon, 16 May 2016 16:23:01 -0400 Subject: [PATCH 2/7] changelog++ --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 758f0cbfed..2072bc6e24 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -78,6 +78,7 @@ IMPROVEMENTS: backend [GH-1404] * credential/ldap: If `groupdn` is not configured, skip searching LDAP and only return policies for local groups, plus a warning [GH-1283] + * credential/ldap: `vault list` support for users and groups [GH-1270] * credential/userpass: Add list support for users [GH-911] * credential/userpass: Remove user configuration paths from requiring sudo, in favor of normal ACL mechanisms [GH-1312] From 624acb34b076510432c4789e6d0e6eb37bafb6a6 Mon Sep 17 00:00:00 2001 From: Seth Vargo Date: Mon, 16 May 2016 16:45:02 -0400 Subject: [PATCH 3/7] Add note about paid training --- website/source/community.html.erb | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/website/source/community.html.erb b/website/source/community.html.erb index 2973efedaf..412fbaee3c 100644 --- a/website/source/community.html.erb +++ b/website/source/community.html.erb @@ -23,6 +23,12 @@ active, dedicated users willing to help you through various mediums. Issue tracker on GitHub. Please only use this for reporting bugs. Do not ask for general help here. Use IRC or the mailing list for that. +

+

+Training: +Paid HashiCorp training courses +are also available in a city near you. Private training courses are also available. +

People

From 8c3866ea16348d49679b6f74b27f1a0b28275e0a Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Tue, 17 May 2016 17:10:12 +0000 Subject: [PATCH 4/7] Rename lease_duration to refresh_interval when there is no lease ID, and output ---- between header and values --- command/format.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/command/format.go b/command/format.go index 93b812312e..ee7145249a 100644 --- a/command/format.go +++ b/command/format.go @@ -129,12 +129,17 @@ func (t TableFormatter) OutputSecret(ui cli.Ui, secret, s *api.Secret) error { input = append(input, fmt.Sprintf("Key %s Value", config.Delim)) + input = append(input, fmt.Sprintf("--- %s -----", config.Delim)) + if s.LeaseDuration > 0 { if s.LeaseID != "" { input = append(input, fmt.Sprintf("lease_id %s %s", config.Delim, s.LeaseID)) + input = append(input, fmt.Sprintf( + "lease_duration %s %d", config.Delim, s.LeaseDuration)) + } else { + input = append(input, fmt.Sprintf( + "refresh_interval %s %d", config.Delim, s.LeaseDuration)) } - input = append(input, fmt.Sprintf( - "lease_duration %s %d", config.Delim, s.LeaseDuration)) if s.LeaseID != "" { input = append(input, fmt.Sprintf( "lease_renewable %s %s", config.Delim, strconv.FormatBool(s.Renewable))) From 83adda998dd75e6baa0bd03967d805440cf5b913 Mon Sep 17 00:00:00 2001 From: vishalnayak Date: Tue, 17 May 2016 20:39:24 -0400 Subject: [PATCH 5/7] Naming of the locked and nonLocked methods --- builtin/credential/aws/backend.go | 4 ++-- builtin/credential/aws/backend_test.go | 6 +++--- builtin/credential/aws/client.go | 2 +- .../credential/aws/path_config_certificate.go | 14 +++++++------- builtin/credential/aws/path_config_client.go | 16 ++++++++-------- .../aws/path_config_tidy_identity_whitelist.go | 12 ++++++------ .../aws/path_config_tidy_roletag_blacklist.go | 12 ++++++------ builtin/credential/aws/path_login.go | 6 +++--- builtin/credential/aws/path_role.go | 12 ++++++------ builtin/credential/aws/path_role_tag.go | 4 ++-- builtin/credential/aws/path_roletag_blacklist.go | 12 ++++++------ 11 files changed, 50 insertions(+), 50 deletions(-) diff --git a/builtin/credential/aws/backend.go b/builtin/credential/aws/backend.go index e0f56d3e8d..2a11f1bf88 100644 --- a/builtin/credential/aws/backend.go +++ b/builtin/credential/aws/backend.go @@ -114,7 +114,7 @@ func (b *backend) periodicFunc(req *logical.Request) error { if b.nextTidyTime.IsZero() || !time.Now().UTC().Before(b.nextTidyTime) { // safety_buffer defaults to 180 days for roletag blacklist safety_buffer := 15552000 - tidyBlacklistConfigEntry, err := b.configTidyRoleTags(req.Storage) + tidyBlacklistConfigEntry, err := b.lockedConfigTidyRoleTags(req.Storage) if err != nil { return err } @@ -135,7 +135,7 @@ func (b *backend) periodicFunc(req *logical.Request) error { // reset the safety_buffer to 72h safety_buffer = 259200 - tidyWhitelistConfigEntry, err := b.configTidyIdentities(req.Storage) + tidyWhitelistConfigEntry, err := b.lockedConfigTidyIdentities(req.Storage) if err != nil { return err } diff --git a/builtin/credential/aws/backend_test.go b/builtin/credential/aws/backend_test.go index 06a27a3084..4a69fa700d 100644 --- a/builtin/credential/aws/backend_test.go +++ b/builtin/credential/aws/backend_test.go @@ -98,7 +98,7 @@ func TestBackend_CreateParseVerifyRoleTag(t *testing.T) { } // read the created role entry - roleEntry, err := b.awsRole(storage, "abcd-123") + roleEntry, err := b.lockedAWSRole(storage, "abcd-123") if err != nil { t.Fatal(err) } @@ -165,7 +165,7 @@ func TestBackend_CreateParseVerifyRoleTag(t *testing.T) { } // get the entry of the newly created role entry - roleEntry2, err := b.awsRole(storage, "ami-6789") + roleEntry2, err := b.lockedAWSRole(storage, "ami-6789") if err != nil { t.Fatal(err) } @@ -1098,7 +1098,7 @@ func TestBackend_PathBlacklistRoleTag(t *testing.T) { } // try to read the deleted entry - tagEntry, err := b.blacklistRoleTagEntry(storage, tag) + tagEntry, err := b.lockedBlacklistRoleTagEntry(storage, tag) if err != nil { t.Fatal(err) } diff --git a/builtin/credential/aws/client.go b/builtin/credential/aws/client.go index f08b7c04ae..01259be690 100644 --- a/builtin/credential/aws/client.go +++ b/builtin/credential/aws/client.go @@ -24,7 +24,7 @@ func (b *backend) getClientConfig(s logical.Storage, region string) (*aws.Config } // Read the configured secret key and access key - config, err := b.clientConfigEntryInternal(s) + config, err := b.nonLockedClientConfigEntry(s) 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 86c91ce9c5..7d1efe8571 100644 --- a/builtin/credential/aws/path_config_certificate.go +++ b/builtin/credential/aws/path_config_certificate.go @@ -96,7 +96,7 @@ func (b *backend) pathConfigCertificateExistenceCheck(req *logical.Request, data return false, fmt.Errorf("missing cert_name") } - entry, err := b.awsPublicCertificateEntry(req.Storage, certName) + entry, err := b.lockedAWSPublicCertificateEntry(req.Storage, certName) if err != nil { return false, err } @@ -161,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.awsPublicCertificateEntryInternal(s, cert) + certEntry, err := b.nonLockedAWSPublicCertificateEntry(s, cert) if err != nil { return nil, err } @@ -180,15 +180,15 @@ func (b *backend) awsPublicCertificates(s logical.Storage) ([]*x509.Certificate, // awsPublicCertificate is used to get the configured AWS Public Key that is used // to verify the PKCS#7 signature of the instance identity document. -func (b *backend) awsPublicCertificateEntry(s logical.Storage, certName string) (*awsPublicCert, error) { +func (b *backend) lockedAWSPublicCertificateEntry(s logical.Storage, certName string) (*awsPublicCert, error) { b.configMutex.RLock() defer b.configMutex.RUnlock() - return b.awsPublicCertificateEntryInternal(s, certName) + return b.nonLockedAWSPublicCertificateEntry(s, certName) } // Internal version of the above that does no locking -func (b *backend) awsPublicCertificateEntryInternal(s logical.Storage, certName string) (*awsPublicCert, error) { +func (b *backend) nonLockedAWSPublicCertificateEntry(s logical.Storage, certName string) (*awsPublicCert, error) { entry, err := s.Get("config/certificate/" + certName) if err != nil { return nil, err @@ -227,7 +227,7 @@ func (b *backend) pathConfigCertificateRead( return logical.ErrorResponse("missing cert_name"), nil } - certificateEntry, err := b.awsPublicCertificateEntry(req.Storage, certName) + certificateEntry, err := b.lockedAWSPublicCertificateEntry(req.Storage, certName) if err != nil { return nil, err } @@ -253,7 +253,7 @@ func (b *backend) pathConfigCertificateCreateUpdate( defer b.configMutex.Unlock() // Check if there is already a certificate entry registered. - certEntry, err := b.awsPublicCertificateEntryInternal(req.Storage, certName) + certEntry, err := b.nonLockedAWSPublicCertificateEntry(req.Storage, certName) if err != nil { return nil, err } diff --git a/builtin/credential/aws/path_config_client.go b/builtin/credential/aws/path_config_client.go index 38e2ad7ffa..eea42546c7 100644 --- a/builtin/credential/aws/path_config_client.go +++ b/builtin/credential/aws/path_config_client.go @@ -48,23 +48,23 @@ func pathConfigClient(b *backend) *framework.Path { func (b *backend) pathConfigClientExistenceCheck( req *logical.Request, data *framework.FieldData) (bool, error) { - entry, err := b.clientConfigEntry(req.Storage) + entry, err := b.lockedClientConfigEntry(req.Storage) if err != nil { return false, err } return entry != nil, nil } -// Fetch the client configuration required to access the AWS API. -func (b *backend) clientConfigEntry(s logical.Storage) (*clientConfig, error) { +// Fetch the client configuration required to access the AWS API, after acquiring an exclusive lock. +func (b *backend) lockedClientConfigEntry(s logical.Storage) (*clientConfig, error) { b.configMutex.RLock() defer b.configMutex.RUnlock() - return b.clientConfigEntryInternal(s) + return b.nonLockedClientConfigEntry(s) } -// Internal version that does no locking -func (b *backend) clientConfigEntryInternal(s logical.Storage) (*clientConfig, error) { +// Fetch the client configuration required to access the AWS API. +func (b *backend) nonLockedClientConfigEntry(s logical.Storage) (*clientConfig, error) { entry, err := s.Get("config/client") if err != nil { return nil, err @@ -82,7 +82,7 @@ func (b *backend) clientConfigEntryInternal(s logical.Storage) (*clientConfig, e func (b *backend) pathConfigClientRead( req *logical.Request, data *framework.FieldData) (*logical.Response, error) { - clientConfig, err := b.clientConfigEntry(req.Storage) + clientConfig, err := b.lockedClientConfigEntry(req.Storage) if err != nil { return nil, err } @@ -118,7 +118,7 @@ func (b *backend) pathConfigClientCreateUpdate( b.configMutex.Lock() defer b.configMutex.Unlock() - configEntry, err := b.clientConfigEntryInternal(req.Storage) + configEntry, err := b.nonLockedClientConfigEntry(req.Storage) if err != nil { return nil, err } diff --git a/builtin/credential/aws/path_config_tidy_identity_whitelist.go b/builtin/credential/aws/path_config_tidy_identity_whitelist.go index 700e5fa21a..1ee5ed7f08 100644 --- a/builtin/credential/aws/path_config_tidy_identity_whitelist.go +++ b/builtin/credential/aws/path_config_tidy_identity_whitelist.go @@ -44,21 +44,21 @@ expiration, before it is removed from the backend storage.`, } func (b *backend) pathConfigTidyIdentityWhitelistExistenceCheck(req *logical.Request, data *framework.FieldData) (bool, error) { - entry, err := b.configTidyIdentities(req.Storage) + entry, err := b.lockedConfigTidyIdentities(req.Storage) if err != nil { return false, err } return entry != nil, nil } -func (b *backend) configTidyIdentities(s logical.Storage) (*tidyWhitelistIdentityConfig, error) { +func (b *backend) lockedConfigTidyIdentities(s logical.Storage) (*tidyWhitelistIdentityConfig, error) { b.configMutex.RLock() defer b.configMutex.RUnlock() - return b.configTidyIdentitiesInternal(s) + return b.nonLockedConfigTidyIdentities(s) } -func (b *backend) configTidyIdentitiesInternal(s logical.Storage) (*tidyWhitelistIdentityConfig, error) { +func (b *backend) nonLockedConfigTidyIdentities(s logical.Storage) (*tidyWhitelistIdentityConfig, error) { entry, err := s.Get(identityWhitelistConfigPath) if err != nil { return nil, err @@ -78,7 +78,7 @@ func (b *backend) pathConfigTidyIdentityWhitelistCreateUpdate(req *logical.Reque b.configMutex.Lock() defer b.configMutex.Unlock() - configEntry, err := b.configTidyIdentitiesInternal(req.Storage) + configEntry, err := b.nonLockedConfigTidyIdentities(req.Storage) if err != nil { return nil, err } @@ -113,7 +113,7 @@ func (b *backend) pathConfigTidyIdentityWhitelistCreateUpdate(req *logical.Reque } func (b *backend) pathConfigTidyIdentityWhitelistRead(req *logical.Request, data *framework.FieldData) (*logical.Response, error) { - clientConfig, err := b.configTidyIdentities(req.Storage) + clientConfig, err := b.lockedConfigTidyIdentities(req.Storage) if err != nil { return nil, err } diff --git a/builtin/credential/aws/path_config_tidy_roletag_blacklist.go b/builtin/credential/aws/path_config_tidy_roletag_blacklist.go index 6932f5ec96..1d834030b0 100644 --- a/builtin/credential/aws/path_config_tidy_roletag_blacklist.go +++ b/builtin/credential/aws/path_config_tidy_roletag_blacklist.go @@ -46,21 +46,21 @@ Defaults to 4320h (180 days).`, } func (b *backend) pathConfigTidyRoletagBlacklistExistenceCheck(req *logical.Request, data *framework.FieldData) (bool, error) { - entry, err := b.configTidyRoleTags(req.Storage) + entry, err := b.lockedConfigTidyRoleTags(req.Storage) if err != nil { return false, err } return entry != nil, nil } -func (b *backend) configTidyRoleTags(s logical.Storage) (*tidyBlacklistRoleTagConfig, error) { +func (b *backend) lockedConfigTidyRoleTags(s logical.Storage) (*tidyBlacklistRoleTagConfig, error) { b.configMutex.RLock() defer b.configMutex.RUnlock() - return b.configTidyRoleTagsInternal(s) + return b.nonLockedConfigTidyRoleTags(s) } -func (b *backend) configTidyRoleTagsInternal(s logical.Storage) (*tidyBlacklistRoleTagConfig, error) { +func (b *backend) nonLockedConfigTidyRoleTags(s logical.Storage) (*tidyBlacklistRoleTagConfig, error) { entry, err := s.Get(roletagBlacklistConfigPath) if err != nil { return nil, err @@ -81,7 +81,7 @@ func (b *backend) pathConfigTidyRoletagBlacklistCreateUpdate(req *logical.Reques b.configMutex.Lock() defer b.configMutex.Unlock() - configEntry, err := b.configTidyRoleTagsInternal(req.Storage) + configEntry, err := b.nonLockedConfigTidyRoleTags(req.Storage) if err != nil { return nil, err } @@ -114,7 +114,7 @@ func (b *backend) pathConfigTidyRoletagBlacklistCreateUpdate(req *logical.Reques } func (b *backend) pathConfigTidyRoletagBlacklistRead(req *logical.Request, data *framework.FieldData) (*logical.Response, error) { - clientConfig, err := b.configTidyRoleTags(req.Storage) + clientConfig, err := b.lockedConfigTidyRoleTags(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 7aecff4461..f2c65b13b4 100644 --- a/builtin/credential/aws/path_login.go +++ b/builtin/credential/aws/path_login.go @@ -236,7 +236,7 @@ func (b *backend) pathLoginUpdate( } // Get the entry for the role used by the instance. - roleEntry, err := b.awsRole(req.Storage, roleName) + roleEntry, err := b.lockedAWSRole(req.Storage, roleName) if err != nil { return nil, err } @@ -442,7 +442,7 @@ func (b *backend) handleRoleTagLogin(s logical.Storage, identityDoc *identityDoc } // Check if the role tag is blacklisted. - blacklistEntry, err := b.blacklistRoleTagEntry(s, rTagValue) + blacklistEntry, err := b.lockedBlacklistRoleTagEntry(s, rTagValue) if err != nil { return nil, err } @@ -487,7 +487,7 @@ func (b *backend) pathLoginRenew( } // Ensure that role entry is not deleted. - roleEntry, err := b.awsRole(req.Storage, storedIdentity.Role) + roleEntry, err := b.lockedAWSRole(req.Storage, storedIdentity.Role) if err != nil { return nil, err } diff --git a/builtin/credential/aws/path_role.go b/builtin/credential/aws/path_role.go index adb8698763..e65bbfa252 100644 --- a/builtin/credential/aws/path_role.go +++ b/builtin/credential/aws/path_role.go @@ -101,7 +101,7 @@ func pathListRoles(b *backend) *framework.Path { // Establishes dichotomy of request operation between CreateOperation and UpdateOperation. // Returning 'true' forces an UpdateOperation, CreateOperation otherwise. func (b *backend) pathRoleExistenceCheck(req *logical.Request, data *framework.FieldData) (bool, error) { - entry, err := b.awsRole(req.Storage, strings.ToLower(data.Get("role").(string))) + entry, err := b.lockedAWSRole(req.Storage, strings.ToLower(data.Get("role").(string))) if err != nil { return false, err } @@ -109,14 +109,14 @@ func (b *backend) pathRoleExistenceCheck(req *logical.Request, data *framework.F } // awsRole is used to get the information registered for the given AMI ID. -func (b *backend) awsRole(s logical.Storage, role string) (*awsRoleEntry, error) { +func (b *backend) lockedAWSRole(s logical.Storage, role string) (*awsRoleEntry, error) { b.roleMutex.RLock() defer b.roleMutex.RUnlock() - return b.awsRoleInternal(s, role) + return b.nonLockedAWSRole(s, role) } -func (b *backend) awsRoleInternal(s logical.Storage, role string) (*awsRoleEntry, error) { +func (b *backend) nonLockedAWSRole(s logical.Storage, role string) (*awsRoleEntry, error) { entry, err := s.Get("role/" + strings.ToLower(role)) if err != nil { return nil, err @@ -162,7 +162,7 @@ func (b *backend) pathRoleList( // pathRoleRead is used to view the information registered for a given AMI ID. func (b *backend) pathRoleRead( req *logical.Request, data *framework.FieldData) (*logical.Response, error) { - roleEntry, err := b.awsRole(req.Storage, strings.ToLower(data.Get("role").(string))) + roleEntry, err := b.lockedAWSRole(req.Storage, strings.ToLower(data.Get("role").(string))) if err != nil { return nil, err } @@ -196,7 +196,7 @@ func (b *backend) pathRoleCreateUpdate( b.roleMutex.Lock() defer b.roleMutex.Unlock() - roleEntry, err := b.awsRoleInternal(req.Storage, roleName) + roleEntry, err := b.nonLockedAWSRole(req.Storage, roleName) 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 4929bc07b8..bf48a14b82 100644 --- a/builtin/credential/aws/path_role_tag.go +++ b/builtin/credential/aws/path_role_tag.go @@ -78,7 +78,7 @@ func (b *backend) pathRoleTagUpdate( } // Fetch the role entry - roleEntry, err := b.awsRole(req.Storage, roleName) + roleEntry, err := b.lockedAWSRole(req.Storage, roleName) if err != nil { return nil, err } @@ -346,7 +346,7 @@ func (b *backend) parseAndVerifyRoleTagValue(s logical.Storage, tag string) (*ro return nil, fmt.Errorf("missing role name") } - roleEntry, err := b.awsRole(s, rTag.Role) + roleEntry, err := b.lockedAWSRole(s, rTag.Role) if err != nil { return nil, err } diff --git a/builtin/credential/aws/path_roletag_blacklist.go b/builtin/credential/aws/path_roletag_blacklist.go index ff7362449d..0bbe33e5c2 100644 --- a/builtin/credential/aws/path_roletag_blacklist.go +++ b/builtin/credential/aws/path_roletag_blacklist.go @@ -72,14 +72,14 @@ func (b *backend) pathRoletagBlacklistsList( // 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 (b *backend) blacklistRoleTagEntry(s logical.Storage, tag string) (*roleTagBlacklistEntry, error) { +func (b *backend) lockedBlacklistRoleTagEntry(s logical.Storage, tag string) (*roleTagBlacklistEntry, error) { b.blacklistMutex.RLock() defer b.blacklistMutex.RUnlock() - return b.blacklistRoleTagEntryInternal(s, tag) + return b.nonLockedBlacklistRoleTagEntry(s, tag) } -func (b *backend) blacklistRoleTagEntryInternal(s logical.Storage, tag string) (*roleTagBlacklistEntry, error) { +func (b *backend) nonLockedBlacklistRoleTagEntry(s logical.Storage, tag string) (*roleTagBlacklistEntry, error) { entry, err := s.Get("blacklist/roletag/" + base64.StdEncoding.EncodeToString([]byte(tag))) if err != nil { return nil, err @@ -119,7 +119,7 @@ func (b *backend) pathRoletagBlacklistRead( return logical.ErrorResponse("missing role_tag"), nil } - entry, err := b.blacklistRoleTagEntry(req.Storage, tag) + entry, err := b.lockedBlacklistRoleTagEntry(req.Storage, tag) if err != nil { return nil, err } @@ -166,7 +166,7 @@ func (b *backend) pathRoletagBlacklistUpdate( } // Get the entry for the role mentioned in the role tag. - roleEntry, err := b.awsRole(req.Storage, rTag.Role) + roleEntry, err := b.lockedAWSRole(req.Storage, rTag.Role) if err != nil { return nil, err } @@ -178,7 +178,7 @@ func (b *backend) pathRoletagBlacklistUpdate( defer b.blacklistMutex.Unlock() // Check if the role tag is already blacklisted. If yes, update it. - blEntry, err := b.blacklistRoleTagEntryInternal(req.Storage, tag) + blEntry, err := b.nonLockedBlacklistRoleTagEntry(req.Storage, tag) if err != nil { return nil, err } From 5330aa734bea2ea65a804b45ef3f5f047fc32b84 Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Wed, 18 May 2016 00:47:42 +0000 Subject: [PATCH 6/7] Use Consul API client's DefaultNonPooledTransport. What we should probably do is create a client with a mutex and invalidate it when parameters change rather than creating a client over and over...that can be a TODO for later but for now this fix suffices. Fixes #1428 --- builtin/logical/consul/client.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/logical/consul/client.go b/builtin/logical/consul/client.go index 528e879793..71f1e1975d 100644 --- a/builtin/logical/consul/client.go +++ b/builtin/logical/consul/client.go @@ -23,7 +23,7 @@ func client(s logical.Storage) (*api.Client, error) { return nil, fmt.Errorf("error reading root configuration: %s", err) } - consulConf := api.DefaultConfig() + consulConf := api.DefaultNonPooledConfig() consulConf.Address = conf.Address consulConf.Scheme = conf.Scheme consulConf.Token = conf.Token From 469aff0d14481f8e784bce7c1004b93152aa8780 Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Wed, 18 May 2016 01:00:58 +0000 Subject: [PATCH 7/7] changelog++ --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2072bc6e24..5a0f25e407 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -115,6 +115,8 @@ BUG FIXES: * credential/various: Fix renewal conditions when `default` policy is not contained in the backend config [GH-1256] * physical/s3: Don't panic in certain error cases from bad S3 responses [GH-1353] + * secret/consul: Use non-pooled Consul API client to avoid leaving files open + [GH-1428] * secret/pki: Don't check whether a certificate is destined to be a CA certificate if sign-verbatim endpoint is used [GH-1250]