Skip to content

Commit

Permalink
Return generic messages if pre-login ldap operations fail (#4700)
Browse files Browse the repository at this point in the history
This avoids leaking any information about valid usernames.
  • Loading branch information
jefferai authored Jun 5, 2018
1 parent d6efc1c commit 2b374b2
Showing 1 changed file with 16 additions and 7 deletions.
23 changes: 16 additions & 7 deletions builtin/credential/ldap/backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,10 @@ func (b *backend) Login(ctx context.Context, req *logical.Request, username stri
return nil, logical.ErrorResponse("ldap backend not configured"), nil, nil
}

if cfg.DenyNullBind && len(password) == 0 {
return nil, logical.ErrorResponse("password cannot be of zero length when passwordless binds are being denied"), nil, nil
}

ldapClient := ldaputil.Client{
Logger: b.Logger(),
LDAP: ldaputil.NewLDAP(),
Expand All @@ -86,32 +90,37 @@ func (b *backend) Login(ctx context.Context, req *logical.Request, username stri

userBindDN, err := ldapClient.GetUserBindDN(cfg, c, username)
if err != nil {
return nil, logical.ErrorResponse(err.Error()), nil, nil
if b.Logger().IsDebug() {
b.Logger().Debug("error getting user bind DN", "error", err)
}
return nil, logical.ErrorResponse("ldap operation failed"), nil, nil
}

if b.Logger().IsDebug() {
b.Logger().Debug("user binddn fetched", "username", username, "binddn", userBindDN)
}

if cfg.DenyNullBind && len(password) == 0 {
return nil, logical.ErrorResponse("password cannot be of zero length when passwordless binds are being denied"), nil, nil
}

// Try to bind as the login user. This is where the actual authentication takes place.
if len(password) > 0 {
err = c.Bind(userBindDN, password)
} else {
err = c.UnauthenticatedBind(userBindDN)
}
if err != nil {
return nil, logical.ErrorResponse(fmt.Sprintf("LDAP bind failed: %v", err)), nil, nil
if b.Logger().IsDebug() {
b.Logger().Debug("ldap bind failed", "error", err)
}
return nil, logical.ErrorResponse("ldap operation failed"), nil, 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.BindDN != "" && cfg.BindPassword != "" {
if err := c.Bind(cfg.BindDN, cfg.BindPassword); err != nil {
return nil, logical.ErrorResponse(fmt.Sprintf("Encountered an error while attempting to re-bind with the BindDN User: %s", err.Error())), nil, nil
if b.Logger().IsDebug() {
b.Logger().Debug("error while attempting to re-bind with the BindDN User", "error", err)
}
return nil, logical.ErrorResponse("ldap operation failed"), nil, nil
}
if b.Logger().IsDebug() {
b.Logger().Debug("re-bound to original binddn")
Expand Down

0 comments on commit 2b374b2

Please sign in to comment.