From 96ed0991b5fe324f502e2e62118c2a706011f2d9 Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Thu, 11 Jun 2020 14:11:30 -0700 Subject: [PATCH] fix: optimize IAM users load, add fallback (#9809) Bonus fix, load service accounts properly when service accounts were generated with LDAP --- cmd/admin-handlers.go | 17 +++++++-- cmd/iam-etcd-store.go | 51 +++++++++++++-------------- cmd/iam-object-store.go | 45 +++++++++++------------- cmd/iam.go | 78 ++++++++++++++++++++++++++++++++++++----- cmd/typed-errors.go | 5 ++- 5 files changed, 135 insertions(+), 61 deletions(-) diff --git a/cmd/admin-handlers.go b/cmd/admin-handlers.go index 0fba12d09..1aeaa5391 100644 --- a/cmd/admin-handlers.go +++ b/cmd/admin-handlers.go @@ -899,13 +899,26 @@ func toAdminAPIErr(ctx context.Context, err error) APIError { HTTPStatusCode: e.StatusCode, } default: - if errors.Is(err, errConfigNotFound) { + switch { + case errors.Is(err, errConfigNotFound): apiErr = APIError{ Code: "XMinioConfigError", Description: err.Error(), HTTPStatusCode: http.StatusNotFound, } - } else { + case errors.Is(err, errIAMActionNotAllowed): + apiErr = APIError{ + Code: "XMinioIAMActionNotAllowed", + Description: err.Error(), + HTTPStatusCode: http.StatusForbidden, + } + case errors.Is(err, errIAMNotInitialized): + apiErr = APIError{ + Code: "XMinioIAMNotInitialized", + Description: err.Error(), + HTTPStatusCode: http.StatusServiceUnavailable, + } + default: apiErr = errorCodes.ToAPIErrWithErr(toAdminAPIErrCode(ctx, err), err) } } diff --git a/cmd/iam-etcd-store.go b/cmd/iam-etcd-store.go index 8916ae894..4c2f20fbf 100644 --- a/cmd/iam-etcd-store.go +++ b/cmd/iam-etcd-store.go @@ -449,51 +449,56 @@ func (ies *IAMEtcdStore) loadMappedPolicies(ctx context.Context, userType IAMUse func (ies *IAMEtcdStore) loadAll(ctx context.Context, sys *IAMSys) error { iamUsersMap := make(map[string]auth.Credentials) iamGroupsMap := make(map[string]GroupInfo) - iamPolicyDocsMap := make(map[string]iampolicy.Policy) iamUserPolicyMap := make(map[string]MappedPolicy) iamGroupPolicyMap := make(map[string]MappedPolicy) - isMinIOUsersSys := false ies.rlock() - if sys.usersSysType == MinIOUsersSysType { - isMinIOUsersSys = true - } + isMinIOUsersSys := sys.usersSysType == MinIOUsersSysType ies.runlock() - if err := ies.loadPolicyDocs(ctx, iamPolicyDocsMap); err != nil { + ies.lock() + if err := ies.loadPolicyDocs(ctx, sys.iamPolicyDocsMap); err != nil { + ies.unlock() return err } + // Sets default canned policies, if none are set. + setDefaultCannedPolicies(sys.iamPolicyDocsMap) - // load STS temp users - if err := ies.loadUsers(ctx, stsUser, iamUsersMap); err != nil { - return err - } + ies.unlock() if isMinIOUsersSys { - // load long term users if err := ies.loadUsers(ctx, regularUser, iamUsersMap); err != nil { return err } - if err := ies.loadUsers(ctx, srvAccUser, iamUsersMap); err != nil { - return err - } if err := ies.loadGroups(ctx, iamGroupsMap); err != nil { return err } - if err := ies.loadMappedPolicies(ctx, regularUser, false, iamUserPolicyMap); err != nil { - return err - } } - // load STS policy mappings into the same map - if err := ies.loadMappedPolicies(ctx, stsUser, false, iamUserPolicyMap); err != nil { + // load polices mapped to users + if err := ies.loadMappedPolicies(ctx, regularUser, false, iamUserPolicyMap); err != nil { return err } + // load policies mapped to groups if err := ies.loadMappedPolicies(ctx, regularUser, true, iamGroupPolicyMap); err != nil { return err } + if err := ies.loadUsers(ctx, srvAccUser, iamUsersMap); err != nil { + return err + } + + // load STS temp users + if err := ies.loadUsers(ctx, stsUser, iamUsersMap); err != nil { + return err + } + + // load STS policy mappings + if err := ies.loadMappedPolicies(ctx, stsUser, false, iamUserPolicyMap); err != nil { + return err + } + ies.lock() defer ies.Unlock() @@ -506,13 +511,6 @@ func (ies *IAMEtcdStore) loadAll(ctx context.Context, sys *IAMSys) error { sys.iamUsersMap[k] = v } - for k, v := range iamPolicyDocsMap { - sys.iamPolicyDocsMap[k] = v - } - - // Sets default canned policies, if none are set. - setDefaultCannedPolicies(sys.iamPolicyDocsMap) - for k, v := range iamUserPolicyMap { sys.iamUserPolicyMap[k] = v } @@ -535,6 +533,7 @@ func (ies *IAMEtcdStore) loadAll(ctx context.Context, sys *IAMSys) error { } sys.buildUserGroupMemberships() + sys.storeFallback = false return nil } diff --git a/cmd/iam-object-store.go b/cmd/iam-object-store.go index 4ec3e8d32..b27d1ad18 100644 --- a/cmd/iam-object-store.go +++ b/cmd/iam-object-store.go @@ -410,33 +410,27 @@ func (iamOS *IAMObjectStore) loadMappedPolicies(ctx context.Context, userType IA func (iamOS *IAMObjectStore) loadAll(ctx context.Context, sys *IAMSys) error { iamUsersMap := make(map[string]auth.Credentials) iamGroupsMap := make(map[string]GroupInfo) - iamPolicyDocsMap := make(map[string]iampolicy.Policy) iamUserPolicyMap := make(map[string]MappedPolicy) iamGroupPolicyMap := make(map[string]MappedPolicy) - isMinIOUsersSys := false iamOS.rlock() - if sys.usersSysType == MinIOUsersSysType { - isMinIOUsersSys = true - } + isMinIOUsersSys := sys.usersSysType == MinIOUsersSysType iamOS.runlock() - if err := iamOS.loadPolicyDocs(ctx, iamPolicyDocsMap); err != nil { + iamOS.lock() + if err := iamOS.loadPolicyDocs(ctx, sys.iamPolicyDocsMap); err != nil { + iamOS.unlock() return err } + // Sets default canned policies, if none are set. + setDefaultCannedPolicies(sys.iamPolicyDocsMap) - // load STS temp users - if err := iamOS.loadUsers(ctx, stsUser, iamUsersMap); err != nil { - return err - } + iamOS.unlock() if isMinIOUsersSys { if err := iamOS.loadUsers(ctx, regularUser, iamUsersMap); err != nil { return err } - if err := iamOS.loadUsers(ctx, srvAccUser, iamUsersMap); err != nil { - return err - } if err := iamOS.loadGroups(ctx, iamGroupsMap); err != nil { return err } @@ -447,13 +441,22 @@ func (iamOS *IAMObjectStore) loadAll(ctx context.Context, sys *IAMSys) error { return err } - // load STS policy mappings - if err := iamOS.loadMappedPolicies(ctx, stsUser, false, iamUserPolicyMap); err != nil { + // load policies mapped to groups + if err := iamOS.loadMappedPolicies(ctx, regularUser, true, iamGroupPolicyMap); err != nil { return err } - // load policies mapped to groups - if err := iamOS.loadMappedPolicies(ctx, regularUser, true, iamGroupPolicyMap); err != nil { + if err := iamOS.loadUsers(ctx, srvAccUser, iamUsersMap); err != nil { + return err + } + + // load STS temp users + if err := iamOS.loadUsers(ctx, stsUser, iamUsersMap); err != nil { + return err + } + + // load STS policy mappings + if err := iamOS.loadMappedPolicies(ctx, stsUser, false, iamUserPolicyMap); err != nil { return err } @@ -469,13 +472,6 @@ func (iamOS *IAMObjectStore) loadAll(ctx context.Context, sys *IAMSys) error { sys.iamUsersMap[k] = v } - for k, v := range iamPolicyDocsMap { - sys.iamPolicyDocsMap[k] = v - } - - // Sets default canned policies, if none are set. - setDefaultCannedPolicies(sys.iamPolicyDocsMap) - for k, v := range iamUserPolicyMap { sys.iamUserPolicyMap[k] = v } @@ -498,6 +494,7 @@ func (iamOS *IAMObjectStore) loadAll(ctx context.Context, sys *IAMSys) error { } sys.buildUserGroupMemberships() + sys.storeFallback = false return nil } diff --git a/cmd/iam.go b/cmd/iam.go index e52d3bc30..18081ae68 100644 --- a/cmd/iam.go +++ b/cmd/iam.go @@ -216,7 +216,8 @@ type IAMSys struct { iamGroupPolicyMap map[string]MappedPolicy // Persistence layer for IAM subsystem - store IAMStorageAPI + store IAMStorageAPI + storeFallback bool } // IAMUserType represents a user type inside MinIO server @@ -413,7 +414,7 @@ func startBackgroundIAMLoad(ctx context.Context) { go globalIAMSys.Init(ctx, newObjectLayerWithoutSafeModeFn()) } -// Init - initializes config system from iam.json +// Init - initializes config system by reading entries from config/iam func (sys *IAMSys) Init(ctx context.Context, objAPI ObjectLayer) { if objAPI == nil { logger.LogIf(ctx, errServerNotInitialized) @@ -461,7 +462,8 @@ func (sys *IAMSys) Init(ctx context.Context, objAPI ObjectLayer) { // IAM sub-system, make sure that we do not move the above codeblock elsewhere. if err := migrateIAMConfigsEtcdToEncrypted(ctx, globalEtcdClient); err != nil { txnLk.Unlock() - logger.LogIf(ctx, fmt.Errorf("Unable to handle encrypted backend for iam and policies: %w", err)) + logger.LogIf(ctx, fmt.Errorf("Unable to decrypt an encrypted ETCD backend for IAM users and policies: %w", err)) + logger.LogIf(ctx, errors.New("IAM sub-system is partially initialized, some users may not be available")) return } } @@ -471,7 +473,7 @@ func (sys *IAMSys) Init(ctx context.Context, objAPI ObjectLayer) { logger.Info("Waiting for all MinIO IAM sub-system to be initialized.. lock acquired") } - // Migrate IAM configuration + // Migrate IAM configuration, if necessary. if err := sys.doIAMConfigMigration(ctx); err != nil { txnLk.Unlock() if errors.Is(err, errDiskNotFound) || @@ -484,20 +486,25 @@ func (sys *IAMSys) Init(ctx context.Context, objAPI ObjectLayer) { logger.Info("Waiting for all MinIO IAM sub-system to be initialized.. possible cause (%v)", err) continue } - logger.LogIf(ctx, fmt.Errorf("Unable to migration IAM users and policies: %w", err)) + logger.LogIf(ctx, fmt.Errorf("Unable to migrate IAM users and policies to new format: %w", err)) + logger.LogIf(ctx, errors.New("IAM sub-system is partially initialized, some users may not be available")) return } - // Successfully migrated + // Successfully migrated, proceed to load the users. txnLk.Unlock() break } - logger.LogIf(ctx, sys.store.loadAll(ctx, sys)) + err := sys.store.loadAll(ctx, sys) - // Invalidate the old cred after finishing IAM initialization + // Invalidate the old cred always, even upon error to avoid any leakage. globalOldCred = auth.Credentials{} + if err != nil { + logger.LogIf(ctx, fmt.Errorf("Unable to initialize IAM sub-system, some users may not be available %w", err)) + } + go sys.store.watch(ctx, sys) } @@ -582,6 +589,10 @@ func (sys *IAMSys) ListPolicies() (map[string]iampolicy.Policy, error) { sys.store.rlock() defer sys.store.runlock() + if sys.storeFallback { + return nil, errIAMNotInitialized + } + policyDocsMap := make(map[string]iampolicy.Policy, len(sys.iamPolicyDocsMap)) for k, v := range sys.iamPolicyDocsMap { policyDocsMap[k] = v @@ -731,6 +742,10 @@ func (sys *IAMSys) ListUsers() (map[string]madmin.UserInfo, error) { return nil, errIAMActionNotAllowed } + if sys.storeFallback { + return nil, errIAMNotInitialized + } + for k, v := range sys.iamUsersMap { if !v.IsTemp() && !v.IsServiceAccount() { users[k] = madmin.UserInfo{ @@ -965,6 +980,10 @@ func (sys *IAMSys) ListServiceAccounts(ctx context.Context, accessKey string) ([ sys.store.rlock() defer sys.store.runlock() + if sys.storeFallback { + return nil, errIAMNotInitialized + } + var serviceAccounts []string for k, v := range sys.iamUsersMap { @@ -1096,6 +1115,44 @@ func (sys *IAMSys) GetUser(accessKey string) (cred auth.Credentials, ok bool) { return cred, false } + sys.store.rlock() + fallback := sys.storeFallback + sys.store.runlock() + if fallback { + sys.store.lock() + // If user is already found proceed. + if _, found := sys.iamUsersMap[accessKey]; !found { + sys.store.loadUser(accessKey, regularUser, sys.iamUsersMap) + if _, found = sys.iamUsersMap[accessKey]; found { + // found user, load its mapped policies + sys.store.loadMappedPolicy(accessKey, regularUser, false, sys.iamUserPolicyMap) + } else { + sys.store.loadUser(accessKey, srvAccUser, sys.iamUsersMap) + if svc, found := sys.iamUsersMap[accessKey]; found { + // Found service account, load its parent user and its mapped policies. + if sys.usersSysType == MinIOUsersSysType { + sys.store.loadUser(svc.ParentUser, regularUser, sys.iamUsersMap) + } + sys.store.loadMappedPolicy(svc.ParentUser, regularUser, false, sys.iamUserPolicyMap) + } else { + // None found fall back to STS users. + sys.store.loadUser(accessKey, stsUser, sys.iamUsersMap) + if _, found = sys.iamUsersMap[accessKey]; found { + // STS user found, load its mapped policy. + sys.store.loadMappedPolicy(accessKey, stsUser, false, sys.iamUserPolicyMap) + } + } + } + } + // Load associated policies if any. + for _, policy := range sys.iamUserPolicyMap[accessKey].toSlice() { + if _, found := sys.iamPolicyDocsMap[policy]; !found { + sys.store.loadPolicyDoc(policy, sys.iamPolicyDocsMap) + } + } + sys.store.unlock() + } + sys.store.rlock() defer sys.store.runlock() @@ -1344,6 +1401,10 @@ func (sys *IAMSys) ListGroups() (r []string, err error) { sys.store.rlock() defer sys.store.runlock() + if sys.storeFallback { + return nil, errIAMNotInitialized + } + if sys.usersSysType != MinIOUsersSysType { return nil, errIAMActionNotAllowed } @@ -1869,5 +1930,6 @@ func NewIAMSys() *IAMSys { iamGroupPolicyMap: make(map[string]MappedPolicy), iamGroupsMap: make(map[string]GroupInfo), iamUserGroupMemberships: make(map[string]set.StringSet), + storeFallback: true, } } diff --git a/cmd/typed-errors.go b/cmd/typed-errors.go index 8b9a941e7..273154725 100644 --- a/cmd/typed-errors.go +++ b/cmd/typed-errors.go @@ -88,7 +88,10 @@ var errGroupNotEmpty = errors.New("Specified group is not empty - cannot remove var errNoSuchPolicy = errors.New("Specified canned policy does not exist") // error returned in IAM subsystem when an external users systems is configured. -var errIAMActionNotAllowed = errors.New("Specified IAM action is not allowed under the current configuration") +var errIAMActionNotAllowed = errors.New("Specified IAM action is not allowed with LDAP configuration") + +// error returned in IAM subsystem when IAM sub-system is still being initialized. +var errIAMNotInitialized = errors.New("IAM sub-system is being initialized, please try again") // error returned when access is denied. var errAccessDenied = errors.New("Do not have enough permissions to access this resource")