From 466e95bb59a4b38b44670d10449da6445afd4efb Mon Sep 17 00:00:00 2001 From: Aditya Manthramurthy Date: Wed, 10 Feb 2021 16:52:49 -0800 Subject: [PATCH] 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 | 24 ++++++------------------ cmd/config/identity/ldap/help.go | 6 ------ cmd/config/identity/ldap/legacy.go | 4 ---- cmd/sts-handlers.go | 22 ++++++++++++++++++++-- docs/sts/ldap.md | 12 ++++-------- 5 files changed, 30 insertions(+), 38 deletions(-) diff --git a/cmd/config/identity/ldap/config.go b/cmd/config/identity/ldap/config.go index 7bf1e4d9d..7c758261e 100644 --- a/cmd/config/identity/ldap/config.go +++ b/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 != "" { - if grpSearchNameAttr == "" || grpSearchBaseDN == "" { - return l, errors.New("All group related parameters must be set") - } - allSet = true + if (grpSearchFilter != "" && grpSearchBaseDN == "") || (grpSearchFilter == "" && grpSearchBaseDN != "") { + return l, errors.New("All group related parameters must be set") } - if allSet { + if grpSearchFilter != "" { l.GroupSearchFilter = grpSearchFilter - l.GroupNameAttribute = grpSearchNameAttr l.GroupSearchBaseDistName = grpSearchBaseDN l.GroupSearchBaseDistNames = strings.Split(l.GroupSearchBaseDistName, dnDelimiter) } diff --git a/cmd/config/identity/ldap/help.go b/cmd/config/identity/ldap/help.go index d89ff764d..1b8bde279 100644 --- a/cmd/config/identity/ldap/help.go +++ b/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"`, diff --git a/cmd/config/identity/ldap/legacy.go b/cmd/config/identity/ldap/legacy.go index 8f4bd90ba..52362d93f 100644 --- a/cmd/config/identity/ldap/legacy.go +++ b/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, diff --git a/cmd/sts-handlers.go b/cmd/sts-handlers.go index 60264f7de..abbdddc07 100644 --- a/cmd/sts-handlers.go +++ b/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 diff --git a/docs/sts/ldap.md b/docs/sts/ldap.md index 8cdcde421..27750ee59 100644 --- a/docs/sts/ldap.md +++ b/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):