Return group DN instead of group name in LDAP STS (#11501)

- Additionally, check if the user or their groups has a policy attached during
the STS call.

- Remove the group name attribute configuration value.

cmd/config/identity/ldap/config.go

@@ -58,7 +58,6 @@ type Config struct {
 	GroupSearchBaseDistName  string   `json:"groupSearchBaseDN"`
 	GroupSearchBaseDistNames []string `json:"-"`
 	GroupSearchFilter        string   `json:"groupSearchFilter"`
-	GroupNameAttribute       string   `json:"groupNameAttribute"`
 
 	// Lookup bind LDAP service account
 	LookupBindDN       string `json:"lookupBindDN"`
@@ -82,7 +81,6 @@ const (
 	UserDNSearchFilter = "user_dn_search_filter"
 	UsernameFormat     = "username_format"
 	GroupSearchFilter  = "group_search_filter"
-	GroupNameAttribute = "group_name_attribute"
 	GroupSearchBaseDN  = "group_search_base_dn"
 	TLSSkipVerify      = "tls_skip_verify"
 	ServerInsecure     = "server_insecure"
@@ -97,7 +95,6 @@ const (
 	EnvUserDNSearchBaseDN = "MINIO_IDENTITY_LDAP_USER_DN_SEARCH_BASE_DN"
 	EnvUserDNSearchFilter = "MINIO_IDENTITY_LDAP_USER_DN_SEARCH_FILTER"
 	EnvGroupSearchFilter  = "MINIO_IDENTITY_LDAP_GROUP_SEARCH_FILTER"
-	EnvGroupNameAttribute = "MINIO_IDENTITY_LDAP_GROUP_NAME_ATTRIBUTE"
 	EnvGroupSearchBaseDN  = "MINIO_IDENTITY_LDAP_GROUP_SEARCH_BASE_DN"
 	EnvLookupBindDN       = "MINIO_IDENTITY_LDAP_LOOKUP_BIND_DN"
 	EnvLookupBindPassword = "MINIO_IDENTITY_LDAP_LOOKUP_BIND_PASSWORD"
@@ -106,6 +103,7 @@ const (
 var removedKeys = []string{
 	"username_search_filter",
 	"username_search_base_dn",
+	"group_name_attribute",
 }
 
 // DefaultKVS - default config for LDAP config
@@ -131,10 +129,6 @@ var (
 			Key:   GroupSearchFilter,
 			Value: "",
 		},
-		config.KV{
-			Key:   GroupNameAttribute,
-			Value: "",
-		},
 		config.KV{
 			Key:   GroupSearchBaseDN,
 			Value: "",
@@ -180,7 +174,7 @@ func getGroups(conn *ldap.Conn, sreq *ldap.SearchRequest) ([]string, error) {
 	for _, entry := range sres.Entries {
 		// We only queried one attribute,
 		// so we only look up the first one.
-		groups = append(groups, entry.Attributes[0].Values...)
+		groups = append(groups, entry.DN)
 	}
 	return groups, nil
 }
@@ -312,7 +306,7 @@ func (l *Config) Bind(username, password string) (string, []string, error) {
 				groupSearchBase,
 				ldap.ScopeWholeSubtree, ldap.NeverDerefAliases, 0, 0, false,
 				filter,
-				[]string{l.GroupNameAttribute},
+				nil,
 				nil,
 			)
 
@@ -463,21 +457,15 @@ func Lookup(kvs config.KVS, rootCAs *x509.CertPool) (l Config, err error) {
 
 	// Group search params configuration
 	grpSearchFilter := env.Get(EnvGroupSearchFilter, kvs.Get(GroupSearchFilter))
-	grpSearchNameAttr := env.Get(EnvGroupNameAttribute, kvs.Get(GroupNameAttribute))
 	grpSearchBaseDN := env.Get(EnvGroupSearchBaseDN, kvs.Get(GroupSearchBaseDN))
 
 	// Either all group params must be set or none must be set.
-	var allSet bool
+	if (grpSearchFilter != "" && grpSearchBaseDN == "") || (grpSearchFilter == "" && grpSearchBaseDN != "") {
-	if grpSearchFilter != "" {
-		if grpSearchNameAttr == "" || grpSearchBaseDN == "" {
 		return l, errors.New("All group related parameters must be set")
 	}
-		allSet = true
-	}
 
-	if allSet {
+	if grpSearchFilter != "" {
 		l.GroupSearchFilter = grpSearchFilter
-		l.GroupNameAttribute = grpSearchNameAttr
 		l.GroupSearchBaseDistName = grpSearchBaseDN
 		l.GroupSearchBaseDistNames = strings.Split(l.GroupSearchBaseDistName, dnDelimiter)
 	}

cmd/config/identity/ldap/help.go

@@ -68,12 +68,6 @@ var (
 			Optional:    true,
 			Type:        "string",
 		},
-		config.HelpKV{
-			Key:         GroupNameAttribute,
-			Description: `search attribute for group name e.g. "cn"`,
-			Optional:    true,
-			Type:        "string",
-		},
 		config.HelpKV{
 			Key:         GroupSearchBaseDN,
 			Description: `";" separated list of group search base DNs e.g. "dc=myldapserver,dc=com"`,

cmd/config/identity/ldap/legacy.go

@@ -41,10 +41,6 @@ func SetIdentityLDAP(s config.Config, ldapArgs Config) {
 			Key:   GroupSearchFilter,
 			Value: ldapArgs.GroupSearchFilter,
 		},
-		config.KV{
-			Key:   GroupNameAttribute,
-			Value: ldapArgs.GroupNameAttribute,
-		},
 		config.KV{
 			Key:   GroupSearchBaseDN,
 			Value: ldapArgs.GroupSearchBaseDistName,

cmd/sts-handlers.go

@@ -490,13 +490,31 @@ func (sts *stsAPIHandlers) AssumeRoleWithLDAPIdentity(w http.ResponseWriter, r *
 		}
 	}
 
-	ldapUserDN, groups, err := globalLDAPConfig.Bind(ldapUsername, ldapPassword)
+	ldapUserDN, groupDistNames, err := globalLDAPConfig.Bind(ldapUsername, ldapPassword)
 	if err != nil {
 		err = fmt.Errorf("LDAP server error: %w", err)
 		writeSTSErrorResponse(ctx, w, true, ErrSTSInvalidParameterValue, err)
 		return
 	}
 
+	// Check if this user or their groups have a policy applied.
+	globalIAMSys.Lock()
+	found := false
+	if _, ok := globalIAMSys.iamUserPolicyMap[ldapUserDN]; ok {
+		found = true
+	}
+	for _, groupDistName := range groupDistNames {
+		if _, ok := globalIAMSys.iamGroupPolicyMap[groupDistName]; ok {
+			found = true
+			break
+		}
+	}
+	globalIAMSys.Unlock()
+	if !found {
+		writeSTSErrorResponse(ctx, w, true, ErrSTSInvalidParameterValue, fmt.Errorf("expecting a policy to be set for user `%s` or one of their groups: `%s` - rejecting this request", ldapUserDN, strings.Join(groupDistNames, "`,`")))
+		return
+	}
+
 	expiryDur := globalLDAPConfig.GetExpiryDuration()
 	m := map[string]interface{}{
 		expClaim: UTCNow().Add(expiryDur).Unix(),
@@ -520,7 +538,7 @@ func (sts *stsAPIHandlers) AssumeRoleWithLDAPIdentity(w http.ResponseWriter, r *
 
 	// Set this value to LDAP groups, LDAP user can be part
 	// of large number of groups
-	cred.Groups = groups
+	cred.Groups = groupDistNames
 
 	// Set the newly generated credentials, policyName is empty on purpose
 	// LDAP policies are applied automatically using their ldapUser, ldapGroups

docs/sts/ldap.md

@@ -57,7 +57,6 @@ MINIO_IDENTITY_LDAP_USER_DN_SEARCH_BASE_DN  (string)    Base LDAP DN to search f
 MINIO_IDENTITY_LDAP_USER_DN_SEARCH_FILTER   (string)    Search filter to lookup user DN
 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_GROUP_SEARCH_FILTER     (string)    search filter for groups e.g. "(&(objectclass=groupOfNames)(memberUid=%s))"
-MINIO_IDENTITY_LDAP_GROUP_NAME_ATTRIBUTE    (string)    search attribute for group name e.g. "cn"
 MINIO_IDENTITY_LDAP_GROUP_SEARCH_BASE_DN    (list)      ";" separated list of group search base DNs e.g. "dc=myldapserver,dc=com"
 MINIO_IDENTITY_LDAP_TLS_SKIP_VERIFY         (on|off)    trust server TLS without verification, defaults to "off" (verify)
 MINIO_IDENTITY_LDAP_SERVER_INSECURE         (on|off)    allow plain text connection to AD/LDAP server, defaults to "off"
@@ -104,11 +103,10 @@ MinIO can be configured to find the groups of a user from AD/LDAP by specifying
 
 ```
 MINIO_IDENTITY_LDAP_GROUP_SEARCH_FILTER     (string)    search filter for groups e.g. "(&(objectclass=groupOfNames)(memberUid=%s))"
-MINIO_IDENTITY_LDAP_GROUP_NAME_ATTRIBUTE    (string)    search attribute for group name e.g. "cn"
 MINIO_IDENTITY_LDAP_GROUP_SEARCH_BASE_DN    (list)      ";" separated list of group search base DNs e.g. "dc=myldapserver,dc=com"
 ```
 
-When a user logs in via the STS API, the MinIO server queries the AD/LDAP server with the given search filter and extracts the given attribute from the search results. These values represent the groups that the user is a member of. On each access MinIO applies the IAM policies attached to these groups in MinIO.
+When a user logs in via the STS API, the MinIO server queries the AD/LDAP server with the given search filter and extracts the DN from the search results. These values represent the groups that the user is a member of. On each access MinIO applies the IAM policies attached to these groups in MinIO.
 
 **MinIO sends LDAP credentials to LDAP server for validation. So we _strongly recommend_ to use MinIO with AD/LDAP server over TLS or StartTLS _only_. Using plain-text connection between MinIO and LDAP server means _credentials can be compromised_ by anyone listening to network traffic.**
 
@@ -119,7 +117,6 @@ export MINIO_IDENTITY_LDAP_SERVER_ADDR=myldapserver.com:636
 export MINIO_IDENTITY_LDAP_USERNAME_FORMAT="uid=%s,cn=accounts,dc=myldapserver,dc=com"
 export MINIO_IDENTITY_LDAP_GROUP_SEARCH_BASE_DN="dc=myldapserver,dc=com"
 export MINIO_IDENTITY_LDAP_GROUP_SEARCH_FILTER="(&(objectclass=groupOfNames)(memberUid=%s)$)"
-export MINIO_IDENTITY_LDAP_GROUP_NAME_ATTRIBUTE=cn
 export MINIO_IDENTITY_LDAP_STS_EXPIRY=60h
 export MINIO_IDENTITY_LDAP_TLS_SKIP_VERIFY=on
 ```
@@ -140,14 +137,14 @@ To define a new policy, you can use the [AWS policy generator](https://awspolicy
 mc admin policy add myminio mypolicy mypolicy.json
 ```
 
-To assign the policy to a user or group, use:
+To assign the policy to a user or group, use the full DN of the user or group:
 
 ```sh
-mc admin policy set myminio mypolicy user=james
+mc admin policy set myminio mypolicy user='uid=james,cn=accounts,dc=myldapserver,dc=com'
 ```
 
 ```sh
-mc admin policy set myminio mypolicy group=bigdatausers
+mc admin policy set myminio mypolicy group='cn=projectx,ou=groups,ou=hwengg,dc=min,dc=io'
 ```
 
 **Please note that when AD/LDAP is configured, MinIO will not support long term users defined internally.** Only AD/LDAP users are allowed. In addition to this, the server will not support operations on users or groups using `mc admin user` or `mc admin group` commands except `mc admin user info` and `mc admin group info` to list set policies for users and groups. This is because users and groups are defined externally in AD/LDAP.
@@ -232,7 +229,6 @@ $ export MINIO_IDENTITY_LDAP_SERVER_ADDR='my.ldap-active-dir-server.com:636'
 $ export MINIO_IDENTITY_LDAP_USERNAME_FORMAT='cn=%s,ou=Users,ou=BUS1,ou=LOB,dc=somedomain,dc=com;cn=%s,ou=Users,ou=BUS2,ou=LOB,dc=somedomain,dc=com'
 $ export MINIO_IDENTITY_LDAP_GROUP_SEARCH_BASE_DN='dc=minioad,dc=local;dc=somedomain,dc=com'
 $ export MINIO_IDENTITY_LDAP_GROUP_SEARCH_FILTER='(&(objectclass=group)(member=%s))'
-$ export MINIO_IDENTITY_LDAP_GROUP_NAME_ATTRIBUTE='cn'
 $ minio server ~/test
 ```
 You can make sure it works appropriately using our [example program](https://raw.githubusercontent.com/minio/minio/master/docs/sts/ldap.go):