LDAP/OpenID must be initialized IAM Init() (#15491)

This allows for LDAP/OpenID to be non-blocking, allowing for unreachable Identity targets to be initialized in IAM.
2024-12-23 21:55:53 -05:00 · 2022-08-08 16:16:27 -07:00 · 2022-08-08 16:16:27 -07:00 · 1823ab6808
commit 1823ab6808
parent 8eec49304d
6 changed files with 117 additions and 91 deletions
--- a/cmd/config-current.go
+++ b/cmd/config-current.go
@ -471,14 +471,12 @@ func lookupConfigs(s config.Config, objAPI ObjectLayer) {
 	}

 	if etcdCfg.Enabled {
-		if globalEtcdClient == nil {
-			globalEtcdClient, err = etcd.New(etcdCfg)
-			if err != nil {
-				if globalIsGateway {
-					logger.FatalIf(err, "Unable to initialize etcd config")
-				} else {
-					logger.LogIf(ctx, fmt.Errorf("Unable to initialize etcd config: %w", err))
-				}
+		globalEtcdClient, err = etcd.New(etcdCfg)
+		if err != nil {
+			if globalIsGateway {
+				logger.FatalIf(err, "Unable to initialize etcd config")
+			} else {
+				logger.LogIf(ctx, fmt.Errorf("Unable to initialize etcd config: %w", err))
 			}
 		}

@ -486,7 +484,7 @@ func lookupConfigs(s config.Config, objAPI ObjectLayer) {
 			if globalDNSConfig != nil {
 				// if global DNS is already configured, indicate with a warning, incase
 				// users are confused.
-				logger.LogIf(ctx, fmt.Errorf("DNS store is already configured with %s, not using etcd for DNS store", globalDNSConfig))
+				logger.LogIf(ctx, fmt.Errorf("DNS store is already configured with %s, etcd is not used for DNS store", globalDNSConfig))
 			} else {
 				globalDNSConfig, err = dns.NewCoreDNS(etcdCfg.Config,
 					dns.DomainNames(globalDomainNames),
@ -518,16 +516,6 @@ func lookupConfigs(s config.Config, objAPI ObjectLayer) {
 		logger.LogIf(ctx, fmt.Errorf("Invalid site configuration: %w", err))
 	}

-	apiConfig, err := api.LookupConfig(s[config.APISubSys][config.Default])
-	if err != nil {
-		logger.LogIf(ctx, fmt.Errorf("Invalid api configuration: %w", err))
-	}
-
-	// Initialize remote instance transport once.
-	getRemoteInstanceTransportOnce.Do(func() {
-		getRemoteInstanceTransport = newGatewayHTTPTransport(apiConfig.RemoteTransportDeadline)
-	})
-
 	globalCacheConfig, err = cache.LookupConfig(s[config.CacheSubSys][config.Default])
 	if err != nil {
 		if globalIsGateway {
@ -560,56 +548,19 @@ func lookupConfigs(s config.Config, objAPI ObjectLayer) {
 		logger.LogIf(ctx, fmt.Errorf("CRITICAL: enabling %s is not recommended in a production environment", xtls.EnvIdentityTLSSkipVerify))
 	}

-	globalOpenIDConfig, err = openid.LookupConfig(s,
-		NewGatewayHTTPTransport(), xhttp.DrainBody, globalSite.Region)
-	if err != nil {
-		logger.LogIf(ctx, fmt.Errorf("Unable to initialize OpenID: %w", err))
-	}
-
-	globalLDAPConfig, err = xldap.Lookup(s[config.IdentityLDAPSubSys][config.Default],
-		globalRootCAs)
-	if err != nil {
-		logger.LogIf(ctx, fmt.Errorf("Unable to parse LDAP configuration: %w", err))
-	}
-
-	authNPluginCfg, err := idplugin.LookupConfig(s[config.IdentityPluginSubSys][config.Default],
-		NewGatewayHTTPTransport(), xhttp.DrainBody, globalSite.Region)
-	if err != nil {
-		logger.LogIf(ctx, fmt.Errorf("Unable to initialize AuthNPlugin: %w", err))
-	}
-	globalAuthNPlugin = idplugin.New(authNPluginCfg)
-
-	authZPluginCfg, err := polplugin.LookupConfig(s[config.PolicyPluginSubSys][config.Default],
-		NewGatewayHTTPTransport(), xhttp.DrainBody)
-	if err != nil {
-		logger.LogIf(ctx, fmt.Errorf("Unable to initialize AuthZPlugin: %w", err))
-	}
-	if authZPluginCfg.URL == nil {
-		opaCfg, err := opa.LookupConfig(s[config.PolicyOPASubSys][config.Default],
-			NewGatewayHTTPTransport(), xhttp.DrainBody)
-		if err != nil {
-			logger.LogIf(ctx, fmt.Errorf("Unable to initialize AuthZPlugin from legacy OPA config: %w", err))
-		} else {
-			authZPluginCfg.URL = opaCfg.URL
-			authZPluginCfg.AuthToken = opaCfg.AuthToken
-			authZPluginCfg.Transport = opaCfg.Transport
-			authZPluginCfg.CloseRespFn = opaCfg.CloseRespFn
-		}
-	}
-
-	setGlobalAuthZPlugin(polplugin.New(authZPluginCfg))
-
 	globalSubnetConfig, err = subnet.LookupConfig(s[config.SubnetSubSys][config.Default], globalProxyTransport)
 	if err != nil {
 		logger.LogIf(ctx, fmt.Errorf("Unable to parse subnet configuration: %w", err))
 	}

-	globalConfigTargetList, err = notify.GetNotificationTargets(GlobalContext, s, NewGatewayHTTPTransport(), false)
+	transport := NewGatewayHTTPTransport()
+
+	globalConfigTargetList, err = notify.GetNotificationTargets(GlobalContext, s, transport, false)
 	if err != nil {
 		logger.LogIf(ctx, fmt.Errorf("Unable to initialize notification target(s): %w", err))
 	}

-	globalEnvTargetList, err = notify.GetNotificationTargets(GlobalContext, newServerConfig(), NewGatewayHTTPTransport(), true)
+	globalEnvTargetList, err = notify.GetNotificationTargets(GlobalContext, newServerConfig(), transport, true)
 	if err != nil {
 		logger.LogIf(ctx, fmt.Errorf("Unable to initialize notification target(s): %w", err))
 	}
@ -636,6 +587,11 @@ func applyDynamicConfigForSubSys(ctx context.Context, objAPI ObjectLayer, s conf
 			setDriveCounts = objAPI.SetDriveCounts()
 		}
 		globalAPIConfig.init(apiConfig, setDriveCounts)
+
+		// Initialize remote instance transport once.
+		getRemoteInstanceTransportOnce.Do(func() {
+			getRemoteInstanceTransport = newGatewayHTTPTransport(apiConfig.RemoteTransportDeadline)
+		})
 	case config.CompressionSubSys:
 		cmpCfg, err := compress.LookupConfig(s[config.CompressionSubSys][config.Default])
 		if err != nil {
@ -678,8 +634,7 @@ func applyDynamicConfigForSubSys(ctx context.Context, objAPI ObjectLayer, s conf
 				loggerCfg.HTTP[n] = l
 			}
 		}
-		err = logger.UpdateSystemTargets(loggerCfg)
-		if err != nil {
+		if err = logger.UpdateSystemTargets(loggerCfg); err != nil {
 			logger.LogIf(ctx, fmt.Errorf("Unable to update logger webhook config: %w", err))
 		}
 	case config.AuditWebhookSubSys:
@ -697,8 +652,7 @@ func applyDynamicConfigForSubSys(ctx context.Context, objAPI ObjectLayer, s conf
 			}
 		}

-		err = logger.UpdateAuditWebhookTargets(loggerCfg)
-		if err != nil {
+		if err = logger.UpdateAuditWebhookTargets(loggerCfg); err != nil {
 			logger.LogIf(ctx, fmt.Errorf("Unable to update audit webhook targets: %w", err))
 		}
 	case config.AuditKafkaSubSys:
@ -712,8 +666,7 @@ func applyDynamicConfigForSubSys(ctx context.Context, objAPI ObjectLayer, s conf
 				loggerCfg.AuditKafka[n] = l
 			}
 		}
-		err = logger.UpdateAuditKafkaTargets(loggerCfg)
-		if err != nil {
+		if err = logger.UpdateAuditKafkaTargets(loggerCfg); err != nil {
 			logger.LogIf(ctx, fmt.Errorf("Unable to update audit kafka targets: %w", err))
 		}
 	case config.StorageClassSubSys:
@ -852,7 +805,7 @@ func newSrvConfig(objAPI ObjectLayer) error {
 	globalServerConfigMu.Unlock()

 	// Save config into file.
-	return saveServerConfig(GlobalContext, objAPI, globalServerConfig)
+	return saveServerConfig(GlobalContext, objAPI, srvCfg)
 }

 func getValidConfig(objAPI ObjectLayer) (config.Config, error) {
--- a/cmd/globals.go
+++ b/cmd/globals.go
@ -383,18 +383,30 @@ var (
 	// Add new variable global values here.
 )

-var globalAuthZPluginMutex sync.Mutex
+var globalAuthPluginMutex sync.Mutex
+
+func newGlobalAuthNPluginFn() *idplugin.AuthNPlugin {
+	globalAuthPluginMutex.Lock()
+	defer globalAuthPluginMutex.Unlock()
+	return globalAuthNPlugin
+}

 func newGlobalAuthZPluginFn() *polplugin.AuthZPlugin {
-	globalAuthZPluginMutex.Lock()
-	defer globalAuthZPluginMutex.Unlock()
+	globalAuthPluginMutex.Lock()
+	defer globalAuthPluginMutex.Unlock()
 	return globalAuthZPlugin
 }

+func setGlobalAuthNPlugin(authn *idplugin.AuthNPlugin) {
+	globalAuthPluginMutex.Lock()
+	globalAuthNPlugin = authn
+	globalAuthPluginMutex.Unlock()
+}
+
 func setGlobalAuthZPlugin(authz *polplugin.AuthZPlugin) {
-	globalAuthZPluginMutex.Lock()
+	globalAuthPluginMutex.Lock()
 	globalAuthZPlugin = authz
-	globalAuthZPluginMutex.Unlock()
+	globalAuthPluginMutex.Unlock()
 }

 var errSelfTestFailure = errors.New("self test failed. unsafe to start server")
--- a/cmd/iam.go
+++ b/cmd/iam.go
@ -38,8 +38,13 @@ import (
 	"github.com/minio/minio/internal/arn"
 	"github.com/minio/minio/internal/auth"
 	"github.com/minio/minio/internal/color"
+	"github.com/minio/minio/internal/config"
 	xldap "github.com/minio/minio/internal/config/identity/ldap"
 	"github.com/minio/minio/internal/config/identity/openid"
+	idplugin "github.com/minio/minio/internal/config/identity/plugin"
+	"github.com/minio/minio/internal/config/policy/opa"
+	polplugin "github.com/minio/minio/internal/config/policy/plugin"
+	xhttp "github.com/minio/minio/internal/http"
 	"github.com/minio/minio/internal/jwt"
 	"github.com/minio/minio/internal/logger"
 	iampolicy "github.com/minio/pkg/iam/policy"
@ -218,6 +223,54 @@ func (sys *IAMSys) Load(ctx context.Context) error {

 // Init - initializes config system by reading entries from config/iam
 func (sys *IAMSys) Init(ctx context.Context, objAPI ObjectLayer, etcdClient *etcd.Client, iamRefreshInterval time.Duration) {
+	globalServerConfigMu.RLock()
+	s := globalServerConfig
+	globalServerConfigMu.RUnlock()
+
+	ldapCfg := s[config.IdentityLDAPSubSys][config.Default]
+
+	var err error
+	globalOpenIDConfig, err = openid.LookupConfig(s,
+		NewGatewayHTTPTransport(), xhttp.DrainBody, globalSite.Region)
+	if err != nil {
+		logger.LogIf(ctx, fmt.Errorf("Unable to initialize OpenID: %w", err))
+	}
+
+	// Initialize if LDAP is enabled
+	globalLDAPConfig, err = xldap.Lookup(ldapCfg, globalRootCAs)
+	if err != nil {
+		logger.LogIf(ctx, fmt.Errorf("Unable to parse LDAP configuration: %w", err))
+	}
+
+	authNPluginCfg, err := idplugin.LookupConfig(s[config.IdentityPluginSubSys][config.Default],
+		NewGatewayHTTPTransport(), xhttp.DrainBody, globalSite.Region)
+	if err != nil {
+		logger.LogIf(ctx, fmt.Errorf("Unable to initialize AuthNPlugin: %w", err))
+	}
+
+	setGlobalAuthNPlugin(idplugin.New(authNPluginCfg))
+
+	authZPluginCfg, err := polplugin.LookupConfig(s[config.PolicyPluginSubSys][config.Default],
+		NewGatewayHTTPTransport(), xhttp.DrainBody)
+	if err != nil {
+		logger.LogIf(ctx, fmt.Errorf("Unable to initialize AuthZPlugin: %w", err))
+	}
+
+	if authZPluginCfg.URL == nil {
+		opaCfg, err := opa.LookupConfig(s[config.PolicyOPASubSys][config.Default],
+			NewGatewayHTTPTransport(), xhttp.DrainBody)
+		if err != nil {
+			logger.LogIf(ctx, fmt.Errorf("Unable to initialize AuthZPlugin from legacy OPA config: %w", err))
+		} else {
+			authZPluginCfg.URL = opaCfg.URL
+			authZPluginCfg.AuthToken = opaCfg.AuthToken
+			authZPluginCfg.Transport = opaCfg.Transport
+			authZPluginCfg.CloseRespFn = opaCfg.CloseRespFn
+		}
+	}
+
+	setGlobalAuthZPlugin(polplugin.New(authZPluginCfg))
+
 	sys.Lock()
 	defer sys.Unlock()

@ -330,8 +383,8 @@ func (sys *IAMSys) Init(ctx context.Context, objAPI ObjectLayer, etcdClient *etc
 	}

 	// From AuthN plugin if enabled.
-	if globalAuthNPlugin != nil {
-		riMap := globalAuthNPlugin.GetRoleInfo()
+	if authn := newGlobalAuthNPluginFn(); authn != nil {
+		riMap := authn.GetRoleInfo()
 		sys.validateAndAddRolePolicyMappings(ctx, riMap)
 	}

@ -352,7 +405,8 @@ func (sys *IAMSys) validateAndAddRolePolicyMappings(ctx context.Context, m map[a
 		knownPoliciesSet := newMappedPolicy(validPolicies).policySet()
 		unknownPoliciesSet := specifiedPoliciesSet.Difference(knownPoliciesSet)
 		if len(unknownPoliciesSet) > 0 {
-			if globalAuthZPlugin == nil {
+			authz := newGlobalAuthZPluginFn()
+			if authz == nil {
 				// Print a warning that some policies mapped to a role are not defined.
 				errMsg := fmt.Errorf(
 					"The policies \"%s\" mapped to role ARN %s are not defined - this role may not work as expected.",
--- a/cmd/sts-handlers.go
+++ b/cmd/sts-handlers.go
@ -843,7 +843,8 @@ func (sts *stsAPIHandlers) AssumeRoleWithCustomToken(w http.ResponseWriter, r *h
 	claims := make(map[string]interface{})
 	defer logger.AuditLog(ctx, w, r, claims)

-	if globalAuthNPlugin == nil {
+	authn := newGlobalAuthNPluginFn()
+	if authn == nil {
 		writeSTSErrorResponse(ctx, w, true, ErrSTSNotInitialized, errors.New("STS API 'AssumeRoleWithCustomToken' is disabled"))
 		return
 	}
@ -879,7 +880,7 @@ func (sts *stsAPIHandlers) AssumeRoleWithCustomToken(w http.ResponseWriter, r *h
 		return
 	}

-	res, err := globalAuthNPlugin.Authenticate(roleArn, token)
+	res, err := authn.Authenticate(roleArn, token)
 	if err != nil {
 		writeSTSErrorResponse(ctx, w, true, ErrSTSInvalidParameterValue, err)
 		return
--- a/internal/config/identity/ldap/ldap.go
+++ b/internal/config/identity/ldap/ldap.go
@ -208,25 +208,29 @@ func (l *Config) Connect() (ldapConn *ldap.Conn, err error) {
 		l.ServerAddr = net.JoinHostPort(l.ServerAddr, "636")
 	}

-	if l.serverInsecure {
-		return ldap.Dial("tcp", l.ServerAddr)
-	}
-
 	tlsConfig := &tls.Config{
 		InsecureSkipVerify: l.tlsSkipVerify,
 		RootCAs:            l.rootCAs,
 	}

-	if l.serverStartTLS {
-		conn, err := ldap.Dial("tcp", l.ServerAddr)
-		if err != nil {
-			return nil, err
+	if l.serverInsecure {
+		ldapConn, err = ldap.Dial("tcp", l.ServerAddr)
+	} else {
+		if l.serverStartTLS {
+			ldapConn, err = ldap.Dial("tcp", l.ServerAddr)
+		} else {
+			ldapConn, err = ldap.DialTLS("tcp", l.ServerAddr, tlsConfig)
 		}
-		err = conn.StartTLS(tlsConfig)
-		return conn, err
 	}

-	return ldap.DialTLS("tcp", l.ServerAddr, tlsConfig)
+	if ldapConn != nil {
+		ldapConn.SetTimeout(30 * time.Second) // Change default timeout to 30 seconds.
+		if l.serverStartTLS {
+			err = ldapConn.StartTLS(tlsConfig)
+		}
+	}
+
+	return ldapConn, err
 }

 // GetExpiryDuration - return parsed expiry duration.
--- a/internal/config/subnet/config.go
+++ b/internal/config/subnet/config.go
@ -55,7 +55,7 @@ type Config struct {
 	ProxyURL *xnet.URL `json:"proxy_url"`

 	// Transport configured with proxy_url if set optionally.
-	transport *http.Transport
+	transport http.RoundTripper
 }

 // LookupConfig - lookup config and override with valid environment settings if any.
@ -83,11 +83,13 @@ func LookupConfig(kvs config.KVS, transport http.RoundTripper) (cfg Config, err
 	}

 	// Make sure to clone the transport before editing the ProxyURL
-	ctransport := transport.(*http.Transport).Clone()
 	if cfg.ProxyURL != nil {
+		ctransport := transport.(*http.Transport).Clone()
 		ctransport.Proxy = http.ProxyURL((*url.URL)(cfg.ProxyURL))
+		cfg.transport = ctransport
+	} else {
+		cfg.transport = transport
 	}
-	cfg.transport = ctransport

 	return cfg, nil
 }