diff --git a/crates/trusted-server-core/build.rs b/crates/trusted-server-core/build.rs index b2803135..6f56e85a 100644 --- a/crates/trusted-server-core/build.rs +++ b/crates/trusted-server-core/build.rs @@ -38,11 +38,6 @@ fn main() { // Merge base TOML with environment variable overrides and write output. // Panics if admin endpoints are not covered by a handler. - // Note: placeholder secret rejection is intentionally NOT done here. - // The base trusted-server.toml ships with placeholder secrets that - // production deployments override via TRUSTED_SERVER__* env vars at - // build time. Runtime startup (get_settings) rejects any remaining - // placeholders so a misconfigured deployment fails fast. let settings = settings::Settings::from_toml_and_env(&toml_content) .expect("Failed to parse settings at build time"); diff --git a/crates/trusted-server-core/src/error.rs b/crates/trusted-server-core/src/error.rs index 0e4a7456..8e352676 100644 --- a/crates/trusted-server-core/src/error.rs +++ b/crates/trusted-server-core/src/error.rs @@ -33,12 +33,6 @@ pub enum TrustedServerError { #[display("GDPR consent error: {message}")] GdprConsent { message: String }, - /// A configuration secret is still set to a known placeholder value. - #[display( - "Configuration field '{field}' is set to a known placeholder value - this is insecure" - )] - InsecureDefault { field: String }, - /// Invalid UTF-8 data encountered. #[display("Invalid UTF-8 data: {message}")] InvalidUtf8 { message: String }, @@ -99,7 +93,6 @@ impl IntoHttpResponse for TrustedServerError { Self::Configuration { .. } | Self::Settings { .. } => StatusCode::INTERNAL_SERVER_ERROR, Self::Gam { .. } => StatusCode::BAD_GATEWAY, Self::GdprConsent { .. } => StatusCode::BAD_REQUEST, - Self::InsecureDefault { .. } => StatusCode::INTERNAL_SERVER_ERROR, Self::InvalidHeaderValue { .. } => StatusCode::BAD_REQUEST, Self::InvalidUtf8 { .. } => StatusCode::BAD_REQUEST, Self::KvStore { .. } => StatusCode::SERVICE_UNAVAILABLE, diff --git a/crates/trusted-server-core/src/settings.rs b/crates/trusted-server-core/src/settings.rs index 512f9a39..f3c0dc3e 100644 --- a/crates/trusted-server-core/src/settings.rs +++ b/crates/trusted-server-core/src/settings.rs @@ -31,18 +31,6 @@ pub struct Publisher { } impl Publisher { - /// Known placeholder values that must not be used in production. - pub const PROXY_SECRET_PLACEHOLDERS: &[&str] = &["change-me-proxy-secret"]; - - /// Returns `true` if `proxy_secret` matches a known placeholder value - /// (case-insensitive). - #[must_use] - pub fn is_placeholder_proxy_secret(proxy_secret: &str) -> bool { - Self::PROXY_SECRET_PLACEHOLDERS - .iter() - .any(|p| p.eq_ignore_ascii_case(proxy_secret)) - } - /// Extracts the host (including port if present) from the `origin_url`. /// /// # Examples @@ -204,25 +192,8 @@ pub struct Synthetic { } impl Synthetic { - /// Known placeholder values that must not be used in production. - pub const SECRET_KEY_PLACEHOLDERS: &[&str] = &["secret-key", "secret_key", "trusted-server"]; - - /// Returns `true` if `secret_key` matches a known placeholder value - /// (case-insensitive). - #[must_use] - pub fn is_placeholder_secret_key(secret_key: &str) -> bool { - Self::SECRET_KEY_PLACEHOLDERS - .iter() - .any(|p| p.eq_ignore_ascii_case(secret_key)) - } - /// Validates that the secret key is not empty. /// - /// Placeholder detection is intentionally **not** performed here because - /// this validator runs at build time (via `from_toml_and_env`) when the - /// config legitimately contains placeholder values. Placeholder rejection - /// happens at runtime via [`Settings::reject_placeholder_secrets`]. - /// /// # Errors /// /// Returns a validation error if the secret key is empty. @@ -419,33 +390,6 @@ impl Settings { Ok(settings) } - /// Checks all secret fields for known placeholder values and returns an - /// error listing every offending field. This centralises the placeholder - /// policy so callers don't need to know which fields are secrets. - /// - /// # Errors - /// - /// Returns [`TrustedServerError::InsecureDefault`] when one or more secret - /// fields still contain a placeholder value. - pub fn reject_placeholder_secrets(&self) -> Result<(), Report> { - let mut insecure_fields: Vec<&str> = Vec::new(); - - if Synthetic::is_placeholder_secret_key(self.synthetic.secret_key.expose()) { - insecure_fields.push("synthetic.secret_key"); - } - if Publisher::is_placeholder_proxy_secret(self.publisher.proxy_secret.expose()) { - insecure_fields.push("publisher.proxy_secret"); - } - - if insecure_fields.is_empty() { - Ok(()) - } else { - Err(Report::new(TrustedServerError::InsecureDefault { - field: insecure_fields.join(", "), - })) - } - } - #[must_use] pub fn handler_for_path(&self, path: &str) -> Option<&Handler> { self.handlers @@ -757,62 +701,6 @@ mod tests { ); } - #[test] - fn is_placeholder_secret_key_rejects_all_known_placeholders() { - for placeholder in Synthetic::SECRET_KEY_PLACEHOLDERS { - assert!( - Synthetic::is_placeholder_secret_key(placeholder), - "should detect placeholder secret_key '{placeholder}'" - ); - } - } - - #[test] - fn is_placeholder_secret_key_is_case_insensitive() { - assert!( - Synthetic::is_placeholder_secret_key("SECRET-KEY"), - "should detect case-insensitive placeholder secret_key" - ); - assert!( - Synthetic::is_placeholder_secret_key("Trusted-Server"), - "should detect mixed-case placeholder secret_key" - ); - } - - #[test] - fn is_placeholder_secret_key_accepts_non_placeholder() { - assert!( - !Synthetic::is_placeholder_secret_key("test-secret-key"), - "should accept non-placeholder secret_key" - ); - } - - #[test] - fn is_placeholder_proxy_secret_rejects_all_known_placeholders() { - for placeholder in Publisher::PROXY_SECRET_PLACEHOLDERS { - assert!( - Publisher::is_placeholder_proxy_secret(placeholder), - "should detect placeholder proxy_secret '{placeholder}'" - ); - } - } - - #[test] - fn is_placeholder_proxy_secret_is_case_insensitive() { - assert!( - Publisher::is_placeholder_proxy_secret("CHANGE-ME-PROXY-SECRET"), - "should detect case-insensitive placeholder proxy_secret" - ); - } - - #[test] - fn is_placeholder_proxy_secret_accepts_non_placeholder() { - assert!( - !Publisher::is_placeholder_proxy_secret("unit-test-proxy-secret"), - "should accept non-placeholder proxy_secret" - ); - } - #[test] fn test_settings_empty_toml() { let toml_str = ""; diff --git a/crates/trusted-server-core/src/settings_data.rs b/crates/trusted-server-core/src/settings_data.rs index d764f0bd..e66b1da0 100644 --- a/crates/trusted-server-core/src/settings_data.rs +++ b/crates/trusted-server-core/src/settings_data.rs @@ -34,124 +34,48 @@ pub fn get_settings() -> Result> { message: "Failed to validate configuration".to_string(), })?; - // Reject known placeholder values for secrets that feed into cryptographic operations. - settings.reject_placeholder_secrets()?; - if !settings.proxy.certificate_check { log::warn!( "INSECURE: proxy.certificate_check is disabled — TLS certificates will NOT be verified" ); } - Ok(settings) -} - -#[cfg(test)] -mod tests { - use crate::error::TrustedServerError; - use crate::settings::Settings; - use crate::test_support::tests::crate_test_settings_str; - - /// Builds a TOML string with the given secret values swapped in. - /// - /// # Panics - /// - /// Panics if the replacement patterns no longer match the test TOML, - /// which would cause the substitution to silently no-op. - fn toml_with_secrets(secret_key: &str, proxy_secret: &str) -> String { - let original = crate_test_settings_str(); - let after_secret_key = original.replace( - r#"secret_key = "test-secret-key""#, - &format!(r#"secret_key = "{secret_key}""#), - ); - assert_ne!( - after_secret_key, original, - "should have replaced secret_key value" - ); - let result = after_secret_key.replace( - r#"proxy_secret = "unit-test-proxy-secret""#, - &format!(r#"proxy_secret = "{proxy_secret}""#), - ); - assert_ne!( - result, after_secret_key, - "should have replaced proxy_secret value" - ); - result - } - - #[test] - fn rejects_placeholder_secret_key() { - let toml = toml_with_secrets("secret-key", "real-proxy-secret"); - let settings = Settings::from_toml(&toml).expect("should parse TOML"); - let err = settings - .reject_placeholder_secrets() - .expect_err("should reject placeholder secret_key"); - let root = err.current_context(); - assert!( - matches!(root, TrustedServerError::InsecureDefault { field } if field.contains("synthetic.secret_key")), - "error should mention synthetic.secret_key, got: {root}" + if settings.synthetic.secret_key.expose() == "trusted-server" { + log::warn!( + "INSECURE: synthetic.secret_key is set to the default placeholder — \ + HMAC-SHA256 signatures can be forged. \ + Override via TRUSTED_SERVER__SYNTHETIC__SECRET_KEY at build time" ); } - #[test] - fn rejects_placeholder_proxy_secret() { - let toml = toml_with_secrets("real-secret-key", "change-me-proxy-secret"); - let settings = Settings::from_toml(&toml).expect("should parse TOML"); - let err = settings - .reject_placeholder_secrets() - .expect_err("should reject placeholder proxy_secret"); - let root = err.current_context(); - assert!( - matches!(root, TrustedServerError::InsecureDefault { field } if field.contains("publisher.proxy_secret")), - "error should mention publisher.proxy_secret, got: {root}" + if settings.publisher.proxy_secret.expose() == "change-me-proxy-secret" { + log::warn!( + "INSECURE: publisher.proxy_secret is set to the default placeholder — \ + XChaCha20-Poly1305 encrypted URLs can be decrypted by anyone. \ + Override via TRUSTED_SERVER__PUBLISHER__PROXY_SECRET at build time" ); } - #[test] - fn rejects_both_placeholders_in_single_error() { - let toml = toml_with_secrets("secret_key", "change-me-proxy-secret"); - let settings = Settings::from_toml(&toml).expect("should parse TOML"); - let err = settings - .reject_placeholder_secrets() - .expect_err("should reject both placeholder secrets"); - let root = err.current_context(); - match root { - TrustedServerError::InsecureDefault { field } => { - assert!( - field.contains("synthetic.secret_key"), - "error should mention synthetic.secret_key, got: {field}" - ); - assert!( - field.contains("publisher.proxy_secret"), - "error should mention publisher.proxy_secret, got: {field}" - ); - } - other => panic!("expected InsecureDefault, got: {other}"), - } - } + Ok(settings) +} - #[test] - fn accepts_non_placeholder_secrets() { - let toml = toml_with_secrets("production-secret-key", "production-proxy-secret"); - let settings = Settings::from_toml(&toml).expect("should parse TOML"); - settings - .reject_placeholder_secrets() - .expect("non-placeholder secrets should pass validation"); - } +#[cfg(test)] +mod tests { + use super::*; - /// Smoke-test the full `get_settings()` pipeline (embedded bytes → UTF-8 → - /// parse → validate → placeholder check). The build-time TOML ships with - /// placeholder secrets, so the expected outcome is an [`InsecureDefault`] - /// error — but reaching that error proves every earlier stage succeeded. #[test] - fn get_settings_rejects_embedded_placeholder_secrets() { - let err = super::get_settings().expect_err("should reject embedded placeholder secrets"); - assert!( - matches!( - err.current_context(), - TrustedServerError::InsecureDefault { .. } - ), - "should fail with InsecureDefault, got: {err}" - ); + fn get_settings_loads_embedded_toml_successfully() { + // The embedded TOML contains placeholder secrets (e.g. "trusted-server", + // "change-me-proxy-secret"). This is expected — production builds override + // them via TRUSTED_SERVER__* env vars at build time. + let settings = get_settings().expect("should load settings from embedded TOML"); + // Verify basic structure is loaded + assert!(!settings.publisher.domain.is_empty()); + assert!(!settings.publisher.cookie_domain.is_empty()); + assert!(!settings.publisher.origin_url.is_empty()); + assert!(!settings.synthetic.counter_store.is_empty()); + assert!(!settings.synthetic.opid_store.is_empty()); + assert!(!settings.synthetic.secret_key.expose().is_empty()); + assert!(!settings.synthetic.template.is_empty()); } } diff --git a/docs/guide/configuration.md b/docs/guide/configuration.md index 583da8eb..146d5e46 100644 --- a/docs/guide/configuration.md +++ b/docs/guide/configuration.md @@ -244,7 +244,6 @@ TRUSTED_SERVER__PUBLISHER__PROXY_SECRET=your-secret-here **Security**: - Keep confidential and secure -- Cannot be the placeholder `"change-me-proxy-secret"` (case-insensitive) — startup will fail - Rotate periodically (90 days recommended) - Use cryptographically random values (32+ bytes) - Never commit to version control @@ -360,7 +359,6 @@ fastly kv-store create --name=opid_store **Security**: - Must be non-empty -- Cannot be a known placeholder: `"secret-key"`, `"secret_key"`, or `"trusted-server"` (case-insensitive) - Rotate periodically for security - Store securely (environment variable recommended) @@ -374,7 +372,6 @@ openssl rand -hex 32 **Validation**: Application startup fails if: - Empty string -- Exactly `"secret-key"`, `"secret_key"`, or `"trusted-server"` (known placeholders, case-insensitive) #### `template` @@ -923,12 +920,10 @@ Configuration is validated at startup: - All fields non-empty - `origin_url` is valid URL -- `proxy_secret` ≠ known placeholder (`"change-me-proxy-secret"` — case-insensitive) **Synthetic Validation**: - `secret_key` ≥ 1 character -- `secret_key` ≠ known placeholders (`"secret-key"`, `"secret_key"`, `"trusted-server"` — case-insensitive) - `template` non-empty **Handler Validation**: @@ -1038,13 +1033,6 @@ trusted-server.dev.toml # Development overrides - Verify all required fields present - Check environment variable format -**"Configuration field '...' is set to a known placeholder value"**: - -- `synthetic.secret_key` cannot be `"secret-key"`, `"secret_key"`, or `"trusted-server"` (case-insensitive) -- `publisher.proxy_secret` cannot be `"change-me-proxy-secret"` (case-insensitive) -- Must be non-empty -- Change to a secure random value (see generation commands above) - **"Invalid regex"**: - Handler `path` must be valid regex