diff --git a/crates/trusted-server-core/src/auth.rs b/crates/trusted-server-core/src/auth.rs index e23fd06d..e41a204b 100644 --- a/crates/trusted-server-core/src/auth.rs +++ b/crates/trusted-server-core/src/auth.rs @@ -8,20 +8,23 @@ use crate::settings::Settings; const BASIC_AUTH_REALM: &str = r#"Basic realm="Trusted Server""#; -/// Enforces Basic-auth for incoming requests. +/// Enforces HTTP Basic authentication for configured handler paths. /// /// Authentication is required when a configured handler's `path` regex matches /// the request path. Paths not covered by any handler pass through without /// authentication. /// -/// Admin endpoints are protected by requiring a handler at build time — see -/// [`Settings::from_toml_and_env`]. +/// Admin endpoints are protected by requiring a handler at build time; see +/// [`Settings::from_toml_and_env`]. Credential checks use constant-time +/// comparison for both username and password, and evaluate both regardless of +/// individual match results to avoid timing oracles. /// /// # Returns /// /// * `Some(Response)` — a `401 Unauthorized` response that should be sent back /// to the client (credentials missing or incorrect). /// * `None` — the request is allowed to proceed. +#[must_use] pub fn enforce_basic_auth(settings: &Settings, req: &Request) -> Option { let path = req.get_path(); @@ -47,6 +50,7 @@ pub fn enforce_basic_auth(settings: &Settings, req: &Request) -> Option StatusCode::BAD_GATEWAY, Self::Integration { .. } => StatusCode::BAD_GATEWAY, Self::Proxy { .. } => StatusCode::BAD_GATEWAY, + Self::Forbidden { .. } => StatusCode::FORBIDDEN, Self::SyntheticId { .. } => StatusCode::INTERNAL_SERVER_ERROR, Self::Template { .. } => StatusCode::INTERNAL_SERVER_ERROR, } diff --git a/crates/trusted-server-core/src/http_util.rs b/crates/trusted-server-core/src/http_util.rs index a53a2e4a..7976c858 100644 --- a/crates/trusted-server-core/src/http_util.rs +++ b/crates/trusted-server-core/src/http_util.rs @@ -3,6 +3,7 @@ use chacha20poly1305::{aead::Aead, aead::KeyInit, XChaCha20Poly1305, XNonce}; use fastly::http::{header, StatusCode}; use fastly::{Request, Response}; use sha2::{Digest, Sha256}; +use subtle::ConstantTimeEq as _; use crate::constants::INTERNAL_HEADERS; use crate::settings::Settings; @@ -318,10 +319,44 @@ pub fn sign_clear_url(settings: &Settings, clear_url: &str) -> String { URL_SAFE_NO_PAD.encode(digest) } +/// Constant-time string comparison. +/// +/// The explicit length check documents the invariant that both values have known, +/// non-secret lengths. Both checks always run — the short-circuit `&&` is safe +/// here because token lengths are public information, not secrets. +/// +/// # Security +/// +/// The length equality check short-circuits (via `&&`), which reveals whether the +/// two strings have equal length via timing. This is safe when both strings have +/// **publicly known, fixed lengths** (e.g. base64url-encoded SHA-256 digests are +/// always 43 bytes). Do **not** use this function to compare secrets of +/// variable or confidential length — use a constant-time comparison that +/// also hides length, such as comparing HMAC outputs. +/// +/// # Examples +/// +/// ``` +/// use trusted_server_core::http_util::ct_str_eq; +/// +/// assert!(ct_str_eq("hello", "hello")); +/// assert!(!ct_str_eq("hello", "world")); +/// assert!(!ct_str_eq("hello", "hell")); +/// ``` +#[must_use] +pub fn ct_str_eq(a: &str, b: &str) -> bool { + a.len() == b.len() && bool::from(a.as_bytes().ct_eq(b.as_bytes())) +} + /// Verify a `tstoken` for the given clear-text URL. +/// +/// Uses constant-time comparison to prevent timing side-channel attacks. +/// Length is not secret (always 43 bytes for base64url-encoded SHA-256), +/// but we check explicitly to document the invariant. #[must_use] pub fn verify_clear_url_signature(settings: &Settings, clear_url: &str, token: &str) -> bool { - sign_clear_url(settings, clear_url) == token + let expected = sign_clear_url(settings, clear_url); + ct_str_eq(&expected, token) } /// Compute tstoken for the new proxy scheme: SHA-256 of the encrypted full URL (including query). @@ -388,6 +423,33 @@ mod tests { )); } + #[test] + fn verify_clear_url_rejects_tampered_token() { + let settings = crate::test_support::tests::create_test_settings(); + let url = "https://cdn.example/a.png?x=1"; + let valid_token = sign_clear_url(&settings, url); + + // Flip one bit in the first byte — same URL, same length, wrong bytes + let mut tampered = valid_token.into_bytes(); + tampered[0] ^= 0x01; + let tampered = + String::from_utf8(tampered).expect("should be valid utf8 after single-bit flip"); + + assert!( + !verify_clear_url_signature(&settings, url, &tampered), + "should reject token with tampered bytes" + ); + } + + #[test] + fn verify_clear_url_rejects_empty_token() { + let settings = crate::test_support::tests::create_test_settings(); + assert!( + !verify_clear_url_signature(&settings, "https://cdn.example/a.png", ""), + "should reject empty token" + ); + } + // RequestInfo tests #[test] diff --git a/crates/trusted-server-core/src/proxy.rs b/crates/trusted-server-core/src/proxy.rs index 27de3510..4cc33de7 100644 --- a/crates/trusted-server-core/src/proxy.rs +++ b/crates/trusted-server-core/src/proxy.rs @@ -1,4 +1,4 @@ -use crate::http_util::compute_encrypted_sha256_token; +use crate::http_util::{compute_encrypted_sha256_token, ct_str_eq}; use error_stack::{Report, ResultExt}; use fastly::http::{header, HeaderValue, Method, StatusCode}; use fastly::{Request, Response}; @@ -1074,8 +1074,11 @@ fn reconstruct_and_validate_signed_target( }; let expected = compute_encrypted_sha256_token(settings, &full_for_token); - if expected != sig { - return Err(Report::new(TrustedServerError::Proxy { + // Constant-time comparison to prevent timing side-channel attacks on the token. + // Length is not secret (always 43 bytes for base64url-encoded SHA-256), + // but we check explicitly to document the invariant. + if !ct_str_eq(&expected, &sig) { + return Err(Report::new(TrustedServerError::Forbidden { message: "invalid tstoken".to_string(), })); } @@ -1275,6 +1278,29 @@ mod tests { assert_eq!(err.current_context().status_code(), StatusCode::BAD_GATEWAY); } + #[tokio::test] + async fn reconstruct_rejects_tampered_tstoken() { + let settings = create_test_settings(); + let tsurl = "https://cdn.example/asset.js"; + let tsurl_encoded = + url::form_urlencoded::byte_serialize(tsurl.as_bytes()).collect::(); + // Syntactically valid base64url token of the right length, but not the correct signature + let bad_token = "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"; + let url = format!( + "https://edge.example/first-party/proxy?tsurl={}&tstoken={}", + tsurl_encoded, bad_token + ); + + let err: Report = + reconstruct_and_validate_signed_target(&settings, &url) + .expect_err("should reject tampered token"); + assert_eq!( + err.current_context().status_code(), + StatusCode::FORBIDDEN, + "should return 403 for invalid tstoken" + ); + } + #[tokio::test] async fn click_missing_params_returns_400() { let settings = create_test_settings(); diff --git a/crates/trusted-server-core/src/redacted.rs b/crates/trusted-server-core/src/redacted.rs index 1cc91b1f..7193a00a 100644 --- a/crates/trusted-server-core/src/redacted.rs +++ b/crates/trusted-server-core/src/redacted.rs @@ -62,18 +62,6 @@ impl fmt::Display for Redacted { } } -impl PartialEq for Redacted { - fn eq(&self, other: &Self) -> bool { - self.0 == other.0 - } -} - -impl PartialEq for Redacted { - fn eq(&self, other: &T) -> bool { - self.0 == *other - } -} - impl From for Redacted { fn from(value: String) -> Self { Self(value) @@ -152,28 +140,6 @@ mod tests { ); } - #[test] - fn partial_eq_with_inner_type() { - let secret = Redacted::new("my-secret".to_string()); - assert!( - secret == "my-secret".to_string(), - "should equal the inner value" - ); - assert!( - secret != "other".to_string(), - "should not equal a different value" - ); - } - - #[test] - fn partial_eq_between_redacted() { - let a = Redacted::new("same".to_string()); - let b = Redacted::new("same".to_string()); - let c = Redacted::new("different".to_string()); - assert!(a == b, "should equal when inner values match"); - assert!(a != c, "should not equal when inner values differ"); - } - #[test] fn struct_with_redacted_field_debug() { #[derive(Debug)]