From eb58609d52c9039a32e77d3ff5c0f2ae06714188 Mon Sep 17 00:00:00 2001 From: Christian Date: Thu, 19 Mar 2026 14:52:50 -0500 Subject: [PATCH 1/3] Use constant-time comparison for admin basic auth credentials Replace standard string equality with subtle::ConstantTimeEq for username and password checks in enforce_basic_auth. This prevents timing side-channel attacks that could leak credential prefix information through response latency differences. Closes #447 --- Cargo.lock | 1 + Cargo.toml | 1 + crates/trusted-server-core/Cargo.toml | 1 + crates/trusted-server-core/src/auth.rs | 67 +++++++++++++++++++++++++- 4 files changed, 69 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index 51837298..de9dd4ea 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2827,6 +2827,7 @@ dependencies = [ "serde", "serde_json", "sha2 0.10.9", + "subtle", "temp-env", "tokio", "tokio-test", diff --git a/Cargo.toml b/Cargo.toml index 424c357d..00655e1e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -82,6 +82,7 @@ regex = "1.12.3" serde = { version = "1.0", features = ["derive"] } serde_json = "1.0.149" sha2 = "0.10.9" +subtle = "2.6" temp-env = "0.3.6" tokio = { version = "1.49", features = ["sync", "macros", "io-util", "rt", "time"] } tokio-test = "0.4" diff --git a/crates/trusted-server-core/Cargo.toml b/crates/trusted-server-core/Cargo.toml index 76b1106d..6a321fd2 100644 --- a/crates/trusted-server-core/Cargo.toml +++ b/crates/trusted-server-core/Cargo.toml @@ -41,6 +41,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/trusted-server-core/src/auth.rs b/crates/trusted-server-core/src/auth.rs index 816891f6..39160b8b 100644 --- a/crates/trusted-server-core/src/auth.rs +++ b/crates/trusted-server-core/src/auth.rs @@ -1,6 +1,7 @@ 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; @@ -30,7 +31,21 @@ pub fn enforce_basic_auth(settings: &Settings, req: &Request) -> Option return Some(unauthorized_response()), }; - if handler.username == username && handler.password == password { + // Use constant-time comparison to prevent timing side-channel attacks + // that could leak password prefix information. Both username and password + // are always fully compared regardless of where the first mismatch occurs. + let username_match = handler + .username + .expose() + .as_bytes() + .ct_eq(username.as_bytes()); + let password_match = handler + .password + .expose() + .as_bytes() + .ct_eq(password.as_bytes()); + + if bool::from(username_match & password_match) { None } else { Some(unauthorized_response()) @@ -169,4 +184,54 @@ mod tests { .expect("should challenge admin path with missing credentials"); assert_eq!(response.get_status(), StatusCode::UNAUTHORIZED); } + + #[test] + fn challenge_when_username_wrong_password_correct() { + let settings = settings_with_handlers(); + let mut req = Request::new(Method::GET, "https://example.com/secure/data"); + let token = STANDARD.encode("wrong:pass"); + req.set_header(header::AUTHORIZATION, format!("Basic {token}")); + + let response = enforce_basic_auth(&settings, &req) + .expect("should challenge when only username is wrong"); + assert_eq!(response.get_status(), StatusCode::UNAUTHORIZED); + } + + #[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"); + req.set_header(header::AUTHORIZATION, format!("Basic {token}")); + + let response = enforce_basic_auth(&settings, &req) + .expect("should challenge when only password is wrong"); + assert_eq!(response.get_status(), StatusCode::UNAUTHORIZED); + } + + /// Both valid and invalid credentials exercise the same constant-time + /// comparison path via `subtle::ConstantTimeEq`, preventing timing + /// side-channels from leaking password prefix information. + #[test] + fn constant_time_comparison_exercises_both_fields() { + let settings = settings_with_handlers(); + + // Valid credentials — both fields evaluated in constant time + let mut valid_req = Request::new(Method::GET, "https://example.com/secure/data"); + let valid_token = STANDARD.encode("user:pass"); + valid_req.set_header(header::AUTHORIZATION, format!("Basic {valid_token}")); + assert!( + enforce_basic_auth(&settings, &valid_req).is_none(), + "should allow valid credentials" + ); + + // Invalid credentials — both fields still fully evaluated in constant time + let mut invalid_req = Request::new(Method::GET, "https://example.com/secure/data"); + let invalid_token = STANDARD.encode("wrong:wrong"); + invalid_req.set_header(header::AUTHORIZATION, format!("Basic {invalid_token}")); + assert!( + enforce_basic_auth(&settings, &invalid_req).is_some(), + "should reject invalid credentials" + ); + } } From 322e77f445e0ba918ee56f3f2dbc245592b0e92b Mon Sep 17 00:00:00 2001 From: Christian Date: Thu, 19 Mar 2026 16:26:33 -0500 Subject: [PATCH 2/3] review changes --- crates/trusted-server-core/src/auth.rs | 85 +++++++--------------- crates/trusted-server-core/src/redacted.rs | 4 + 2 files changed, 31 insertions(+), 58 deletions(-) diff --git a/crates/trusted-server-core/src/auth.rs b/crates/trusted-server-core/src/auth.rs index 39160b8b..e23fd06d 100644 --- a/crates/trusted-server-core/src/auth.rs +++ b/crates/trusted-server-core/src/auth.rs @@ -1,6 +1,7 @@ use base64::{engine::general_purpose::STANDARD, Engine as _}; use fastly::http::{header, StatusCode}; use fastly::{Request, Response}; +use sha2::{Digest as _, Sha256}; use subtle::ConstantTimeEq as _; use crate::settings::Settings; @@ -31,19 +32,17 @@ pub fn enforce_basic_auth(settings: &Settings, req: &Request) -> Option return Some(unauthorized_response()), }; - // Use constant-time comparison to prevent timing side-channel attacks - // that could leak password prefix information. Both username and password - // are always fully compared regardless of where the first mismatch occurs. - let username_match = handler - .username - .expose() - .as_bytes() - .ct_eq(username.as_bytes()); - let password_match = handler - .password - .expose() - .as_bytes() - .ct_eq(password.as_bytes()); + // Hash before comparing to normalise lengths — `ct_eq` on raw byte slices + // short-circuits when lengths differ, which would leak credential length. + // SHA-256 produces fixed-size digests so the comparison is truly constant-time. + // + // Note: constant-time guarantees are best-effort on WASM targets because the + // runtime optimiser/JIT may re-introduce variable-time paths. This is an + // inherent limitation of all constant-time code in managed runtimes. + let username_match = Sha256::digest(handler.username.expose().as_bytes()) + .ct_eq(&Sha256::digest(username.as_bytes())); + let password_match = Sha256::digest(handler.password.expose().as_bytes()) + .ct_eq(&Sha256::digest(password.as_bytes())); if bool::from(username_match & password_match) { None @@ -91,16 +90,11 @@ mod tests { use base64::engine::general_purpose::STANDARD; use fastly::http::{header, Method}; - use crate::test_support::tests::crate_test_settings_str; - - fn settings_with_handlers() -> Settings { - let config = crate_test_settings_str(); - Settings::from_toml(&config).expect("should parse settings with handlers") - } + use crate::test_support::tests::create_test_settings; #[test] fn no_challenge_for_non_protected_path() { - let settings = settings_with_handlers(); + let settings = create_test_settings(); let req = Request::new(Method::GET, "https://example.com/open"); assert!(enforce_basic_auth(&settings, &req).is_none()); @@ -108,7 +102,7 @@ mod tests { #[test] fn challenge_when_missing_credentials() { - let settings = settings_with_handlers(); + let settings = create_test_settings(); let req = Request::new(Method::GET, "https://example.com/secure"); let response = enforce_basic_auth(&settings, &req).expect("should challenge"); @@ -121,7 +115,7 @@ mod tests { #[test] fn allow_when_credentials_match() { - let settings = settings_with_handlers(); + let settings = create_test_settings(); let mut req = Request::new(Method::GET, "https://example.com/secure/data"); let token = STANDARD.encode("user:pass"); req.set_header(header::AUTHORIZATION, format!("Basic {token}")); @@ -130,19 +124,20 @@ mod tests { } #[test] - fn challenge_when_credentials_mismatch() { - let settings = settings_with_handlers(); + fn challenge_when_both_credentials_wrong() { + let settings = create_test_settings(); let mut req = Request::new(Method::GET, "https://example.com/secure/data"); - let token = STANDARD.encode("user:wrong"); + let token = STANDARD.encode("wrong:wrong"); req.set_header(header::AUTHORIZATION, format!("Basic {token}")); - let response = enforce_basic_auth(&settings, &req).expect("should challenge"); + let response = enforce_basic_auth(&settings, &req) + .expect("should challenge when both username and password are wrong"); assert_eq!(response.get_status(), StatusCode::UNAUTHORIZED); } #[test] fn challenge_when_scheme_is_not_basic() { - let settings = settings_with_handlers(); + let settings = create_test_settings(); let mut req = Request::new(Method::GET, "https://example.com/secure"); req.set_header(header::AUTHORIZATION, "Bearer token"); @@ -152,7 +147,7 @@ mod tests { #[test] fn allow_admin_path_with_valid_credentials() { - let settings = settings_with_handlers(); + let settings = create_test_settings(); let mut req = Request::new(Method::POST, "https://example.com/admin/keys/rotate"); let token = STANDARD.encode("admin:admin-pass"); req.set_header(header::AUTHORIZATION, format!("Basic {token}")); @@ -165,7 +160,7 @@ mod tests { #[test] fn challenge_admin_path_with_wrong_credentials() { - let settings = settings_with_handlers(); + let settings = create_test_settings(); let mut req = Request::new(Method::POST, "https://example.com/admin/keys/rotate"); let token = STANDARD.encode("admin:wrong"); req.set_header(header::AUTHORIZATION, format!("Basic {token}")); @@ -177,7 +172,7 @@ mod tests { #[test] fn challenge_admin_path_with_missing_credentials() { - let settings = settings_with_handlers(); + let settings = create_test_settings(); let req = Request::new(Method::POST, "https://example.com/admin/keys/rotate"); let response = enforce_basic_auth(&settings, &req) @@ -187,7 +182,7 @@ mod tests { #[test] fn challenge_when_username_wrong_password_correct() { - let settings = settings_with_handlers(); + let settings = create_test_settings(); let mut req = Request::new(Method::GET, "https://example.com/secure/data"); let token = STANDARD.encode("wrong:pass"); req.set_header(header::AUTHORIZATION, format!("Basic {token}")); @@ -199,7 +194,7 @@ mod tests { #[test] fn challenge_when_username_correct_password_wrong() { - let settings = settings_with_handlers(); + let settings = create_test_settings(); let mut req = Request::new(Method::GET, "https://example.com/secure/data"); let token = STANDARD.encode("user:wrong"); req.set_header(header::AUTHORIZATION, format!("Basic {token}")); @@ -208,30 +203,4 @@ mod tests { .expect("should challenge when only password is wrong"); assert_eq!(response.get_status(), StatusCode::UNAUTHORIZED); } - - /// Both valid and invalid credentials exercise the same constant-time - /// comparison path via `subtle::ConstantTimeEq`, preventing timing - /// side-channels from leaking password prefix information. - #[test] - fn constant_time_comparison_exercises_both_fields() { - let settings = settings_with_handlers(); - - // Valid credentials — both fields evaluated in constant time - let mut valid_req = Request::new(Method::GET, "https://example.com/secure/data"); - let valid_token = STANDARD.encode("user:pass"); - valid_req.set_header(header::AUTHORIZATION, format!("Basic {valid_token}")); - assert!( - enforce_basic_auth(&settings, &valid_req).is_none(), - "should allow valid credentials" - ); - - // Invalid credentials — both fields still fully evaluated in constant time - let mut invalid_req = Request::new(Method::GET, "https://example.com/secure/data"); - let invalid_token = STANDARD.encode("wrong:wrong"); - invalid_req.set_header(header::AUTHORIZATION, format!("Basic {invalid_token}")); - assert!( - enforce_basic_auth(&settings, &invalid_req).is_some(), - "should reject invalid credentials" - ); - } } diff --git a/crates/trusted-server-core/src/redacted.rs b/crates/trusted-server-core/src/redacted.rs index 1cc91b1f..b088159d 100644 --- a/crates/trusted-server-core/src/redacted.rs +++ b/crates/trusted-server-core/src/redacted.rs @@ -62,6 +62,10 @@ impl fmt::Display for Redacted { } } +// TODO(#447): These `PartialEq` impls use standard (non-constant-time) comparison. +// Security-sensitive call-sites should use `.expose()` with `subtle::ConstantTimeEq` +// instead. Consider removing these impls to prevent accidental non-constant-time +// comparisons of secrets. impl PartialEq for Redacted { fn eq(&self, other: &Self) -> bool { self.0 == other.0 From 6de09722314f14e2f72359dc299b76ac83b30efa Mon Sep 17 00:00:00 2001 From: Christian Date: Thu, 19 Mar 2026 16:34:12 -0500 Subject: [PATCH 3/3] Remove stale TODO referencing #447 from Redacted PartialEq --- crates/trusted-server-core/src/redacted.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/crates/trusted-server-core/src/redacted.rs b/crates/trusted-server-core/src/redacted.rs index b088159d..1cc91b1f 100644 --- a/crates/trusted-server-core/src/redacted.rs +++ b/crates/trusted-server-core/src/redacted.rs @@ -62,10 +62,6 @@ impl fmt::Display for Redacted { } } -// TODO(#447): These `PartialEq` impls use standard (non-constant-time) comparison. -// Security-sensitive call-sites should use `.expose()` with `subtle::ConstantTimeEq` -// instead. Consider removing these impls to prevent accidental non-constant-time -// comparisons of secrets. impl PartialEq for Redacted { fn eq(&self, other: &Self) -> bool { self.0 == other.0