Fix support for multiple LDAP user formats (#11276)

Fixes support for using multiple base DNs for user search in the LDAP directory
allowing users from different subtrees in the LDAP hierarchy to request
credentials.

- The username in the produced credentials is now the full DN of the LDAP user
to disambiguate users in different base DNs.

cmd/config/identity/ldap/config.go

@@ -47,11 +47,9 @@ type Config struct {
 	// Format string for usernames
 	UsernameFormat  string   `json:"usernameFormat"`
 	UsernameFormats []string `json:"-"`
-	UsernameSearchFilter  string   `json:"-"`
-	UsernameSearchBaseDNS []string `json:"-"`
 
-	GroupSearchBaseDN  string   `json:"groupSearchBaseDN"`
+	GroupSearchBaseDistName  string   `json:"groupSearchBaseDN"`
-	GroupSearchBaseDNS []string `json:"-"`
+	GroupSearchBaseDistNames []string `json:"-"`
 	GroupSearchFilter        string   `json:"groupSearchFilter"`
 	GroupNameAttribute       string   `json:"groupNameAttribute"`
 
@@ -82,8 +80,6 @@ const (
 	EnvServerInsecure     = "MINIO_IDENTITY_LDAP_SERVER_INSECURE"
 	EnvServerStartTLS     = "MINIO_IDENTITY_LDAP_SERVER_STARTTLS"
 	EnvUsernameFormat     = "MINIO_IDENTITY_LDAP_USERNAME_FORMAT"
-	EnvUsernameSearchFilter = "MINIO_IDENTITY_LDAP_USERNAME_SEARCH_FILTER"
-	EnvUsernameSearchBaseDN = "MINIO_IDENTITY_LDAP_USERNAME_SEARCH_BASE_DN"
 	EnvGroupSearchFilter  = "MINIO_IDENTITY_LDAP_GROUP_SEARCH_FILTER"
 	EnvGroupNameAttribute = "MINIO_IDENTITY_LDAP_GROUP_NAME_ATTRIBUTE"
 	EnvGroupSearchBaseDN  = "MINIO_IDENTITY_LDAP_GROUP_SEARCH_BASE_DN"
@@ -157,96 +153,82 @@ func getGroups(conn *ldap.Conn, sreq *ldap.SearchRequest) ([]string, error) {
 	return groups, nil
 }
 
-func (l *Config) bind(conn *ldap.Conn, username, password string) ([]string, error) {
+// bind - Iterates over all given username formats and expects that only one
-	var bindDNS = make([]string, len(l.UsernameFormats))
+// will succeed if the credentials are valid. The succeeding bindDN is returned
+// or an error.
+//
+// In the rare case that multiple username formats succeed, implying that two
+// (or more) distinct users in the LDAP directory have the same username and
+// password, we return an error as we cannot identify the account intended by
+// the user.
+func (l *Config) bind(conn *ldap.Conn, username, password string) (string, error) {
+	var bindDistNames []string
+	var errs = make([]error, len(l.UsernameFormats))
+	var successCount = 0
 	for i, usernameFormat := range l.UsernameFormats {
 		bindDN := fmt.Sprintf(usernameFormat, username)
 		// Bind with user credentials to validate the password
-		if err := conn.Bind(bindDN, password); err != nil {
+		errs[i] = conn.Bind(bindDN, password)
-			return nil, err
+		if errs[i] == nil {
+			bindDistNames = append(bindDistNames, bindDN)
+			successCount++
 		}
-		bindDNS[i] = bindDN
 	}
-	return bindDNS, nil
+	if successCount == 0 {
+		var errStrings []string = []string{"All username formats failed with: "}
+		for _, err := range errs {
+			if err != nil {
+				errStrings = append(errStrings, err.Error())
+			}
+		}
+		outErr := strings.Join(errStrings, "; ")
+		return "", errors.New(outErr)
+	}
+	if successCount > 1 {
+		successDistNames := strings.Join(bindDistNames, ", ")
+		errMsg := fmt.Sprintf("Multiple username formats succeeded - ambiguous user login (succeeded for: %s)", successDistNames)
+		return "", errors.New(errMsg)
+	}
+	return bindDistNames[0], nil
 }
 
-var standardAttributes = []string{
+// Bind - binds to ldap, searches LDAP and returns the distinguished name of the
-	"givenName",
+// user and the list of groups.
-	"sn",
+func (l *Config) Bind(username, password string) (string, []string, error) {
-	"cn",
-	"memberOf",
-	"email",
-}
-
-// Bind - binds to ldap, searches LDAP and returns list of groups.
-func (l *Config) Bind(username, password string) ([]string, error) {
 	conn, err := l.Connect()
 	if err != nil {
-		return nil, err
+		return "", nil, err
 	}
 	defer conn.Close()
 
-	bindDNS, err := l.bind(conn, username, password)
+	bindDN, err := l.bind(conn, username, password)
 	if err != nil {
-		return nil, err
+		return "", nil, err
 	}
 
 	var groups []string
-	if l.UsernameSearchFilter != "" {
-		for _, userSearchBase := range l.UsernameSearchBaseDNS {
-			filter := strings.Replace(l.UsernameSearchFilter, "%s",
-				ldap.EscapeFilter(username), -1)
-
-			searchRequest := ldap.NewSearchRequest(
-				userSearchBase,
-				ldap.ScopeWholeSubtree, ldap.NeverDerefAliases, 0, 0, false,
-				filter,
-				standardAttributes,
-				nil,
-			)
-
-			groups, err = getGroups(conn, searchRequest)
-			if err != nil {
-				return nil, err
-			}
-		}
-	}
-
 	if l.GroupSearchFilter != "" {
-		for _, groupSearchBase := range l.GroupSearchBaseDNS {
+		for _, groupSearchBase := range l.GroupSearchBaseDistNames {
-			var filters []string
+			filter := strings.Replace(l.GroupSearchFilter, "%s", ldap.EscapeFilter(bindDN), -1)
-			if l.GroupNameAttribute == "" {
-				filters = []string{strings.Replace(l.GroupSearchFilter, "%s",
-					ldap.EscapeFilter(username), -1)}
-			} else {
-				// With group name attribute specified, make sure to
-				// include search queries for CN distinguished name
-				for _, bindDN := range bindDNS {
-					filters = append(filters, strings.Replace(l.GroupSearchFilter, "%s",
-						ldap.EscapeFilter(bindDN), -1))
-				}
-			}
-			for _, filter := range filters {
 			searchRequest := ldap.NewSearchRequest(
 				groupSearchBase,
 				ldap.ScopeWholeSubtree, ldap.NeverDerefAliases, 0, 0, false,
 				filter,
-					standardAttributes,
+				[]string{l.GroupNameAttribute},
 				nil,
 			)
 
 			var newGroups []string
 			newGroups, err = getGroups(conn, searchRequest)
 			if err != nil {
-					return nil, err
+				return "", nil, err
 			}
 
 			groups = append(groups, newGroups...)
 		}
 	}
-	}
 
-	return groups, nil
+	return bindDN, groups, nil
 }
 
 // Connect connect to ldap server.
@@ -343,17 +325,6 @@ func Lookup(kvs config.KVS, rootCAs *x509.CertPool) (l Config, err error) {
 		return l, fmt.Errorf("'%s' cannot be empty and must have a value", UsernameFormat)
 	}
 
-	if v := env.Get(EnvUsernameSearchFilter, kvs.Get(UsernameSearchFilter)); v != "" {
-		if !strings.Contains(v, "%s") {
-			return l, errors.New("LDAP username search filter doesn't have '%s' substitution")
-		}
-		l.UsernameSearchFilter = v
-	}
-
-	if v := env.Get(EnvUsernameSearchBaseDN, kvs.Get(UsernameSearchBaseDN)); v != "" {
-		l.UsernameSearchBaseDNS = strings.Split(v, dnDelimiter)
-	}
-
 	grpSearchFilter := env.Get(EnvGroupSearchFilter, kvs.Get(GroupSearchFilter))
 	grpSearchNameAttr := env.Get(EnvGroupNameAttribute, kvs.Get(GroupNameAttribute))
 	grpSearchBaseDN := env.Get(EnvGroupSearchBaseDN, kvs.Get(GroupSearchBaseDN))
@@ -370,7 +341,7 @@ func Lookup(kvs config.KVS, rootCAs *x509.CertPool) (l Config, err error) {
 	if allSet {
 		l.GroupSearchFilter = grpSearchFilter
 		l.GroupNameAttribute = grpSearchNameAttr
-		l.GroupSearchBaseDNS = strings.Split(grpSearchBaseDN, dnDelimiter)
+		l.GroupSearchBaseDistNames = strings.Split(l.GroupSearchBaseDistName, dnDelimiter)
 	}
 
 	l.rootCAs = rootCAs

cmd/config/identity/ldap/legacy.go

@@ -47,7 +47,7 @@ func SetIdentityLDAP(s config.Config, ldapArgs Config) {
 		},
 		config.KV{
 			Key:   GroupSearchBaseDN,
-			Value: ldapArgs.GroupSearchBaseDN,
+			Value: ldapArgs.GroupSearchBaseDistName,
 		},
 	}
 }

cmd/sts-handlers.go

@@ -490,9 +490,9 @@ func (sts *stsAPIHandlers) AssumeRoleWithLDAPIdentity(w http.ResponseWriter, r *
 		}
 	}
 
-	groups, err := globalLDAPConfig.Bind(ldapUsername, ldapPassword)
+	ldapUserDN, groups, err := globalLDAPConfig.Bind(ldapUsername, ldapPassword)
 	if err != nil {
-		err = fmt.Errorf("LDAP server connection failure: %w", err)
+		err = fmt.Errorf("LDAP server error: %w", err)
 		writeSTSErrorResponse(ctx, w, true, ErrSTSInvalidParameterValue, err)
 		return
 	}
@@ -500,7 +500,7 @@ func (sts *stsAPIHandlers) AssumeRoleWithLDAPIdentity(w http.ResponseWriter, r *
 	expiryDur := globalLDAPConfig.GetExpiryDuration()
 	m := map[string]interface{}{
 		expClaim: UTCNow().Add(expiryDur).Unix(),
-		ldapUser: ldapUsername,
+		ldapUser: ldapUserDN,
 	}
 
 	if len(sessionPolicyStr) > 0 {
@@ -516,7 +516,7 @@ func (sts *stsAPIHandlers) AssumeRoleWithLDAPIdentity(w http.ResponseWriter, r *
 
 	// Set the parent of the temporary access key, this is useful
 	// in obtaining service accounts by this cred.
-	cred.ParentUser = ldapUsername
+	cred.ParentUser = ldapUserDN
 
 	// Set this value to LDAP groups, LDAP user can be part
 	// of large number of groups

docs/sts/ldap.md

@@ -47,10 +47,8 @@ identity_ldap  enable LDAP SSO support
 ARGS:
 MINIO_IDENTITY_LDAP_SERVER_ADDR*             (address)   AD/LDAP server address e.g. "myldapserver.com:636"
 MINIO_IDENTITY_LDAP_USERNAME_FORMAT*         (list)      ";" separated list of username bind DNs e.g. "uid=%s,cn=accounts,dc=myldapserver,dc=com"
-MINIO_IDENTITY_LDAP_USERNAME_SEARCH_FILTER*  (string)    user search filter, for example "(cn=%s)" or "(sAMAccountName=%s)" or "(uid=%s)"
 MINIO_IDENTITY_LDAP_GROUP_SEARCH_FILTER*     (string)    search filter for groups e.g. "(&(objectclass=groupOfNames)(memberUid=%s))"
 MINIO_IDENTITY_LDAP_GROUP_SEARCH_BASE_DN*    (list)      ";" separated list of group search base DNs e.g. "dc=myldapserver,dc=com"
-MINIO_IDENTITY_LDAP_USERNAME_SEARCH_BASE_DN  (list)      ";" separated list of username search DNs
 MINIO_IDENTITY_LDAP_GROUP_NAME_ATTRIBUTE     (string)    search attribute for group name e.g. "cn"
 MINIO_IDENTITY_LDAP_STS_EXPIRY               (duration)  temporary credentials validity duration in s,m,h,d. Default is "1h"
 MINIO_IDENTITY_LDAP_TLS_SKIP_VERIFY          (on|off)    trust server TLS without verification, defaults to "off" (verify)