From de00b641dafe0f5f2eeba952dfe0769a49f5e719 Mon Sep 17 00:00:00 2001 From: Aditya Manthramurthy Date: Sat, 24 Jul 2021 11:57:36 -0700 Subject: [PATCH] [LDAP] Support syncing user-group memberships with LDAP service (#12785) When configured in Lookup Bind mode, the server now periodically queries the LDAP IDP service to find changes to a user's group memberships, and saves this info to update the access policies for all temporary and service account credentials belonging to LDAP users. --- cmd/admin-handlers-users.go | 5 + cmd/iam.go | 120 +++++++++++++++++++++--- internal/config/identity/ldap/config.go | 42 ++++++++- 3 files changed, 147 insertions(+), 20 deletions(-) diff --git a/cmd/admin-handlers-users.go b/cmd/admin-handlers-users.go index 229829287..5e3046c2e 100644 --- a/cmd/admin-handlers-users.go +++ b/cmd/admin-handlers-users.go @@ -562,10 +562,12 @@ func (a adminAPIHandlers) AddServiceAccount(w http.ResponseWriter, r *http.Reque } } + var ldapUsername string if globalLDAPConfig.Enabled && targetUser != "" { // If LDAP enabled, service accounts need // to be created only for LDAP users. var err error + ldapUsername = targetUser targetUser, targetGroups, err = globalLDAPConfig.LookupUserDN(targetUser) if err != nil { writeErrorResponseJSON(ctx, w, toAdminAPIErr(ctx, err), r.URL) @@ -604,6 +606,9 @@ func (a adminAPIHandlers) AddServiceAccount(w http.ResponseWriter, r *http.Reque secretKey: createReq.SecretKey, sessionPolicy: sp, } + if ldapUsername != "" { + opts.ldapUsername = ldapUsername + } newCred, err := globalIAMSys.NewServiceAccount(ctx, targetUser, targetGroups, opts) if err != nil { writeErrorResponseJSON(ctx, w, toAdminAPIErr(ctx, err), r.URL) diff --git a/cmd/iam.go b/cmd/iam.go index 54ef43c1b..525242131 100644 --- a/cmd/iam.go +++ b/cmd/iam.go @@ -664,6 +664,7 @@ func (sys *IAMSys) Init(ctx context.Context, objAPI ObjectLayer) { for { time.Sleep(globalRefreshIAMInterval) sys.purgeExpiredCredentialsForLDAP(ctx) + sys.updateGroupMembershipsForLDAP(ctx) } }() } @@ -1165,6 +1166,9 @@ type newServiceAccountOpts struct { sessionPolicy *iampolicy.Policy accessKey string secretKey string + + // LDAP username + ldapUsername string } // NewServiceAccount - create a new service account @@ -1222,6 +1226,11 @@ func (sys *IAMSys) NewServiceAccount(ctx context.Context, parentUser string, gro m[iamPolicyClaimNameSA()] = "inherited-policy" } + // For LDAP service account, save the ldap username in the metadata. + if opts.ldapUsername != "" { + m[ldapUserN] = opts.ldapUsername + } + var ( cred auth.Credentials ) @@ -1597,7 +1606,7 @@ func (sys *IAMSys) purgeExpiredCredentialsForLDAP(ctx context.Context) { } sys.store.unlock() - expiredUsers, err := globalLDAPConfig.GetNonExistentUserDNS(parentUsers) + expiredUsers, err := globalLDAPConfig.GetNonExistentUserDistNames(parentUsers) if err != nil { // Log and return on error - perhaps it'll work the next time. logger.LogIf(GlobalContext, err) @@ -1627,6 +1636,92 @@ func (sys *IAMSys) purgeExpiredCredentialsForLDAP(ctx context.Context) { sys.store.unlock() } +// updateGroupMembershipsForLDAP - updates the list of groups associated with the credential. +func (sys *IAMSys) updateGroupMembershipsForLDAP(ctx context.Context) { + // 1. Collect all LDAP users with active creds. + sys.store.lock() + // List of unique LDAP (parent) user DNs that have active creds + parentUsers := make([]string, 0, len(sys.iamUsersMap)) + // Map of LDAP user to list of active credential objects + parentUserToCredsMap := make(map[string][]auth.Credentials, len(sys.iamUsersMap)) + // DN to ldap username mapping for each LDAP user + parentUserToLDAPUsernameMap := make(map[string]string, len(sys.iamUsersMap)) + for _, cred := range sys.iamUsersMap { + if cred.IsServiceAccount() || cred.IsTemp() { + if globalLDAPConfig.IsLDAPUserDN(cred.ParentUser) { + // Check if this is the first time we are + // encountering this LDAP user. + if _, ok := parentUserToCredsMap[cred.ParentUser]; !ok { + // Try to find the ldapUsername for this + // parentUser by extracting JWT claims + jwtClaims, err := auth.ExtractClaims(cred.SessionToken, globalActiveCred.SecretKey) + if err != nil { + // skip this cred - session token seems + // invalid + continue + } + ldapUsername, ok := jwtClaims.Lookup(ldapUserN) + if !ok { + // skip this cred - we dont have the + // username info needed + continue + } + + // Collect each new cred.ParentUser into parentUsers + parentUsers = append(parentUsers, cred.ParentUser) + + // Update the ldapUsernameMap + parentUserToLDAPUsernameMap[cred.ParentUser] = ldapUsername + } + parentUserToCredsMap[cred.ParentUser] = append(parentUserToCredsMap[cred.ParentUser], cred) + } + } + } + sys.store.unlock() + + // 2. Query LDAP server for groups of the LDAP users collected. + updatedGroups, err := globalLDAPConfig.LookupGroupMemberships(parentUsers, parentUserToLDAPUsernameMap) + if err != nil { + // Log and return on error - perhaps it'll work the next time. + logger.LogIf(GlobalContext, err) + return + } + + // 3. Update creds for those users whose groups are changed + sys.store.lock() + defer sys.store.unlock() + for _, parentUser := range parentUsers { + currGroupsSet := updatedGroups[parentUser] + currGroups := currGroupsSet.ToSlice() + for _, cred := range parentUserToCredsMap[parentUser] { + gSet := set.CreateStringSet(cred.Groups...) + if gSet.Equals(currGroupsSet) { + // No change to groups memberships for this + // credential. + continue + } + + cred.Groups = currGroups + userType := regUser + if cred.IsServiceAccount() { + userType = svcUser + } else if cred.IsTemp() { + userType = stsUser + } + // Overwrite the user identity here. As store should be + // atomic, it shouldn't cause any corruption. + if err := sys.store.saveUserIdentity(ctx, cred.AccessKey, userType, newUserIdentity(cred)); err != nil { + // Log and continue error - perhaps it'll work the next time. + logger.LogIf(GlobalContext, err) + continue + } + // If we wrote the updated creds to IAM storage, we can + // update the in memory map. + sys.iamUsersMap[cred.AccessKey] = cred + } + } +} + // GetUser - get user credentials func (sys *IAMSys) GetUser(accessKey string) (cred auth.Credentials, ok bool) { if !sys.Initialized() { @@ -2205,20 +2300,15 @@ func (sys *IAMSys) IsAllowedServiceAccount(args iampolicy.Args, parentUser strin // IsAllowedLDAPSTS - checks for LDAP specific claims and values func (sys *IAMSys) IsAllowedLDAPSTS(args iampolicy.Args, parentUser string) bool { - parentInClaimIface, ok := args.Claims[ldapUser] - if ok { - parentInClaim, ok := parentInClaimIface.(string) - if !ok { - // ldap parentInClaim name is not a string reject it. - return false - } - - if parentInClaim != parentUser { - // ldap claim has been modified maliciously reject it. - return false - } - } else { - // no ldap parentInClaim claim present reject it. + // parentUser value must match the ldap user in the claim. + if parentInClaimIface, ok := args.Claims[ldapUser]; !ok { + // no ldapUser claim present reject it. + return false + } else if parentInClaim, ok := parentInClaimIface.(string); !ok { + // not the right type, reject it. + return false + } else if parentInClaim != parentUser { + // ldap claim has been modified maliciously reject it. return false } diff --git a/internal/config/identity/ldap/config.go b/internal/config/identity/ldap/config.go index 1ae8a47a9..0b2783a4b 100644 --- a/internal/config/identity/ldap/config.go +++ b/internal/config/identity/ldap/config.go @@ -29,6 +29,7 @@ import ( "time" ldap "github.com/go-ldap/ldap/v3" + "github.com/minio/minio-go/v7/pkg/set" "github.com/minio/minio/internal/auth" "github.com/minio/minio/internal/config" "github.com/minio/minio/internal/logger" @@ -296,7 +297,7 @@ func (l *Config) searchForUserGroups(conn *ldap.Conn, username, bindDN string) ( return groups, nil } -// LookupUserDN searches for the full DN ang groups of a given username +// LookupUserDN searches for the full DN and groups of a given username func (l *Config) LookupUserDN(username string) (string, []string, error) { if !l.isUsingLookupBind { return "", nil, errors.New("current lookup mode does not support searching for User DN") @@ -477,9 +478,9 @@ func (l Config) IsLDAPUserDN(user string) bool { return strings.HasSuffix(user, ","+l.UserDNSearchBaseDN) } -// GetNonExistentUserDNS - find user accounts that are no longer present in the -// LDAP server. -func (l *Config) GetNonExistentUserDNS(userDNS []string) ([]string, error) { +// GetNonExistentUserDistNames - find user accounts (DNs) that are no longer +// present in the LDAP server. +func (l *Config) GetNonExistentUserDistNames(userDistNames []string) ([]string, error) { if !l.isUsingLookupBind { return nil, errors.New("current LDAP configuration does not permit looking for expired user accounts") } @@ -496,7 +497,7 @@ func (l *Config) GetNonExistentUserDNS(userDNS []string) ([]string, error) { } nonExistentUsers := []string{} - for _, dn := range userDNS { + for _, dn := range userDistNames { searchRequest := ldap.NewSearchRequest( dn, ldap.ScopeBaseObject, ldap.NeverDerefAliases, 0, 0, false, @@ -523,6 +524,37 @@ func (l *Config) GetNonExistentUserDNS(userDNS []string) ([]string, error) { return nonExistentUsers, nil } +// LookupGroupMemberships - for each DN finds the set of LDAP groups they are a +// member of. +func (l *Config) LookupGroupMemberships(userDistNames []string, userDNToUsernameMap map[string]string) (map[string]set.StringSet, error) { + if !l.isUsingLookupBind { + return nil, errors.New("current LDAP configuration does not permit this lookup") + } + + conn, err := l.Connect() + if err != nil { + return nil, err + } + defer conn.Close() + + // Bind to the lookup user account + if err = l.lookupBind(conn); err != nil { + return nil, err + } + + res := make(map[string]set.StringSet, len(userDistNames)) + for _, userDistName := range userDistNames { + username := userDNToUsernameMap[userDistName] + groups, err := l.searchForUserGroups(conn, username, userDistName) + if err != nil { + return nil, err + } + res[userDistName] = set.CreateStringSet(groups...) + } + + return res, nil +} + // EnabledWithLookupBind - checks if ldap IDP is enabled in lookup bind mode. func (l Config) EnabledWithLookupBind() bool { return l.Enabled && l.isUsingLookupBind