fix: etcd IAM encryption fails due to incorrect kms.Context (#12431)

Due to incorrect KMS context constructed, we need to add
additional fallbacks and also fix the original root cause
to fix already migrated deployments.

Bonus remove double migration is avoided in gateway mode
for etcd, instead do it once in iam.Init(), also simplify
the migration by not migrating STS users instead let the
clients regenerate them.

cmd/config-encrypted.go

@@ -69,14 +69,12 @@ func migrateIAMConfigsEtcdToEncrypted(ctx context.Context, client *etcd.Client)
 		return err
 	}
 
-	if encrypted {
-		if GlobalKMS != nil {
-			stat, err := GlobalKMS.Stat()
-			if err != nil {
-				return err
-			}
-			logger.Info("Attempting to re-encrypt config, IAM users and policies on MinIO with %q (%s)", stat.DefaultKey, stat.Name)
+	if encrypted && GlobalKMS != nil {
+		stat, err := GlobalKMS.Stat()
+		if err != nil {
+			return err
 		}
+		logger.Info("Attempting to re-encrypt IAM users and policies on etcd with %q (%s)", stat.DefaultKey, stat.Name)
 	}
 
 	listCtx, cancel := context.WithTimeout(ctx, 1*time.Minute)
@@ -97,15 +95,26 @@ func migrateIAMConfigsEtcdToEncrypted(ctx context.Context, client *etcd.Client)
 		}
 
 		if !utf8.Valid(data) {
-			data, err = madmin.DecryptData(globalActiveCred.String(), bytes.NewReader(data))
+			pdata, err := madmin.DecryptData(globalActiveCred.String(), bytes.NewReader(data))
 			if err != nil {
-				return fmt.Errorf("Decrypting config failed %w, possibly credentials are incorrect", err)
+				pdata, err = config.DecryptBytes(GlobalKMS, data, kms.Context{
+					minioMetaBucket: path.Join(minioMetaBucket, string(kv.Key)),
+				})
+				if err != nil {
+					pdata, err = config.DecryptBytes(GlobalKMS, data, kms.Context{
+						minioMetaBucket: string(kv.Key),
+					})
+					if err != nil {
+						return fmt.Errorf("Decrypting IAM config failed %w, possibly credentials are incorrect", err)
+					}
+				}
 			}
+			data = pdata
 		}
 
 		if GlobalKMS != nil {
 			data, err = config.EncryptBytes(GlobalKMS, data, kms.Context{
-				minioMetaBucket: string(kv.Key),
+				minioMetaBucket: path.Join(minioMetaBucket, string(kv.Key)),
 			})
 			if err != nil {
 				return err
@@ -117,10 +126,8 @@ func migrateIAMConfigsEtcdToEncrypted(ctx context.Context, client *etcd.Client)
 		}
 	}
 
-	if encrypted {
-		if GlobalKMS != nil {
-			logger.Info("Migration of encrypted config data completed. All config data is now encrypted.")
-		}
+	if encrypted && GlobalKMS != nil {
+		logger.Info("Migration of encrypted IAM config data completed. All data is now encrypted on etcd.")
 	}
 	return deleteKeyEtcd(ctx, client, backendEncryptedFile)
 }
@@ -129,14 +136,12 @@ func migrateConfigPrefixToEncrypted(objAPI ObjectLayer, encrypted bool) error {
 	if !encrypted {
 		return nil
 	}
-	if encrypted {
-		if GlobalKMS != nil {
-			stat, err := GlobalKMS.Stat()
-			if err != nil {
-				return err
-			}
-			logger.Info("Attempting to re-encrypt config, IAM users and policies on MinIO with %q (%s)", stat.DefaultKey, stat.Name)
+	if encrypted && GlobalKMS != nil {
+		stat, err := GlobalKMS.Stat()
+		if err != nil {
+			return err
 		}
+		logger.Info("Attempting to re-encrypt config, IAM users and policies on MinIO with %q (%s)", stat.DefaultKey, stat.Name)
 	}
 
 	var marker string
@@ -175,10 +180,8 @@ func migrateConfigPrefixToEncrypted(objAPI ObjectLayer, encrypted bool) error {
 		}
 		marker = res.NextMarker
 	}
-	if encrypted {
-		if GlobalKMS != nil {
-			logger.Info("Migration of encrypted config data completed. All config data is now encrypted.")
-		}
+	if encrypted && GlobalKMS != nil {
+		logger.Info("Migration of encrypted config data completed. All config data is now encrypted.")
 	}
 	return deleteConfig(GlobalContext, globalObjectAPI, backendEncryptedFile)
 }

cmd/gateway-main.go

@@ -317,14 +317,6 @@ func StartGateway(ctx *cli.Context, gw Gateway) {
 		logger.FatalIf(globalNotificationSys.Init(GlobalContext, buckets, newObject), "Unable to initialize notification system")
 	}
 
-	if globalEtcdClient != nil {
-		// ****  WARNING ****
-		// Migrating to encrypted backend on etcd should happen before initialization of
-		// IAM sub-systems, make sure that we do not move the above codeblock elsewhere.
-		logger.FatalIf(migrateIAMConfigsEtcdToEncrypted(GlobalContext, globalEtcdClient),
-			"Unable to handle encrypted backend for iam and policies")
-	}
-
 	if enableIAMOps {
 		// Initialize users credentials and policies in background.
 		globalIAMSys.InitStore(newObject)

cmd/globals.go

@@ -212,9 +212,6 @@ var (
 
 	globalActiveCred auth.Credentials
 
-	// Hold the old server credentials passed by the environment
-	globalOldCred auth.Credentials
-
 	globalPublicCerts []*x509.Certificate
 
 	globalDomainNames []string      // Root domains for virtual host style requests

cmd/iam-etcd-store.go

@@ -20,14 +20,12 @@ package cmd
 import (
 	"context"
 	"encoding/json"
-	"errors"
 	"path"
 	"strings"
 	"sync"
 	"time"
 	"unicode/utf8"
 
-	jwtgo "github.com/dgrijalva/jwt-go"
 	jsoniter "github.com/json-iterator/go"
 	"github.com/minio/minio-go/v7/pkg/set"
 	"github.com/minio/minio/internal/auth"
@@ -110,7 +108,14 @@ func getIAMConfig(item interface{}, data []byte, itemPath string) error {
 			minioMetaBucket: path.Join(minioMetaBucket, itemPath),
 		})
 		if err != nil {
-			return err
+			// This fallback is needed because of a bug, in kms.Context{}
+			// construction during migration.
+			data, err = config.DecryptBytes(GlobalKMS, data, kms.Context{
+				minioMetaBucket: itemPath,
+			})
+			if err != nil {
+				return err
+			}
 		}
 	}
 	var json = jsoniter.ConfigCompatibleWithStandardLibrary
@@ -129,12 +134,8 @@ func (ies *IAMEtcdStore) deleteIAMConfig(ctx context.Context, path string) error
 	return deleteKeyEtcd(ctx, ies.client, path)
 }
 
-func (ies *IAMEtcdStore) migrateUsersConfigToV1(ctx context.Context, isSTS bool) error {
+func (ies *IAMEtcdStore) migrateUsersConfigToV1(ctx context.Context) error {
 	basePrefix := iamConfigUsersPrefix
-	if isSTS {
-		basePrefix = iamConfigSTSPrefix
-	}
-
 	ctx, cancel := context.WithTimeout(ctx, defaultContextTimeout)
 	defer cancel()
 	r, err := ies.client.Get(ctx, basePrefix, etcd.WithPrefix(), etcd.WithKeysOnly())
@@ -162,9 +163,6 @@ func (ies *IAMEtcdStore) migrateUsersConfigToV1(ctx context.Context, isSTS bool)
 			// 2. copy policy to new loc.
 			mp := newMappedPolicy(policyName)
 			userType := regularUser
-			if isSTS {
-				userType = stsUser
-			}
 			path := getMappedPolicyPath(user, userType, false)
 			if err := ies.saveIAMConfig(ctx, mp, path); err != nil {
 				return err
@@ -220,34 +218,27 @@ func (ies *IAMEtcdStore) migrateToV1(ctx context.Context) error {
 		case errConfigNotFound:
 			// Need to migrate to V1.
 		default:
+			// if IAM format
 			return err
 		}
-	} else {
-		if iamFmt.Version >= iamFormatVersion1 {
-			// Already migrated to V1 of higher!
-			return nil
-		}
-		// This case should not happen
-		// (i.e. Version is 0 or negative.)
-		return errors.New("got an invalid IAM format version")
-
 	}
 
-	// Migrate long-term users
-	if err := ies.migrateUsersConfigToV1(ctx, false); err != nil {
+	if iamFmt.Version >= iamFormatVersion1 {
+		// Nothing to do.
+		return nil
+	}
+
+	if err := ies.migrateUsersConfigToV1(ctx); err != nil {
 		logger.LogIf(ctx, err)
 		return err
 	}
-	// Migrate STS users
-	if err := ies.migrateUsersConfigToV1(ctx, true); err != nil {
-		logger.LogIf(ctx, err)
-		return err
-	}
-	// Save iam version file.
+
+	// Save iam format to version 1.
 	if err := ies.saveIAMConfig(ctx, newIAMFormatVersion1(), path); err != nil {
 		logger.LogIf(ctx, err)
 		return err
 	}
+
 	return nil
 }
 
@@ -322,27 +313,6 @@ func (ies *IAMEtcdStore) addUser(ctx context.Context, user string, userType IAMU
 		deleteKeyEtcd(ctx, ies.client, getMappedPolicyPath(user, userType, false))
 		return nil
 	}
-
-	// If this is a service account, rotate the session key if we are changing the server creds
-	if globalOldCred.IsValid() && u.Credentials.IsServiceAccount() {
-		if !globalOldCred.Equal(globalActiveCred) {
-			m := jwtgo.MapClaims{}
-			stsTokenCallback := func(t *jwtgo.Token) (interface{}, error) {
-				return []byte(globalOldCred.SecretKey), nil
-			}
-			if _, err := jwtgo.ParseWithClaims(u.Credentials.SessionToken, m, stsTokenCallback); err == nil {
-				jwt := jwtgo.NewWithClaims(jwtgo.SigningMethodHS512, m)
-				if token, err := jwt.SignedString([]byte(globalActiveCred.SecretKey)); err == nil {
-					u.Credentials.SessionToken = token
-					err := ies.saveIAMConfig(ctx, &u, getUserIdentityPath(user, userType))
-					if err != nil {
-						return err
-					}
-				}
-			}
-		}
-	}
-
 	if u.Credentials.AccessKey == "" {
 		u.Credentials.AccessKey = user
 	}

cmd/iam.go

@@ -668,8 +668,6 @@ func (sys *IAMSys) Init(ctx context.Context, objAPI ObjectLayer) {
 		break
 	}
 
-	// Invalidate the old cred always, even upon error to avoid any leakage.
-	globalOldCred = auth.Credentials{}
 	go sys.store.watch(ctx, sys)
 
 	logger.Info("IAM initialization complete")

cmd/server-main.go

@@ -548,6 +548,9 @@ func serverMain(ctx *cli.Context) {
 		logger.LogIf(GlobalContext, err)
 	}
 
+	// Initialize users credentials and policies in background right after config has initialized.
+	go globalIAMSys.Init(GlobalContext, newObject)
+
 	initDataScanner(GlobalContext, newObject)
 
 	if globalIsErasure { // to be done after config init
@@ -567,9 +570,6 @@ func serverMain(ctx *cli.Context) {
 		setCacheObjectLayer(cacheAPI)
 	}
 
-	// Initialize users credentials and policies in background right after config has initialized.
-	go globalIAMSys.Init(GlobalContext, newObject)
-
 	// Prints the formatted startup message, if err is not nil then it prints additional information as well.
 	printStartupMessage(getAPIEndpoints(), err)