From 113c7ff49a99bc967f7c3fc94408d32094e85280 Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Mon, 13 Dec 2021 18:23:31 -0800 Subject: [PATCH] add code to parse secrets natively instead of shell scripts (#13883) --- Dockerfile | 1 + Dockerfile.dev | 1 + cmd/common-main.go | 163 +++++++++++++++++++++++++++-- cmd/common-main_test.go | 93 ++++++++++++++++ dockerscripts/docker-entrypoint.sh | 87 --------------- internal/config/constants.go | 27 +++-- 6 files changed, 271 insertions(+), 101 deletions(-) create mode 100644 cmd/common-main_test.go diff --git a/Dockerfile b/Dockerfile index 3db2c622e..68b4bb6c8 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,6 +1,7 @@ FROM minio/minio:latest COPY ./minio /opt/bin/minio +COPY dockerscripts/docker-entrypoint.sh /usr/bin/docker-entrypoint.sh ENTRYPOINT ["/usr/bin/docker-entrypoint.sh"] diff --git a/Dockerfile.dev b/Dockerfile.dev index 3db2c622e..68b4bb6c8 100644 --- a/Dockerfile.dev +++ b/Dockerfile.dev @@ -1,6 +1,7 @@ FROM minio/minio:latest COPY ./minio /opt/bin/minio +COPY dockerscripts/docker-entrypoint.sh /usr/bin/docker-entrypoint.sh ENTRYPOINT ["/usr/bin/docker-entrypoint.sh"] diff --git a/cmd/common-main.go b/cmd/common-main.go index 125b976a3..6903fce36 100644 --- a/cmd/common-main.go +++ b/cmd/common-main.go @@ -18,12 +18,14 @@ package cmd import ( + "bufio" "context" "crypto/tls" "crypto/x509" "encoding/gob" "errors" "fmt" + "io/ioutil" "math/rand" "net" "net/http" @@ -433,7 +435,153 @@ func handleCommonCmdArgs(ctx *cli.Context) { logger.FatalIf(mkdirAllIgnorePerm(globalCertsCADir.Get()), "Unable to create certs CA directory at %s", globalCertsCADir.Get()) } +type envKV struct { + Key string + Value string + Skip bool +} + +func (e envKV) String() string { + if e.Skip { + return "" + } + return fmt.Sprintf("%s=%s", e.Key, e.Value) +} + +func parsEnvEntry(envEntry string) (envKV, error) { + if strings.TrimSpace(envEntry) == "" { + // Skip all empty lines + return envKV{ + Skip: true, + }, nil + } + const envSeparator = "=" + envTokens := strings.SplitN(strings.TrimSpace(strings.TrimPrefix(envEntry, "export")), envSeparator, 2) + if len(envTokens) != 2 { + return envKV{}, fmt.Errorf("envEntry malformed; %s, expected to be of form 'KEY=value'", envEntry) + } + return envKV{ + Key: envTokens[0], + Value: envTokens[1], + }, nil +} + +// Similar to os.Environ returns a copy of strings representing +// the environment values from a file, in the form "key, value". +// in a structured form. +func minioEnvironFromFile(envConfigFile string) ([]envKV, error) { + f, err := os.Open(envConfigFile) + if err != nil { + if os.IsNotExist(err) { // ignore if file doesn't exist. + return nil, nil + } + return nil, err + } + defer f.Close() + var ekvs []envKV + scanner := bufio.NewScanner(f) + for scanner.Scan() { + ekv, err := parsEnvEntry(scanner.Text()) + if err != nil { + return nil, err + } + if ekv.Skip { + // Skips empty lines + continue + } + ekvs = append(ekvs, ekv) + } + if err = scanner.Err(); err != nil { + return nil, err + } + return ekvs, nil +} + +func readFromSecret(sp string) (string, error) { + // Supports reading path from docker secrets, filename is + // relative to /run/secrets/ position. + if isFile(pathJoin("/run/secrets/", sp)) { + sp = pathJoin("/run/secrets/", sp) + } + credBuf, err := ioutil.ReadFile(sp) + if err != nil { + if os.IsNotExist(err) { // ignore if file doesn't exist. + return "", nil + } + return "", err + } + return string(credBuf), nil +} + +func loadEnvVarsFromFiles() { + if env.IsSet(config.EnvAccessKeyFile) { + accessKey, err := readFromSecret(env.Get(config.EnvAccessKeyFile, "")) + if err != nil { + logger.Fatal(config.ErrInvalidCredentials(err), + "Unable to validate credentials inherited from the secret file(s)") + } + if accessKey != "" { + os.Setenv(config.EnvRootUser, accessKey) + } + } + + if env.IsSet(config.EnvSecretKeyFile) { + secretKey, err := readFromSecret(env.Get(config.EnvSecretKeyFile, "")) + if err != nil { + logger.Fatal(config.ErrInvalidCredentials(err), + "Unable to validate credentials inherited from the secret file(s)") + } + if secretKey != "" { + os.Setenv(config.EnvRootPassword, secretKey) + } + } + + if env.IsSet(config.EnvRootUserFile) { + rootUser, err := readFromSecret(env.Get(config.EnvRootUserFile, "")) + if err != nil { + logger.Fatal(config.ErrInvalidCredentials(err), + "Unable to validate credentials inherited from the secret file(s)") + } + if rootUser != "" { + os.Setenv(config.EnvRootUser, rootUser) + } + } + + if env.IsSet(config.EnvRootPasswordFile) { + rootPassword, err := readFromSecret(env.Get(config.EnvRootPasswordFile, "")) + if err != nil { + logger.Fatal(config.ErrInvalidCredentials(err), + "Unable to validate credentials inherited from the secret file(s)") + } + if rootPassword != "" { + os.Setenv(config.EnvRootPassword, rootPassword) + } + } + + if env.IsSet(config.EnvKMSSecretKeyFile) { + kmsSecret, err := readFromSecret(env.Get(config.EnvKMSSecretKeyFile, "")) + if err != nil { + logger.Fatal(err, "Unable to read the KMS secret key inherited from secret file") + } + if kmsSecret != "" { + os.Setenv(config.EnvKMSSecretKey, kmsSecret) + } + } + + if env.IsSet(config.EnvConfigEnvFile) { + ekvs, err := minioEnvironFromFile(env.Get(config.EnvConfigEnvFile, "")) + if err != nil { + logger.Fatal(err, "Unable to read the config environment file") + } + for _, ekv := range ekvs { + os.Setenv(ekv.Key, ekv.Value) + } + } +} + func handleCommonEnvVars() { + loadEnvVarsFromFiles() + var err error globalBrowserEnabled, err = config.ParseBool(env.Get(config.EnvBrowser, config.EnableOn)) if err != nil { @@ -530,8 +678,8 @@ func handleCommonEnvVars() { // in-place update is off. globalInplaceUpdateDisabled = strings.EqualFold(env.Get(config.EnvUpdate, config.EnableOn), config.EnableOff) - // Check if the supported credential env vars, "MINIO_ROOT_USER" and - // "MINIO_ROOT_PASSWORD" are provided + // Check if the supported credential env vars, + // "MINIO_ROOT_USER" and "MINIO_ROOT_PASSWORD" are provided // Warn user if deprecated environment variables, // "MINIO_ACCESS_KEY" and "MINIO_SECRET_KEY", are defined // Check all error conditions first @@ -552,25 +700,24 @@ func handleCommonEnvVars() { // are defined or both are not defined. // Check both cases and authenticate them if correctly defined var user, password string - haveRootCredentials := false - haveAccessCredentials := false + var hasCredentials bool //nolint:gocritic if env.IsSet(config.EnvRootUser) && env.IsSet(config.EnvRootPassword) { user = env.Get(config.EnvRootUser, "") password = env.Get(config.EnvRootPassword, "") - haveRootCredentials = true + hasCredentials = true } else if env.IsSet(config.EnvAccessKey) && env.IsSet(config.EnvSecretKey) { user = env.Get(config.EnvAccessKey, "") password = env.Get(config.EnvSecretKey, "") - haveAccessCredentials = true + hasCredentials = true } - if haveRootCredentials || haveAccessCredentials { + if hasCredentials { cred, err := auth.CreateCredentials(user, password) if err != nil { logger.Fatal(config.ErrInvalidCredentials(err), "Unable to validate credentials inherited from the shell environment") } - if haveAccessCredentials { + 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, diff --git a/cmd/common-main_test.go b/cmd/common-main_test.go new file mode 100644 index 000000000..b22abc6a6 --- /dev/null +++ b/cmd/common-main_test.go @@ -0,0 +1,93 @@ +// Copyright (c) 2015-2021 MinIO, Inc. +// +// This file is part of MinIO Object Storage stack +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +package cmd + +import ( + "errors" + "io/ioutil" + "reflect" + "testing" +) + +func Test_minioEnvironFromFile(t *testing.T) { + testCases := []struct { + content string + expectedErr bool + expectedEkvs []envKV + }{ + {` +export MINIO_ROOT_USER=minio +export MINIO_ROOT_PASSWORD=minio123`, + false, + []envKV{ + { + Key: "MINIO_ROOT_USER", + Value: "minio", + }, + { + Key: "MINIO_ROOT_PASSWORD", + Value: "minio123", + }, + }, + }, + {` +MINIO_ROOT_USER=minio +MINIO_ROOT_PASSWORD=minio123`, + false, + []envKV{ + { + Key: "MINIO_ROOT_USER", + Value: "minio", + }, + { + Key: "MINIO_ROOT_PASSWORD", + Value: "minio123", + }, + }, + }, + {` +export MINIO_ROOT_USERminio +export MINIO_ROOT_PASSWORD=minio123`, + true, + nil, + }, + } + for _, testCase := range testCases { + testCase := testCase + t.Run("", func(t *testing.T) { + tmpfile, err := ioutil.TempFile("", "testfile") + if err != nil { + t.Error(err) + } + tmpfile.WriteString(testCase.content) + tmpfile.Sync() + tmpfile.Close() + + ekvs, err := minioEnvironFromFile(tmpfile.Name()) + if err != nil && !testCase.expectedErr { + t.Error(err) + } + if err == nil && testCase.expectedErr { + t.Error(errors.New("expected error, found success")) + } + if !reflect.DeepEqual(ekvs, testCase.expectedEkvs) { + t.Errorf("expected %v, got %v", testCase.expectedEkvs, ekvs) + } + }) + } +} diff --git a/dockerscripts/docker-entrypoint.sh b/dockerscripts/docker-entrypoint.sh index 15317d366..e7dd5ea1e 100755 --- a/dockerscripts/docker-entrypoint.sh +++ b/dockerscripts/docker-entrypoint.sh @@ -8,79 +8,6 @@ if [ "${1}" != "minio" ]; then fi fi -## look for specific a `config.env` file to load all the -## minio settings from -docker_minio_env() { - if [ -f "$MINIO_CONFIG_ENV_FILE" ]; then - config_env_file="${MINIO_CONFIG_ENV_FILE}" - else - config_env_file="/run/secrets/${MINIO_CONFIG_ENV_FILE}" - fi - if [ -f "$config_env_file" ]; then - # shellcheck source=/dev/null - . "${config_env_file}" - fi -} - -## Look for docker secrets at given absolute path or in default documented location. -docker_secrets_env_old() { - if [ -f "$MINIO_ACCESS_KEY_FILE" ]; then - ACCESS_KEY_FILE="$MINIO_ACCESS_KEY_FILE" - else - ACCESS_KEY_FILE="/run/secrets/$MINIO_ACCESS_KEY_FILE" - fi - if [ -f "$MINIO_SECRET_KEY_FILE" ]; then - SECRET_KEY_FILE="$MINIO_SECRET_KEY_FILE" - else - SECRET_KEY_FILE="/run/secrets/$MINIO_SECRET_KEY_FILE" - fi - - if [ -f "$ACCESS_KEY_FILE" ]; then - MINIO_ACCESS_KEY="$(cat "$ACCESS_KEY_FILE")" - export MINIO_ACCESS_KEY - fi - if [ -f "$SECRET_KEY_FILE" ]; then - MINIO_SECRET_KEY="$(cat "$SECRET_KEY_FILE")" - export MINIO_SECRET_KEY - fi -} - -docker_secrets_env() { - if [ -f "$MINIO_ROOT_USER_FILE" ]; then - ROOT_USER_FILE="$MINIO_ROOT_USER_FILE" - else - ROOT_USER_FILE="/run/secrets/$MINIO_ROOT_USER_FILE" - fi - if [ -f "$MINIO_ROOT_PASSWORD_FILE" ]; then - ROOT_PASSWORD_FILE="$MINIO_ROOT_PASSWORD_FILE" - else - ROOT_PASSWORD_FILE="/run/secrets/$MINIO_ROOT_PASSWORD_FILE" - fi - - if [ -f "$ROOT_USER_FILE" ]; then - MINIO_ROOT_USER="$(cat "$ROOT_USER_FILE")" - export MINIO_ROOT_USER - fi - if [ -f "$ROOT_PASSWORD_FILE" ]; then - MINIO_ROOT_PASSWORD="$(cat "$ROOT_PASSWORD_FILE")" - export MINIO_ROOT_PASSWORD - fi -} - -## Set KMS_SECRET_KEY from docker secrets if provided -docker_kms_secret_encryption_env() { - if [ -f "$MINIO_KMS_SECRET_KEY_FILE" ]; then - KMS_SECRET_KEY_FILE="$MINIO_KMS_SECRET_KEY_FILE" - else - KMS_SECRET_KEY_FILE="/run/secrets/$MINIO_KMS_SECRET_KEY_FILE" - fi - - if [ -f "$KMS_SECRET_KEY_FILE" ]; then - MINIO_KMS_SECRET_KEY="$(cat "$KMS_SECRET_KEY_FILE")" - export MINIO_KMS_SECRET_KEY - fi -} - # su-exec to requested user, if service cannot run exec will fail. docker_switch_user() { if [ -n "${MINIO_USERNAME}" ] && [ -n "${MINIO_GROUPNAME}" ]; then @@ -98,19 +25,5 @@ docker_switch_user() { fi } -## Set access env from secrets if necessary. Legacy -docker_secrets_env_old - -## Set access env from secrets if necessary. Override -docker_secrets_env - -## Set kms encryption from secrets if necessary. Override -docker_kms_secret_encryption_env - -## Set all config environment variables from 'config.env' if necessary. -## Overrides all previous settings and also overrides all -## environment values passed from 'podman run -e ENV=value' -docker_minio_env - ## Switch to user if applicable. docker_switch_user "$@" diff --git a/internal/config/constants.go b/internal/config/constants.go index 7b87d9a75..af06452f9 100644 --- a/internal/config/constants.go +++ b/internal/config/constants.go @@ -29,6 +29,20 @@ const ( EnvRootUser = "MINIO_ROOT_USER" EnvRootPassword = "MINIO_ROOT_PASSWORD" + // Legacy files + EnvAccessKeyFile = "MINIO_ACCESS_KEY_FILE" + EnvSecretKeyFile = "MINIO_SECRET_KEY_FILE" + + // Current files + EnvRootUserFile = "MINIO_ROOT_USER_FILE" + EnvRootPasswordFile = "MINIO_ROOT_PASSWORD_FILE" + + // Set all config environment variables from 'config.env' + // if necessary. Overrides all previous settings and also + // overrides all environment values passed from + // 'podman run -e ENV=value' + EnvConfigEnvFile = "MINIO_CONFIG_ENV_FILE" + EnvBrowser = "MINIO_BROWSER" EnvDomain = "MINIO_DOMAIN" EnvPublicIPs = "MINIO_PUBLIC_IPS" @@ -47,12 +61,13 @@ const ( EnvUpdate = "MINIO_UPDATE" - EnvKMSSecretKey = "MINIO_KMS_SECRET_KEY" - EnvKESEndpoint = "MINIO_KMS_KES_ENDPOINT" - EnvKESKeyName = "MINIO_KMS_KES_KEY_NAME" - EnvKESClientKey = "MINIO_KMS_KES_KEY_FILE" - EnvKESClientCert = "MINIO_KMS_KES_CERT_FILE" - EnvKESServerCA = "MINIO_KMS_KES_CAPATH" + EnvKMSSecretKey = "MINIO_KMS_SECRET_KEY" + EnvKMSSecretKeyFile = "MINIO_KMS_SECRET_KEY_FILE" + EnvKESEndpoint = "MINIO_KMS_KES_ENDPOINT" + EnvKESKeyName = "MINIO_KMS_KES_KEY_NAME" + EnvKESClientKey = "MINIO_KMS_KES_KEY_FILE" + EnvKESClientCert = "MINIO_KMS_KES_CERT_FILE" + EnvKESServerCA = "MINIO_KMS_KES_CAPATH" EnvEndpoints = "MINIO_ENDPOINTS" // legacy EnvWorm = "MINIO_WORM" // legacy