From 71fd6e8bc2066553cb234424150b97cfbfdf875d Mon Sep 17 00:00:00 2001 From: Mitch Davis Date: Mon, 27 Mar 2017 21:03:16 -0500 Subject: [PATCH 1/6] Updating the ldap backend to address issue #2387 --- builtin/credential/ldap/backend.go | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/builtin/credential/ldap/backend.go b/builtin/credential/ldap/backend.go index 85ea2d88b12b..e41d94c13f9a 100644 --- a/builtin/credential/ldap/backend.go +++ b/builtin/credential/ldap/backend.go @@ -122,6 +122,17 @@ func (b *backend) Login(req *logical.Request, username string, password string) return nil, logical.ErrorResponse(fmt.Sprintf("LDAP bind failed: %v", err)), nil } + // We re-bind to the BindDN if it's defined because we assume + // the BindDN should be the one to search, not the user logging in. + if cfg.DiscoverDN || (cfg.BindDN != "" && cfg.BindPassword != "") { + if err := c.Bind(cfg.BindDN, cfg.BindPassword); err != nil { + return nil, logical.ErrorResponse(err.Error()), nil + } + if b.Logger().IsDebug() { + b.Logger().Debug("auth/ldap: Re-Bound to original BindDN") + } + } + userDN, err := b.getUserDN(cfg, c, bindDN) if err != nil { return nil, logical.ErrorResponse(err.Error()), nil @@ -165,7 +176,7 @@ func (b *backend) Login(req *logical.Request, username string, password string) policies = append(policies, group.Policies...) } } - if user !=nil && user.Policies != nil { + if user != nil && user.Policies != nil { policies = append(policies, user.Policies...) } // Policies from each group may overlap From 7583c60ca13c0618d0f6940bed56933e8d996765 Mon Sep 17 00:00:00 2001 From: Mitch Davis Date: Wed, 5 Apr 2017 14:21:28 -0500 Subject: [PATCH 2/6] Addressing lines 127 and 129 from @shomron's comments --- builtin/credential/ldap/backend.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/builtin/credential/ldap/backend.go b/builtin/credential/ldap/backend.go index e41d94c13f9a..aab402d97928 100644 --- a/builtin/credential/ldap/backend.go +++ b/builtin/credential/ldap/backend.go @@ -124,9 +124,9 @@ func (b *backend) Login(req *logical.Request, username string, password string) // We re-bind to the BindDN if it's defined because we assume // the BindDN should be the one to search, not the user logging in. - if cfg.DiscoverDN || (cfg.BindDN != "" && cfg.BindPassword != "") { + if cfg.BindDN != "" && cfg.BindPassword != "" { if err := c.Bind(cfg.BindDN, cfg.BindPassword); err != nil { - return nil, logical.ErrorResponse(err.Error()), nil + return nil, logical.ErrorResponse("Encountered an error while attempting to bind with the BindDN User: " + err.Error()), nil } if b.Logger().IsDebug() { b.Logger().Debug("auth/ldap: Re-Bound to original BindDN") From ab7f99c03e7d9b9f0a3fa35ac521f014c6638609 Mon Sep 17 00:00:00 2001 From: Mitch Davis Date: Wed, 5 Apr 2017 14:34:25 -0500 Subject: [PATCH 3/6] Changing 'bind' to 're-bind'. --- builtin/credential/ldap/backend.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/credential/ldap/backend.go b/builtin/credential/ldap/backend.go index aab402d97928..a02d8a7ed0d4 100644 --- a/builtin/credential/ldap/backend.go +++ b/builtin/credential/ldap/backend.go @@ -126,7 +126,7 @@ func (b *backend) Login(req *logical.Request, username string, password string) // the BindDN should be the one to search, not the user logging in. if cfg.BindDN != "" && cfg.BindPassword != "" { if err := c.Bind(cfg.BindDN, cfg.BindPassword); err != nil { - return nil, logical.ErrorResponse("Encountered an error while attempting to bind with the BindDN User: " + err.Error()), nil + return nil, logical.ErrorResponse("Encountered an error while attempting to re-bind with the BindDN User: " + err.Error()), nil } if b.Logger().IsDebug() { b.Logger().Debug("auth/ldap: Re-Bound to original BindDN") From ee62b8f2e08fd5bc9198f2e9e4b7a7b5f389463a Mon Sep 17 00:00:00 2001 From: Mitch Davis Date: Wed, 5 Apr 2017 17:17:01 -0500 Subject: [PATCH 4/6] Addressing line 136 of @shomron's comments --- builtin/credential/ldap/backend.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/builtin/credential/ldap/backend.go b/builtin/credential/ldap/backend.go index a02d8a7ed0d4..8c325f19e4f8 100644 --- a/builtin/credential/ldap/backend.go +++ b/builtin/credential/ldap/backend.go @@ -104,13 +104,13 @@ func (b *backend) Login(req *logical.Request, username string, password string) // Clean connection defer c.Close() - bindDN, err := b.getBindDN(cfg, c, username) + userBindDN, err := b.getUserBindDN(cfg, c, username) if err != nil { return nil, logical.ErrorResponse(err.Error()), nil } if b.Logger().IsDebug() { - b.Logger().Debug("auth/ldap: BindDN fetched", "username", username, "binddn", bindDN) + b.Logger().Debug("auth/ldap: BindDN fetched", "username", username, "binddn", userBindDN) } if cfg.DenyNullBind && len(password) == 0 { @@ -118,7 +118,7 @@ func (b *backend) Login(req *logical.Request, username string, password string) } // Try to bind as the login user. This is where the actual authentication takes place. - if err = c.Bind(bindDN, password); err != nil { + if err = c.Bind(userBindDN, password); err != nil { return nil, logical.ErrorResponse(fmt.Sprintf("LDAP bind failed: %v", err)), nil } @@ -133,7 +133,7 @@ func (b *backend) Login(req *logical.Request, username string, password string) } } - userDN, err := b.getUserDN(cfg, c, bindDN) + userDN, err := b.getUserDN(cfg, c, userBindDN) if err != nil { return nil, logical.ErrorResponse(err.Error()), nil } @@ -229,7 +229,7 @@ func (b *backend) getCN(dn string) string { * 2. If upndomain is set, the user dn is constructed as 'username@upndomain'. See https://msdn.microsoft.com/en-us/library/cc223499.aspx * */ -func (b *backend) getBindDN(cfg *ConfigEntry, c *ldap.Conn, username string) (string, error) { +func (b *backend) getUserBindDN(cfg *ConfigEntry, c *ldap.Conn, username string) (string, error) { bindDN := "" if cfg.DiscoverDN || (cfg.BindDN != "" && cfg.BindPassword != "") { if err := c.Bind(cfg.BindDN, cfg.BindPassword); err != nil { From 286c3b85e263779540463c1f8757993db1fd8ac5 Mon Sep 17 00:00:00 2001 From: Mitch Davis Date: Tue, 11 Apr 2017 09:47:24 -0500 Subject: [PATCH 5/6] Addressing @somron's review requests on formatting and clarity. --- builtin/credential/ldap/backend.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/builtin/credential/ldap/backend.go b/builtin/credential/ldap/backend.go index 8c325f19e4f8..9250fce6c939 100644 --- a/builtin/credential/ldap/backend.go +++ b/builtin/credential/ldap/backend.go @@ -110,7 +110,7 @@ func (b *backend) Login(req *logical.Request, username string, password string) } if b.Logger().IsDebug() { - b.Logger().Debug("auth/ldap: BindDN fetched", "username", username, "binddn", userBindDN) + b.Logger().Debug("auth/ldap: User BindDN fetched", "username", username, "binddn", userBindDN) } if cfg.DenyNullBind && len(password) == 0 { @@ -126,7 +126,7 @@ func (b *backend) Login(req *logical.Request, username string, password string) // the BindDN should be the one to search, not the user logging in. if cfg.BindDN != "" && cfg.BindPassword != "" { if err := c.Bind(cfg.BindDN, cfg.BindPassword); err != nil { - return nil, logical.ErrorResponse("Encountered an error while attempting to re-bind with the BindDN User: " + err.Error()), nil + return nil, logical.ErrorResponse(fmt.Sprintf("Encountered an error while attempting to re-bind with the BindDN User: %s", err.Error())), nil } if b.Logger().IsDebug() { b.Logger().Debug("auth/ldap: Re-Bound to original BindDN") From 9ec9434073d27a1eb123f9d39a82a601dd38e831 Mon Sep 17 00:00:00 2001 From: Mitch Davis Date: Fri, 14 Apr 2017 07:59:22 -0500 Subject: [PATCH 6/6] Updating the docs to add a little more clarity on which user is performing the group search. --- website/source/docs/auth/ldap.html.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/website/source/docs/auth/ldap.html.md b/website/source/docs/auth/ldap.html.md index 7ec754f2a8ba..3912192ba86c 100644 --- a/website/source/docs/auth/ldap.html.md +++ b/website/source/docs/auth/ldap.html.md @@ -122,7 +122,7 @@ There are two alternate methods of resolving the user object used to authenticat #### Binding - Authenticated Search -* `binddn` (string, optional) - Distinguished name of object to bind when performing user search. Example: `cn=vault,ou=Users,dc=example,dc=com` +* `binddn` (string, optional) - Distinguished name of object to bind when performing user and group search. Example: `cn=vault,ou=Users,dc=example,dc=com` * `bindpass` (string, optional) - Password to use along with `binddn` when performing user search. * `userdn` (string, optional) - Base DN under which to perform user search. Example: `ou=Users,dc=example,dc=com` * `userattr` (string, optional) - Attribute on user attribute object matching the username passed when authenticating. Examples: `sAMAccountName`, `cn`, `uid` @@ -146,6 +146,7 @@ Once a user has been authenticated, the LDAP auth backend must know how to resol * `groupdn` (string, required) - LDAP search base to use for group membership search. This can be the root containing either groups or users. Example: `ou=Groups,dc=example,dc=com` * `groupattr` (string, optional) - LDAP attribute to follow on objects returned by `groupfilter` in order to enumerate user group membership. Examples: for groupfilter queries returning _group_ objects, use: `cn`. For queries returning _user_ objects, use: `memberOf`. The default is `cn`. +*Note*: When using _Authenticated Search_ for binding parameters (see above) the distinguished name defined for `binddn` is used for the group search. Otherwise, the authenticating user is used to perform the group search. Use `vault path-help` for more details.