diff --git a/crates/trusted-server-core/src/cookies.rs b/crates/trusted-server-core/src/cookies.rs index 8757cae5..acbffce8 100644 --- a/crates/trusted-server-core/src/cookies.rs +++ b/crates/trusted-server-core/src/cookies.rs @@ -126,12 +126,34 @@ pub fn forward_cookie_header(from: &Request, to: &mut Request, strip_consent: bo } } -/// Creates a synthetic ID cookie string. +/// Returns `true` if every byte in `value` is a valid RFC 6265 `cookie-octet`. +/// An empty string is always rejected. /// -/// Generates a properly formatted cookie with security attributes -/// for storing the synthetic ID. +/// RFC 6265 restricts cookie values to printable US-ASCII excluding whitespace, +/// double-quote, comma, semicolon, and backslash. Rejecting these characters +/// prevents header-injection attacks where a crafted value could append +/// spurious cookie attributes (e.g. `evil; Domain=.attacker.com`). +/// +/// Non-ASCII characters (multi-byte UTF-8) are always rejected because their +/// byte values exceed `0x7E`. +#[must_use] +fn is_safe_cookie_value(value: &str) -> bool { + // RFC 6265 §4.1.1 cookie-octet: + // 0x21 — '!' + // 0x23–0x2B — '#' through '+' (excludes 0x22 DQUOTE) + // 0x2D–0x3A — '-' through ':' (excludes 0x2C comma) + // 0x3C–0x5B — '<' through '[' (excludes 0x3B semicolon) + // 0x5D–0x7E — ']' through '~' (excludes 0x5C backslash, 0x7F DEL) + // All control characters (0x00–0x20) and non-ASCII (0x80+) are also excluded. + !value.is_empty() + && value + .bytes() + .all(|b| matches!(b, 0x21 | 0x23..=0x2B | 0x2D..=0x3A | 0x3C..=0x5B | 0x5D..=0x7E)) +} + +/// Formats the `Set-Cookie` header value for the synthetic ID cookie. #[must_use] -pub fn create_synthetic_cookie(settings: &Settings, synthetic_id: &str) -> String { +fn create_synthetic_cookie(settings: &Settings, synthetic_id: &str) -> String { format!( "{}={}; Domain={}; Path=/; Secure; SameSite=Lax; Max-Age={}", COOKIE_SYNTHETIC_ID, synthetic_id, settings.publisher.cookie_domain, COOKIE_MAX_AGE, @@ -140,13 +162,24 @@ pub fn create_synthetic_cookie(settings: &Settings, synthetic_id: &str) -> Strin /// Sets the synthetic ID cookie on the given response. /// -/// This helper abstracts the logic of creating the cookie string and appending -/// the Set-Cookie header to the response. +/// Validates `synthetic_id` against RFC 6265 `cookie-octet` rules before +/// interpolation. If the value contains unsafe characters (e.g. semicolons), +/// the cookie is not set and a warning is logged. This prevents an attacker +/// from injecting spurious cookie attributes via a controlled ID value. +/// +/// `cookie_domain` comes from operator configuration and is considered trusted. pub fn set_synthetic_cookie( settings: &Settings, response: &mut fastly::Response, synthetic_id: &str, ) { + if !is_safe_cookie_value(synthetic_id) { + log::warn!( + "Rejecting synthetic_id for Set-Cookie: value of {} bytes contains characters illegal in a cookie value", + synthetic_id.len() + ); + return; + } response.append_header( header::SET_COOKIE, create_synthetic_cookie(settings, synthetic_id), @@ -191,7 +224,7 @@ mod tests { } #[test] - fn test_parse_cookies_to_jar_emtpy() { + fn test_parse_cookies_to_jar_empty() { let cookie_str = ""; let jar = parse_cookies_to_jar(cookie_str); @@ -247,35 +280,102 @@ mod tests { } #[test] - fn test_create_synthetic_cookie() { + fn test_set_synthetic_cookie() { let settings = create_test_settings(); - let result = create_synthetic_cookie(&settings, "12345"); + let mut response = fastly::Response::new(); + set_synthetic_cookie(&settings, &mut response, "abc123.XyZ789"); + + let cookie_str = response + .get_header(header::SET_COOKIE) + .expect("Set-Cookie header should be present") + .to_str() + .expect("header should be valid UTF-8"); + assert_eq!( - result, + cookie_str, format!( - "{}=12345; Domain={}; Path=/; Secure; SameSite=Lax; Max-Age={}", + "{}=abc123.XyZ789; Domain={}; Path=/; Secure; SameSite=Lax; Max-Age={}", COOKIE_SYNTHETIC_ID, settings.publisher.cookie_domain, COOKIE_MAX_AGE, - ) + ), + "Set-Cookie header should match expected format" ); } #[test] - fn test_set_synthetic_cookie() { + fn test_set_synthetic_cookie_rejects_semicolon() { let settings = create_test_settings(); let mut response = fastly::Response::new(); - set_synthetic_cookie(&settings, &mut response, "test-id-123"); + set_synthetic_cookie(&settings, &mut response, "evil; Domain=.attacker.com"); - let cookie_header = response - .get_header(header::SET_COOKIE) - .expect("Set-Cookie header should be present"); - let cookie_str = cookie_header - .to_str() - .expect("header should be valid UTF-8"); + assert!( + response.get_header(header::SET_COOKIE).is_none(), + "Set-Cookie should not be set when value contains a semicolon" + ); + } - let expected = create_synthetic_cookie(&settings, "test-id-123"); - assert_eq!( - cookie_str, expected, - "Set-Cookie header should match create_synthetic_cookie output" + #[test] + fn test_set_synthetic_cookie_rejects_crlf() { + let settings = create_test_settings(); + let mut response = fastly::Response::new(); + set_synthetic_cookie(&settings, &mut response, "evil\r\nX-Injected: header"); + + assert!( + response.get_header(header::SET_COOKIE).is_none(), + "Set-Cookie should not be set when value contains CRLF" + ); + } + + #[test] + fn test_set_synthetic_cookie_rejects_space() { + let settings = create_test_settings(); + let mut response = fastly::Response::new(); + set_synthetic_cookie(&settings, &mut response, "bad value"); + + assert!( + response.get_header(header::SET_COOKIE).is_none(), + "Set-Cookie should not be set when value contains whitespace" + ); + } + + #[test] + fn test_is_safe_cookie_value_rejects_empty_string() { + assert!(!is_safe_cookie_value(""), "should reject empty string"); + } + + #[test] + fn test_is_safe_cookie_value_accepts_valid_synthetic_id_characters() { + // Hex digits, dot separator, alphanumeric suffix — the full synthetic ID character set + assert!( + is_safe_cookie_value("abcdef0123456789.ABCDEFabcdef"), + "should accept hex digits, dots, and alphanumeric characters" + ); + } + + #[test] + fn test_is_safe_cookie_value_rejects_non_ascii() { + assert!( + !is_safe_cookie_value("valüe"), + "should reject non-ASCII UTF-8 characters" + ); + } + + #[test] + fn test_is_safe_cookie_value_rejects_illegal_characters() { + assert!(!is_safe_cookie_value("val;ue"), "should reject semicolon"); + assert!(!is_safe_cookie_value("val,ue"), "should reject comma"); + assert!( + !is_safe_cookie_value("val\"ue"), + "should reject double-quote" + ); + assert!(!is_safe_cookie_value("val\\ue"), "should reject backslash"); + assert!(!is_safe_cookie_value("val ue"), "should reject space"); + assert!( + !is_safe_cookie_value("val\x00ue"), + "should reject null byte" + ); + assert!( + !is_safe_cookie_value("val\x7fue"), + "should reject DEL character" ); } diff --git a/crates/trusted-server-core/src/integrations/registry.rs b/crates/trusted-server-core/src/integrations/registry.rs index 158a1e29..7d87d4f8 100644 --- a/crates/trusted-server-core/src/integrations/registry.rs +++ b/crates/trusted-server-core/src/integrations/registry.rs @@ -658,7 +658,9 @@ impl IntegrationRegistry { // Generate synthetic ID before consuming request let synthetic_id_result = get_or_generate_synthetic_id(settings, &req); - // Set synthetic ID header on the request so integrations can read it + // Set synthetic ID header on the request so integrations can read it. + // Header injection: Fastly's HeaderValue API rejects values containing \r, \n, or \0, + // so a crafted synthetic_id cannot inject additional request headers. if let Ok(ref synthetic_id) = synthetic_id_result { req.set_header(HEADER_X_SYNTHETIC_ID, synthetic_id.as_str()); } @@ -669,7 +671,13 @@ impl IntegrationRegistry { if let Ok(ref mut response) = result { match synthetic_id_result { Ok(ref synthetic_id) => { + // Response-header injection: Fastly's HeaderValue API rejects values + // containing \r, \n, or \0, so a crafted synthetic_id cannot inject + // additional response headers. response.set_header(HEADER_X_SYNTHETIC_ID, synthetic_id.as_str()); + // Cookie is intentionally not set when synthetic_id contains RFC 6265-illegal + // characters (e.g. a crafted x-synthetic-id header value). The response header + // is still emitted; only cookie persistence is skipped. set_synthetic_cookie(settings, response, synthetic_id.as_str()); } Err(ref err) => { diff --git a/crates/trusted-server-core/src/publisher.rs b/crates/trusted-server-core/src/publisher.rs index cec56b25..a2f54441 100644 --- a/crates/trusted-server-core/src/publisher.rs +++ b/crates/trusted-server-core/src/publisher.rs @@ -370,7 +370,11 @@ pub fn handle_publisher_request( // - Consent absent + existing cookie → revoke (expire cookie + delete KV entry). // - Consent absent + no cookie → do nothing. if ssc_allowed { + // Fastly's HeaderValue API rejects \r, \n, and \0, so the synthetic ID + // cannot inject additional response headers. response.set_header(HEADER_X_SYNTHETIC_ID, synthetic_id.as_str()); + // Cookie persistence is skipped if the synthetic ID contains RFC 6265-illegal + // characters. The header is still emitted when consent allows it. set_synthetic_cookie(settings, &mut response, synthetic_id.as_str()); } else if let Some(cookie_synthetic_id) = existing_ssc_cookie.as_deref() { log::info!(