From aa04f97f9594443242069a6e8c96f514ac28fd10 Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Mon, 11 Nov 2019 12:01:21 -0800 Subject: [PATCH] Config migration should handle plain-text (#8506) This PR fixes issues found in config migration - StorageClass migration error when rrs is empty - Plain-text migration of older config - Do not run in safe mode with incorrect credentials - Update logger_http documentation for _STATE env Refer more reported issues at #8434 --- cmd/admin-handlers-config-kv.go | 12 ++-- cmd/config-encrypted.go | 85 +++++++++++++----------- cmd/config-encrypted_test.go | 75 +++++++++++++++++++++ cmd/config/storageclass/storage-class.go | 14 ++-- cmd/gateway/s3/gateway-s3.go | 2 + cmd/logger/config.go | 4 +- cmd/server-main.go | 14 ++-- docs/logging/README.md | 2 + 8 files changed, 153 insertions(+), 55 deletions(-) create mode 100644 cmd/config-encrypted_test.go diff --git a/cmd/admin-handlers-config-kv.go b/cmd/admin-handlers-config-kv.go index 2ec085ac1..c65a13d8a 100644 --- a/cmd/admin-handlers-config-kv.go +++ b/cmd/admin-handlers-config-kv.go @@ -195,10 +195,14 @@ func (a adminAPIHandlers) GetConfigKVHandler(w http.ResponseWriter, r *http.Requ return } - cfg, err := getValidConfig(objectAPI) - if err != nil { - writeErrorResponseJSON(ctx, w, toAdminAPIErr(ctx, err), r.URL) - return + cfg := globalServerConfig + if globalSafeMode { + var err error + cfg, err = getValidConfig(objectAPI) + if err != nil { + writeErrorResponseJSON(ctx, w, toAdminAPIErr(ctx, err), r.URL) + return + } } vars := mux.Vars(r) diff --git a/cmd/config-encrypted.go b/cmd/config-encrypted.go index 47f24b6b3..0d05e445b 100644 --- a/cmd/config-encrypted.go +++ b/cmd/config-encrypted.go @@ -91,16 +91,9 @@ func handleEncryptedConfigBackend(objAPI ObjectLayer, server bool) error { } } - accessKeyOld := env.Get(config.EnvAccessKeyOld, "") - secretKeyOld := env.Get(config.EnvSecretKeyOld, "") - var activeCredOld auth.Credentials - if accessKeyOld != "" && secretKeyOld != "" { - activeCredOld, err = auth.CreateCredentials(accessKeyOld, secretKeyOld) - if err != nil { - return err - } - os.Unsetenv(config.EnvAccessKeyOld) - os.Unsetenv(config.EnvSecretKeyOld) + activeCredOld, err := getOldCreds() + if err != nil { + return err } // Migrating Config backend needs a retry mechanism for @@ -148,6 +141,38 @@ func checkBackendEncrypted(objAPI ObjectLayer) (bool, error) { return err == nil && bytes.Equal(data, backendEncryptedMigrationComplete), nil } +// decryptData - decrypts input data with more that one credentials, +func decryptData(edata []byte, creds ...auth.Credentials) ([]byte, error) { + var err error + var data []byte + for _, cred := range creds { + data, err = madmin.DecryptData(cred.String(), bytes.NewReader(edata)) + if err != nil { + if err == madmin.ErrMaliciousData { + continue + } + return nil, err + } + break + } + return data, err +} + +func getOldCreds() (activeCredOld auth.Credentials, err error) { + accessKeyOld := env.Get(config.EnvAccessKeyOld, "") + secretKeyOld := env.Get(config.EnvSecretKeyOld, "") + if accessKeyOld != "" && secretKeyOld != "" { + activeCredOld, err = auth.CreateCredentials(accessKeyOld, secretKeyOld) + if err != nil { + return activeCredOld, err + } + // Once we have obtained the rotating creds + os.Unsetenv(config.EnvAccessKeyOld) + os.Unsetenv(config.EnvSecretKeyOld) + } + return activeCredOld, nil +} + func migrateIAMConfigsEtcdToEncrypted(client *etcd.Client) error { ctx, cancel := context.WithTimeout(context.Background(), defaultContextTimeout) defer cancel() @@ -175,17 +200,9 @@ func migrateIAMConfigsEtcdToEncrypted(client *etcd.Client) error { } } - accessKeyOld := env.Get(config.EnvAccessKeyOld, "") - secretKeyOld := env.Get(config.EnvSecretKeyOld, "") - var activeCredOld auth.Credentials - if accessKeyOld != "" && secretKeyOld != "" { - activeCredOld, err = auth.CreateCredentials(accessKeyOld, secretKeyOld) - if err != nil { - return err - } - // Once we have obtained the rotating creds - os.Unsetenv(config.EnvAccessKeyOld) - os.Unsetenv(config.EnvSecretKeyOld) + activeCredOld, err := getOldCreds() + if err != nil { + return err } if encrypted { @@ -232,18 +249,15 @@ func migrateIAMConfigsEtcdToEncrypted(client *etcd.Client) error { var data []byte // Is rotating of creds requested? if activeCredOld.IsValid() { - data, err = madmin.DecryptData(activeCredOld.String(), bytes.NewReader(cdata)) + data, err = decryptData(cdata, activeCredOld, globalActiveCred) if err != nil { if err == madmin.ErrMaliciousData { - data, err = madmin.DecryptData(globalActiveCred.String(), - bytes.NewReader(cdata)) - if err != nil { - return config.ErrInvalidRotatingCredentialsBackendEncrypted(nil) - } - } else { - return err + return config.ErrInvalidRotatingCredentialsBackendEncrypted(nil) } + return err } + } else { + data = cdata } // Attempt to unmarshal JSON content @@ -310,18 +324,15 @@ func migrateConfigPrefixToEncrypted(objAPI ObjectLayer, activeCredOld auth.Crede var data []byte // Is rotating of creds requested? if activeCredOld.IsValid() { - data, err = madmin.DecryptData(activeCredOld.String(), bytes.NewReader(cdata)) + data, err = decryptData(cdata, activeCredOld, globalActiveCred) if err != nil { if err == madmin.ErrMaliciousData { - data, err = madmin.DecryptData(globalActiveCred.String(), - bytes.NewReader(cdata)) - if err != nil { - return config.ErrInvalidRotatingCredentialsBackendEncrypted(nil) - } - } else { - return err + return config.ErrInvalidRotatingCredentialsBackendEncrypted(nil) } + return err } + } else { + data = cdata } // Attempt to unmarshal JSON content diff --git a/cmd/config-encrypted_test.go b/cmd/config-encrypted_test.go new file mode 100644 index 000000000..860e16822 --- /dev/null +++ b/cmd/config-encrypted_test.go @@ -0,0 +1,75 @@ +/* + * MinIO Cloud Storage, (C) 2019 MinIO, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package cmd + +import ( + "bytes" + "testing" + + "github.com/minio/minio/pkg/auth" + "github.com/minio/minio/pkg/madmin" +) + +func TestDecryptData(t *testing.T) { + cred1 := auth.Credentials{ + AccessKey: "minio", + SecretKey: "minio123", + } + + cred2 := auth.Credentials{ + AccessKey: "minio", + SecretKey: "minio1234", + } + + data := []byte(`config data`) + edata1, err := madmin.EncryptData(cred1.String(), data) + if err != nil { + t.Fatal(err) + } + + edata2, err := madmin.EncryptData(cred2.String(), data) + if err != nil { + t.Fatal(err) + } + + tests := []struct { + edata []byte + creds []auth.Credentials + success bool + }{ + {edata1, []auth.Credentials{cred1, cred2}, true}, + {edata2, []auth.Credentials{cred1, cred2}, true}, + {data, []auth.Credentials{cred1, cred2}, false}, + } + + for _, test := range tests { + t.Run("", func(t *testing.T) { + ddata, err := decryptData(test.edata, test.creds...) + if err != nil && test.success { + t.Errorf("Expected success, saw failure %v", err) + } + if err == nil && !test.success { + t.Error("Expected failure, saw success") + } + if test.success { + if !bytes.Equal(ddata, data) { + t.Errorf("Expected %s, got %s", string(data), string(ddata)) + } + } + }) + } +} diff --git a/cmd/config/storageclass/storage-class.go b/cmd/config/storageclass/storage-class.go index 5d42ab71a..cc6c285b5 100644 --- a/cmd/config/storageclass/storage-class.go +++ b/cmd/config/storageclass/storage-class.go @@ -224,17 +224,17 @@ func LookupConfig(kvs config.KVS, drivesPerSet int) (cfg Config, err error) { if err != nil { return cfg, err } + ssc := env.Get(StandardEnv, kvs.Get(ClassStandard)) + rrsc := env.Get(RRSEnv, kvs.Get(ClassRRS)) if stateBool { - if ssc := env.Get(StandardEnv, kvs.Get(ClassStandard)); ssc == "" { - return cfg, config.Error("'standard' key cannot be empty if you wish to enable storage class") - } - if rrsc := env.Get(RRSEnv, kvs.Get(ClassRRS)); rrsc == "" { - return cfg, config.Error("'rrs' key cannot be empty if you wish to enable storage class") + if ssc == "" && rrsc == "" { + return cfg, config.Error("'standard' and 'rrs' key cannot be empty for enabled storage class") } + // if one of storage class is not empty proceed. } // Check for environment variables and parse into storageClass struct - if ssc := env.Get(StandardEnv, kvs.Get(ClassStandard)); ssc != "" { + if ssc != "" { cfg.Standard, err = parseStorageClass(ssc) if err != nil { return cfg, err @@ -244,7 +244,7 @@ func LookupConfig(kvs config.KVS, drivesPerSet int) (cfg Config, err error) { cfg.Standard.Parity = drivesPerSet / 2 } - if rrsc := env.Get(RRSEnv, kvs.Get(ClassRRS)); rrsc != "" { + if rrsc != "" { cfg.RRS, err = parseStorageClass(rrsc) if err != nil { return cfg, err diff --git a/cmd/gateway/s3/gateway-s3.go b/cmd/gateway/s3/gateway-s3.go index 906439936..a4b856989 100644 --- a/cmd/gateway/s3/gateway-s3.go +++ b/cmd/gateway/s3/gateway-s3.go @@ -73,6 +73,7 @@ ENVIRONMENT VARIABLES: MINIO_CACHE_QUOTA: Maximum permitted usage of the cache in percentage (0-100). LOGGER: + MINIO_LOGGER_HTTP_STATE: Set this to "on" to enable HTTP logging target. MINIO_LOGGER_HTTP_ENDPOINT: HTTP endpoint URL to log all incoming requests. EXAMPLES: @@ -89,6 +90,7 @@ EXAMPLES: 3. Start minio gateway server for AWS S3 backend logging all requests to http endpoint. {{.Prompt}} {{.EnvVarSetCommand}} MINIO_ACCESS_KEY{{.AssignmentOperator}}Q3AM3UQ867SPQQA43P2F {{.Prompt}} {{.EnvVarSetCommand}} MINIO_SECRET_KEY{{.AssignmentOperator}}zuf+tfteSlswRu7BJ86wekitnifILbZam1KYY3TG + {{.Prompt}} {{.EnvVarSetCommand}} MINIO_LOGGER_HTTP_STATE{{.AssignmenOperator}}"on" {{.Prompt}} {{.EnvVarSetCommand}} MINIO_LOGGER_HTTP_ENDPOINT{{.AssignmentOperator}}"http://localhost:8000/" {{.Prompt}} {{.HelpName}} https://play.min.io:9000 diff --git a/cmd/logger/config.go b/cmd/logger/config.go index 4493edfa9..978a7dcc9 100644 --- a/cmd/logger/config.go +++ b/cmd/logger/config.go @@ -131,7 +131,7 @@ func LookupConfig(scfg config.Config) (Config, error) { for starget, kv := range scfg[config.LoggerHTTPSubSys] { subSysTarget := config.LoggerHTTPSubSys if starget != config.Default { - subSysTarget = config.LoggerHTTPSubSys + ":" + starget + subSysTarget = config.LoggerHTTPSubSys + config.SubSystemSeparator + starget } if err := config.CheckValidKeys(subSysTarget, kv, DefaultKVS); err != nil { return cfg, err @@ -233,7 +233,7 @@ func LookupConfig(scfg config.Config) (Config, error) { if target != config.Default { authTokenEnv = EnvLoggerHTTPAuditAuthToken + config.Default + target } - cfg.HTTP[target] = HTTP{ + cfg.Audit[target] = HTTP{ Enabled: true, Endpoint: endpoint, AuthToken: env.Get(authTokenEnv, ""), diff --git a/cmd/server-main.go b/cmd/server-main.go index 5fcfa8b29..46800adb1 100644 --- a/cmd/server-main.go +++ b/cmd/server-main.go @@ -200,6 +200,10 @@ func newAllSubsystems() { func initSafeModeInit(buckets []BucketInfo) (err error) { defer func() { if err != nil { + switch err.(type) { + case config.Err: + return + } // Enable logger logger.Disable = false @@ -246,25 +250,25 @@ func initSafeModeInit(buckets []BucketInfo) (err error) { func initAllSubsystems(buckets []BucketInfo, newObject ObjectLayer) (err error) { // Initialize config system. if err = globalConfigSys.Init(newObject); err != nil { - return fmt.Errorf("Unable to initialize config system: %v", err) + return fmt.Errorf("Unable to initialize config system: %w", err) } if err = globalNotificationSys.AddNotificationTargetsFromConfig(globalServerConfig); err != nil { - return fmt.Errorf("Unable to initialize notification target(s) from config: %v", err) + return fmt.Errorf("Unable to initialize notification target(s) from config: %w", err) } if err = globalIAMSys.Init(newObject); err != nil { - return fmt.Errorf("Unable to initialize IAM system: %v", err) + return fmt.Errorf("Unable to initialize IAM system: %w", err) } // Initialize notification system. if err = globalNotificationSys.Init(buckets, newObject); err != nil { - return fmt.Errorf("Unable to initialize notification system: %v", err) + return fmt.Errorf("Unable to initialize notification system: %w", err) } // Initialize policy system. if err = globalPolicySys.Init(buckets, newObject); err != nil { - return fmt.Errorf("Unable to initialize policy system; %v", err) + return fmt.Errorf("Unable to initialize policy system; %w", err) } // Initialize lifecycle system. diff --git a/docs/logging/README.md b/docs/logging/README.md index 93f427431..daa252c8d 100644 --- a/docs/logging/README.md +++ b/docs/logging/README.md @@ -28,6 +28,7 @@ NOTE: `http://endpoint:port/path` is a placeholder value to indicate the URL for MinIO also honors environment variable for HTTP target logging as shown below, this setting will override the endpoint settings in the MinIO server config. ``` +export MINIO_LOGGER_HTTP_STATE_target1="on" export MINIO_LOGGER_HTTP_AUTH_TOKEN_target1="token" export MINIO_LOGGER_HTTP_ENDPOINT_target1=http://localhost:8080/minio/logs minio server /mnt/data @@ -49,6 +50,7 @@ NOTE: `http://endpoint:port/path` is a placeholder value to indicate the URL for MinIO also honors environment variable for HTTP target Audit logging as shown below, this setting will override the endpoint settings in the MinIO server config. ``` +export MINIO_LOGGER_HTTP_AUDIT_STATE_target1="on" export MINIO_LOGGER_HTTP_AUDIT_AUTH_TOKEN_target1="token" export MINIO_LOGGER_HTTP_AUDIT_ENDPOINT_target1=http://localhost:8080/minio/logs minio server /mnt/data