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

@@ -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)

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

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))
+				}
+			}
+		})
+	}
+}

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

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
 

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, ""),

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.

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