diff --git a/Cargo.lock b/Cargo.lock index 96c38234..1dd449b5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4705,6 +4705,7 @@ dependencies = [ "smol_str", "socket2 0.6.4", "sqlx", + "subtle", "tantivy", "tempfile", "testcontainers-modules", diff --git a/Cargo.toml b/Cargo.toml index 16cbb6c6..aeb0fa2d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -125,6 +125,13 @@ mp3-duration = "0.1" kamadak-exif = "0.6.1" md-5 = "0.11.0" sha2 = "0.11.0" +# Constant-time equality primitives for security-sensitive comparisons. +# Direct dep is free: `subtle` is already pulled in transitively via +# sqlx-postgres → sha2 → digest, so this doesn't add a compile unit or +# bytes — just makes the import explicit for our own callsites (WebDAV +# lock-token comparison in `evaluate_if_header`, and any future +# token/secret comparisons). +subtle = "2.6" unicode-normalization = "0.1.25" blake3 = { version = "1.8.5", features = ["rayon", "mmap"] } hex = "0.4.3" diff --git a/src/infrastructure/services/webdav_lock_service.rs b/src/infrastructure/services/webdav_lock_service.rs index 9bedaf99..fc15bdb5 100644 --- a/src/infrastructure/services/webdav_lock_service.rs +++ b/src/infrastructure/services/webdav_lock_service.rs @@ -19,8 +19,26 @@ use std::sync::Arc; use std::time::{Duration, Instant}; +use subtle::ConstantTimeEq; + use crate::application::adapters::webdav_adapter::{LockInfo, LockScope}; +/// Constant-time equality for lock tokens. Same rationale as +/// `webdav_handler::ct_str_eq` — see that helper's doc-comment. +/// +/// The two callsites in this file (`refresh` at :169, `release` +/// at :191) are already gated by `self.by_token.get(token)?`, so +/// the attacker CANNOT reach these checks without already having +/// presented a valid token — the practical timing-attack surface is +/// nil. Kept constant-time for defense-in-depth consistency across +/// every token comparison in the WebDAV surface, so a future +/// auditor doesn't have to re-derive "this one is safe because…" +/// for each individual callsite. +#[inline] +fn ct_str_eq(a: &str, b: &str) -> bool { + a.len() == b.len() && a.as_bytes().ct_eq(b.as_bytes()).into() +} + /// Default lock timeout when the client does not specify one (RFC 4918 §10.7). const DEFAULT_LOCK_TIMEOUT_SECS: u64 = 1800; // 30 minutes @@ -166,7 +184,7 @@ impl WebDavLockStore { let path = self.by_token.get(token)?; let mut entry = self.by_path.get(&path)?; - if entry.info.token != token { + if !ct_str_eq(&entry.info.token, token) { return None; // token mismatch — lock was replaced } @@ -188,7 +206,7 @@ impl WebDavLockStore { if let Some(path) = self.by_token.get(token) { // Only remove from by_path if the token still matches if let Some(entry) = self.by_path.get(&path) - && entry.info.token == token + && ct_str_eq(&entry.info.token, token) { self.by_path.invalidate(&path); } diff --git a/src/interfaces/api/handlers/webdav_handler.rs b/src/interfaces/api/handlers/webdav_handler.rs index 22fd0374..780d9815 100644 --- a/src/interfaces/api/handlers/webdav_handler.rs +++ b/src/interfaces/api/handlers/webdav_handler.rs @@ -46,6 +46,7 @@ use crate::interfaces::upload_ingest::{IngestedBlob, RangeSegment, discard_inges use percent_encoding::{AsciiSet, NON_ALPHANUMERIC, percent_decode_str, utf8_percent_encode}; use std::collections::HashMap; use std::sync::Arc; +use subtle::ConstantTimeEq; /// Characters that MUST NOT be percent-encoded inside a URI path segment. /// RFC 3986 §3.3 pchar = unreserved / pct-encoded / sub-delims / ":" / "@" @@ -1581,6 +1582,34 @@ fn parse_if_header(header: &str) -> IfLists { lists } +/// Constant-time string equality for security-sensitive tokens +/// (WebDAV lock State-tokens today; extend for future session / +/// secret-adjacent comparisons if any). +/// +/// Rust's built-in `str::eq` compares byte-wise with early exit on +/// mismatch — the position of the differing byte is observable via +/// timing. For WebDAV lock tokens the practical exploit is not +/// realistic (ns-scale signal buried under ms-scale network jitter, +/// plus ~5×10⁸ samples needed to average through the noise before +/// the lock expires), but the fix is a 5-line change with zero +/// measurable perf cost and matches the "constant-time compare on +/// any token that gates access" hygiene rule the rest of the code +/// follows on session tokens. Reported responsibly on 2026-09-05. +/// +/// Length leaks are acceptable here — WebDAV lock tokens have a +/// fixed public format (`opaquelocktoken:`), so the length is +/// not secret and any timing distinguishability from a length +/// mismatch reveals nothing an attacker doesn't already know from +/// the URI grammar. +#[inline] +fn ct_str_eq(a: &str, b: &str) -> bool { + // `ct_eq` returns 1 on match, 0 on mismatch — same length always, + // no early exit within the byte compare. Different-length inputs + // still short-circuit at the length check (see doc note above), + // and equal-length inputs run the full constant-time compare. + a.len() == b.len() && a.as_bytes().ct_eq(b.as_bytes()).into() +} + /// Evaluate a parsed `If:` header against the current resource state. /// /// Returns `(header_true, submitted_active_lock)`: @@ -1614,7 +1643,7 @@ fn evaluate_if_header( negated: false, token, } = cond - && token == active + && ct_str_eq(token, active) { submitted_active_lock = true; } @@ -1627,7 +1656,14 @@ fn evaluate_if_header( list.iter().all(|cond| { let (negated, natural) = match cond { IfCondition::StateToken { negated, token } => { - let is_active = active_lock_token == Some(token.as_str()); + // Constant-time compare (see `ct_str_eq` above). + // `active_lock_token = None` short-circuits at the + // outer `Some(_)` match — that branch is only + // reachable when a lock actually exists, so the + // "no lock present" fast path stays public info. + let is_active = active_lock_token + .map(|a| ct_str_eq(token, a)) + .unwrap_or(false); (*negated, is_active) } IfCondition::EntityTag {