From fd46a1c3b3c314d8d98d95fc63ed1c5bf84bf9c0 Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Wed, 25 May 2022 18:32:53 -0700 Subject: [PATCH] fix: some races when accessing ldap/openid config globally (#14978) --- cmd/auth-handler.go | 2 +- cmd/config-current.go | 3 ++- cmd/globals.go | 14 ++++++++++++ cmd/iam.go | 24 +++++++++++++-------- internal/config/identity/ldap/config.go | 25 ++++++++++++++++++++++ internal/config/identity/openid/openid.go | 26 +++++++++++++++++++++++ 6 files changed, 83 insertions(+), 11 deletions(-) diff --git a/cmd/auth-handler.go b/cmd/auth-handler.go index 28320724a..efab56ddb 100644 --- a/cmd/auth-handler.go +++ b/cmd/auth-handler.go @@ -218,7 +218,7 @@ func getClaimsFromTokenWithSecret(token, secret string) (map[string]interface{}, } // If AuthZPlugin is set, return without any further checks. - if globalAuthZPlugin != nil { + if newGlobalAuthZPluginFn() != nil { return claims.Map(), nil } diff --git a/cmd/config-current.go b/cmd/config-current.go index 76b428967..586450c81 100644 --- a/cmd/config-current.go +++ b/cmd/config-current.go @@ -558,7 +558,8 @@ func lookupConfigs(s config.Config, objAPI ObjectLayer) { authZPluginCfg.CloseRespFn = opaCfg.CloseRespFn } } - globalAuthZPlugin = polplugin.New(authZPluginCfg) + + setGlobalAuthZPlugin(polplugin.New(authZPluginCfg)) globalLDAPConfig, err = xldap.Lookup(s[config.IdentityLDAPSubSys][config.Default], globalRootCAs) diff --git a/cmd/globals.go b/cmd/globals.go index df2e066d9..ba75e872b 100644 --- a/cmd/globals.go +++ b/cmd/globals.go @@ -359,4 +359,18 @@ var ( // Add new variable global values here. ) +var globalAuthZPluginMutex sync.Mutex + +func newGlobalAuthZPluginFn() *polplugin.AuthZPlugin { + globalAuthZPluginMutex.Lock() + defer globalAuthZPluginMutex.Unlock() + return globalAuthZPlugin +} + +func setGlobalAuthZPlugin(authz *polplugin.AuthZPlugin) { + globalAuthZPluginMutex.Lock() + globalAuthZPlugin = authz + globalAuthZPluginMutex.Unlock() +} + var errSelfTestFailure = errors.New("self test failed. unsafe to start server") diff --git a/cmd/iam.go b/cmd/iam.go index c3d9d8210..7ce131915 100644 --- a/cmd/iam.go +++ b/cmd/iam.go @@ -38,6 +38,8 @@ import ( "github.com/minio/minio/internal/arn" "github.com/minio/minio/internal/auth" "github.com/minio/minio/internal/color" + xldap "github.com/minio/minio/internal/config/identity/ldap" + "github.com/minio/minio/internal/config/identity/openid" "github.com/minio/minio/internal/jwt" "github.com/minio/minio/internal/logger" iampolicy "github.com/minio/pkg/iam/policy" @@ -80,6 +82,8 @@ type IAMSys struct { sync.Mutex iamRefreshInterval time.Duration + ldapConfig xldap.Config // only valid if usersSysType is LDAPUsers + openIDConfig openid.Config // only valid if OpenID is configured usersSysType UsersSysType @@ -164,7 +168,7 @@ func (sys *IAMSys) doIAMConfigMigration(ctx context.Context) error { // initStore initializes IAM stores func (sys *IAMSys) initStore(objAPI ObjectLayer, etcdClient *etcd.Client) { - if globalLDAPConfig.Enabled { + if sys.ldapConfig.Enabled { sys.EnableLDAPSys() } @@ -224,6 +228,8 @@ func (sys *IAMSys) Init(ctx context.Context, objAPI ObjectLayer, etcdClient *etc sys.Lock() defer sys.Unlock() + sys.ldapConfig = globalLDAPConfig.Clone() + sys.openIDConfig = globalOpenIDConfig.Clone() sys.iamRefreshInterval = iamRefreshInterval // Initialize IAM store @@ -310,7 +316,7 @@ func (sys *IAMSys) Init(ctx context.Context, objAPI ObjectLayer, etcdClient *etc // Set up polling for expired accounts and credentials purging. switch { - case globalOpenIDConfig.ProviderEnabled(): + case sys.openIDConfig.ProviderEnabled(): go func() { timer := time.NewTimer(sys.iamRefreshInterval) defer timer.Stop() @@ -325,7 +331,7 @@ func (sys *IAMSys) Init(ctx context.Context, objAPI ObjectLayer, etcdClient *etc } } }() - case globalLDAPConfig.Enabled: + case sys.ldapConfig.Enabled: go func() { timer := time.NewTimer(sys.iamRefreshInterval) defer timer.Stop() @@ -348,7 +354,7 @@ func (sys *IAMSys) Init(ctx context.Context, objAPI ObjectLayer, etcdClient *etc go sys.watch(ctx) // Load RoleARN - if rolePolicyMap := globalOpenIDConfig.GetRoleInfo(); rolePolicyMap != nil { + if rolePolicyMap := sys.openIDConfig.GetRoleInfo(); rolePolicyMap != nil { // Validate that policies associated with roles are defined. for _, rolePolicies := range rolePolicyMap { ps := newMappedPolicy(rolePolicies).toSlice() @@ -1138,7 +1144,7 @@ func (sys *IAMSys) purgeExpiredCredentialsForExternalSSO(ctx context.Context) { continue } roleArn = roleArns[0] - u, err := globalOpenIDConfig.LookupUser(roleArn, puInfo.subClaimValue) + u, err := sys.openIDConfig.LookupUser(roleArn, puInfo.subClaimValue) if err != nil { logger.LogIf(GlobalContext, err) continue @@ -1160,14 +1166,14 @@ func (sys *IAMSys) purgeExpiredCredentialsForLDAP(ctx context.Context) { parentUsers := sys.store.GetAllParentUsers() var allDistNames []string for parentUser := range parentUsers { - if !globalLDAPConfig.IsLDAPUserDN(parentUser) { + if !sys.ldapConfig.IsLDAPUserDN(parentUser) { continue } allDistNames = append(allDistNames, parentUser) } - expiredUsers, err := globalLDAPConfig.GetNonEligibleUserDistNames(allDistNames) + expiredUsers, err := sys.ldapConfig.GetNonEligibleUserDistNames(allDistNames) if err != nil { // Log and return on error - perhaps it'll work the next time. logger.LogIf(GlobalContext, err) @@ -1189,7 +1195,7 @@ func (sys *IAMSys) updateGroupMembershipsForLDAP(ctx context.Context) { // DN to ldap username mapping for each LDAP user parentUserToLDAPUsernameMap := make(map[string]string) for _, cred := range allCreds { - if !globalLDAPConfig.IsLDAPUserDN(cred.ParentUser) { + if !sys.ldapConfig.IsLDAPUserDN(cred.ParentUser) { continue } // Check if this is the first time we are @@ -1237,7 +1243,7 @@ func (sys *IAMSys) updateGroupMembershipsForLDAP(ctx context.Context) { } // 2. Query LDAP server for groups of the LDAP users collected. - updatedGroups, err := globalLDAPConfig.LookupGroupMemberships(parentUsers, parentUserToLDAPUsernameMap) + updatedGroups, err := sys.ldapConfig.LookupGroupMemberships(parentUsers, parentUserToLDAPUsernameMap) if err != nil { // Log and return on error - perhaps it'll work the next time. logger.LogIf(GlobalContext, err) diff --git a/internal/config/identity/ldap/config.go b/internal/config/identity/ldap/config.go index 059345731..ae3034d28 100644 --- a/internal/config/identity/ldap/config.go +++ b/internal/config/identity/ldap/config.go @@ -62,6 +62,31 @@ type Config struct { rootCAs *x509.CertPool } +// Clone returns a cloned copy of LDAP config. +func (l *Config) Clone() Config { + if l == nil { + return Config{} + } + cfg := Config{ + Enabled: l.Enabled, + ServerAddr: l.ServerAddr, + UserDNSearchBaseDistName: l.UserDNSearchBaseDistName, + UserDNSearchBaseDistNames: l.UserDNSearchBaseDistNames, + UserDNSearchFilter: l.UserDNSearchFilter, + GroupSearchBaseDistName: l.GroupSearchBaseDistName, + GroupSearchBaseDistNames: l.GroupSearchBaseDistNames, + GroupSearchFilter: l.GroupSearchFilter, + LookupBindDN: l.LookupBindDN, + LookupBindPassword: l.LookupBindPassword, + stsExpiryDuration: l.stsExpiryDuration, + tlsSkipVerify: l.tlsSkipVerify, + serverInsecure: l.serverInsecure, + serverStartTLS: l.serverStartTLS, + rootCAs: l.rootCAs, + } + return cfg +} + // LDAP keys and envs. const ( ServerAddr = "server_addr" diff --git a/internal/config/identity/openid/openid.go b/internal/config/identity/openid/openid.go index 3771daa8e..d9d98a307 100644 --- a/internal/config/identity/openid/openid.go +++ b/internal/config/identity/openid/openid.go @@ -164,6 +164,32 @@ type Config struct { closeRespFn func(io.ReadCloser) } +// Clone returns a cloned copy of OpenID config. +func (r *Config) Clone() Config { + if r == nil { + return Config{} + } + cfg := Config{ + Enabled: r.Enabled, + arnProviderCfgsMap: make(map[arn.ARN]*providerCfg, len(r.arnProviderCfgsMap)), + ProviderCfgs: make(map[string]*providerCfg, len(r.ProviderCfgs)), + pubKeys: r.pubKeys, + roleArnPolicyMap: make(map[arn.ARN]string, len(r.roleArnPolicyMap)), + transport: r.transport, + closeRespFn: r.closeRespFn, + } + for k, v := range r.arnProviderCfgsMap { + cfg.arnProviderCfgsMap[k] = v + } + for k, v := range r.ProviderCfgs { + cfg.ProviderCfgs[k] = v + } + for k, v := range r.roleArnPolicyMap { + cfg.roleArnPolicyMap[k] = v + } + return cfg +} + // LookupConfig lookup jwks from config, override with any ENVs. func LookupConfig(kvsMap map[string]config.KVS, transport http.RoundTripper, closeRespFn func(io.ReadCloser), serverRegion string) (c Config, err error) { openIDClientTransport := http.DefaultTransport