support autogenerated credentials for KMS_SECRET_KEY properly (#21223)

we had a chicken and egg problem with this feature even when used with kes the credentials generation would not work in correct sequence causing setup/deployment disruptions. This PR streamlines all of this properly to ensure that this functionality works as advertised.
2025-11-07 12:52:58 -05:00 · 2025-04-21 09:23:51 -07:00
parent e2ed696619
commit 43aa8e4259
6 changed files with 132 additions and 128 deletions
--- a/cmd/common-main.go
+++ b/cmd/common-main.go
@@ -47,6 +47,7 @@ import (
 	"github.com/minio/console/api/operations"
 	consoleoauth2 "github.com/minio/console/pkg/auth/idp/oauth2"
 	consoleCerts "github.com/minio/console/pkg/certs"
+	"github.com/minio/kms-go/kes"
 	"github.com/minio/madmin-go/v3"
 	"github.com/minio/minio-go/v7"
 	"github.com/minio/minio-go/v7/pkg/set"
@@ -831,55 +832,83 @@ func serverHandleEnvVars() {
 	globalEnableSyncBoot = env.Get("MINIO_SYNC_BOOT", config.EnableOff) == config.EnableOn
 }

-func loadRootCredentials() {
+func loadRootCredentials() auth.Credentials {
 	// At this point, either both environment variables
 	// are defined or both are not defined.
 	// Check both cases and authenticate them if correctly defined
 	var user, password string
-	var hasCredentials bool
 	var legacyCredentials bool
 	//nolint:gocritic
 	if env.IsSet(config.EnvRootUser) && env.IsSet(config.EnvRootPassword) {
 		user = env.Get(config.EnvRootUser, "")
 		password = env.Get(config.EnvRootPassword, "")
-		hasCredentials = true
 	} else if env.IsSet(config.EnvAccessKey) && env.IsSet(config.EnvSecretKey) {
 		user = env.Get(config.EnvAccessKey, "")
 		password = env.Get(config.EnvSecretKey, "")
 		legacyCredentials = true
-		hasCredentials = true
 	} else if globalServerCtxt.RootUser != "" && globalServerCtxt.RootPwd != "" {
 		user, password = globalServerCtxt.RootUser, globalServerCtxt.RootPwd
-		hasCredentials = true
 	}
-	if hasCredentials {
-		cred, err := auth.CreateCredentials(user, password)
-		if err != nil {
-			if legacyCredentials {
-				logger.Fatal(config.ErrInvalidCredentials(err),
-					"Unable to validate credentials inherited from the shell environment")
-			} else {
-				logger.Fatal(config.ErrInvalidRootUserCredentials(err),
-					"Unable to validate credentials inherited from the shell environment")
-			}
+	if user == "" || password == "" {
+		return auth.Credentials{}
+	}
+	cred, err := auth.CreateCredentials(user, password)
+	if err != nil {
+		if legacyCredentials {
+			logger.Fatal(config.ErrInvalidCredentials(err),
+				"Unable to validate credentials inherited from the shell environment")
+		} else {
+			logger.Fatal(config.ErrInvalidRootUserCredentials(err),
+				"Unable to validate credentials inherited from the shell environment")
 		}
-		if env.IsSet(config.EnvAccessKey) && env.IsSet(config.EnvSecretKey) {
-			msg := fmt.Sprintf("WARNING: %s and %s are deprecated.\n"+
-				"         Please use %s and %s",
-				config.EnvAccessKey, config.EnvSecretKey,
-				config.EnvRootUser, config.EnvRootPassword)
-			logger.Info(color.RedBold(msg))
-		}
-		globalActiveCred = cred
-		globalCredViaEnv = true
-	} else {
-		globalActiveCred = auth.DefaultCredentials
+	}
+	if env.IsSet(config.EnvAccessKey) && env.IsSet(config.EnvSecretKey) {
+		msg := fmt.Sprintf("WARNING: %s and %s are deprecated.\n"+
+			"         Please use %s and %s",
+			config.EnvAccessKey, config.EnvSecretKey,
+			config.EnvRootUser, config.EnvRootPassword)
+		logger.Info(color.RedBold(msg))
+	}
+	globalCredViaEnv = true
+	return cred
+}
+
+// autoGenerateRootCredentials generates root credentials deterministically if
+// a KMS is configured, no manual credentials have been specified and if root
+// access is disabled.
+func autoGenerateRootCredentials() auth.Credentials {
+	if GlobalKMS == nil {
+		return globalActiveCred
 	}

-	var err error
-	globalNodeAuthToken, err = authenticateNode(globalActiveCred.AccessKey, globalActiveCred.SecretKey)
+	aKey, err := GlobalKMS.MAC(GlobalContext, &kms.MACRequest{Message: []byte("root access key")})
+	if IsErrIgnored(err, kes.ErrNotAllowed, kms.ErrNotSupported, errors.ErrUnsupported, kms.ErrPermission) {
+		// If we don't have permission to compute the HMAC, don't change the cred.
+		return globalActiveCred
+	}
 	if err != nil {
-		logger.Fatal(err, "Unable to generate internode credentials")
+		logger.Fatal(err, "Unable to generate root access key using KMS")
+	}
+
+	sKey, err := GlobalKMS.MAC(GlobalContext, &kms.MACRequest{Message: []byte("root secret key")})
+	if err != nil {
+		// Here, we must have permission. Otherwise, we would have failed earlier.
+		logger.Fatal(err, "Unable to generate root secret key using KMS")
+	}
+
+	accessKey, err := auth.GenerateAccessKey(20, bytes.NewReader(aKey))
+	if err != nil {
+		logger.Fatal(err, "Unable to generate root access key")
+	}
+	secretKey, err := auth.GenerateSecretKey(32, bytes.NewReader(sKey))
+	if err != nil {
+		logger.Fatal(err, "Unable to generate root secret key")
+	}
+
+	logger.Info("Automatically generated root access key and secret key with the KMS")
+	return auth.Credentials{
+		AccessKey: accessKey,
+		SecretKey: secretKey,
 	}
 }

--- a/cmd/config-current.go
+++ b/cmd/config-current.go
@@ -18,17 +18,13 @@
 package cmd

 import (
-	"bytes"
 	"context"
 	"errors"
 	"fmt"
 	"strings"
 	"sync"

-	"github.com/minio/kms-go/kes"
-	"github.com/minio/minio/internal/auth"
 	"github.com/minio/minio/internal/config/browser"
-	"github.com/minio/minio/internal/kms"

 	"github.com/minio/madmin-go/v3"
 	"github.com/minio/minio/internal/config"
@@ -570,7 +566,6 @@ func applyDynamicConfigForSubSys(ctx context.Context, objAPI ObjectLayer, s conf
 		}

 		globalAPIConfig.init(apiConfig, setDriveCounts, objAPI.Legacy())
-		autoGenerateRootCredentials() // Generate the KMS root credentials here since we don't know whether API root access is disabled until now.
 		setRemoteInstanceTransport(NewHTTPTransportWithTimeout(apiConfig.RemoteTransportDeadline))
 	case config.CompressionSubSys:
 		cmpCfg, err := compress.LookupConfig(s[config.CompressionSubSys][config.Default])
@@ -729,47 +724,6 @@ func applyDynamicConfigForSubSys(ctx context.Context, objAPI ObjectLayer, s conf
 	return nil
 }

-// autoGenerateRootCredentials generates root credentials deterministically if
-// a KMS is configured, no manual credentials have been specified and if root
-// access is disabled.
-func autoGenerateRootCredentials() {
-	if GlobalKMS == nil {
-		return
-	}
-	if globalAPIConfig.permitRootAccess() || !globalActiveCred.Equal(auth.DefaultCredentials) {
-		return
-	}
-
-	aKey, err := GlobalKMS.MAC(GlobalContext, &kms.MACRequest{Message: []byte("root access key")})
-	if errors.Is(err, kes.ErrNotAllowed) || errors.Is(err, errors.ErrUnsupported) {
-		return // If we don't have permission to compute the HMAC, don't change the cred.
-	}
-	if err != nil {
-		logger.Fatal(err, "Unable to generate root access key using KMS")
-	}
-
-	sKey, err := GlobalKMS.MAC(GlobalContext, &kms.MACRequest{Message: []byte("root secret key")})
-	if err != nil {
-		// Here, we must have permission. Otherwise, we would have failed earlier.
-		logger.Fatal(err, "Unable to generate root secret key using KMS")
-	}
-
-	accessKey, err := auth.GenerateAccessKey(20, bytes.NewReader(aKey))
-	if err != nil {
-		logger.Fatal(err, "Unable to generate root access key")
-	}
-	secretKey, err := auth.GenerateSecretKey(32, bytes.NewReader(sKey))
-	if err != nil {
-		logger.Fatal(err, "Unable to generate root secret key")
-	}
-
-	logger.Info("Automatically generated root access key and secret key with the KMS")
-	globalActiveCred = auth.Credentials{
-		AccessKey: accessKey,
-		SecretKey: secretKey,
-	}
-}
-
 // applyDynamicConfig will apply dynamic config values.
 // Dynamic systems should be in config.SubSystemsDynamic as well.
 func applyDynamicConfig(ctx context.Context, objAPI ObjectLayer, s config.Config) error {
--- a/cmd/server-main.go
+++ b/cmd/server-main.go
@@ -47,6 +47,7 @@ import (
 	"github.com/minio/minio/internal/bucket/bandwidth"
 	"github.com/minio/minio/internal/color"
 	"github.com/minio/minio/internal/config"
+	"github.com/minio/minio/internal/config/api"
 	"github.com/minio/minio/internal/handlers"
 	"github.com/minio/minio/internal/hash/sha256"
 	xhttp "github.com/minio/minio/internal/http"
@@ -792,10 +793,6 @@ func serverMain(ctx *cli.Context) {
 	// Handle all server environment vars.
 	serverHandleEnvVars()

-	// Load the root credentials from the shell environment or from
-	// the config file if not defined, set the default one.
-	loadRootCredentials()
-
 	// Perform any self-tests
 	bootstrapTrace("selftests", func() {
 		bitrotSelfTest()
@@ -806,6 +803,29 @@ func serverMain(ctx *cli.Context) {
 	// Initialize KMS configuration
 	bootstrapTrace("handleKMSConfig", handleKMSConfig)

+	// Load the root credentials from the shell environment or from
+	// the config file if not defined, set the default one.
+	bootstrapTrace("rootCredentials", func() {
+		cred := loadRootCredentials()
+		if !cred.IsValid() && (env.Get(api.EnvAPIRootAccess, config.EnableOn) == config.EnableOff) {
+			// Generate KMS based credentials if root access is disabled
+			// and no ENV is set.
+			cred = autoGenerateRootCredentials()
+		}
+
+		if !cred.IsValid() {
+			cred = auth.DefaultCredentials
+		}
+
+		var err error
+		globalNodeAuthToken, err = authenticateNode(cred.AccessKey, cred.SecretKey)
+		if err != nil {
+			logger.Fatal(err, "Unable to generate internode credentials")
+		}
+
+		globalActiveCred = cred
+	})
+
 	// Initialize all help
 	bootstrapTrace("initHelp", initHelp)