diff --git a/Cargo.lock b/Cargo.lock index 5792c2bb..191f1848 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2369,6 +2369,7 @@ dependencies = [ "serde", "serde_json", "sha2 0.10.9", + "subtle", "temp-env", "tokio", "tokio-test", diff --git a/Cargo.toml b/Cargo.toml index 67dc7668..8fc2feef 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -75,6 +75,7 @@ regex = "1.12.3" serde = { version = "1.0", features = ["derive"] } serde_json = "1.0.149" sha2 = "0.10.9" +subtle = "2" temp-env = "0.3.6" tokio = { version = "1.49", features = ["sync", "macros", "io-util", "rt", "time"] } tokio-test = "0.4" diff --git a/crates/common/Cargo.toml b/crates/common/Cargo.toml index c360474e..cb7dedc1 100644 --- a/crates/common/Cargo.toml +++ b/crates/common/Cargo.toml @@ -40,6 +40,7 @@ regex = { workspace = true } serde = { workspace = true } serde_json = { workspace = true } sha2 = { workspace = true } +subtle = { workspace = true } tokio = { workspace = true } toml = { workspace = true } trusted-server-js = { path = "../js" } diff --git a/crates/common/src/auth.rs b/crates/common/src/auth.rs index 020f648c..2ad0697d 100644 --- a/crates/common/src/auth.rs +++ b/crates/common/src/auth.rs @@ -1,25 +1,29 @@ use base64::{engine::general_purpose::STANDARD, Engine as _}; use fastly::http::{header, StatusCode}; use fastly::{Request, Response}; +use subtle::ConstantTimeEq as _; 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(); @@ -30,9 +34,14 @@ pub fn enforce_basic_auth(settings: &Settings, req: &Request) -> Option return Some(unauthorized_response()), }; - if username == handler.username && password == handler.password { + // Use bitwise & (not &&) so both sides always evaluate — eliminates the + // username-existence oracle that short-circuit evaluation would create. + let username_ok = username.as_bytes().ct_eq(handler.username.as_bytes()); + let password_ok = password.as_bytes().ct_eq(handler.password.as_bytes()); + if (username_ok & password_ok).into() { None } else { + log::warn!("Basic auth failed for path: {}", req.get_path()); Some(unauthorized_response()) } } @@ -125,6 +134,37 @@ mod tests { assert_eq!(response.get_status(), StatusCode::UNAUTHORIZED); } + #[test] + fn challenge_when_username_wrong_password_correct() { + // Validates that both fields are always evaluated — no short-circuit username oracle. + let settings = settings_with_handlers(); + let mut req = Request::new(Method::GET, "https://example.com/secure/data"); + let token = STANDARD.encode("wrong-user:pass"); + req.set_header(header::AUTHORIZATION, format!("Basic {token}")); + + let response = enforce_basic_auth(&settings, &req).expect("should challenge"); + assert_eq!( + response.get_status(), + StatusCode::UNAUTHORIZED, + "should reject wrong username even with correct password" + ); + } + + #[test] + fn challenge_when_username_correct_password_wrong() { + let settings = settings_with_handlers(); + let mut req = Request::new(Method::GET, "https://example.com/secure/data"); + let token = STANDARD.encode("user:wrong-pass"); + req.set_header(header::AUTHORIZATION, format!("Basic {token}")); + + let response = enforce_basic_auth(&settings, &req).expect("should challenge"); + assert_eq!( + response.get_status(), + StatusCode::UNAUTHORIZED, + "should reject correct username with wrong password" + ); + } + #[test] fn challenge_when_scheme_is_not_basic() { let settings = settings_with_handlers(); diff --git a/crates/common/src/http_util.rs b/crates/common/src/http_util.rs index 5b3c4b8b..b0546c82 100644 --- a/crates/common/src/http_util.rs +++ b/crates/common/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; @@ -319,9 +320,14 @@ pub fn sign_clear_url(settings: &Settings, clear_url: &str) -> String { } /// 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); + expected.len() == token.len() && bool::from(expected.as_bytes().ct_eq(token.as_bytes())) } /// Compute tstoken for the new proxy scheme: SHA-256 of the encrypted full URL (including query). @@ -388,6 +394,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/common/src/proxy.rs b/crates/common/src/proxy.rs index 27de3510..89b889f0 100644 --- a/crates/common/src/proxy.rs +++ b/crates/common/src/proxy.rs @@ -4,6 +4,7 @@ use fastly::http::{header, HeaderValue, Method, StatusCode}; use fastly::{Request, Response}; use serde::{Deserialize, Serialize}; use std::time::{Duration, SystemTime, UNIX_EPOCH}; +use subtle::ConstantTimeEq as _; use crate::constants::{ HEADER_ACCEPT, HEADER_ACCEPT_ENCODING, HEADER_ACCEPT_LANGUAGE, HEADER_REFERER, @@ -1074,7 +1075,12 @@ fn reconstruct_and_validate_signed_target( }; let expected = compute_encrypted_sha256_token(settings, &full_for_token); - if expected != sig { + // 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. + let valid = + expected.len() == sig.len() && bool::from(expected.as_bytes().ct_eq(sig.as_bytes())); + if !valid { return Err(Report::new(TrustedServerError::Proxy { message: "invalid tstoken".to_string(), })); @@ -1275,6 +1281,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::BAD_GATEWAY, + "should return 502 for invalid tstoken" + ); + } + #[tokio::test] async fn click_missing_params_returns_400() { let settings = create_test_settings();