From 801dc98c66a1be88371d5ba6727c8c530fcb05f8 Mon Sep 17 00:00:00 2001 From: GatewayJ <18332154+GatewayJ@users.noreply.github.com> Date: Wed, 5 Aug 2026 23:30:56 +0800 Subject: [PATCH 1/2] fix(security): reject empty credential secrets --- src/context.rs | 209 ++++++++++++++++++++++++++++++++++++------------- 1 file changed, 154 insertions(+), 55 deletions(-) diff --git a/src/context.rs b/src/context.rs index 765dd6b..f87e9c1 100755 --- a/src/context.rs +++ b/src/context.rs @@ -297,6 +297,38 @@ fn validate_secret_utf8_non_blank( const RUSTFS_DEFAULT_CREDENTIAL_VALUE: &str = "rustfsadmin"; +const CREDENTIAL_SECRET_KEYS: [&str; 2] = ["accesskey", "secretkey"]; +const MIN_CREDENTIAL_LENGTH: usize = 8; + +fn validate_credential_secret_data(secret: &Secret, secret_name: &str) -> Result<(), Error> { + for key in CREDENTIAL_SECRET_KEYS { + let Some(value) = secret.data.as_ref().and_then(|data| data.get(key)) else { + return CredentialSecretMissingKeySnafu { + secret_name: secret_name.to_string(), + key: key.to_string(), + } + .fail(); + }; + + let value = + std::str::from_utf8(&value.0).map_err(|_| Error::CredentialSecretInvalidEncoding { + secret_name: secret_name.to_string(), + key: key.to_string(), + })?; + let length = value.len(); + if length < MIN_CREDENTIAL_LENGTH { + return CredentialSecretTooShortSnafu { + secret_name: secret_name.to_string(), + key: key.to_string(), + length, + } + .fail(); + } + } + + Ok(()) +} + fn validate_rpc_secret_ref(secret_ref: &RpcSecretRef) -> Result<(), Error> { for (field, value) in [("name", &secret_ref.name), ("key", &secret_ref.key)] { if value.trim().is_empty() { @@ -638,61 +670,7 @@ impl Context { } }; - // Validate Secret has required keys - if let Some(data) = secret.data { - let access_key = "accesskey".to_string(); - let secret_key = "secretkey".to_string(); - - // Validate accesskey exists, is valid UTF-8, and meets minimum length - if let Some(accesskey_bytes) = data.get(&access_key) { - let accesskey = String::from_utf8(accesskey_bytes.0.clone()).map_err(|_| { - Error::CredentialSecretInvalidEncoding { - secret_name: cfg.name.clone(), - key: access_key.clone(), - } - })?; - - if accesskey.len() < 8 { - return CredentialSecretTooShortSnafu { - secret_name: cfg.name.clone(), - key: access_key.clone(), - length: accesskey.len(), - } - .fail(); - } - } else { - return CredentialSecretMissingKeySnafu { - secret_name: cfg.name.clone(), - key: access_key, - } - .fail(); - } - - // Validate secretkey exists, is valid UTF-8, and meets minimum length - if let Some(secretkey_bytes) = data.get(&secret_key) { - let secretkey = String::from_utf8(secretkey_bytes.0.clone()).map_err(|_| { - Error::CredentialSecretInvalidEncoding { - secret_name: cfg.name.clone(), - key: secret_key.clone(), - } - })?; - - if secretkey.len() < 8 { - return CredentialSecretTooShortSnafu { - secret_name: cfg.name.clone(), - key: secret_key.clone(), - length: secretkey.len(), - } - .fail(); - } - } else { - return CredentialSecretMissingKeySnafu { - secret_name: cfg.name.clone(), - key: secret_key, - } - .fail(); - } - } + validate_credential_secret_data(&secret, &cfg.name)?; } Ok(()) @@ -901,6 +879,127 @@ impl Context { } } +#[cfg(test)] +mod credential_secret_validation_tests { + use super::{Error, validate_credential_secret_data}; + use k8s_openapi::ByteString; + use k8s_openapi::api::core::v1::Secret; + use std::collections::BTreeMap; + + #[test] + fn credential_secret_accepts_valid_values() { + let secret = Secret { + data: Some(BTreeMap::from([ + ("accesskey".to_string(), ByteString(b"access01".to_vec())), + ("secretkey".to_string(), ByteString(b"secret01".to_vec())), + ])), + ..Default::default() + }; + + assert!(validate_credential_secret_data(&secret, "creds").is_ok()); + } + + #[test] + fn credential_secret_requires_data() { + for data in [None, Some(BTreeMap::new())] { + let secret = Secret { + data, + ..Default::default() + }; + + let err = validate_credential_secret_data(&secret, "creds").unwrap_err(); + assert!(matches!( + err, + Error::CredentialSecretMissingKey { secret_name, key } + if secret_name == "creds" && key == "accesskey" + )); + } + } + + #[test] + fn credential_secret_requires_both_keys() { + let valid_value = ByteString(b"valid-key".to_vec()); + for (data, missing_key) in [ + ( + BTreeMap::from([("secretkey".to_string(), valid_value.clone())]), + "accesskey", + ), + ( + BTreeMap::from([("accesskey".to_string(), valid_value.clone())]), + "secretkey", + ), + ] { + let secret = Secret { + data: Some(data), + ..Default::default() + }; + + let err = validate_credential_secret_data(&secret, "creds").unwrap_err(); + assert!(matches!( + err, + Error::CredentialSecretMissingKey { secret_name, key } + if secret_name == "creds" && key == missing_key + )); + } + } + + #[test] + fn credential_secret_values_must_be_valid_utf8() { + let valid_value = ByteString(b"valid-key".to_vec()); + for (access_key, secret_key, invalid_key) in [ + (ByteString(vec![0xff]), valid_value.clone(), "accesskey"), + (valid_value.clone(), ByteString(vec![0xff]), "secretkey"), + ] { + let secret = Secret { + data: Some(BTreeMap::from([ + ("accesskey".to_string(), access_key), + ("secretkey".to_string(), secret_key), + ])), + ..Default::default() + }; + + let err = validate_credential_secret_data(&secret, "creds").unwrap_err(); + assert!(matches!( + err, + Error::CredentialSecretInvalidEncoding { secret_name, key } + if secret_name == "creds" && key == invalid_key + )); + } + } + + #[test] + fn credential_secret_values_must_be_at_least_eight_bytes() { + let valid_value = ByteString(b"valid-key".to_vec()); + for (access_key, secret_key, invalid_key) in [ + (ByteString(Vec::new()), valid_value.clone(), "accesskey"), + (valid_value.clone(), ByteString(Vec::new()), "secretkey"), + ( + ByteString(b"short".to_vec()), + valid_value.clone(), + "accesskey", + ), + ] { + let secret = Secret { + data: Some(BTreeMap::from([ + ("accesskey".to_string(), access_key), + ("secretkey".to_string(), secret_key), + ])), + ..Default::default() + }; + + let err = validate_credential_secret_data(&secret, "creds").unwrap_err(); + assert!(matches!( + err, + Error::CredentialSecretTooShort { + secret_name, + key, + .. + } if secret_name == "creds" && key == invalid_key + )); + } + } +} + #[cfg(test)] mod validate_local_kms_tests { use super::Error; From 2a9d7501e46ddb8a1ae7f7e6871c266cc73859f1 Mon Sep 17 00:00:00 2001 From: GatewayJ <18332154+GatewayJ@users.noreply.github.com> Date: Sun, 9 Aug 2026 20:30:44 +0800 Subject: [PATCH 2/2] fix(security): align credential validation with runtime --- deploy/rustfs-operator/crds/tenant-crd.yaml | 3 +- docs/operator-user-guide.md | 5 +- docs/operator-user-guide.zh-CN.md | 5 +- examples/secret-credentials-tenant.yaml | 11 +-- examples/tenant-4nodes.yaml | 3 +- src/context.rs | 82 +++++++++++++++++---- src/status.rs | 4 +- src/types/v1alpha1/tenant.rs | 3 +- 8 files changed, 88 insertions(+), 28 deletions(-) diff --git a/deploy/rustfs-operator/crds/tenant-crd.yaml b/deploy/rustfs-operator/crds/tenant-crd.yaml index 4c6437d..6f28965 100644 --- a/deploy/rustfs-operator/crds/tenant-crd.yaml +++ b/deploy/rustfs-operator/crds/tenant-crd.yaml @@ -174,7 +174,8 @@ spec: credsSecret: description: |- Optional reference to a Secret containing RustFS credentials. - The Secret must contain 'accesskey' and 'secretkey' keys (both required, minimum 8 characters each). + The Secret must contain 'accesskey' and 'secretkey' keys. Both values must be valid + UTF-8 without NUL bytes and at least 8 UTF-8 bytes after trimming. If not specified, credentials can be provided via environment variables in 'env'. Priority: Secret credentials > Environment variables > RustFS built-in defaults. For production use, always configure credentials via Secret or environment variables. diff --git a/docs/operator-user-guide.md b/docs/operator-user-guide.md index 8829943..db73826 100644 --- a/docs/operator-user-guide.md +++ b/docs/operator-user-guide.md @@ -447,7 +447,7 @@ spec: ### 7.3 Credentials -For production, use `spec.credsSecret`. The Secret must be in the same namespace as the Tenant and contain UTF-8 `accesskey` and `secretkey` keys. Both values must be at least 8 characters. +For production, use `spec.credsSecret`. The Secret must be in the same namespace as the Tenant and contain `accesskey` and `secretkey` keys. Both values must be valid UTF-8 without NUL bytes and at least 8 UTF-8 bytes after trimming. ```yaml apiVersion: v1 @@ -1146,7 +1146,8 @@ Common blocked reasons: | `InvalidPoolSpec` | Pool count, total volume count, pool name, and immutable fields. | | `CredentialSecretNotFound` | Secret exists in the Tenant namespace. | | `CredentialSecretMissingKey` | Secret contains `accesskey` and `secretkey`. | -| `CredentialSecretTooShort` | Both credential values are at least 8 characters. | +| `CredentialSecretInvalidEncoding` | Both credential values are valid UTF-8 without NUL bytes. | +| `CredentialSecretTooShort` | Both credential values are at least 8 UTF-8 bytes after trimming. | | `KmsSecretNotFound` / `KmsSecretMissingKey` | KMS Secret exists and contains required keys, such as Vault `vault-token` or the Local KMS `masterKeySecretRef.key`. | | `CertManagerCrdMissing` / `CertManagerIssuerNotFound` | cert-manager is installed and the issuer exists. | | `InvalidWorkloadSecurityProfile` | Fix the seccomp or AppArmor profile type and its `localhostProfile` pairing. | diff --git a/docs/operator-user-guide.zh-CN.md b/docs/operator-user-guide.zh-CN.md index 36280b8..a010ef2 100644 --- a/docs/operator-user-guide.zh-CN.md +++ b/docs/operator-user-guide.zh-CN.md @@ -424,7 +424,7 @@ spec: ### 7.3 凭据配置 -生产环境建议使用 `spec.credsSecret`。Secret 必须与 Tenant 在同一 namespace，并包含 UTF-8 编码的 `accesskey` 和 `secretkey` 两个 key，两个值长度都至少为 8 个字符。 +生产环境建议使用 `spec.credsSecret`。Secret 必须与 Tenant 在同一 namespace，并包含 `accesskey` 和 `secretkey` 两个 key；两个值都必须是不含 NUL 字节的有效 UTF-8，且 trim 后至少为 8 个 UTF-8 字节。 ```yaml apiVersion: v1 @@ -1104,7 +1104,8 @@ kubectl logs -n rustfs-system \ | `InvalidPoolSpec` | Pool 数量、总卷数、pool 名称和不可变字段。 | | `CredentialSecretNotFound` | Secret 是否存在于 Tenant namespace。 | | `CredentialSecretMissingKey` | Secret 是否包含 `accesskey` 和 `secretkey`。 | -| `CredentialSecretTooShort` | 两个凭据值是否都至少 8 个字符。 | +| `CredentialSecretInvalidEncoding` | 两个凭据值是否都是不含 NUL 字节的有效 UTF-8。 | +| `CredentialSecretTooShort` | 两个凭据值 trim 后是否都至少为 8 个 UTF-8 字节。 | | `KmsSecretNotFound` / `KmsSecretMissingKey` | KMS Secret 是否存在，并包含必要 key，例如 Vault 的 `vault-token` 或 Local KMS 的 `masterKeySecretRef.key`。 | | `CertManagerCrdMissing` / `CertManagerIssuerNotFound` | cert-manager 是否安装，issuer 是否存在。 | | `InvalidWorkloadSecurityProfile` | 修正 seccomp 或 AppArmor profile 类型及其与 `localhostProfile` 的组合。 | diff --git a/examples/secret-credentials-tenant.yaml b/examples/secret-credentials-tenant.yaml index a60fac2..9e4abf8 100755 --- a/examples/secret-credentials-tenant.yaml +++ b/examples/secret-credentials-tenant.yaml @@ -23,11 +23,11 @@ type: Opaque stringData: # RustFS admin access key (username) # ⚠️ IMPORTANT: Change these default values for production deployments! - # REQUIRED: Must be at least 8 characters long + # REQUIRED: Must be at least 8 UTF-8 bytes after trimming accesskey: "rustfsadmin123" # RustFS admin secret key (password) - # REQUIRED: Must be at least 8 characters long + # REQUIRED: Must be at least 8 UTF-8 bytes after trimming # Recommendation: Use at least 16 characters with mixed case, numbers, and symbols secretkey: "rustfsadmin123" @@ -199,7 +199,8 @@ spec: # Note: The operator will retry every 60 seconds until keys are present # Issue: Credential validation error - keys too short -# Solution: Ensure both accesskey and secretkey are at least 8 characters +# Solution: Ensure both values are valid UTF-8 without NUL bytes and at least +# 8 UTF-8 bytes after trimming # kubectl get secret rustfs-credentials -o jsonpath='{.data.accesskey}' | base64 -d | wc -c # kubectl get secret rustfs-credentials -o jsonpath='{.data.secretkey}' | base64 -d | wc -c # Note: The operator will retry every 60 seconds until credentials meet requirements @@ -232,8 +233,8 @@ spec: # Credential validation errors (require user action): # - Secret not found # - Missing required keys (accesskey/secretkey) -# - Invalid UTF-8 encoding -# - Credentials too short (less than 8 characters) +# - Invalid UTF-8 encoding or NUL bytes +# - Credentials too short (less than 8 UTF-8 bytes after trimming) # Retry interval: 60 seconds (reduces log/event spam while you fix the issue) # # Transient errors (may self-resolve): diff --git a/examples/tenant-4nodes.yaml b/examples/tenant-4nodes.yaml index 2b46cfc..342a63e 100644 --- a/examples/tenant-4nodes.yaml +++ b/examples/tenant-4nodes.yaml @@ -68,7 +68,8 @@ spec: value: "true" --- -# Credentials Secret for RustFS (keys must be at least 8 characters) +# Credentials Secret for RustFS (values must be valid UTF-8 without NUL bytes +# and at least 8 UTF-8 bytes after trimming) # Example: accesskey admin123, secretkey admin12345 (dev only) apiVersion: v1 kind: Secret diff --git a/src/context.rs b/src/context.rs index f87e9c1..7204da6 100755 --- a/src/context.rs +++ b/src/context.rs @@ -54,7 +54,7 @@ pub enum Error { CredentialSecretInvalidEncoding { secret_name: String, key: String }, #[snafu(display( - "credential secret '{}' key '{}' must be at least 8 characters (got {} characters)", + "credential secret '{}' key '{}' must be at least 8 UTF-8 bytes after trimming (got {} bytes)", secret_name, key, length @@ -298,7 +298,7 @@ fn validate_secret_utf8_non_blank( const RUSTFS_DEFAULT_CREDENTIAL_VALUE: &str = "rustfsadmin"; const CREDENTIAL_SECRET_KEYS: [&str; 2] = ["accesskey", "secretkey"]; -const MIN_CREDENTIAL_LENGTH: usize = 8; +const MIN_CREDENTIAL_LENGTH_BYTES: usize = 8; fn validate_credential_secret_data(secret: &Secret, secret_name: &str) -> Result<(), Error> { for key in CREDENTIAL_SECRET_KEYS { @@ -315,8 +315,15 @@ fn validate_credential_secret_data(secret: &Secret, secret_name: &str) -> Result secret_name: secret_name.to_string(), key: key.to_string(), })?; - let length = value.len(); - if length < MIN_CREDENTIAL_LENGTH { + if value.contains('\0') { + return Err(Error::CredentialSecretInvalidEncoding { + secret_name: secret_name.to_string(), + key: key.to_string(), + }); + } + + let length = value.trim().len(); + if length < MIN_CREDENTIAL_LENGTH_BYTES { return CredentialSecretTooShortSnafu { secret_name: secret_name.to_string(), key: key.to_string(), @@ -643,8 +650,8 @@ impl Context { /// # Validation Rules /// - Secret must exist in the same namespace as the Tenant /// - Secret must contain both `accesskey` and `secretkey` keys - /// - Both keys must be valid UTF-8 strings - /// - Both keys must be at least 8 characters long + /// - Both keys must be valid UTF-8 strings without NUL bytes + /// - Both keys must be at least 8 UTF-8 bytes after trimming /// /// # Returns /// - `Ok(())` if Secret is valid or not configured @@ -890,8 +897,8 @@ mod credential_secret_validation_tests { fn credential_secret_accepts_valid_values() { let secret = Secret { data: Some(BTreeMap::from([ - ("accesskey".to_string(), ByteString(b"access01".to_vec())), - ("secretkey".to_string(), ByteString(b"secret01".to_vec())), + ("accesskey".to_string(), ByteString(b" access01 ".to_vec())), + ("secretkey".to_string(), ByteString(b" secret01 ".to_vec())), ])), ..Default::default() }; @@ -944,11 +951,21 @@ mod credential_secret_validation_tests { } #[test] - fn credential_secret_values_must_be_valid_utf8() { + fn credential_secret_values_must_be_environment_safe_utf8() { let valid_value = ByteString(b"valid-key".to_vec()); for (access_key, secret_key, invalid_key) in [ (ByteString(vec![0xff]), valid_value.clone(), "accesskey"), (valid_value.clone(), ByteString(vec![0xff]), "secretkey"), + ( + ByteString(b"valid\0key".to_vec()), + valid_value.clone(), + "accesskey", + ), + ( + valid_value.clone(), + ByteString(b"valid\0key".to_vec()), + "secretkey", + ), ] { let secret = Secret { data: Some(BTreeMap::from([ @@ -970,13 +987,32 @@ mod credential_secret_validation_tests { #[test] fn credential_secret_values_must_be_at_least_eight_bytes() { let valid_value = ByteString(b"valid-key".to_vec()); - for (access_key, secret_key, invalid_key) in [ - (ByteString(Vec::new()), valid_value.clone(), "accesskey"), - (valid_value.clone(), ByteString(Vec::new()), "secretkey"), + for (access_key, secret_key, invalid_key, expected_length) in [ + (ByteString(Vec::new()), valid_value.clone(), "accesskey", 0), + (valid_value.clone(), ByteString(Vec::new()), "secretkey", 0), ( ByteString(b"short".to_vec()), valid_value.clone(), "accesskey", + 5, + ), + ( + ByteString(b" ".to_vec()), + valid_value.clone(), + "accesskey", + 0, + ), + ( + ByteString(b" short ".to_vec()), + valid_value.clone(), + "accesskey", + 5, + ), + ( + valid_value.clone(), + ByteString(b" short ".to_vec()), + "secretkey", + 5, ), ] { let secret = Secret { @@ -993,11 +1029,29 @@ mod credential_secret_validation_tests { Error::CredentialSecretTooShort { secret_name, key, - .. - } if secret_name == "creds" && key == invalid_key + length, + } if secret_name == "creds" + && key == invalid_key + && length == expected_length )); } } + + #[test] + fn credential_secret_length_is_measured_in_trimmed_utf8_bytes() { + let secret = Secret { + data: Some(BTreeMap::from([ + ( + "accesskey".to_string(), + ByteString(" \u{1f510}\u{1f510} ".as_bytes().to_vec()), + ), + ("secretkey".to_string(), ByteString(b"secret01".to_vec())), + ])), + ..Default::default() + }; + + assert!(validate_credential_secret_data(&secret, "creds").is_ok()); + } } #[cfg(test)] diff --git a/src/status.rs b/src/status.rs index 6715d95..1baa699 100644 --- a/src/status.rs +++ b/src/status.rs @@ -60,7 +60,7 @@ impl StatusError { Reason::CredentialSecretInvalidEncoding, ConditionType::CredentialsReady, format!( - "Credential Secret '{}' key '{}' must contain valid UTF-8", + "Credential Secret '{}' key '{}' must contain valid UTF-8 without NUL bytes", secret_name, key ), ), @@ -70,7 +70,7 @@ impl StatusError { Reason::CredentialSecretTooShort, ConditionType::CredentialsReady, format!( - "Credential Secret '{}' key '{}' must be at least 8 characters", + "Credential Secret '{}' key '{}' must be at least 8 UTF-8 bytes after trimming", secret_name, key ), ), diff --git a/src/types/v1alpha1/tenant.rs b/src/types/v1alpha1/tenant.rs index f63a3e8..c62a22d 100755 --- a/src/types/v1alpha1/tenant.rs +++ b/src/types/v1alpha1/tenant.rs @@ -181,7 +181,8 @@ pub struct TenantSpec { // // #[serde(default, skip_serializing_if = "Option::is_none")] // // pub side_cars: Option, /// Optional reference to a Secret containing RustFS credentials. - /// The Secret must contain 'accesskey' and 'secretkey' keys (both required, minimum 8 characters each). + /// The Secret must contain 'accesskey' and 'secretkey' keys. Both values must be valid + /// UTF-8 without NUL bytes and at least 8 UTF-8 bytes after trimming. /// If not specified, credentials can be provided via environment variables in 'env'. /// Priority: Secret credentials > Environment variables > RustFS built-in defaults. /// For production use, always configure credentials via Secret or environment variables.