diff --git a/.docker/nginx/nginx.conf b/.docker/nginx/nginx.conf index 2e574e04c..88ff9f418 100644 --- a/.docker/nginx/nginx.conf +++ b/.docker/nginx/nginx.conf @@ -74,6 +74,17 @@ http { # ssl_certificate /etc/nginx/ssl/server.crt; # ssl_certificate_key /etc/nginx/ssl/server.key; # + # # Restrict to modern TLS versions and ciphers. Operators copying this + # # example must keep at least these directives — without them, nginx + # # may negotiate older protocol versions that have known weaknesses. + # ssl_protocols TLSv1.2 TLSv1.3; + # ssl_prefer_server_ciphers on; + # ssl_ciphers 'ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-RSA-CHACHA20-POLY1305'; + # ssl_session_timeout 1d; + # ssl_session_cache shared:SSL:10m; + # ssl_session_tickets off; + # # add_header Strict-Transport-Security "max-age=63072000" always; + # # location / { # proxy_pass http://rustfs:9000; # ... diff --git a/crates/config/src/constants/console.rs b/crates/config/src/constants/console.rs index c484372d3..617aa020d 100644 --- a/crates/config/src/constants/console.rs +++ b/crates/config/src/constants/console.rs @@ -24,8 +24,14 @@ pub const DEFAULT_CORS_ALLOWED_ORIGINS: &str = ""; /// Comma-separated list of origins or "*" for all origins pub const ENV_CONSOLE_CORS_ALLOWED_ORIGINS: &str = "RUSTFS_CONSOLE_CORS_ALLOWED_ORIGINS"; -/// Default CORS allowed origins for the console service -pub const DEFAULT_CONSOLE_CORS_ALLOWED_ORIGINS: &str = "*"; +/// Default CORS allowed origins for the console service. +/// +/// Empty string means same-origin only — no `Access-Control-Allow-Origin` +/// header is emitted, so browsers will not allow cross-origin reads of +/// console responses by default. Operators that need cross-origin access set +/// `RUSTFS_CONSOLE_CORS_ALLOWED_ORIGINS` to a comma-separated allow-list, or +/// to `*` to keep the previous permissive behavior. +pub const DEFAULT_CONSOLE_CORS_ALLOWED_ORIGINS: &str = ""; /// Enable or disable the console service pub const ENV_CONSOLE_ENABLE: &str = "RUSTFS_CONSOLE_ENABLE"; @@ -100,8 +106,8 @@ mod tests { } #[test] - fn console_cors_default_remains_wildcard() { + fn console_cors_default_is_same_origin_only() { assert_eq!(ENV_CONSOLE_CORS_ALLOWED_ORIGINS, "RUSTFS_CONSOLE_CORS_ALLOWED_ORIGINS"); - assert_eq!(DEFAULT_CONSOLE_CORS_ALLOWED_ORIGINS, "*"); + assert_eq!(DEFAULT_CONSOLE_CORS_ALLOWED_ORIGINS, ""); } } diff --git a/deny.toml b/deny.toml new file mode 100644 index 000000000..8708c7498 --- /dev/null +++ b/deny.toml @@ -0,0 +1,103 @@ +# cargo-deny configuration +# +# Run with `cargo deny check` (advisories, sources, bans, licenses). +# Schema: https://embarkstudios.github.io/cargo-deny/checks/cfg.html +# +# This file codifies what was previously implicit policy: +# - which RustSec advisories we knowingly accept and why, +# - which non-crates.io sources we trust, +# - which duplicate crate versions we tolerate vs. flag. +# +# When adding an exception, include an `# owner: review: ` +# comment so future audits know who signed off and when to revisit. + +[graph] +all-features = true +no-default-features = false + +[advisories] +version = 2 +yanked = "deny" +ignore = [ + # `instant 0.1.13` — unmaintained. No direct dependency; pulled in + # transitively. Tracked for upgrade as part of broader dep refresh. + # owner: rustfs-maintainers review: 2026-07 + { id = "RUSTSEC-2024-0384", reason = "instant unmaintained; transitive only; tracked for upgrade" }, + + # `paste 1.0.15` — unmaintained. No direct dependency. + # owner: rustfs-maintainers review: 2026-07 + { id = "RUSTSEC-2024-0436", reason = "paste unmaintained; transitive only; tracked for upgrade" }, + + # `rsa` Marvin timing sidechannel (RUSTSEC-2023-0071). Pulled in via + # `openidconnect` (transitive) and historically used directly. No upstream + # fix is available yet. Tracked separately for follow-up; remove this + # entry once a patched `rsa` lands in the dependency graph and any + # in-process RSA decryption oracles are removed. + # owner: rustfs-maintainers review: 2026-07 + { id = "RUSTSEC-2023-0071", reason = "rsa Marvin timing sidechannel; no fixed upstream version; tracked separately" }, +] + +[sources] +unknown-registry = "deny" +unknown-git = "deny" +allow-registry = ["https://github.com/rust-lang/crates.io-index"] +allow-git = [ + # Custom S3 server library with minio compatibility patches not yet upstreamed. + # Pinned to a specific commit in workspace Cargo.toml. + "https://github.com/rustfs/s3s", +] + +[bans] +# Multiple-versions of the same crate are permitted with a warning so the +# graph remains buildable while we work the chains down. Crypto- and +# transport-sensitive crates are tracked separately below. +multiple-versions = "warn" +wildcards = "warn" +highlight = "all" + +# Any future crate we want to forbid outright belongs here. +deny = [] + +# Crates whose duplicate versions are most worth eliminating, because they +# touch crypto, parsing, or networking trust boundaries. Not currently a +# build error — the graph still has duplicates — but tracking the list keeps +# them visible. +[[bans.skip-tree]] +# `windows-sys` notoriously has many old versions in dependency closures; +# don't flood the report with it. +name = "windows-sys" + +[licenses] +version = 2 +allow = [ + "Apache-2.0", + "Apache-2.0 WITH LLVM-exception", + "BSD-2-Clause", + "BSD-3-Clause", + "BSL-1.0", # boost; tracing-related crates + "CC0-1.0", + "CDLA-Permissive-2.0",# webpki / linux-raw-sys metadata + "ISC", + "MIT", + "MIT-0", + "MPL-2.0", + "Unicode-3.0", + "Zlib", +] +confidence-threshold = 0.93 +exceptions = [ + # `ring` ships a custom license combining ISC, MIT, and an OpenSSL-style + # notice that does not parse cleanly as SPDX OpenSSL. + { allow = ["ISC", "MIT"], crate = "ring" }, + + # `inferno` is CDDL-1.0 (copyleft). Used only by profiling tooling + # (pyroscope / jemalloc_pprof) which is opt-in and never linked into the + # default S3 path. Tracked as an exception rather than a blanket allow. + # owner: rustfs-maintainers review: 2026-07 + { allow = ["CDDL-1.0"], crate = "inferno" }, + + # `libbz2-rs-sys` carries the upstream bzip2-1.0.6 license. It's used + # transitively via `bzip2`. Not on a hot path. + # owner: rustfs-maintainers review: 2026-07 + { allow = ["bzip2-1.0.6"], crate = "libbz2-rs-sys" }, +] diff --git a/docker-compose.yml b/docker-compose.yml index 410d62849..c1c2b26ee 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -33,6 +33,8 @@ services: - RUSTFS_CONSOLE_ADDRESS=0.0.0.0:9001 - RUSTFS_CONSOLE_ENABLE=true - RUSTFS_CONSOLE_CORS_ALLOWED_ORIGINS=* + # SECURITY: these defaults are public and well-known. Override before + # exposing the listener beyond localhost. - RUSTFS_ACCESS_KEY=rustfsadmin - RUSTFS_SECRET_KEY=rustfsadmin - RUSTFS_OBS_LOGGER_LEVEL=info diff --git a/helm/rustfs/templates/secret.yaml b/helm/rustfs/templates/secret.yaml index 76348c202..773530782 100644 --- a/helm/rustfs/templates/secret.yaml +++ b/helm/rustfs/templates/secret.yaml @@ -1,4 +1,23 @@ {{- if not .Values.secret.existingSecret }} +{{- $accessKey := .Values.secret.rustfs.access_key | default "" }} +{{- $secretKey := .Values.secret.rustfs.secret_key | default "" }} +{{- $allowInsecure := .Values.secret.allowInsecureDefaults | default false }} +{{/* Either key set to the well-known default counts as insecure. */}} +{{- $hasDefaultKey := or (eq $accessKey "rustfsadmin") (eq $secretKey "rustfsadmin") }} +{{- $bothEmpty := and (eq $accessKey "") (eq $secretKey "") }} +{{- $oneEmpty := and (not $bothEmpty) (or (eq $accessKey "") (eq $secretKey "")) }} +{{/* Always fail when only one of the two keys is supplied — never silently + auto-fill a single missing key with the well-known default. */}} +{{- if $oneEmpty }} +{{- fail (printf "secret.rustfs.access_key and secret.rustfs.secret_key must both be set, or both be left empty. Setting only one of the two is ambiguous and is rejected to avoid silently using the well-known default for the missing key.") }} +{{- end }} +{{- if and (not $allowInsecure) (or $bothEmpty $hasDefaultKey) }} +{{- fail (printf "secret.rustfs.access_key and secret.rustfs.secret_key must be set to non-default, non-empty values, or set secret.existingSecret to a Secret you control. To opt into the well-known default credentials for local development only, set secret.allowInsecureDefaults=true.") }} +{{- end }} +{{- if and $allowInsecure $bothEmpty }} +{{- $accessKey = "rustfsadmin" }} +{{- $secretKey = "rustfsadmin" }} +{{- end }} apiVersion: v1 kind: Secret metadata: @@ -8,8 +27,8 @@ metadata: {{- toYaml .Values.commonLabels | nindent 4 }} type: Opaque data: - RUSTFS_ACCESS_KEY: {{ .Values.secret.rustfs.access_key | b64enc | quote }} - RUSTFS_SECRET_KEY: {{ .Values.secret.rustfs.secret_key | b64enc | quote }} + RUSTFS_ACCESS_KEY: {{ $accessKey | b64enc | quote }} + RUSTFS_SECRET_KEY: {{ $secretKey | b64enc | quote }} {{- end }} --- diff --git a/helm/rustfs/values.yaml b/helm/rustfs/values.yaml index ce76851c7..0b9840367 100644 --- a/helm/rustfs/values.yaml +++ b/helm/rustfs/values.yaml @@ -48,9 +48,17 @@ mode: secret: existingSecret: "" + # SECURITY: rendering fails by default unless one of the following is true: + # 1. `secret.existingSecret` names a Kubernetes Secret you control, or + # 2. `secret.rustfs.access_key` and `secret.rustfs.secret_key` are both + # set to non-empty, non-default values, or + # 3. `secret.allowInsecureDefaults: true` is set (only for local dev). + # This prevents accidental deployment with the well-known default + # `rustfsadmin/rustfsadmin` credentials. + allowInsecureDefaults: false rustfs: - access_key: rustfsadmin - secret_key: rustfsadmin + access_key: "" + secret_key: "" config: rustfs: diff --git a/rustfs/src/admin/console.rs b/rustfs/src/admin/console.rs index e0dc95cab..150212ab8 100644 --- a/rustfs/src/admin/console.rs +++ b/rustfs/src/admin/console.rs @@ -602,12 +602,18 @@ async fn health_route_disabled() -> StatusCode { StatusCode::NOT_FOUND } -/// Parse CORS allowed origins from configuration +/// Parse CORS allowed origins from configuration. /// -/// # Arguments: +/// When no origins are configured (None or an empty string), the layer is +/// left without `Access-Control-Allow-Origin` so browsers treat responses +/// as same-origin only. Operators that need cross-origin access set +/// `RUSTFS_CONSOLE_CORS_ALLOWED_ORIGINS` to a comma-separated allow-list, +/// or to `*` to allow any origin. +/// +/// # Arguments /// - `origins`: An optional reference to a string containing allowed origins. /// -/// # Returns: +/// # Returns /// - A `CorsLayer` configured with the specified origins. pub fn parse_cors_origins(origins: Option<&String>) -> CorsLayer { let cors_layer = CorsLayer::new() @@ -615,38 +621,28 @@ pub fn parse_cors_origins(origins: Option<&String>) -> CorsLayer { .allow_headers(Any); match origins { - Some(origins_str) if origins_str == "*" => cors_layer.allow_origin(Any).expose_headers(Any), - Some(origins_str) => { - let origins: Vec<&str> = origins_str.split(',').map(|s| s.trim()).collect(); - if origins.is_empty() { - warn!("Empty CORS origins provided, using permissive CORS"); - cors_layer.allow_origin(Any).expose_headers(Any) - } else { - // Parse origins with proper error handling - let mut valid_origins = Vec::new(); - for origin in origins { - match origin.parse::() { - Ok(header_value) => { - valid_origins.push(header_value); - } - Err(e) => { - warn!("Invalid CORS origin '{}': {}", origin, e); - } - } - } - - if valid_origins.is_empty() { - warn!("No valid CORS origins found, using permissive CORS"); - cors_layer.allow_origin(Any).expose_headers(Any) - } else { - info!("Console CORS origins configured: {:?}", valid_origins); - cors_layer.allow_origin(AllowOrigin::list(valid_origins)).expose_headers(Any) + Some(origins_str) if origins_str.trim() == "*" => cors_layer.allow_origin(Any).expose_headers(Any), + Some(origins_str) if !origins_str.trim().is_empty() => { + let origins: Vec<&str> = origins_str.split(',').map(|s| s.trim()).filter(|s| !s.is_empty()).collect(); + let mut valid_origins = Vec::new(); + for origin in origins { + match origin.parse::() { + Ok(header_value) => valid_origins.push(header_value), + Err(e) => warn!("Invalid CORS origin '{}': {}", origin, e), } } + + if valid_origins.is_empty() { + warn!("No valid CORS origins parsed from configuration; defaulting to same-origin only"); + cors_layer + } else { + info!("Console CORS origins configured: {:?}", valid_origins); + cors_layer.allow_origin(AllowOrigin::list(valid_origins)).expose_headers(Any) + } } - None => { - debug!("No CORS origins configured for console, using permissive CORS"); - cors_layer.allow_origin(Any) + _ => { + debug!("No CORS origins configured for console; same-origin only"); + cors_layer } } } @@ -676,6 +672,7 @@ mod tests { use axum::body::Body; use http::{Request, StatusCode}; use http_body_util::BodyExt; + use serial_test::serial; use std::net::{IpAddr, Ipv4Addr}; use temp_env::async_with_vars; use tower::ServiceExt; @@ -709,7 +706,10 @@ mod tests { assert!(!is_console_path("/rustfs/admin/v3/info")); } + // setup_console_middleware_stack reads ENV_HEALTH_ENDPOINT_ENABLE; serialise + // with other tests that override that env var to avoid cross-task leakage. #[tokio::test] + #[serial] async fn console_middleware_stack_propagates_request_id_header() { let app = setup_console_middleware_stack(parse_cors_origins(None), false, 0, 30); let request = Request::builder() @@ -725,7 +725,95 @@ mod tests { ); } + /// Regression: when no console CORS origins are configured (the new + /// default), the layer must NOT emit `Access-Control-Allow-Origin`, so + /// browsers treat responses as same-origin only. #[tokio::test] + #[serial] + async fn default_console_cors_is_same_origin_only() { + let app = setup_console_middleware_stack(parse_cors_origins(None), false, 0, 30); + + let request = Request::builder() + .method("OPTIONS") + .uri(format!("{CONSOLE_PREFIX}/license")) + .header("origin", "https://example.com") + .header("access-control-request-method", "GET") + .body(Body::empty()) + .expect("build preflight"); + + let response = app.oneshot(request).await.expect("preflight should complete"); + + assert!( + response.headers().get("access-control-allow-origin").is_none(), + "default console CORS must not emit Access-Control-Allow-Origin" + ); + assert!( + response.headers().get("access-control-allow-credentials").is_none(), + "default console CORS must not emit Access-Control-Allow-Credentials" + ); + } + + /// Operators that opt in to wildcard origins (via `*`) keep the previous + /// permissive behavior. + #[tokio::test] + #[serial] + async fn explicit_wildcard_console_cors_allows_any_origin() { + let star = "*".to_string(); + let app = setup_console_middleware_stack(parse_cors_origins(Some(&star)), false, 0, 30); + + let request = Request::builder() + .method("OPTIONS") + .uri(format!("{CONSOLE_PREFIX}/license")) + .header("origin", "https://example.com") + .header("access-control-request-method", "GET") + .body(Body::empty()) + .expect("build preflight"); + + let response = app.oneshot(request).await.expect("preflight should complete"); + + assert_eq!( + response + .headers() + .get("access-control-allow-origin") + .and_then(|v| v.to_str().ok()), + Some("*"), + "explicit `*` origin must produce wildcard Allow-Origin" + ); + } + + /// Whitespace-padded wildcard ("` * `") must still be treated as wildcard + /// rather than falling into the comma-separated parser. Common when the + /// origin string is templated through env vars. + #[tokio::test] + #[serial] + async fn whitespace_padded_wildcard_console_cors_allows_any_origin() { + let star = " * ".to_string(); + let app = setup_console_middleware_stack(parse_cors_origins(Some(&star)), false, 0, 30); + + let request = Request::builder() + .method("OPTIONS") + .uri(format!("{CONSOLE_PREFIX}/license")) + .header("origin", "https://example.com") + .header("access-control-request-method", "GET") + .body(Body::empty()) + .expect("build preflight"); + + let response = app.oneshot(request).await.expect("preflight should complete"); + + assert_eq!( + response + .headers() + .get("access-control-allow-origin") + .and_then(|v| v.to_str().ok()), + Some("*"), + "whitespace-padded `*` origin must produce wildcard Allow-Origin" + ); + } + + // Mutates the global ENV_HEALTH_ENDPOINT_ENABLE env var; serialise to + // avoid leaking the override into other async tests in the same module. + #[tokio::test] + #[serial] async fn console_middleware_stack_hides_health_routes_when_disabled() { async_with_vars([(rustfs_config::ENV_HEALTH_ENDPOINT_ENABLE, Some("false"))], async { let app = setup_console_middleware_stack(parse_cors_origins(None), false, 0, 30); diff --git a/rustfs/src/admin/route_registration_test.rs b/rustfs/src/admin/route_registration_test.rs index 9bdc7966c..3374d7894 100644 --- a/rustfs/src/admin/route_registration_test.rs +++ b/rustfs/src/admin/route_registration_test.rs @@ -21,6 +21,7 @@ use crate::admin::{ }; use crate::server::{ADMIN_PREFIX, HEALTH_PREFIX, HEALTH_READY_PATH, MINIO_ADMIN_PREFIX, PROFILE_CPU_PATH, PROFILE_MEMORY_PATH}; use hyper::Method; +use serial_test::serial; use temp_env::with_var; fn admin_path(path: &str) -> String { @@ -60,7 +61,11 @@ fn register_admin_routes(router: &mut S3Router) { oidc::register_oidc_route(router).expect("register oidc route"); } +// register_admin_routes reads ENV_HEALTH_ENDPOINT_ENABLE to decide whether +// to register /health; serialise with the env-mutating test below to avoid +// cross-thread leakage of that override. #[test] +#[serial] fn test_register_routes_cover_representative_admin_paths() { let mut router: S3Router = S3Router::new(false); register_admin_routes(&mut router); @@ -171,6 +176,7 @@ fn test_register_routes_cover_representative_admin_paths() { } #[test] +#[serial] fn test_admin_alias_paths_match_existing_admin_routes() { let mut router: S3Router = S3Router::new(false); register_admin_routes(&mut router); @@ -233,6 +239,7 @@ fn test_admin_alias_paths_match_existing_admin_routes() { } #[test] +#[serial] fn test_health_routes_not_registered_when_disabled_by_env() { with_var(rustfs_config::ENV_HEALTH_ENDPOINT_ENABLE, Some("false"), || { let mut router: S3Router = S3Router::new(false); diff --git a/scripts/test_helm_templates.sh b/scripts/test_helm_templates.sh index a4609b88d..0e7dc2297 100755 --- a/scripts/test_helm_templates.sh +++ b/scripts/test_helm_templates.sh @@ -4,12 +4,18 @@ set -euo pipefail ROOT_DIR=$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd) CHART_DIR="$ROOT_DIR/helm/rustfs" -render_standalone_deployment() { +render_chart() { helm template rustfs "$CHART_DIR" \ --namespace rustfs \ --set mode.distributed.enabled=false \ --set mode.standalone.enabled=true \ - "$@" | + --set secret.rustfs.access_key=test-access-key \ + --set secret.rustfs.secret_key=test-secret-key \ + "$@" +} + +render_standalone_deployment() { + render_chart "$@" | awk ' /^# Source: rustfs\/templates\/deployment.yaml$/ { in_deployment = 1 } in_deployment && /^---$/ { exit } @@ -27,3 +33,84 @@ fi rolling_output=$(render_standalone_deployment) grep -q "type: RollingUpdate" <<<"$rolling_output" grep -q "rollingUpdate:" <<<"$rolling_output" + +# Fail-closed credential checks. Rendering must fail when no credentials, +# existingSecret, or allowInsecureDefaults override is supplied. +default_render_status=0 +helm template rustfs "$CHART_DIR" \ + --namespace rustfs \ + --set mode.distributed.enabled=false \ + --set mode.standalone.enabled=true \ + >/dev/null 2>&1 || default_render_status=$? +if [[ $default_render_status -eq 0 ]]; then + echo "Default credentials must fail to render without an explicit override" >&2 + exit 1 +fi + +# Rendering must also fail if someone re-supplies the well-known defaults. +default_creds_status=0 +helm template rustfs "$CHART_DIR" \ + --namespace rustfs \ + --set mode.distributed.enabled=false \ + --set mode.standalone.enabled=true \ + --set secret.rustfs.access_key=rustfsadmin \ + --set secret.rustfs.secret_key=rustfsadmin \ + >/dev/null 2>&1 || default_creds_status=$? +if [[ $default_creds_status -eq 0 ]]; then + echo "Setting the well-known defaults must fail without allowInsecureDefaults" >&2 + exit 1 +fi + +# allowInsecureDefaults=true must succeed and emit the dev creds. +insecure_output=$(helm template rustfs "$CHART_DIR" \ + --namespace rustfs \ + --set mode.distributed.enabled=false \ + --set mode.standalone.enabled=true \ + --set secret.allowInsecureDefaults=true) +expected_b64=$(printf 'rustfsadmin' | base64) +if ! grep -q "RUSTFS_ACCESS_KEY: \"$expected_b64\"" <<<"$insecure_output"; then + echo "allowInsecureDefaults=true must emit the well-known dev access key" >&2 + exit 1 +fi + +# existingSecret must skip rendering the chart-managed Secret entirely. +existing_output=$(helm template rustfs "$CHART_DIR" \ + --namespace rustfs \ + --set mode.distributed.enabled=false \ + --set mode.standalone.enabled=true \ + --set secret.existingSecret=my-existing-secret) +if grep -q "RUSTFS_ACCESS_KEY:" <<<"$existing_output"; then + echo "existingSecret must suppress chart-managed Secret rendering" >&2 + exit 1 +fi + +# Partial-default credentials (one key set to the well-known default) must +# fail rendering even when the other key is non-default. +partial_default_status=0 +helm template rustfs "$CHART_DIR" \ + --namespace rustfs \ + --set mode.distributed.enabled=false \ + --set mode.standalone.enabled=true \ + --set secret.rustfs.access_key=rustfsadmin \ + --set secret.rustfs.secret_key=some-other-secret \ + >/dev/null 2>&1 || partial_default_status=$? +if [[ $partial_default_status -eq 0 ]]; then + echo "Partial-default credentials (access_key=rustfsadmin) must fail rendering" >&2 + exit 1 +fi + +# Partial-empty credentials (only one of the two keys set) must fail rendering +# even when allowInsecureDefaults=true — never silently auto-fill a single +# missing key with the well-known default. +partial_empty_status=0 +helm template rustfs "$CHART_DIR" \ + --namespace rustfs \ + --set mode.distributed.enabled=false \ + --set mode.standalone.enabled=true \ + --set secret.allowInsecureDefaults=true \ + --set secret.rustfs.access_key=user-supplied-access-key \ + >/dev/null 2>&1 || partial_empty_status=$? +if [[ $partial_empty_status -eq 0 ]]; then + echo "Partial-empty credentials (only access_key set) must fail rendering" >&2 + exit 1 +fi