From 36b2f6d11d47496bdcacc425581ce4d433937c6b Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Fri, 4 Jun 2021 11:15:13 -0700 Subject: [PATCH] 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 | 53 +++++++++++++++++--------------- cmd/gateway-main.go | 8 ----- cmd/globals.go | 3 -- cmd/iam-etcd-store.go | 68 ++++++++++++----------------------------- cmd/iam.go | 2 -- cmd/server-main.go | 6 ++-- 6 files changed, 50 insertions(+), 90 deletions(-) diff --git a/cmd/config-encrypted.go b/cmd/config-encrypted.go index c09552b11..81ecaa692 100644 --- a/cmd/config-encrypted.go +++ b/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) } diff --git a/cmd/gateway-main.go b/cmd/gateway-main.go index 5921186e9..c63ce8929 100644 --- a/cmd/gateway-main.go +++ b/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) diff --git a/cmd/globals.go b/cmd/globals.go index c7e66708c..a302a4143 100644 --- a/cmd/globals.go +++ b/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 diff --git a/cmd/iam-etcd-store.go b/cmd/iam-etcd-store.go index 0ec58563f..bd70e253c 100644 --- a/cmd/iam-etcd-store.go +++ b/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 } diff --git a/cmd/iam.go b/cmd/iam.go index 3f0c4d1a6..97964d65c 100644 --- a/cmd/iam.go +++ b/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") diff --git a/cmd/server-main.go b/cmd/server-main.go index 3d5e9cd27..adec098c6 100644 --- a/cmd/server-main.go +++ b/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)