From a9426dcb37b20e927c2f5f8d53a1a714abc07648 Mon Sep 17 00:00:00 2001 From: Jesse Jaggars Date: Fri, 30 Oct 2026 10:04:21 -0400 Subject: [PATCH 1/3] feat(cli): support fixed callback port for browser login in containers Browser login (OIDC Authorization Code + PKCE and the edge/mTLS browser flow) binds its callback listener to an ephemeral port on 127.0.0.1. When the CLI runs inside a container, the wrapper cannot know a random port in advance, so it cannot publish it to the host and the browser never reaches the callback. Add a shared bind helper honoring two environment variables: - OPENSHELL_CALLBACK_PORT pins the listener to a fixed port (validated integer 1..=65535) so a wrapper can publish -p 127.0.0.1:: before login starts. - OPENSHELL_CALLBACK_BIND_ADDRESS overrides the bind address (validated IP address) so user-mode container networking (pasta/slirp4netns), which can only reach 0.0.0.0 listeners, can forward to it. Defaults are unchanged: with both variables unset the listener binds 127.0.0.1:0 exactly as before, and the bind address never widens to 0.0.0.0 without the explicit variable, since a wildcard bind on a host machine would expose the callback server to the local network. The browser-facing redirect URL stays http://127.0.0.1:/callback even for a wildcard bind, so no identity-provider allowlist change is needed. A busy fixed port or an invalid value fails with a clear error instead of falling back silently. Signed-off-by: Jesse Jaggars --- crates/openshell-cli/src/auth.rs | 102 +++++- crates/openshell-cli/src/callback.rs | 296 ++++++++++++++++++ crates/openshell-cli/src/lib.rs | 1 + crates/openshell-cli/src/oidc_auth.rs | 96 +++++- docs/how-it-works/gateways/authentication.mdx | 23 ++ 5 files changed, 510 insertions(+), 8 deletions(-) create mode 100644 crates/openshell-cli/src/callback.rs diff --git a/crates/openshell-cli/src/auth.rs b/crates/openshell-cli/src/auth.rs index bc1baae263..39f4f3a2e9 100644 --- a/crates/openshell-cli/src/auth.rs +++ b/crates/openshell-cli/src/auth.rs @@ -81,6 +81,15 @@ fn generate_confirmation_code() -> String { code } +/// Build the gateway `/auth/connect` URL carrying the local callback port +/// and the one-time confirmation code. +fn auth_connect_url(gateway_endpoint: &str, callback_port: u16, code: &str) -> String { + format!( + "{}/auth/connect?callback_port={callback_port}&code={code}", + gateway_endpoint.trim_end_matches('/') + ) +} + /// Run the browser-based CF Access auth flow. /// /// 1. Generates a one-time confirmation code @@ -111,16 +120,13 @@ 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 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_port, &code); // Channel to receive the token from the callback handler. let (tx, rx) = oneshot::channel::(); @@ -879,4 +885,90 @@ 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}; + + /// 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, addr.port(), "ABC-1234"); + assert!( + url.contains(&format!("callback_port={}", addr.port())), + "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 so the wrapper's + // published `-p 127.0.0.1::` mapping reaches the listener. + let url = auth_connect_url(TEST_GATEWAY, addr.port(), "ABC-1234"); + assert!( + url.contains(&format!("callback_port={expected}")), + "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..b5de2b5499 --- /dev/null +++ b/crates/openshell-cli/src/callback.rs @@ -0,0 +1,296 @@ +// 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. +//! The browser-facing redirect URL stays `http://127.0.0.1:/...` +//! either way, so no identity-provider allowlist change is needed. +//! +//! 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, 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)) +} + +/// 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; + use std::net::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, 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 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..fd96d32070 100644 --- a/crates/openshell-cli/src/oidc_auth.rs +++ b/crates/openshell-cli/src/oidc_auth.rs @@ -171,6 +171,17 @@ fn device_token_form( ] } +/// Build the OIDC redirect URI for a bound callback listener. +/// +/// The URI always points at IPv4 loopback, even when the listener binds +/// `0.0.0.0` inside a container: the host-published port and the identity +/// provider allowlist (`http://127.0.0.1:*`) both target host loopback. See +/// the `callback` module docs for the environment-variable controls. +fn callback_redirect_uri(listener: &TcpListener) -> Result { + let port = listener.local_addr().into_diagnostic()?.port(); + Ok(format!("http://127.0.0.1:{port}/callback")) +} + /// Run the OIDC Authorization Code + PKCE browser flow. /// /// Opens the user's browser to the Keycloak login page and waits for @@ -185,9 +196,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 +1044,84 @@ 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}; + + /// 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 even for a 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_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/docs/how-it-works/gateways/authentication.mdx b/docs/how-it-works/gateways/authentication.mdx index b764d3fbfe..dea0502a99 100644 --- a/docs/how-it-works/gateways/authentication.mdx +++ b/docs/how-it-works/gateways/authentication.mdx @@ -326,6 +326,29 @@ This is transparent to the user. All CLI commands work the same regardless of wh **Re-authentication**: If the token expires, run `openshell gateway login` to open the browser flow again and update the stored token. +### Browser Login Callbacks + +Both browser login flows (OIDC Authorization Code + PKCE and edge JWT) receive their result on a short-lived local HTTP server. By default the CLI binds this listener to an ephemeral port on `127.0.0.1`, which requires no configuration on a normal host machine. + +When the CLI runs inside a container, the wrapper cannot know a randomly chosen port in advance, so it cannot publish the port to the host and the browser cannot reach the callback. Two environment variables make browser login work in that setting: + +| Environment variable | Purpose | Default | +|---|---|---| +| `OPENSHELL_CALLBACK_PORT` | Pin the callback listener to a fixed TCP port (an integer from 1 to 65535) so a container wrapper can publish `-p 127.0.0.1::` before login starts. | Unset (ephemeral port) | +| `OPENSHELL_CALLBACK_BIND_ADDRESS` | Override the listener bind address (an IP 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. | `127.0.0.1` | + +For example, a wrapper running the CLI in a container sets both variables and publishes the fixed port to host loopback only: + +```shell +OPENSHELL_CALLBACK_PORT=18080 OPENSHELL_CALLBACK_BIND_ADDRESS=0.0.0.0 openshell gateway login +``` + +The browser-facing redirect URL stays `http://127.0.0.1:/callback` (or the edge flow's `callback_port`) even when the listener binds `0.0.0.0`, so no identity-provider redirect allowlist change is needed. + +Binding `0.0.0.0` on a host machine would expose the callback server to the local network, so the CLI never does it by default: the bind address only changes when `OPENSHELL_CALLBACK_BIND_ADDRESS` is set explicitly. With both variables unset, the listener binds `127.0.0.1` on an ephemeral port exactly as before. An invalid port or address value, or a fixed port that is already in use, fails the login command with a clear error instead of falling back silently. + +`--network=host` is an alternative on Linux, but it does not work with a podman machine VM on macOS, which is why the fixed callback port is the portable approach. + ### Plaintext When a gateway is deployed with `disable_tls = true` (set through Helm as From b67620015d1bf3bc62e330cdc233b2d760f8497d Mon Sep 17 00:00:00 2001 From: Jesse Jaggars Date: Fri, 30 Oct 2026 17:08:48 -0400 Subject: [PATCH 2/3] fix(auth): support IPv6 callback targets in browser login flows An IPv6-only callback bind (OPENSHELL_CALLBACK_BIND_ADDRESS=::1) was accepted but both browser flows still directed callbacks at 127.0.0.1, so the listener was unreachable and login timed out. Route callbacks to the address the listener actually bound: - The CLI derives the browser-facing callback target from the bound address. Wildcard binds map to the matching loopback (0.0.0.0 to 127.0.0.1, :: to [::1]) because a wildcard listener accepts loopback connections of its family; IPv6 addresses render bracketed per RFC 3986/RFC 8252. - The OIDC redirect URI therefore becomes http://[::1]:/callback for ::1-bound listeners, and the bundled Keycloak realm now allows http://[::1]:* redirects for the CLI client alongside the existing IPv4 entries. - The edge flow passes the unbracketed loopback literal to the gateway connect page as callback_host. The page strictly validates it to a loopback IP literal (anything missing, non-loopback, or malformed falls back to 127.0.0.1) before embedding it in the fetch URL and CSP connect-src, so a crafted parameter can never direct the embedded edge token to an external host. The host is preserved across the waiting-page refresh, and older gateways that ignore the new parameter keep the historical 127.0.0.1 behavior. Defaults are unchanged: the listener still binds 127.0.0.1 on an ephemeral port unless the environment variables say otherwise. Adds a callback-reachability test (connecting to the advertised [::1] target succeeds while 127.0.0.1 is refused for a ::1 bind), host-derivation table tests, OIDC redirect tests for ::1 and :: binds, edge URL tests, and server-side validation and render tests. Signed-off-by: Jesse Jaggars --- crates/openshell-cli/src/auth.rs | 56 +++++-- crates/openshell-cli/src/callback.rs | 97 +++++++++++- crates/openshell-cli/src/oidc_auth.rs | 59 ++++++-- crates/openshell-server/src/auth/http.rs | 140 ++++++++++++++++-- docs/how-it-works/gateways/authentication.mdx | 4 +- scripts/keycloak-realm.json | 4 +- 6 files changed, 322 insertions(+), 38 deletions(-) diff --git a/crates/openshell-cli/src/auth.rs b/crates/openshell-cli/src/auth.rs index 39f4f3a2e9..f84e203d7c 100644 --- a/crates/openshell-cli/src/auth.rs +++ b/crates/openshell-cli/src/auth.rs @@ -81,11 +81,21 @@ fn generate_confirmation_code() -> String { code } -/// Build the gateway `/auth/connect` URL carrying the local callback port -/// and the one-time confirmation code. -fn auth_connect_url(gateway_endpoint: &str, callback_port: u16, code: &str) -> String { +/// 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}&code={code}", + "{}/auth/connect?callback_port={callback_port}&callback_host={callback_host}&code={code}", gateway_endpoint.trim_end_matches('/') ) } @@ -123,10 +133,11 @@ pub async fn browser_auth_flow(gateway_endpoint: &str) -> Result { 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 = auth_connect_url(gateway_endpoint, callback_port, &code); + 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::(); @@ -892,7 +903,7 @@ mod tests { use crate::callback::{CALLBACK_BIND_ADDRESS_ENV, CALLBACK_PORT_ENV}; use crate::test_utils::Environment; - use std::net::{IpAddr, Ipv4Addr}; + use std::net::{IpAddr, Ipv4Addr, Ipv6Addr}; /// Bind an ephemeral loopback listener, return its (now freed) port. async fn free_loopback_port() -> u16 { @@ -923,11 +934,12 @@ mod tests { assert_eq!(addr.ip(), IpAddr::V4(Ipv4Addr::LOCALHOST)); assert_ne!(addr.port(), 0); - let url = auth_connect_url(TEST_GATEWAY, addr.port(), "ABC-1234"); + 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; } @@ -947,13 +959,37 @@ mod tests { assert_eq!(addr.ip(), IpAddr::V4(Ipv4Addr::UNSPECIFIED)); assert_eq!(addr.port(), expected); - // The gateway auth URL advertises the fixed port so the wrapper's - // published `-p 127.0.0.1::` mapping reaches the listener. - let url = auth_connect_url(TEST_GATEWAY, addr.port(), "ABC-1234"); + // 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; } diff --git a/crates/openshell-cli/src/callback.rs b/crates/openshell-cli/src/callback.rs index b5de2b5499..1045c66269 100644 --- a/crates/openshell-cli/src/callback.rs +++ b/crates/openshell-cli/src/callback.rs @@ -20,15 +20,17 @@ //! 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. -//! The browser-facing redirect URL stays `http://127.0.0.1:/...` -//! either way, so no identity-provider allowlist change is needed. +//! 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, SocketAddr}; +use std::net::{IpAddr, Ipv4Addr, Ipv6Addr, SocketAddr}; use miette::Result; use tokio::net::TcpListener; @@ -67,6 +69,33 @@ pub(crate) async fn bind_callback_listener() -> Result { .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 = || { @@ -108,7 +137,6 @@ fn bind_error(addr: SocketAddr, source: std::io::Error) -> miette::Report { mod tests { use super::*; use crate::test_utils::Environment; - use std::net::Ipv6Addr; /// Bind an ephemeral loopback listener, return its (now freed) port. async fn free_loopback_port() -> u16 { @@ -159,6 +187,67 @@ mod tests { } } + #[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"); diff --git a/crates/openshell-cli/src/oidc_auth.rs b/crates/openshell-cli/src/oidc_auth.rs index fd96d32070..fa46f67086 100644 --- a/crates/openshell-cli/src/oidc_auth.rs +++ b/crates/openshell-cli/src/oidc_auth.rs @@ -173,13 +173,16 @@ fn device_token_form( /// Build the OIDC redirect URI for a bound callback listener. /// -/// The URI always points at IPv4 loopback, even when the listener binds -/// `0.0.0.0` inside a container: the host-published port and the identity -/// provider allowlist (`http://127.0.0.1:*`) both target host loopback. See -/// the `callback` module docs for the environment-variable controls. +/// 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 port = listener.local_addr().into_diagnostic()?.port(); - Ok(format!("http://127.0.0.1:{port}/callback")) + 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. @@ -1051,7 +1054,7 @@ mod tests { use crate::callback::{CALLBACK_BIND_ADDRESS_ENV, CALLBACK_PORT_ENV}; use crate::test_utils::Environment; - use std::net::{IpAddr, Ipv4Addr}; + use std::net::{IpAddr, Ipv4Addr, Ipv6Addr}; /// Bind an ephemeral loopback listener, return its (now freed) port. async fn free_loopback_port() -> u16 { @@ -1103,14 +1106,52 @@ mod tests { assert_eq!(addr.ip(), IpAddr::V4(Ipv4Addr::UNSPECIFIED)); assert_eq!(addr.port(), expected); - // The redirect URI stays on 127.0.0.1 even for a wildcard bind so the - // identity-provider allowlist keeps matching. + // 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() diff --git a/crates/openshell-server/src/auth/http.rs b/crates/openshell-server/src/auth/http.rs index 8f0bb7d11c..4a02a3f9bf 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,18 @@ async fn auth_connect( if let Some(token) = cf_token { let nonce = uuid::Uuid::new_v4().to_string(); + // The page's only network egress is the POST to the CLI callback + // server, so CSP `connect-src` targets the validated callback host. + let (url_host, _) = sanitize_callback_host(¶ms.callback_host); let csp = format!( - "default-src 'none'; script-src 'nonce-{nonce}'; style-src 'unsafe-inline'; connect-src http://127.0.0.1:*" + "default-src 'none'; script-src 'nonce-{nonce}'; style-src 'unsafe-inline'; connect-src http://{url_host}:*" ); ( [(header::CONTENT_SECURITY_POLICY, csp)], Html(render_connect_page( &safe_gateway, params.callback_port, + &url_host, &token, ¶ms.code, &nonce, @@ -227,9 +265,14 @@ 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() } @@ -258,6 +301,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 +322,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 +487,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 +496,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 +550,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