diff --git a/src/config.rs b/src/config.rs index 14146c9..0afd54e 100644 --- a/src/config.rs +++ b/src/config.rs @@ -506,23 +506,43 @@ pub(crate) fn livekit_scheme(url: &str) -> Option<&'static (&'static str, bool, }) } -/// Rejects what `url_origin` in `web.rs` would otherwise have to cope with: a -/// scheme nothing here knows, or a scheme with no host after it. The URL never -/// appears in the message, because the message reaches stderr and a LiveKit URL -/// can carry a query string. +pub(crate) fn livekit_host_is_csp_safe(authority: &str) -> bool { + let Some((host, port)) = split_authority(authority) else { + return false; + }; + let host_is_safe = if authority.starts_with('[') { + host.parse::().is_ok() + } else { + !host.is_empty() + && host + .bytes() + .all(|byte| byte.is_ascii_alphanumeric() || matches!(byte, b'.' | b'-')) + }; + host_is_safe && port.is_none_or(|port| port.parse::().is_ok_and(|port| port != 0)) +} + +/// Rejects what the URL's consumers would otherwise have to cope with: an +/// unknown scheme, no host, or host syntax that cannot become a CSP source. +/// The URL never appears in the message, because the message reaches stderr +/// and a LiveKit URL can carry a query string. pub fn validate_livekit_url(url: &str, production: bool) -> Result<(), String> { let trimmed = url.trim(); let Some((scheme, encrypted, _)) = livekit_scheme(trimmed) else { return Err("LIVEKIT_URL must start with wss://, https://, ws:// or http://".to_string()); }; - if trimmed[scheme.len()..] + let host = trimmed[scheme.len()..] .split(['/', '?', '#']) .next() .unwrap_or_default() - .is_empty() - { + .rsplit('@') + .next() + .unwrap_or_default(); + if host.is_empty() { return Err("LIVEKIT_URL has a scheme but no host".to_string()); } + if !livekit_host_is_csp_safe(host) { + return Err("LIVEKIT_URL host contains invalid characters".to_string()); + } if production && !encrypted { return Err("LIVEKIT_URL must use wss:// or https:// when NODE_ENV=production".to_string()); } diff --git a/src/livekit.rs b/src/livekit.rs index 43b4fef..ce9ff75 100644 --- a/src/livekit.rs +++ b/src/livekit.rs @@ -1863,16 +1863,15 @@ async fn on_interruption( // What Gemini heard is the whole diagnosis. It interrupts on its own voice // activity detection, so a cut with the candidate mid-sentence is barge-in - // working, and a cut with nothing transcribed is the microphone hearing the - // interviewer through the candidate's speakers. The line reported the size - // of the loss and left the cause to guesswork across a whole session of - // them. + // working. A cut with nothing transcribed is usually speaker echo or room + // noise, and naming those makes the log actionable without pretending the + // server can tell them apart. let heard = context.turns.candidate.tail(80); eprintln!( "timing: Gemini cut its own turn, {:.1}s of it unplayed; candidate audio so far: {}", unplayed.as_secs_f64(), if heard.is_empty() { - "(nothing transcribed)" + "(nothing transcribed; check speaker echo or background noise)" } else { heard } diff --git a/src/web/policy.rs b/src/web/policy.rs index 882fb52..4a69635 100644 --- a/src/web/policy.rs +++ b/src/web/policy.rs @@ -81,20 +81,14 @@ pub(crate) fn content_security_policy(config: &WebServerConfig) -> String { // LiveKit Cloud answers `/settings/regions` with a regional hostname and // the SDK retries there, so the configured host alone is not enough. - // Self-hosted deployments have no such indirection and keep their exact - // origins. - // - // One pass, so a third source of endpoints is added to the iterator above - // and both the exact origins and the wildcard follow from it. The wildcard - // names the domain it was granted for, because a deployment on one cloud - // domain has no reason to reach the other. - let mut cloud: Vec<&'static str> = Vec::new(); + // Keeping the wildcard beside the endpoint's two exact origins means a + // future endpoint source cannot add only the initial signaling host and + // bring this browser-only failure back. for url in endpoints { - connect.extend(livekit_origins(url)); - if let Some(domain) = livekit_cloud_domain(url).filter(|it| !cloud.contains(it)) { - cloud.push(domain); - connect.push(format!("https://*.{domain}")); - connect.push(format!("wss://*.{domain}")); + for source in livekit_connect_sources(url) { + if !connect.contains(&source) { + connect.push(source); + } } } if !config.production { @@ -141,10 +135,7 @@ pub(crate) fn content_security_policy(config: &WebServerConfig) -> String { /// HTTPS calls to the same host, region settings among them. Naming only the /// `wss://` origin lets the socket open and then blocks those, which surfaces /// as a connection that fails for no stated reason. -pub(crate) fn livekit_origins(url: &str) -> Vec { - let Some(origin) = url_origin(url) else { - return Vec::new(); - }; +fn livekit_origins_for_csp_origin(origin: &str) -> Vec { let Some((scheme, host)) = origin.split_once("://") else { return Vec::new(); }; @@ -153,9 +144,36 @@ pub(crate) fn livekit_origins(url: &str) -> Vec { "ws" => "http", "https" => "wss", "http" => "ws", - _ => return vec![origin], + _ => return Vec::new(), }; - vec![format!("{sibling}://{host}"), origin] + vec![format!("{sibling}://{host}"), origin.to_string()] +} + +/// The origin spelling CSP accepts. A LiveKit URL may carry userinfo for a +/// server-side client, but credentials are not part of a CSP host source and +/// would make the browser discard the source that needs to admit the socket. +fn csp_origin(url: &str) -> Option { + let origin = url_origin(url)?; + crate::config::livekit_scheme(&origin)?; + let (scheme, authority) = origin.split_once("://")?; + let host = authority.rsplit('@').next()?; + crate::config::livekit_host_is_csp_safe(host).then(|| format!("{scheme}://{host}")) +} + +/// Every CSP source a LiveKit endpoint needs. Cloud projects redirect the +/// browser to a regional hostname, while a self-hosted project has only the +/// exact HTTP and WebSocket origins from the CSP-safe endpoint. +fn livekit_connect_sources(url: &str) -> Vec { + let Some(origin) = csp_origin(url) else { + return Vec::new(); + }; + + let mut sources = livekit_origins_for_csp_origin(&origin); + if let Some(domain) = livekit_cloud_domain(&origin) { + sources.push(format!("https://*.{domain}")); + sources.push(format!("wss://*.{domain}")); + } + sources } /// The two domains `livekit-client.js` treats as LiveKit Cloud, which is what @@ -207,7 +225,7 @@ pub(crate) fn livekit_http_origin(url: &str) -> Option { /// configure and the server will start on. Every reader below then matches the /// scheme exactly, and each one fails differently on the uppercase form: the /// quota probe finds no HTTP origin and treats the project as available, so an -/// exhausted project is never passed over, and `livekit_origins` finds no +/// exhausted project is never passed over, and the CSP origin pairing finds no /// sibling, so the CSP omits the `https://` origin the SDK needs for /// `/settings/regions` and the socket opens onto blocked requests. One /// normalization at the boundary is the alternative to three readers each diff --git a/tests/config.rs b/tests/config.rs index eabbf0d..73f68f5 100644 --- a/tests/config.rs +++ b/tests/config.rs @@ -737,12 +737,24 @@ fn plaintext_livekit_is_local_only() { for url in [ "wss://example.livekit.cloud", "https://example.livekit.cloud", + "wss://example.livekit.cloud:443", + "wss://[2001:db8::1]:7880", ] { assert!(codetrial::config::validate_livekit_url(url, true).is_ok()); } // Prefix matching alone let all of these through. for url in [ "wss://", + "wss://@", + "wss://project.example;script-src=*", + "wss://project.example'", + "wss://project.example,evil.example", + "wss://project.example*", + "wss://project.example:notaport", + "wss://project.example:0", + "wss://project.example:65536", + "wss://[2001:db8::1", + "wss://[not-ipv6]", "https://", "example.livekit.cloud", "ftp://x.example", diff --git a/tests/unit/web/policy.rs b/tests/unit/web/policy.rs index e2c5f10..30666e2 100644 --- a/tests/unit/web/policy.rs +++ b/tests/unit/web/policy.rs @@ -56,19 +56,31 @@ fn the_policy_names_the_host_the_avatar_model_is_fetched_from() { assert!(connect.contains(AVATAR_MODEL_ORIGIN), "{connect}"); } -/// both fail over to a regional host and both need the wildcard. Each gets -/// its own: a deployment on one has no reason to reach the other. +/// Both cloud domains fail over to regional hosts and need their own wildcard. +/// A deployment on one has no reason to reach the other. #[test] -fn each_cloud_domain_gets_only_its_own_wildcard() { - let cloud = policy_for("wss://example.livekit.cloud"); - assert!(cloud.contains("https://*.livekit.cloud"), "{cloud}"); - assert!(cloud.contains("wss://*.livekit.cloud"), "{cloud}"); - assert!(!cloud.contains("*.livekit.run"), "{cloud}"); +fn cloud_domains_admit_regional_hosts_without_widening_to_each_other() { + for (url, domain, other_domain) in [ + ( + "wss://example.livekit.cloud", + "livekit.cloud", + "livekit.run", + ), + ("wss://example.livekit.run", "livekit.run", "livekit.cloud"), + ] { + let policy = policy_for(url); + assert!(policy.contains(&format!("https://*.{domain}")), "{policy}"); + assert!(policy.contains(&format!("wss://*.{domain}")), "{policy}"); + assert!(!policy.contains(&format!("*.{other_domain}")), "{policy}"); + } - let run = policy_for("wss://example.livekit.run"); - assert!(run.contains("https://*.livekit.run"), "{run}"); - assert!(run.contains("wss://*.livekit.run"), "{run}"); - assert!(!run.contains("*.livekit.cloud"), "{run}"); + // Regional signaling hosts have labels below both the project and cloud + // domains. The configured project's wildcard must therefore cover this + // shape, not merely a sibling project directly under `livekit.cloud`. + assert_eq!( + livekit_cloud_domain("wss://conversation-xxx.otokyo1b.production.livekit.cloud"), + Some("livekit.cloud") + ); } /// A self-hosted server has no region indirection, so widening its policy @@ -132,6 +144,17 @@ fn a_malformed_url_is_refused_one_term_at_a_time() { ); } +/// A server-side URL may need userinfo, but a CSP source is only a scheme and +/// host. Leaving the userinfo in makes the browser discard the entry, which is +/// a silent connection failure for a self-hosted deployment. +#[test] +fn csp_origins_strip_userinfo() { + let policy = policy_for("wss://key:secret@project.example:7880"); + assert!(policy.contains("https://project.example:7880"), "{policy}"); + assert!(policy.contains("wss://project.example:7880"), "{policy}"); + assert!(!policy.contains("key:secret"), "{policy}"); +} + /// `validate_livekit_url` accepts the scheme case-insensitively, so an /// uppercase one is a URL the server starts on. Every reader here matches /// the scheme exactly, and each fails differently: the quota probe finds no @@ -149,14 +172,6 @@ fn an_uppercase_scheme_is_normalized_before_anything_matches_on_it() { Some("https://host.example".to_string()), "without this the quota probe never runs and the project is assumed available" ); - assert_eq!( - livekit_origins("WSS://host.example"), - vec![ - "https://host.example".to_string(), - "wss://host.example".to_string() - ], - "the sibling origin is what the SDK reaches for regions" - ); // All four schemes, not just the one a cloud deployment uses. A self-hosted // LiveKit is reached over `ws://` in development, and the arm that pairs it @@ -168,11 +183,9 @@ fn an_uppercase_scheme_is_normalized_before_anything_matches_on_it() { ("https://host.example", "wss://host.example"), ("http://host.example", "ws://host.example"), ] { - assert_eq!( - livekit_origins(configured), - vec![sibling.to_string(), configured.to_string()], - "{configured} must also name {sibling}" - ); + let policy = policy_for(configured); + assert!(policy.contains(sibling), "{configured}: {policy}"); + assert!(policy.contains(configured), "{configured}: {policy}"); } let policy = policy_for("WSS://uppercase-scheme.livekit.cloud"); diff --git a/tests/web.rs b/tests/web.rs index 537f29e..df837be 100644 --- a/tests/web.rs +++ b/tests/web.rs @@ -677,6 +677,33 @@ async fn responses_carry_baseline_security_headers() { server.abort(); } +/// `web_service` is public, so its caller may not have used the binary's +/// configuration loader. Invalid endpoints must cost only their CSP sources. +#[tokio::test] +async fn public_web_service_omits_malformed_livekit_origins() { + for (url, forbidden) in [ + ("wss://project.example;frame-src=*", "project.example"), + ("wss://project.example:notaport", "project.example"), + ("wss://[2001:db8::1", "2001:db8::1"), + ("ftp://project.livekit.cloud", "livekit.cloud"), + ] { + let mut config = web_config(); + config.pool = primary_pool(url, "devkey", "devsecret"); + let (base, server) = spawn_web_server(config).await; + let policy = reqwest::get(&base) + .await + .unwrap() + .headers() + .get("content-security-policy") + .unwrap() + .to_str() + .unwrap() + .to_string(); + assert!(!policy.contains(forbidden), "{url}: {policy}"); + server.abort(); + } +} + #[tokio::test] async fn production_policy_names_no_loopback_origins() { let (base, server) = spawn_web_server(WebServerConfig {