Use constant-time comparison for token and credential verification

[prk-Jr]

Summary

• Replace == comparisons with subtle::ConstantTimeEq in all three secret-verification sites (tstoken, clear-URL signature, Basic Auth), eliminating timing side-channel attacks
• Fix a secondary short-circuit oracle in Basic Auth: && allowed an attacker to distinguish wrong-username from wrong-password via timing — replaced with bitwise & on subtle::Choice so both fields are always evaluated
• Add log::warn! on auth failure (path only, no credentials) and five new targeted tests covering each fix

Changes

File	Change
Cargo.toml	Add subtle = "2" to workspace dependencies
crates/common/Cargo.toml	Add subtle as direct dependency
crates/common/src/auth.rs	CT comparison with & instead of &&; log::warn! on failure; 2 new tests
crates/common/src/http_util.rs	CT comparison in verify_clear_url_signature; 2 new tests
crates/common/src/proxy.rs	CT comparison for tstoken in reconstruct_and_validate_signed_target; 1 new test

Closes

Closes #410

Test plan

• cargo test --workspace
• cargo clippy --all-targets --all-features -- -D warnings
• cargo fmt --all -- --check
• JS tests: cd crates/js/lib && npx vitest run
• JS format: cd crates/js/lib && npm run format
• Docs format: cd docs && npm run format
• WASM build: cargo build --bin trusted-server-fastly --release --target wasm32-wasip1
• Manual testing via fastly compute serve

JS tests are failing on main due to a pre-existing ESM/CJS incompatibility in html-encoding-sniffer node_modules — unrelated to this PR.

Checklist

• Changes follow CLAUDE.md conventions
• No unwrap() in production code — use expect("should ...")
• Uses tracing macros (not println!)
• New code has tests
• No secrets or credentials committed

[ChristianPavilonis]

See below

[ChristianPavilonis]

Verdict: Clean, well-focused security hardening PR. Ready to merge with minor improvements.

Excellent anti-oracle design in auth.rs — bitwise & on subtle::Choice instead of && prevents username-existence oracle. Minimal, focused changes — only touches the 3 comparison sites + targeted tests. Good test coverage (5 new tests). subtle v2 is the right dependency choice.

[ChristianPavilonis]

P1 — ct_eq on different-length inputs leaks length via timing

subtle::ConstantTimeEq for [u8] returns 0 immediately when lengths differ — the comparison short-circuits on length mismatch. Practically low risk since token length is public knowledge (SHA-256 → base64url = always 43 chars), but it technically doesn't achieve fully constant-time behavior.

Suggestion — either add a length guard or document the invariant:

pub fn verify_clear_url_signature(settings: &Settings, clear_url: &str, token: &str) -> bool {
    let expected = sign_clear_url(settings, clear_url);
    // Length is not secret (always 43 bytes for base64url-encoded SHA-256),
    // but we check explicitly to document the invariant.
    expected.len() == token.len()
        && bool::from(expected.as_bytes().ct_eq(token.as_bytes()))
}

Same applies to the proxy.rs site.

[ChristianPavilonis]

P2 — Doc comment should mention constant-time behavior

The existing doc comment doesn't mention CT comparison. Future maintainers need to know not to regress this:

/// Verify a `tstoken` for the given clear-text URL.
///
/// Uses constant-time comparison to prevent timing side-channel attacks.

[ChristianPavilonis]

P2 — Missing doc comment on enforce_basic_auth

Public function with no doc comment (CLAUDE.md convention). The timing-attack mitigation and bitwise-AND design should be documented:

/// Enforce HTTP Basic Authentication for paths that require credentials.
///
/// Returns `None` if the request is authorized (or the path has no handler),
/// `Some(401 response)` otherwise. Uses constant-time comparison for both
/// username and password, and evaluates both regardless of individual match
/// results to prevent timing oracles.

[ChristianPavilonis]

P2 — Add inline comment explaining why ct_eq is used here

Unlike auth.rs which has a nice comment about the bitwise &, this site has no comment explaining why ct_eq is used. A one-liner would help future maintainers:

// Constant-time comparison to prevent timing side-channel attacks on the token.
let expected = compute_encrypted_sha256_token(settings, &full_for_token);

[ChristianPavilonis]

P3 — Convention: use Trait as _

Per CLAUDE.md: "Use use Trait as _ when you only need trait methods, not the trait name." ConstantTimeEq is only used via .ct_eq() — the trait name is never referenced directly.

use subtle::ConstantTimeEq as _;

Same applies to the imports in http_util.rs and proxy.rs.

[ChristianPavilonis]

P3 — Alphabetical ordering

subtle should go between sha2 and temp-env to maintain the alphabetical ordering of workspace dependencies.