diff --git a/crates/openshell-cli/src/auth.rs b/crates/openshell-cli/src/auth.rs index bc1baae263..f84e203d7c 100644 --- a/crates/openshell-cli/src/auth.rs +++ b/crates/openshell-cli/src/auth.rs @@ -81,6 +81,25 @@ fn generate_confirmation_code() -> String { code } +/// Build the gateway `/auth/connect` URL carrying the local callback host +/// and port plus the one-time confirmation code. +/// +/// `callback_host` is the unbracketed IP literal the browser should use to +/// reach the CLI callback server (for example `127.0.0.1` or `::1`). The +/// gateway page brackets IPv6 literals when building the fetch URL and only +/// accepts loopback addresses. +fn auth_connect_url( + gateway_endpoint: &str, + callback_host: &str, + callback_port: u16, + code: &str, +) -> String { + format!( + "{}/auth/connect?callback_port={callback_port}&callback_host={callback_host}&code={code}", + gateway_endpoint.trim_end_matches('/') + ) +} + /// Run the browser-based CF Access auth flow. /// /// 1. Generates a one-time confirmation code @@ -111,16 +130,14 @@ pub async fn browser_auth_flow(gateway_endpoint: &str) -> Result { )); } - let listener = TcpListener::bind("127.0.0.1:0").await.into_diagnostic()?; + let listener = crate::callback::bind_callback_listener().await?; let local_addr = listener.local_addr().into_diagnostic()?; let callback_port = local_addr.port(); + let callback_host = crate::callback::callback_target(local_addr).to_string(); let code = generate_confirmation_code(); - let auth_url = format!( - "{}/auth/connect?callback_port={callback_port}&code={code}", - gateway_endpoint.trim_end_matches('/') - ); + let auth_url = auth_connect_url(gateway_endpoint, &callback_host, callback_port, &code); // Channel to receive the token from the callback handler. let (tx, rx) = oneshot::channel::(); @@ -879,4 +896,115 @@ mod tests { "GET should return 405:\n{response}" ); } + + // --------------------------------------------------------------- + // Browser-flow callback listener binding + // --------------------------------------------------------------- + + use crate::callback::{CALLBACK_BIND_ADDRESS_ENV, CALLBACK_PORT_ENV}; + use crate::test_utils::Environment; + use std::net::{IpAddr, Ipv4Addr, Ipv6Addr}; + + /// Bind an ephemeral loopback listener, return its (now freed) port. + async fn free_loopback_port() -> u16 { + let listener = TcpListener::bind("127.0.0.1:0").await.expect("probe bind"); + listener.local_addr().expect("probe addr").port() + } + + /// Read the fixed port under test back from the environment (the + /// `Environment` harness re-enters the test in a child process with the + /// variables applied). + fn expected_fixed_port() -> u16 { + std::env::var(CALLBACK_PORT_ENV) + .expect("callback port env") + .parse() + .expect("callback port env parses") + } + + #[tokio::test] + async fn browser_flow_binds_ephemeral_loopback_by_default() { + Environment::new() + .remove(CALLBACK_PORT_ENV) + .remove(CALLBACK_BIND_ADDRESS_ENV) + .run_async(async { + let listener = crate::callback::bind_callback_listener() + .await + .expect("default bind"); + let addr = listener.local_addr().expect("local addr"); + + assert_eq!(addr.ip(), IpAddr::V4(Ipv4Addr::LOCALHOST)); + assert_ne!(addr.port(), 0); + let url = auth_connect_url(TEST_GATEWAY, "127.0.0.1", addr.port(), "ABC-1234"); + assert!( + url.contains(&format!("callback_port={}", addr.port())), + "auth URL: {url}" + ); + assert!(url.contains("callback_host=127.0.0.1"), "auth URL: {url}"); + }) + .await; + } + + #[tokio::test] + async fn browser_flow_honors_fixed_port_and_wildcard_bind() { + let port = free_loopback_port().await; + Environment::new() + .set(CALLBACK_PORT_ENV, port.to_string()) + .set(CALLBACK_BIND_ADDRESS_ENV, "0.0.0.0") + .run_async(async { + let expected = expected_fixed_port(); + let listener = crate::callback::bind_callback_listener() + .await + .expect("fixed-port bind"); + let addr = listener.local_addr().expect("local addr"); + + assert_eq!(addr.ip(), IpAddr::V4(Ipv4Addr::UNSPECIFIED)); + assert_eq!(addr.port(), expected); + // The gateway auth URL advertises the fixed port and the + // IPv4 loopback target so the wrapper's published + // `-p 127.0.0.1::` mapping reaches the listener. + let url = auth_connect_url(TEST_GATEWAY, "127.0.0.1", addr.port(), "ABC-1234"); + assert!( + url.contains(&format!("callback_port={expected}")), + "auth URL: {url}" + ); + assert!(url.contains("callback_host=127.0.0.1"), "auth URL: {url}"); + }) + .await; + } + + #[tokio::test] + async fn browser_flow_advertises_ipv6_loopback_callback_host() { + Environment::new() + .remove(CALLBACK_PORT_ENV) + .set(CALLBACK_BIND_ADDRESS_ENV, "::1") + .run_async(async { + let listener = crate::callback::bind_callback_listener() + .await + .expect("ipv6 bind"); + let addr = listener.local_addr().expect("local addr"); + + assert_eq!(addr.ip(), IpAddr::V6(Ipv6Addr::LOCALHOST)); + // The auth URL carries the unbracketed IPv6 literal; the + // gateway page brackets it when building the fetch URL. + let host = crate::callback::callback_target(addr).to_string(); + assert_eq!(host, "::1"); + let url = auth_connect_url(TEST_GATEWAY, &host, addr.port(), "ABC-1234"); + assert!(url.contains("callback_host=::1"), "auth URL: {url}"); + }) + .await; + } + + #[tokio::test] + async fn browser_flow_rejects_invalid_callback_port() { + Environment::new() + .set(CALLBACK_PORT_ENV, "https") + .run_async(async { + let err = crate::callback::bind_callback_listener() + .await + .expect_err("invalid port must fail"); + let msg = format!("{err:#}"); + assert!(msg.contains(CALLBACK_PORT_ENV), "message: {msg}"); + }) + .await; + } } diff --git a/crates/openshell-cli/src/callback.rs b/crates/openshell-cli/src/callback.rs new file mode 100644 index 0000000000..1045c66269 --- /dev/null +++ b/crates/openshell-cli/src/callback.rs @@ -0,0 +1,385 @@ +// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +//! Browser-login callback listener binding. +//! +//! Both browser-based login flows (the OIDC Authorization Code + PKCE flow +//! and the edge/mTLS browser flow) receive their result on a short-lived +//! local HTTP server. By default that listener binds an ephemeral port on +//! IPv4 loopback (`127.0.0.1:0`), which is the safest option on a normal +//! host machine. +//! +//! When the CLI runs inside a container, a wrapper cannot know a randomly +//! chosen port in advance, so it cannot publish the port to the host and the +//! browser can never reach the callback. Two environment variables make the +//! flows container-friendly: +//! +//! - [`CALLBACK_PORT_ENV`] pins the listener to a fixed TCP port so the +//! wrapper can publish `-p 127.0.0.1::` before login starts. +//! - [`CALLBACK_BIND_ADDRESS_ENV`] overrides the bind address. Container +//! runtimes with user-mode networking (pasta, slirp4netns) can only reach +//! listeners bound to `0.0.0.0`, so a wrapper sets this to `0.0.0.0` +//! inside the container while publishing the port to host loopback only. +//! IPv6 addresses work too (for example `::1` or `::`): the browser-facing +//! callback URL derives from the bound address (see [`callback_host`]), so +//! an IPv6-bound listener directs the browser to `http://[::1]:/...` +//! instead of the unreachable `127.0.0.1`. +//! +//! Both variables default to unset, preserving the historical +//! `127.0.0.1:` behavior. Binding `0.0.0.0` on a host machine +//! would expose the callback server to the local network, so it is never the +//! default and always requires the explicit variable. + +use std::net::{IpAddr, Ipv4Addr, Ipv6Addr, SocketAddr}; + +use miette::Result; +use tokio::net::TcpListener; + +/// Environment variable pinning the browser-login callback listener to a +/// fixed TCP port (an integer from 1 to 65535). +pub(crate) const CALLBACK_PORT_ENV: &str = "OPENSHELL_CALLBACK_PORT"; + +/// Environment variable overriding the browser-login callback listener bind +/// address (an IP address; defaults to `127.0.0.1`). +pub(crate) const CALLBACK_BIND_ADDRESS_ENV: &str = "OPENSHELL_CALLBACK_BIND_ADDRESS"; + +/// Default callback listener bind address: IPv4 loopback. +const DEFAULT_BIND_ADDRESS: IpAddr = IpAddr::V4(Ipv4Addr::LOCALHOST); + +/// Bind the TCP listener that receives browser-login callbacks. +/// +/// Both browser login flows call this instead of binding a listener +/// directly. See the [module documentation][self] for the environment +/// variable controls and their security rationale. +pub(crate) async fn bind_callback_listener() -> Result { + let port = match std::env::var(CALLBACK_PORT_ENV) { + Ok(raw) => Some(parse_callback_port(&raw)?), + Err(_) => None, + }; + + let address = match std::env::var(CALLBACK_BIND_ADDRESS_ENV) { + Ok(raw) => Some(parse_bind_address(&raw)?), + Err(_) => None, + }; + + let addr = SocketAddr::new(address.unwrap_or(DEFAULT_BIND_ADDRESS), port.unwrap_or(0)); + + TcpListener::bind(addr) + .await + .map_err(|e| bind_error(addr, e)) +} + +/// The IP address a browser uses to reach a bound callback listener. +/// +/// Callback URLs target the bound address, except that wildcard binds are +/// reached through the matching loopback address: a listener on `0.0.0.0` +/// accepts `127.0.0.1` connections and a listener on `::` accepts `::1` +/// connections, so the browser is sent to the loopback literal of that +/// family (the container wrapper publishes the port to host loopback). +pub(crate) fn callback_target(local_addr: SocketAddr) -> IpAddr { + match local_addr.ip() { + IpAddr::V4(v4) if v4.is_unspecified() => IpAddr::V4(Ipv4Addr::LOCALHOST), + IpAddr::V6(v6) if v6.is_unspecified() => IpAddr::V6(Ipv6Addr::LOCALHOST), + ip => ip, + } +} + +/// The host a browser uses to reach a bound callback listener, in URL form. +/// +/// IPv4 addresses render bare (`127.0.0.1`); IPv6 addresses are bracketed +/// (`[::1]`) as required by RFC 3986/RFC 8252. Use +/// [`callback_target`] for the unbracketed literal. +pub(crate) fn callback_host(local_addr: SocketAddr) -> String { + match callback_target(local_addr) { + IpAddr::V4(v4) => v4.to_string(), + IpAddr::V6(v6) => format!("[{v6}]"), + } +} + +/// Parse and validate [`CALLBACK_PORT_ENV`]. +fn parse_callback_port(raw: &str) -> Result { + let invalid = || { + miette::miette!("{CALLBACK_PORT_ENV} must be an integer between 1 and 65535, got {raw:?}") + }; + let port: u16 = raw.trim().parse().map_err(|_| invalid())?; + if port == 0 { + return Err(invalid()); + } + Ok(port) +} + +/// Parse and validate [`CALLBACK_BIND_ADDRESS_ENV`]. +fn parse_bind_address(raw: &str) -> Result { + raw.trim().parse().map_err(|_| { + miette::miette!( + "{CALLBACK_BIND_ADDRESS_ENV} must be an IP address \ + (e.g. 127.0.0.1, 0.0.0.0, or ::1), got {raw:?}" + ) + }) +} + +/// Build a helpful error for a failed callback listener bind. +fn bind_error(addr: SocketAddr, source: std::io::Error) -> miette::Report { + if addr.port() != 0 && source.kind() == std::io::ErrorKind::AddrInUse { + miette::miette!( + "browser-login callback port {} is already in use on {}.\n\ + Set {CALLBACK_PORT_ENV} to a free port, or unset it to let the \ + CLI pick an ephemeral port", + addr.port(), + addr.ip() + ) + } else { + miette::miette!("failed to bind browser-login callback listener on {addr}: {source}") + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::test_utils::Environment; + + /// Bind an ephemeral loopback listener, return its (now freed) port. + async fn free_loopback_port() -> u16 { + let listener = TcpListener::bind("127.0.0.1:0").await.expect("probe bind"); + listener.local_addr().expect("probe addr").port() + } + + /// Read the fixed port under test back from the environment. + /// + /// The `Environment` harness re-enters the test in a child process with + /// the variables applied, so the expected value is read from the + /// environment rather than the parent's local variable. + fn expected_fixed_port() -> u16 { + std::env::var(CALLBACK_PORT_ENV) + .expect("callback port env") + .parse() + .expect("callback port env parses") + } + + #[test] + fn invalid_callback_port_values_are_rejected() { + for raw in ["", "0", "-1", "65536", "999999", "abc", "80.5", "8 0"] { + let err = parse_callback_port(raw).expect_err(raw); + let msg = format!("{err:#}"); + assert!(msg.contains(CALLBACK_PORT_ENV), "value {raw:?}: {msg}"); + assert!(msg.contains("between 1 and 65535"), "value {raw:?}: {msg}"); + } + } + + #[test] + fn valid_callback_port_values_are_accepted() { + assert_eq!(parse_callback_port("1").unwrap(), 1); + assert_eq!(parse_callback_port("8080").unwrap(), 8080); + assert_eq!(parse_callback_port(" 8443 ").unwrap(), 8443); + assert_eq!(parse_callback_port("65535").unwrap(), 65535); + } + + #[test] + fn invalid_bind_address_values_are_rejected() { + for raw in ["", " ", "banana", "localhost", "127.0.0.1:8080"] { + let err = parse_bind_address(raw).expect_err(raw); + let msg = format!("{err:#}"); + assert!( + msg.contains(CALLBACK_BIND_ADDRESS_ENV), + "value {raw:?}: {msg}" + ); + assert!(msg.contains("IP address"), "value {raw:?}: {msg}"); + } + } + + #[test] + fn callback_host_maps_bindings_to_reachable_url_hosts() { + let parse = |s: &str| -> SocketAddr { s.parse().expect("addr") }; + // Wildcard binds are reached through the matching loopback literal. + assert_eq!(callback_host(parse("0.0.0.0:8080")), "127.0.0.1"); + assert_eq!(callback_host(parse("[::]:8080")), "[::1]"); + // Loopback binds keep their address; IPv6 renders bracketed. + assert_eq!(callback_host(parse("127.0.0.1:8080")), "127.0.0.1"); + assert_eq!(callback_host(parse("[::1]:8080")), "[::1]"); + // Explicit specific addresses are addressed literally. + assert_eq!(callback_host(parse("192.168.1.5:8080")), "192.168.1.5"); + assert_eq!(callback_host(parse("[2001:db8::5]:8080")), "[2001:db8::5]"); + } + + #[test] + fn callback_target_maps_bindings_to_reachable_ips() { + let parse = |s: &str| -> SocketAddr { s.parse().expect("addr") }; + assert_eq!( + callback_target(parse("0.0.0.0:8080")), + IpAddr::V4(Ipv4Addr::LOCALHOST) + ); + assert_eq!( + callback_target(parse("[::]:8080")), + IpAddr::V6(Ipv6Addr::LOCALHOST) + ); + assert_eq!( + callback_target(parse("[2001:db8::5]:8080")), + IpAddr::V6("2001:db8::5".parse::().expect("v6")) + ); + } + + #[tokio::test] + async fn ipv6_callback_is_reachable_at_advertised_host() { + // Callback reachability: an IPv6-bound listener must be reachable at + // exactly the host the browser flows advertise, and (on a host + // without the address) must NOT be reachable via IPv4 loopback. + Environment::new() + .remove(CALLBACK_PORT_ENV) + .set(CALLBACK_BIND_ADDRESS_ENV, "::1") + .run_async(async { + let listener = bind_callback_listener().await.expect("ipv6 bind"); + let addr = listener.local_addr().expect("local addr"); + let host = callback_host(addr); + assert_eq!(host, "[::1]"); + + let v6 = SocketAddr::new( + IpAddr::V6("::1".parse::().expect("v6")), + addr.port(), + ); + let stream = tokio::net::TcpStream::connect(v6) + .await + .expect("[::1] callback must be reachable"); + drop(stream); + + let v4 = SocketAddr::new(IpAddr::V4(Ipv4Addr::LOCALHOST), addr.port()); + let refused = tokio::net::TcpStream::connect(v4).await; + assert!(refused.is_err(), "IPv4 loopback must not reach a ::1 bind"); + }) + .await; + } + + #[test] + fn busy_fixed_port_error_names_the_variable() { + let addr: SocketAddr = "127.0.0.1:18080".parse().expect("parse addr"); + let err = bind_error(addr, std::io::Error::from(std::io::ErrorKind::AddrInUse)); + let msg = format!("{err:#}"); + assert!(msg.contains("already in use"), "message: {msg}"); + assert!(msg.contains("18080"), "message: {msg}"); + assert!(msg.contains(CALLBACK_PORT_ENV), "message: {msg}"); + } + + #[test] + fn other_bind_errors_keep_the_listener_address() { + let addr: SocketAddr = "127.0.0.1:18080".parse().expect("parse addr"); + let err = bind_error( + addr, + std::io::Error::from(std::io::ErrorKind::PermissionDenied), + ); + let msg = format!("{err:#}"); + assert!(msg.contains("127.0.0.1:18080"), "message: {msg}"); + } + + #[tokio::test] + async fn binds_ephemeral_loopback_port_by_default() { + Environment::new() + .remove(CALLBACK_PORT_ENV) + .remove(CALLBACK_BIND_ADDRESS_ENV) + .run_async(async { + let listener = bind_callback_listener().await.expect("default bind"); + let addr = listener.local_addr().expect("local addr"); + + assert_eq!(addr.ip(), IpAddr::V4(Ipv4Addr::LOCALHOST)); + assert_ne!(addr.port(), 0, "default bind must use an ephemeral port"); + }) + .await; + } + + #[tokio::test] + async fn binds_fixed_port_when_callback_port_is_set() { + let port = free_loopback_port().await; + Environment::new() + .set(CALLBACK_PORT_ENV, port.to_string()) + .remove(CALLBACK_BIND_ADDRESS_ENV) + .run_async(async { + let expected = expected_fixed_port(); + let listener = bind_callback_listener().await.expect("fixed-port bind"); + let addr = listener.local_addr().expect("local addr"); + + assert_eq!(addr.port(), expected); + assert_eq!(addr.ip(), IpAddr::V4(Ipv4Addr::LOCALHOST)); + }) + .await; + } + + #[tokio::test] + async fn fixed_port_in_use_reports_clear_error() { + let port = free_loopback_port().await; + Environment::new() + .set(CALLBACK_PORT_ENV, port.to_string()) + .remove(CALLBACK_BIND_ADDRESS_ENV) + .run_async(async { + let expected = expected_fixed_port(); + let _holder = TcpListener::bind(("127.0.0.1", expected)) + .await + .expect("hold port"); + let err = bind_callback_listener() + .await + .expect_err("busy port must fail"); + let msg = format!("{err:#}"); + assert!(msg.contains("already in use"), "message: {msg}"); + assert!(msg.contains(CALLBACK_PORT_ENV), "message: {msg}"); + }) + .await; + } + + #[tokio::test] + async fn invalid_callback_port_env_fails_before_binding() { + Environment::new() + .set(CALLBACK_PORT_ENV, "not-a-port") + .run_async(async { + let err = bind_callback_listener() + .await + .expect_err("invalid port must fail"); + let msg = format!("{err:#}"); + assert!(msg.contains(CALLBACK_PORT_ENV), "message: {msg}"); + assert!(msg.contains("between 1 and 65535"), "message: {msg}"); + }) + .await; + } + + #[tokio::test] + async fn binds_all_interfaces_when_bind_address_is_set() { + Environment::new() + .remove(CALLBACK_PORT_ENV) + .set(CALLBACK_BIND_ADDRESS_ENV, "0.0.0.0") + .run_async(async { + let listener = bind_callback_listener().await.expect("wildcard bind"); + let addr = listener.local_addr().expect("local addr"); + + assert_eq!(addr.ip(), IpAddr::V4(Ipv4Addr::UNSPECIFIED)); + assert_ne!(addr.port(), 0, "wildcard bind still uses ephemeral port"); + }) + .await; + } + + #[tokio::test] + async fn fixed_port_and_bind_address_combine() { + let port = free_loopback_port().await; + Environment::new() + .set(CALLBACK_PORT_ENV, port.to_string()) + .set(CALLBACK_BIND_ADDRESS_ENV, "0.0.0.0") + .run_async(async { + let expected = expected_fixed_port(); + let listener = bind_callback_listener().await.expect("combined bind"); + let addr = listener.local_addr().expect("local addr"); + + assert_eq!(addr.ip(), IpAddr::V4(Ipv4Addr::UNSPECIFIED)); + assert_eq!(addr.port(), expected); + }) + .await; + } + + #[tokio::test] + async fn ipv6_loopback_bind_address_is_supported() { + Environment::new() + .remove(CALLBACK_PORT_ENV) + .set(CALLBACK_BIND_ADDRESS_ENV, "::1") + .run_async(async { + let listener = bind_callback_listener().await.expect("ipv6 bind"); + let addr = listener.local_addr().expect("local addr"); + + assert_eq!(addr.ip(), IpAddr::V6(Ipv6Addr::LOCALHOST)); + }) + .await; + } +} diff --git a/crates/openshell-cli/src/lib.rs b/crates/openshell-cli/src/lib.rs index 2eebd61f5a..d1a4b3be89 100644 --- a/crates/openshell-cli/src/lib.rs +++ b/crates/openshell-cli/src/lib.rs @@ -13,6 +13,7 @@ pub(crate) static TEST_ENV_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(() pub(crate) mod test_utils; pub mod auth; +pub mod callback; pub mod color; pub(crate) mod commands; pub mod completers; diff --git a/crates/openshell-cli/src/oidc_auth.rs b/crates/openshell-cli/src/oidc_auth.rs index d9bf347f6b..fa46f67086 100644 --- a/crates/openshell-cli/src/oidc_auth.rs +++ b/crates/openshell-cli/src/oidc_auth.rs @@ -171,6 +171,20 @@ fn device_token_form( ] } +/// Build the OIDC redirect URI for a bound callback listener. +/// +/// The URI derives from the listener's bound address (see +/// [`crate::callback::callback_host`]): IPv4 binds and the default bind use +/// `http://127.0.0.1:/callback`, while IPv6 binds use the bracketed +/// literal (`http://[::1]:/callback`), matching RFC 8252 loopback +/// redirects. Identity providers that allowlist `http://127.0.0.1:*` for +/// native apps should also allowlist `http://[::1]:*`. +fn callback_redirect_uri(listener: &TcpListener) -> Result { + let local_addr = listener.local_addr().into_diagnostic()?; + let host = crate::callback::callback_host(local_addr); + Ok(format!("http://{host}:{}/callback", local_addr.port())) +} + /// Run the OIDC Authorization Code + PKCE browser flow. /// /// Opens the user's browser to the Keycloak login page and waits for @@ -185,9 +199,8 @@ pub async fn oidc_browser_auth_flow( ) -> Result { let discovery = discover(issuer, insecure).await?; - let listener = TcpListener::bind("127.0.0.1:0").await.into_diagnostic()?; - let port = listener.local_addr().into_diagnostic()?.port(); - let redirect_uri = format!("http://127.0.0.1:{port}/callback"); + let listener = crate::callback::bind_callback_listener().await?; + let redirect_uri = callback_redirect_uri(&listener)?; let client = BasicClient::new(ClientId::new(client_id.to_string())) .set_auth_uri(AuthUrl::new(discovery.authorization_endpoint).into_diagnostic()?) @@ -1034,4 +1047,122 @@ mod tests { bundle_from_device_token_response(&response, "https://issuer.example", "test-client"); assert!(result.is_err()); } + + // --------------------------------------------------------------- + // Browser-flow callback listener binding + // --------------------------------------------------------------- + + use crate::callback::{CALLBACK_BIND_ADDRESS_ENV, CALLBACK_PORT_ENV}; + use crate::test_utils::Environment; + use std::net::{IpAddr, Ipv4Addr, Ipv6Addr}; + + /// Bind an ephemeral loopback listener, return its (now freed) port. + async fn free_loopback_port() -> u16 { + let listener = TcpListener::bind("127.0.0.1:0").await.expect("probe bind"); + listener.local_addr().expect("probe addr").port() + } + + /// Read the fixed port under test back from the environment (the + /// `Environment` harness re-enters the test in a child process with the + /// variables applied). + fn expected_fixed_port() -> u16 { + std::env::var(CALLBACK_PORT_ENV) + .expect("callback port env") + .parse() + .expect("callback port env parses") + } + + #[tokio::test] + async fn browser_flow_binds_ephemeral_loopback_by_default() { + Environment::new() + .remove(CALLBACK_PORT_ENV) + .remove(CALLBACK_BIND_ADDRESS_ENV) + .run_async(async { + let listener = crate::callback::bind_callback_listener() + .await + .expect("default bind"); + let addr = listener.local_addr().expect("local addr"); + + assert_eq!(addr.ip(), IpAddr::V4(Ipv4Addr::LOCALHOST)); + assert_ne!(addr.port(), 0); + let uri = callback_redirect_uri(&listener).expect("redirect uri"); + assert_eq!(uri, format!("http://127.0.0.1:{}/callback", addr.port())); + }) + .await; + } + + #[tokio::test] + async fn browser_flow_honors_fixed_port_and_wildcard_bind() { + let port = free_loopback_port().await; + Environment::new() + .set(CALLBACK_PORT_ENV, port.to_string()) + .set(CALLBACK_BIND_ADDRESS_ENV, "0.0.0.0") + .run_async(async { + let expected = expected_fixed_port(); + let listener = crate::callback::bind_callback_listener() + .await + .expect("fixed-port bind"); + let addr = listener.local_addr().expect("local addr"); + + assert_eq!(addr.ip(), IpAddr::V4(Ipv4Addr::UNSPECIFIED)); + assert_eq!(addr.port(), expected); + // The redirect URI stays on 127.0.0.1 for an IPv4 wildcard + // bind so the identity-provider allowlist keeps matching. + let uri = callback_redirect_uri(&listener).expect("redirect uri"); + assert_eq!(uri, format!("http://127.0.0.1:{expected}/callback")); + }) + .await; + } + + #[tokio::test] + async fn browser_flow_redirect_uri_follows_ipv6_bind() { + Environment::new() + .remove(CALLBACK_PORT_ENV) + .set(CALLBACK_BIND_ADDRESS_ENV, "::1") + .run_async(async { + let listener = crate::callback::bind_callback_listener() + .await + .expect("ipv6 bind"); + let addr = listener.local_addr().expect("local addr"); + + assert_eq!(addr.ip(), IpAddr::V6(Ipv6Addr::LOCALHOST)); + // RFC 8252 loopback redirect with the bracketed IPv6 literal, + // so the browser actually reaches the ::1-bound listener. + let uri = callback_redirect_uri(&listener).expect("redirect uri"); + assert_eq!(uri, format!("http://[::1]:{}/callback", addr.port())); + }) + .await; + } + + #[tokio::test] + async fn browser_flow_redirect_uri_uses_ipv6_loopback_for_wildcard_v6_bind() { + Environment::new() + .remove(CALLBACK_PORT_ENV) + .set(CALLBACK_BIND_ADDRESS_ENV, "::") + .run_async(async { + let listener = crate::callback::bind_callback_listener() + .await + .expect("wildcard ipv6 bind"); + let addr = listener.local_addr().expect("local addr"); + + assert_eq!(addr.ip(), IpAddr::V6(Ipv6Addr::UNSPECIFIED)); + let uri = callback_redirect_uri(&listener).expect("redirect uri"); + assert_eq!(uri, format!("http://[::1]:{}/callback", addr.port())); + }) + .await; + } + + #[tokio::test] + async fn browser_flow_rejects_invalid_callback_port() { + Environment::new() + .set(CALLBACK_PORT_ENV, "99999") + .run_async(async { + let err = crate::callback::bind_callback_listener() + .await + .expect_err("invalid port must fail"); + let msg = format!("{err:#}"); + assert!(msg.contains(CALLBACK_PORT_ENV), "message: {msg}"); + }) + .await; + } } diff --git a/crates/openshell-server/src/auth/http.rs b/crates/openshell-server/src/auth/http.rs index 8f0bb7d11c..90dae5046f 100644 --- a/crates/openshell-server/src/auth/http.rs +++ b/crates/openshell-server/src/auth/http.rs @@ -12,8 +12,10 @@ //! The handler reads the `CF_Authorization` cookie from the request headers //! (required because the cookie is typically `HttpOnly`) and serves a styled //! confirmation page. When the user clicks "Connect", JavaScript POSTs -//! the token to `http://127.0.0.1:/callback`, where the -//! CLI's ephemeral localhost server captures and stores the token. +//! the token to `http://:/callback`, where the +//! CLI's ephemeral localhost server captures and stores the token. The CLI +//! supplies `callback_host` (validated to a loopback IP literal; IPv4 +//! loopback by default) so IPv6-bound listeners are reached at `[::1]`. use axum::{ Json, Router, @@ -52,6 +54,38 @@ struct ConnectParams { /// clicking Connect. Sent back in the XHR POST to the CLI callback /// server for cryptographic binding. code: String, + /// Unbracketed IP literal the browser should use to reach the CLI's + /// callback server (for example `127.0.0.1` or `::1`). Optional for + /// backward compatibility with older CLIs, which only bind IPv4 + /// loopback. Strictly validated to a loopback literal by + /// [`sanitize_callback_host`] before use. + callback_host: Option, +} + +/// Validate the CLI-provided `callback_host` query parameter. +/// +/// Returns the host in URL form (IPv4 bare, IPv6 bracketed) for use in the +/// page's fetch URL and CSP, plus the unbracketed literal for round-tripping +/// through the waiting-page refresh. Only loopback IP literals are accepted: +/// the confirmation page embeds the edge auth token, and a crafted +/// `callback_host` must never direct the browser to POST that token to an +/// external host. Anything missing or untrusted falls back to the historical +/// `127.0.0.1` target. +fn sanitize_callback_host(raw: &Option) -> (String, String) { + let fallback = ("127.0.0.1".to_string(), "127.0.0.1".to_string()); + let Some(raw) = raw.as_deref().map(str::trim).filter(|h| !h.is_empty()) else { + return fallback; + }; + let Ok(ip) = raw.parse::() else { + return fallback; + }; + if !ip.is_loopback() { + return fallback; + } + match ip { + std::net::IpAddr::V4(v4) => (v4.to_string(), v4.to_string()), + std::net::IpAddr::V6(v6) => (format!("[{v6}]"), v6.to_string()), + } } /// Create the auth router. @@ -211,14 +245,14 @@ async fn auth_connect( if let Some(token) = cf_token { let nonce = uuid::Uuid::new_v4().to_string(); - let csp = format!( - "default-src 'none'; script-src 'nonce-{nonce}'; style-src 'unsafe-inline'; connect-src http://127.0.0.1:*" - ); + let (url_host, _) = sanitize_callback_host(¶ms.callback_host); + let csp = connect_page_csp(&nonce, &url_host); ( [(header::CONTENT_SECURITY_POLICY, csp)], Html(render_connect_page( &safe_gateway, params.callback_port, + &url_host, &token, ¶ms.code, &nonce, @@ -227,14 +261,39 @@ async fn auth_connect( .into_response() } else { let csp = "default-src 'none'; style-src 'unsafe-inline'".to_string(); + let (_, wire_host) = sanitize_callback_host(¶ms.callback_host); ( [(header::CONTENT_SECURITY_POLICY, csp)], - Html(render_waiting_page(params.callback_port, ¶ms.code)), + Html(render_waiting_page( + params.callback_port, + &wire_host, + ¶ms.code, + )), ) .into_response() } } +/// Build the Content-Security-Policy for the connect page. +/// +/// The page's only network egress is the POST to the CLI callback server, +/// so `connect-src` targets the validated callback host. The IPv4 loopback +/// source is always included: Chrome's CSP implementation does not support +/// IPv6 literals in host-source expressions, so an IPv6 target's source is +/// dropped there and the policy must not degrade to empty. The IPv6 +/// edge-login limitation this implies is documented in the CLI auth +/// reference. +fn connect_page_csp(nonce: &str, url_host: &str) -> String { + let connect_src = if url_host == "127.0.0.1" { + "http://127.0.0.1:*".to_string() + } else { + format!("http://127.0.0.1:* http://{url_host}:*") + }; + format!( + "default-src 'none'; script-src 'nonce-{nonce}'; style-src 'unsafe-inline'; connect-src {connect_src}" + ) +} + /// Extract a named cookie value from a `Cookie` header string. fn extract_cookie(cookies: &str, name: &str) -> Option { cookies.split(';').find_map(|c| { @@ -258,6 +317,7 @@ fn extract_cookie(cookies: &str, name: &str) -> Option { fn render_connect_page( gateway_addr: &str, callback_port: u16, + callback_host: &str, cf_token: &str, code: &str, nonce: &str, @@ -278,6 +338,13 @@ fn render_connect_page( .unwrap_or_else(|_| "\"\"".to_string()) .replace('<', "\\u003c") .replace('>', "\\u003e"); + // `callback_host` is validated by `sanitize_callback_host` to a loopback + // IP literal in URL form (IPv6 bracketed), so embedding it via JSON + // serialization is inject-safe. + let json_host = serde_json::to_string(callback_host) + .unwrap_or_else(|_| "\"127.0.0.1\"".to_string()) + .replace('<', "\\u003c") + .replace('>', "\\u003e"); // HTML-safe version of the code for display in the page body. let html_code = html_escape(code); @@ -436,6 +503,7 @@ fn render_connect_page( var token = {json_token}; var code = {json_code}; var port = {callback_port}; + var host = {json_host}; function connect() {{ var btn = document.getElementById('connectBtn'); var status = document.getElementById('status'); @@ -444,7 +512,7 @@ fn render_connect_page( status.textContent = ''; status.className = 'status'; - fetch('http://127.0.0.1:' + port + '/callback', {{ + fetch('http://' + host + ':' + port + '/callback', {{ method: 'POST', headers: {{ 'Content-Type': 'application/json' }}, body: JSON.stringify({{ token: token, code: code }}) @@ -498,16 +566,19 @@ fn html_escape(s: &str) -> String { /// /// This can happen if the edge proxy hasn't completed authentication yet. The /// page auto-reloads after a short delay to pick up the cookie once login -/// finishes. The `code` parameter is preserved across reloads. -fn render_waiting_page(callback_port: u16, code: &str) -> String { +/// finishes. The `code` and `callback_host` parameters are preserved across +/// reloads; `callback_host` is the unbracketed loopback literal validated by +/// [`sanitize_callback_host`]. +fn render_waiting_page(callback_port: u16, callback_host: &str, code: &str) -> String { let safe_code = html_escape(code); + let safe_host = html_escape(callback_host); format!( r#" - + OpenShell — Authenticating