From 3f1acfe8a709288b0e16d6e6dcc4235d6ad16e71 Mon Sep 17 00:00:00 2001 From: Zhengchao An Date: Sat, 11 Jul 2026 19:13:04 +0800 Subject: [PATCH] fix(docker): warn instead of rejecting default or missing credentials (#4728) * fix(docker): warn instead of rejecting default or missing credentials The entrypoint hard-reject from #4278 broke the container-first UX and the repo's own scheduled e2e lanes (e2e-s3tests, mint boot rustfs-ci images with default credentials and died at startup). Maintainer decision: ship no baked-in credentials, warn instead of block. - missing credentials: warn and start; wording accounts for the RUSTFS_ROOT_*/MINIO_* alias sources the entrypoint does not inspect - default rustfsadmin via env/CLI/file: warn and start; the warning notes all-default pairs cannot derive an internode RPC secret - malformed config stays fatal: source conflicts, unreadable files, empty or whitespace-only values, flags missing their argument - present-but-empty env vars now hit the empty-value hard failure instead of running the binary with an empty root credential - empty/default checks trim CR and blanks like the binary; files without a trailing newline are no longer falsely rejected as empty - the no-baked-credentials guard covers all four Dockerfiles, and the test harness refuses hosts where /usr/bin/rustfs exists - e2e-s3tests/mint move to non-default credentials (rustfsadmin-ci), which also restores RPC-secret derivation for the multi-node lane GHSA-68cw fail-closed RPC derivation (#4402) is untouched; helm stays fail-closed by design. * chore(docker): reword entrypoint comment flagged by typos check --- .github/workflows/e2e-s3tests.yml | 4 +- .github/workflows/mint.yml | 4 +- entrypoint.sh | 52 ++++-- scripts/test_entrypoint_credentials.sh | 222 +++++++++++++++++++++---- 4 files changed, 230 insertions(+), 52 deletions(-) diff --git a/.github/workflows/e2e-s3tests.yml b/.github/workflows/e2e-s3tests.yml index 2b3ac5f19..c4f02c4d1 100644 --- a/.github/workflows/e2e-s3tests.yml +++ b/.github/workflows/e2e-s3tests.yml @@ -85,8 +85,8 @@ on: env: # main user - S3_ACCESS_KEY: rustfsadmin - S3_SECRET_KEY: rustfsadmin + S3_ACCESS_KEY: rustfsadmin-ci + S3_SECRET_KEY: rustfssecret-ci # alt user (must be different from main for many s3-tests) S3_ALT_ACCESS_KEY: rustfsalt S3_ALT_SECRET_KEY: rustfsalt diff --git a/.github/workflows/mint.yml b/.github/workflows/mint.yml index 91e6801cb..71515b4ef 100644 --- a/.github/workflows/mint.yml +++ b/.github/workflows/mint.yml @@ -72,8 +72,8 @@ on: - cron: "0 6 * * 0" env: - S3_ACCESS_KEY: rustfsadmin - S3_SECRET_KEY: rustfsadmin + S3_ACCESS_KEY: rustfsadmin-ci + S3_SECRET_KEY: rustfssecret-ci S3_REGION: us-east-1 RUST_LOG: info diff --git a/entrypoint.sh b/entrypoint.sh index 3ce6c74e6..a302dcffa 100755 --- a/entrypoint.sh +++ b/entrypoint.sh @@ -35,15 +35,21 @@ resolve_credential_source() { CREDENTIAL_FILE_SET=false CREDENTIAL_FILE_VALUE="" + # ${VAR+x} distinguishes unset from present-but-empty: an env var explicitly + # set to "" (e.g. an unexpanded compose interpolation) is a malformed value + # that must hit the empty-value hard failure, not the missing-credential + # warning — the binary would otherwise run with an empty root credential. + eval "CREDENTIAL_ENV_PRESENT=\${$CREDENTIAL_ENV_NAME+x}" eval "CREDENTIAL_ENV_VALUE=\${$CREDENTIAL_ENV_NAME:-}" + eval "CREDENTIAL_ENV_FILE_PRESENT=\${$CREDENTIAL_FILE_ENV_NAME+x}" eval "CREDENTIAL_ENV_FILE_VALUE=\${$CREDENTIAL_FILE_ENV_NAME:-}" - if [ -n "$CREDENTIAL_ENV_VALUE" ]; then + if [ -n "$CREDENTIAL_ENV_PRESENT" ]; then CREDENTIAL_DIRECT_SET=true CREDENTIAL_DIRECT_VALUE="$CREDENTIAL_ENV_VALUE" fi - if [ -n "$CREDENTIAL_ENV_FILE_VALUE" ]; then + if [ -n "$CREDENTIAL_ENV_FILE_PRESENT" ]; then CREDENTIAL_FILE_SET=true CREDENTIAL_FILE_VALUE="$CREDENTIAL_ENV_FILE_VALUE" fi @@ -103,10 +109,26 @@ resolve_credential_source() { echo "missing" } +# Mirrors the binary's .trim() so the empty/default checks below agree with +# what the server will actually use: strips CR (CRLF-edited files) and +# surrounding blanks. Comparison only — the value passed to the binary is +# untouched. +trim_credential() { + printf '%s' "$1" | tr -d '\r' | sed 's/^[[:space:]]*//;s/[[:space:]]*$//' +} + +# Credential policy: images ship no baked-in credentials, but default or +# missing credentials only WARN — the container still starts (the binary falls +# back to its built-in default, and warns when both keys end up default). +# Hard failures (ERROR, exit 1) are reserved for malformed configuration +# (conflicting sources, unreadable files, empty values). The binary also accepts credentials via +# legacy/compat envs (RUSTFS_ROOT_*, MINIO_*) that this script does not +# inspect, so the missing-credential warning is phrased conditionally. validate_credential_source() { CREDENTIAL_NAME="$1" FILE_NAME="$2" - CREDENTIAL_SOURCE="$3" + ALIAS_HINT="$3" + CREDENTIAL_SOURCE="$4" case "$CREDENTIAL_SOURCE" in error:*) @@ -118,18 +140,16 @@ validate_credential_source() { exit 1 ;; missing) - echo "ERROR: $CREDENTIAL_NAME or $FILE_NAME must be set explicitly for container startup." >&2 - exit 1 + echo "WARNING: $CREDENTIAL_NAME or $FILE_NAME is not set; unless credentials are provided via another supported source (e.g. $ALIAS_HINT), rustfs falls back to its built-in default credential. Set non-default credentials for production deployments." >&2 ;; value:*) - CREDENTIAL_VALUE="${CREDENTIAL_SOURCE#value:}" + CREDENTIAL_VALUE=$(trim_credential "${CREDENTIAL_SOURCE#value:}") if [ -z "$CREDENTIAL_VALUE" ]; then echo "ERROR: $CREDENTIAL_NAME must not be empty." >&2 exit 1 fi if [ "$CREDENTIAL_VALUE" = "$DEFAULT_ROOT_CREDENTIAL" ]; then - echo "ERROR: $CREDENTIAL_NAME must not use the default $DEFAULT_ROOT_CREDENTIAL credential." >&2 - exit 1 + echo "WARNING: $CREDENTIAL_NAME uses the default $DEFAULT_ROOT_CREDENTIAL credential. Set non-default credentials for production deployments; with an all-default pair, multi-node clusters additionally need RUSTFS_RPC_SECRET to derive internode RPC auth." >&2 fi ;; file:*) @@ -142,18 +162,16 @@ validate_credential_source() { echo "ERROR: $FILE_NAME points to an unreadable file." >&2 exit 1 fi - if IFS= read -r CREDENTIAL_FILE_CONTENT < "$CREDENTIAL_FILE"; then - : - else - CREDENTIAL_FILE_CONTENT="" - fi + # `read` fails at EOF-without-newline but still fills the variable; + # keep the partial line so newline-less secret files stay valid. + IFS= read -r CREDENTIAL_FILE_CONTENT < "$CREDENTIAL_FILE" || : + CREDENTIAL_FILE_CONTENT=$(trim_credential "$CREDENTIAL_FILE_CONTENT") if [ -z "$CREDENTIAL_FILE_CONTENT" ]; then echo "ERROR: $FILE_NAME must not be empty." >&2 exit 1 fi if [ "$CREDENTIAL_FILE_CONTENT" = "$DEFAULT_ROOT_CREDENTIAL" ]; then - echo "ERROR: $FILE_NAME must not contain the default $DEFAULT_ROOT_CREDENTIAL credential." >&2 - exit 1 + echo "WARNING: $FILE_NAME contains the default $DEFAULT_ROOT_CREDENTIAL credential. Set non-default credentials for production deployments; with an all-default pair, multi-node clusters additionally need RUSTFS_RPC_SECRET to derive internode RPC auth." >&2 fi ;; esac @@ -162,8 +180,8 @@ validate_credential_source() { if [ "$1" = "/usr/bin/rustfs" ]; then ACCESS_SOURCE=$(resolve_credential_source "RUSTFS_ACCESS_KEY" "RUSTFS_ACCESS_KEY_FILE" "access-key" "access-key-file" "$@") SECRET_SOURCE=$(resolve_credential_source "RUSTFS_SECRET_KEY" "RUSTFS_SECRET_KEY_FILE" "secret-key" "secret-key-file" "$@") - validate_credential_source "RUSTFS_ACCESS_KEY" "RUSTFS_ACCESS_KEY_FILE" "$ACCESS_SOURCE" - validate_credential_source "RUSTFS_SECRET_KEY" "RUSTFS_SECRET_KEY_FILE" "$SECRET_SOURCE" + validate_credential_source "RUSTFS_ACCESS_KEY" "RUSTFS_ACCESS_KEY_FILE" "RUSTFS_ROOT_USER or MINIO_ROOT_USER" "$ACCESS_SOURCE" + validate_credential_source "RUSTFS_SECRET_KEY" "RUSTFS_SECRET_KEY_FILE" "RUSTFS_ROOT_PASSWORD or MINIO_ROOT_PASSWORD" "$SECRET_SOURCE" fi # 2) Process data volumes (separate from log directory) diff --git a/scripts/test_entrypoint_credentials.sh b/scripts/test_entrypoint_credentials.sh index b33399de5..7d3cc4e71 100755 --- a/scripts/test_entrypoint_credentials.sh +++ b/scripts/test_entrypoint_credentials.sh @@ -6,13 +6,56 @@ TMP_DIR=$(mktemp -d) trap 'rm -rf "$TMP_DIR"' EXIT ENTRYPOINT="$ROOT_DIR/entrypoint.sh" -DOCKERFILE_GLIBC="$ROOT_DIR/Dockerfile.glibc" -if grep -Eq 'RUSTFS_(ACCESS|SECRET)_KEY=' "$DOCKERFILE_GLIBC"; then - echo "Dockerfile.glibc must not bake root credentials into image ENV" >&2 +# Warn-path cases run the entrypoint through to `exec /usr/bin/rustfs`; on a +# host that actually has the binary they would start real servers and hang. +if [ -e /usr/bin/rustfs ]; then + echo "refusing to run: /usr/bin/rustfs exists on this host" >&2 exit 1 fi +# Every image that ships the entrypoint must not bake root credentials into +# its ENV — with hard-fail enforcement gone, this guard is the remaining +# teeth of the "no built-in default credentials" policy. +for dockerfile in Dockerfile Dockerfile.glibc Dockerfile.source Dockerfile.decommission-local; do + if grep -Eq 'RUSTFS_(ACCESS|SECRET)_KEY=' "$ROOT_DIR/$dockerfile"; then + echo "$dockerfile must not bake root credentials into image ENV" >&2 + exit 1 + fi +done + +assert_contains() { + if ! grep -q "$2" "$1"; then + echo "Expected ${1##*/} to contain: $2" >&2 + cat "$1" >&2 + exit 1 + fi +} + +assert_lacks() { + if grep -q "$2" "$1"; then + echo "Expected ${1##*/} to not contain: $2" >&2 + cat "$1" >&2 + exit 1 + fi +} + +# The test environment has no /usr/bin/rustfs, so a run that passes credential +# validation still exits non-zero at exec time. "Starting: /usr/bin/rustfs" on +# stdout is the proof that validation let startup proceed. +run_expect_start() { + label="$1" + expected="$2" + shift 2 + + log_file="$TMP_DIR/$label.log" + env -i PATH="$PATH" RUSTFS_VOLUMES="$TMP_DIR/data" RUSTFS_OBS_LOG_DIRECTORY= "$@" sh "$ENTRYPOINT" rustfs >"$log_file" 2>&1 || true + + assert_contains "$log_file" "$expected" + assert_contains "$log_file" "Starting: /usr/bin/rustfs" + assert_lacks "$log_file" "^ERROR:" +} + run_expect_failure() { label="$1" expected="$2" @@ -24,54 +67,171 @@ run_expect_failure() { exit 1 fi - if ! grep -q "$expected" "$log_file"; then - echo "Expected $label output to contain: $expected" >&2 - cat "$log_file" >&2 - exit 1 - fi + assert_contains "$log_file" "$expected" + assert_lacks "$log_file" "Starting: /usr/bin/rustfs" } -missing_log="$TMP_DIR/missing.log" -if env -i PATH="$PATH" RUSTFS_VOLUMES="$TMP_DIR/data" RUSTFS_OBS_LOG_DIRECTORY= sh "$ENTRYPOINT" >"$missing_log" 2>&1; then - echo "Expected missing credentials to fail" >&2 - exit 1 -fi -grep -q "RUSTFS_ACCESS_KEY or RUSTFS_ACCESS_KEY_FILE must be set explicitly" "$missing_log" +# CLI-flag variants cannot go through the env-pair helpers, so they run +# inline with the same assertions. +run_cli_expect_start() { + label="$1" + expected="$2" + shift 2 -run_expect_failure \ + log_file="$TMP_DIR/$label.log" + env -i PATH="$PATH" RUSTFS_VOLUMES="$TMP_DIR/data" RUSTFS_OBS_LOG_DIRECTORY= RUSTFS_SECRET_KEY=custom-secret sh "$ENTRYPOINT" rustfs "$@" >"$log_file" 2>&1 || true + + assert_contains "$log_file" "$expected" + assert_contains "$log_file" "Starting: /usr/bin/rustfs" + assert_lacks "$log_file" "^ERROR:" +} + +run_cli_expect_failure() { + label="$1" + expected="$2" + shift 2 + + log_file="$TMP_DIR/$label.log" + if env -i PATH="$PATH" RUSTFS_VOLUMES="$TMP_DIR/data" RUSTFS_OBS_LOG_DIRECTORY= RUSTFS_SECRET_KEY=custom-secret sh "$ENTRYPOINT" rustfs "$@" >"$log_file" 2>&1; then + echo "Expected $label to fail" >&2 + exit 1 + fi + + assert_contains "$log_file" "$expected" + assert_lacks "$log_file" "Starting: /usr/bin/rustfs" +} + +# Missing credentials warn for both keys but still start. +run_expect_start \ + "missing-both" \ + "WARNING: RUSTFS_ACCESS_KEY or RUSTFS_ACCESS_KEY_FILE is not set" +assert_contains "$TMP_DIR/missing-both.log" "WARNING: RUSTFS_SECRET_KEY or RUSTFS_SECRET_KEY_FILE is not set" + +run_expect_start \ "missing-secret" \ - "RUSTFS_SECRET_KEY or RUSTFS_SECRET_KEY_FILE must be set explicitly" \ + "WARNING: RUSTFS_SECRET_KEY or RUSTFS_SECRET_KEY_FILE is not set" \ RUSTFS_ACCESS_KEY=custom-access -run_expect_failure \ +# Default credentials warn but still start, via env, CLI flag (both forms), +# and file. +run_expect_start \ "default-access-env" \ - "RUSTFS_ACCESS_KEY must not use the default rustfsadmin credential" \ + "WARNING: RUSTFS_ACCESS_KEY uses the default rustfsadmin credential" \ RUSTFS_ACCESS_KEY=rustfsadmin \ RUSTFS_SECRET_KEY=custom-secret -default_access_cli_log="$TMP_DIR/default-access-cli.log" -if env -i PATH="$PATH" RUSTFS_VOLUMES="$TMP_DIR/data" RUSTFS_OBS_LOG_DIRECTORY= RUSTFS_SECRET_KEY=custom-secret sh "$ENTRYPOINT" rustfs --access-key rustfsadmin >"$default_access_cli_log" 2>&1; then - echo "Expected default-access-cli to fail" >&2 - exit 1 -fi -grep -q "RUSTFS_ACCESS_KEY must not use the default rustfsadmin credential" "$default_access_cli_log" +run_expect_start \ + "default-secret-env" \ + "WARNING: RUSTFS_SECRET_KEY uses the default rustfsadmin credential" \ + RUSTFS_ACCESS_KEY=custom-access \ + RUSTFS_SECRET_KEY=rustfsadmin + +run_cli_expect_start \ + "default-access-cli" \ + "WARNING: RUSTFS_ACCESS_KEY uses the default rustfsadmin credential" \ + --access-key rustfsadmin + +run_cli_expect_start \ + "default-access-cli-equals" \ + "WARNING: RUSTFS_ACCESS_KEY uses the default rustfsadmin credential" \ + --access-key=rustfsadmin default_access_file="$TMP_DIR/default-access-key" printf 'rustfsadmin\n' >"$default_access_file" -run_expect_failure \ +run_expect_start \ "default-access-file" \ - "RUSTFS_ACCESS_KEY_FILE must not contain the default rustfsadmin credential" \ + "WARNING: RUSTFS_ACCESS_KEY_FILE contains the default rustfsadmin credential" \ RUSTFS_ACCESS_KEY_FILE="$default_access_file" \ RUSTFS_SECRET_KEY=custom-secret +# CRLF-edited default-credential files must still be detected (the binary +# trims before comparing, so must we). +crlf_default_file="$TMP_DIR/crlf-default-access-key" +printf 'rustfsadmin\r\n' >"$crlf_default_file" +run_expect_start \ + "default-access-file-crlf" \ + "WARNING: RUSTFS_ACCESS_KEY_FILE contains the default rustfsadmin credential" \ + RUSTFS_ACCESS_KEY_FILE="$crlf_default_file" \ + RUSTFS_SECRET_KEY=custom-secret + +# A newline-less secret file (printf/echo -n style) is well-formed: the +# binary accepts it, so the entrypoint must not reject it as empty. +no_newline_file="$TMP_DIR/no-newline-access-key" +printf 'custom-access' >"$no_newline_file" +run_expect_start \ + "proper-access-file-no-newline" \ + "Starting: /usr/bin/rustfs" \ + RUSTFS_ACCESS_KEY_FILE="$no_newline_file" \ + RUSTFS_SECRET_KEY=custom-secret +assert_lacks "$TMP_DIR/proper-access-file-no-newline.log" "WARNING:" + +# Proper credentials start with no credential diagnostics at all. +run_expect_start \ + "proper-creds" \ + "Starting: /usr/bin/rustfs" \ + RUSTFS_ACCESS_KEY=custom-access \ + RUSTFS_SECRET_KEY=custom-secret +assert_lacks "$TMP_DIR/proper-creds.log" "WARNING:" + +# Malformed configuration still fails hard: conflicting sources, unreadable +# credential files, empty values, and flags missing their argument must stop +# the container before startup. +conflict_file="$TMP_DIR/conflict-access-key" +printf 'custom-access\n' >"$conflict_file" +run_expect_failure \ + "conflict-access-sources" \ + "ERROR: Set either RUSTFS_ACCESS_KEY or RUSTFS_ACCESS_KEY_FILE, not both." \ + RUSTFS_ACCESS_KEY=custom-access \ + RUSTFS_ACCESS_KEY_FILE="$conflict_file" \ + RUSTFS_SECRET_KEY=custom-secret + +run_expect_failure \ + "unreadable-access-file" \ + "ERROR: RUSTFS_ACCESS_KEY_FILE points to an unreadable file." \ + RUSTFS_ACCESS_KEY_FILE="$TMP_DIR/does-not-exist" \ + RUSTFS_SECRET_KEY=custom-secret + +run_cli_expect_failure \ + "empty-access-cli-value" \ + "ERROR: RUSTFS_ACCESS_KEY must not be empty." \ + --access-key= + +# Present-but-empty env vars (e.g. an unexpanded compose interpolation) are +# malformed values, not missing credentials — the binary would otherwise run +# with an empty root credential. +run_expect_failure \ + "empty-access-env-value" \ + "ERROR: RUSTFS_ACCESS_KEY must not be empty." \ + RUSTFS_ACCESS_KEY= \ + RUSTFS_SECRET_KEY=custom-secret + +run_expect_failure \ + "empty-access-file-env-value" \ + "ERROR: RUSTFS_ACCESS_KEY_FILE must not be empty." \ + RUSTFS_ACCESS_KEY_FILE= \ + RUSTFS_SECRET_KEY=custom-secret + +# Whitespace-only values trim to empty in the binary, so they must hit the +# same empty-value hard failure instead of starting with an empty credential. +run_expect_failure \ + "whitespace-access-env-value" \ + "ERROR: RUSTFS_ACCESS_KEY must not be empty." \ + RUSTFS_ACCESS_KEY=" " \ + RUSTFS_SECRET_KEY=custom-secret + +run_cli_expect_failure \ + "access-flag-missing-argument" \ + "ERROR: --access-key requires a value." \ + --access-key + +# Non-server commands skip credential validation entirely. cargo_log="$TMP_DIR/cargo.log" if ! env -i PATH="$PATH" RUSTFS_VOLUMES="$TMP_DIR/data" RUSTFS_OBS_LOG_DIRECTORY= sh "$ENTRYPOINT" cargo --version >"$cargo_log" 2>&1; then echo "Expected cargo passthrough to skip server credential checks" >&2 cat "$cargo_log" >&2 exit 1 fi -if grep -q "must be set explicitly" "$cargo_log"; then - echo "Cargo passthrough must not require server credentials" >&2 - cat "$cargo_log" >&2 - exit 1 -fi +assert_lacks "$cargo_log" "RUSTFS_ACCESS_KEY" +assert_lacks "$cargo_log" "RUSTFS_SECRET_KEY" + +echo "entrypoint credential policy tests passed"