Skip to content

Commit b676200

Browse files
committed
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]:<port>/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 <jjaggars@redhat.com>
1 parent a9426dc commit b676200

6 files changed

Lines changed: 322 additions & 38 deletions

File tree

‎crates/openshell-cli/src/auth.rs‎

Lines changed: 46 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -81,11 +81,21 @@ fn generate_confirmation_code() -> String {
8181
code
8282
}
8383

84-
/// Build the gateway `/auth/connect` URL carrying the local callback port
85-
/// and the one-time confirmation code.
86-
fn auth_connect_url(gateway_endpoint: &str, callback_port: u16, code: &str) -> String {
84+
/// Build the gateway `/auth/connect` URL carrying the local callback host
85+
/// and port plus the one-time confirmation code.
86+
///
87+
/// `callback_host` is the unbracketed IP literal the browser should use to
88+
/// reach the CLI callback server (for example `127.0.0.1` or `::1`). The
89+
/// gateway page brackets IPv6 literals when building the fetch URL and only
90+
/// accepts loopback addresses.
91+
fn auth_connect_url(
92+
gateway_endpoint: &str,
93+
callback_host: &str,
94+
callback_port: u16,
95+
code: &str,
96+
) -> String {
8797
format!(
88-
"{}/auth/connect?callback_port={callback_port}&code={code}",
98+
"{}/auth/connect?callback_port={callback_port}&callback_host={callback_host}&code={code}",
8999
gateway_endpoint.trim_end_matches('/')
90100
)
91101
}
@@ -123,10 +133,11 @@ pub async fn browser_auth_flow(gateway_endpoint: &str) -> Result<String> {
123133
let listener = crate::callback::bind_callback_listener().await?;
124134
let local_addr = listener.local_addr().into_diagnostic()?;
125135
let callback_port = local_addr.port();
136+
let callback_host = crate::callback::callback_target(local_addr).to_string();
126137

127138
let code = generate_confirmation_code();
128139

129-
let auth_url = auth_connect_url(gateway_endpoint, callback_port, &code);
140+
let auth_url = auth_connect_url(gateway_endpoint, &callback_host, callback_port, &code);
130141

131142
// Channel to receive the token from the callback handler.
132143
let (tx, rx) = oneshot::channel::<String>();
@@ -892,7 +903,7 @@ mod tests {
892903

893904
use crate::callback::{CALLBACK_BIND_ADDRESS_ENV, CALLBACK_PORT_ENV};
894905
use crate::test_utils::Environment;
895-
use std::net::{IpAddr, Ipv4Addr};
906+
use std::net::{IpAddr, Ipv4Addr, Ipv6Addr};
896907

897908
/// Bind an ephemeral loopback listener, return its (now freed) port.
898909
async fn free_loopback_port() -> u16 {
@@ -923,11 +934,12 @@ mod tests {
923934

924935
assert_eq!(addr.ip(), IpAddr::V4(Ipv4Addr::LOCALHOST));
925936
assert_ne!(addr.port(), 0);
926-
let url = auth_connect_url(TEST_GATEWAY, addr.port(), "ABC-1234");
937+
let url = auth_connect_url(TEST_GATEWAY, "127.0.0.1", addr.port(), "ABC-1234");
927938
assert!(
928939
url.contains(&format!("callback_port={}", addr.port())),
929940
"auth URL: {url}"
930941
);
942+
assert!(url.contains("callback_host=127.0.0.1"), "auth URL: {url}");
931943
})
932944
.await;
933945
}
@@ -947,13 +959,37 @@ mod tests {
947959

948960
assert_eq!(addr.ip(), IpAddr::V4(Ipv4Addr::UNSPECIFIED));
949961
assert_eq!(addr.port(), expected);
950-
// The gateway auth URL advertises the fixed port so the wrapper's
951-
// published `-p 127.0.0.1:<port>:<port>` mapping reaches the listener.
952-
let url = auth_connect_url(TEST_GATEWAY, addr.port(), "ABC-1234");
962+
// The gateway auth URL advertises the fixed port and the
963+
// IPv4 loopback target so the wrapper's published
964+
// `-p 127.0.0.1:<port>:<port>` mapping reaches the listener.
965+
let url = auth_connect_url(TEST_GATEWAY, "127.0.0.1", addr.port(), "ABC-1234");
953966
assert!(
954967
url.contains(&format!("callback_port={expected}")),
955968
"auth URL: {url}"
956969
);
970+
assert!(url.contains("callback_host=127.0.0.1"), "auth URL: {url}");
971+
})
972+
.await;
973+
}
974+
975+
#[tokio::test]
976+
async fn browser_flow_advertises_ipv6_loopback_callback_host() {
977+
Environment::new()
978+
.remove(CALLBACK_PORT_ENV)
979+
.set(CALLBACK_BIND_ADDRESS_ENV, "::1")
980+
.run_async(async {
981+
let listener = crate::callback::bind_callback_listener()
982+
.await
983+
.expect("ipv6 bind");
984+
let addr = listener.local_addr().expect("local addr");
985+
986+
assert_eq!(addr.ip(), IpAddr::V6(Ipv6Addr::LOCALHOST));
987+
// The auth URL carries the unbracketed IPv6 literal; the
988+
// gateway page brackets it when building the fetch URL.
989+
let host = crate::callback::callback_target(addr).to_string();
990+
assert_eq!(host, "::1");
991+
let url = auth_connect_url(TEST_GATEWAY, &host, addr.port(), "ABC-1234");
992+
assert!(url.contains("callback_host=::1"), "auth URL: {url}");
957993
})
958994
.await;
959995
}

‎crates/openshell-cli/src/callback.rs‎

Lines changed: 93 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,15 +20,17 @@
2020
//! runtimes with user-mode networking (pasta, slirp4netns) can only reach
2121
//! listeners bound to `0.0.0.0`, so a wrapper sets this to `0.0.0.0`
2222
//! inside the container while publishing the port to host loopback only.
23-
//! The browser-facing redirect URL stays `http://127.0.0.1:<port>/...`
24-
//! either way, so no identity-provider allowlist change is needed.
23+
//! IPv6 addresses work too (for example `::1` or `::`): the browser-facing
24+
//! callback URL derives from the bound address (see [`callback_host`]), so
25+
//! an IPv6-bound listener directs the browser to `http://[::1]:<port>/...`
26+
//! instead of the unreachable `127.0.0.1`.
2527
//!
2628
//! Both variables default to unset, preserving the historical
2729
//! `127.0.0.1:<ephemeral>` behavior. Binding `0.0.0.0` on a host machine
2830
//! would expose the callback server to the local network, so it is never the
2931
//! default and always requires the explicit variable.
3032
31-
use std::net::{IpAddr, Ipv4Addr, SocketAddr};
33+
use std::net::{IpAddr, Ipv4Addr, Ipv6Addr, SocketAddr};
3234

3335
use miette::Result;
3436
use tokio::net::TcpListener;
@@ -67,6 +69,33 @@ pub(crate) async fn bind_callback_listener() -> Result<TcpListener> {
6769
.map_err(|e| bind_error(addr, e))
6870
}
6971

72+
/// The IP address a browser uses to reach a bound callback listener.
73+
///
74+
/// Callback URLs target the bound address, except that wildcard binds are
75+
/// reached through the matching loopback address: a listener on `0.0.0.0`
76+
/// accepts `127.0.0.1` connections and a listener on `::` accepts `::1`
77+
/// connections, so the browser is sent to the loopback literal of that
78+
/// family (the container wrapper publishes the port to host loopback).
79+
pub(crate) fn callback_target(local_addr: SocketAddr) -> IpAddr {
80+
match local_addr.ip() {
81+
IpAddr::V4(v4) if v4.is_unspecified() => IpAddr::V4(Ipv4Addr::LOCALHOST),
82+
IpAddr::V6(v6) if v6.is_unspecified() => IpAddr::V6(Ipv6Addr::LOCALHOST),
83+
ip => ip,
84+
}
85+
}
86+
87+
/// The host a browser uses to reach a bound callback listener, in URL form.
88+
///
89+
/// IPv4 addresses render bare (`127.0.0.1`); IPv6 addresses are bracketed
90+
/// (`[::1]`) as required by RFC 3986/RFC 8252. Use
91+
/// [`callback_target`] for the unbracketed literal.
92+
pub(crate) fn callback_host(local_addr: SocketAddr) -> String {
93+
match callback_target(local_addr) {
94+
IpAddr::V4(v4) => v4.to_string(),
95+
IpAddr::V6(v6) => format!("[{v6}]"),
96+
}
97+
}
98+
7099
/// Parse and validate [`CALLBACK_PORT_ENV`].
71100
fn parse_callback_port(raw: &str) -> Result<u16> {
72101
let invalid = || {
@@ -108,7 +137,6 @@ fn bind_error(addr: SocketAddr, source: std::io::Error) -> miette::Report {
108137
mod tests {
109138
use super::*;
110139
use crate::test_utils::Environment;
111-
use std::net::Ipv6Addr;
112140

113141
/// Bind an ephemeral loopback listener, return its (now freed) port.
114142
async fn free_loopback_port() -> u16 {
@@ -159,6 +187,67 @@ mod tests {
159187
}
160188
}
161189

190+
#[test]
191+
fn callback_host_maps_bindings_to_reachable_url_hosts() {
192+
let parse = |s: &str| -> SocketAddr { s.parse().expect("addr") };
193+
// Wildcard binds are reached through the matching loopback literal.
194+
assert_eq!(callback_host(parse("0.0.0.0:8080")), "127.0.0.1");
195+
assert_eq!(callback_host(parse("[::]:8080")), "[::1]");
196+
// Loopback binds keep their address; IPv6 renders bracketed.
197+
assert_eq!(callback_host(parse("127.0.0.1:8080")), "127.0.0.1");
198+
assert_eq!(callback_host(parse("[::1]:8080")), "[::1]");
199+
// Explicit specific addresses are addressed literally.
200+
assert_eq!(callback_host(parse("192.168.1.5:8080")), "192.168.1.5");
201+
assert_eq!(callback_host(parse("[2001:db8::5]:8080")), "[2001:db8::5]");
202+
}
203+
204+
#[test]
205+
fn callback_target_maps_bindings_to_reachable_ips() {
206+
let parse = |s: &str| -> SocketAddr { s.parse().expect("addr") };
207+
assert_eq!(
208+
callback_target(parse("0.0.0.0:8080")),
209+
IpAddr::V4(Ipv4Addr::LOCALHOST)
210+
);
211+
assert_eq!(
212+
callback_target(parse("[::]:8080")),
213+
IpAddr::V6(Ipv6Addr::LOCALHOST)
214+
);
215+
assert_eq!(
216+
callback_target(parse("[2001:db8::5]:8080")),
217+
IpAddr::V6("2001:db8::5".parse::<Ipv6Addr>().expect("v6"))
218+
);
219+
}
220+
221+
#[tokio::test]
222+
async fn ipv6_callback_is_reachable_at_advertised_host() {
223+
// Callback reachability: an IPv6-bound listener must be reachable at
224+
// exactly the host the browser flows advertise, and (on a host
225+
// without the address) must NOT be reachable via IPv4 loopback.
226+
Environment::new()
227+
.remove(CALLBACK_PORT_ENV)
228+
.set(CALLBACK_BIND_ADDRESS_ENV, "::1")
229+
.run_async(async {
230+
let listener = bind_callback_listener().await.expect("ipv6 bind");
231+
let addr = listener.local_addr().expect("local addr");
232+
let host = callback_host(addr);
233+
assert_eq!(host, "[::1]");
234+
235+
let v6 = SocketAddr::new(
236+
IpAddr::V6("::1".parse::<Ipv6Addr>().expect("v6")),
237+
addr.port(),
238+
);
239+
let stream = tokio::net::TcpStream::connect(v6)
240+
.await
241+
.expect("[::1] callback must be reachable");
242+
drop(stream);
243+
244+
let v4 = SocketAddr::new(IpAddr::V4(Ipv4Addr::LOCALHOST), addr.port());
245+
let refused = tokio::net::TcpStream::connect(v4).await;
246+
assert!(refused.is_err(), "IPv4 loopback must not reach a ::1 bind");
247+
})
248+
.await;
249+
}
250+
162251
#[test]
163252
fn busy_fixed_port_error_names_the_variable() {
164253
let addr: SocketAddr = "127.0.0.1:18080".parse().expect("parse addr");

‎crates/openshell-cli/src/oidc_auth.rs‎

Lines changed: 50 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -173,13 +173,16 @@ fn device_token_form(
173173

174174
/// Build the OIDC redirect URI for a bound callback listener.
175175
///
176-
/// The URI always points at IPv4 loopback, even when the listener binds
177-
/// `0.0.0.0` inside a container: the host-published port and the identity
178-
/// provider allowlist (`http://127.0.0.1:*`) both target host loopback. See
179-
/// the `callback` module docs for the environment-variable controls.
176+
/// The URI derives from the listener's bound address (see
177+
/// [`crate::callback::callback_host`]): IPv4 binds and the default bind use
178+
/// `http://127.0.0.1:<port>/callback`, while IPv6 binds use the bracketed
179+
/// literal (`http://[::1]:<port>/callback`), matching RFC 8252 loopback
180+
/// redirects. Identity providers that allowlist `http://127.0.0.1:*` for
181+
/// native apps should also allowlist `http://[::1]:*`.
180182
fn callback_redirect_uri(listener: &TcpListener) -> Result<String> {
181-
let port = listener.local_addr().into_diagnostic()?.port();
182-
Ok(format!("http://127.0.0.1:{port}/callback"))
183+
let local_addr = listener.local_addr().into_diagnostic()?;
184+
let host = crate::callback::callback_host(local_addr);
185+
Ok(format!("http://{host}:{}/callback", local_addr.port()))
183186
}
184187

185188
/// Run the OIDC Authorization Code + PKCE browser flow.
@@ -1051,7 +1054,7 @@ mod tests {
10511054

10521055
use crate::callback::{CALLBACK_BIND_ADDRESS_ENV, CALLBACK_PORT_ENV};
10531056
use crate::test_utils::Environment;
1054-
use std::net::{IpAddr, Ipv4Addr};
1057+
use std::net::{IpAddr, Ipv4Addr, Ipv6Addr};
10551058

10561059
/// Bind an ephemeral loopback listener, return its (now freed) port.
10571060
async fn free_loopback_port() -> u16 {
@@ -1103,14 +1106,52 @@ mod tests {
11031106

11041107
assert_eq!(addr.ip(), IpAddr::V4(Ipv4Addr::UNSPECIFIED));
11051108
assert_eq!(addr.port(), expected);
1106-
// The redirect URI stays on 127.0.0.1 even for a wildcard bind so the
1107-
// identity-provider allowlist keeps matching.
1109+
// The redirect URI stays on 127.0.0.1 for an IPv4 wildcard
1110+
// bind so the identity-provider allowlist keeps matching.
11081111
let uri = callback_redirect_uri(&listener).expect("redirect uri");
11091112
assert_eq!(uri, format!("http://127.0.0.1:{expected}/callback"));
11101113
})
11111114
.await;
11121115
}
11131116

1117+
#[tokio::test]
1118+
async fn browser_flow_redirect_uri_follows_ipv6_bind() {
1119+
Environment::new()
1120+
.remove(CALLBACK_PORT_ENV)
1121+
.set(CALLBACK_BIND_ADDRESS_ENV, "::1")
1122+
.run_async(async {
1123+
let listener = crate::callback::bind_callback_listener()
1124+
.await
1125+
.expect("ipv6 bind");
1126+
let addr = listener.local_addr().expect("local addr");
1127+
1128+
assert_eq!(addr.ip(), IpAddr::V6(Ipv6Addr::LOCALHOST));
1129+
// RFC 8252 loopback redirect with the bracketed IPv6 literal,
1130+
// so the browser actually reaches the ::1-bound listener.
1131+
let uri = callback_redirect_uri(&listener).expect("redirect uri");
1132+
assert_eq!(uri, format!("http://[::1]:{}/callback", addr.port()));
1133+
})
1134+
.await;
1135+
}
1136+
1137+
#[tokio::test]
1138+
async fn browser_flow_redirect_uri_uses_ipv6_loopback_for_wildcard_v6_bind() {
1139+
Environment::new()
1140+
.remove(CALLBACK_PORT_ENV)
1141+
.set(CALLBACK_BIND_ADDRESS_ENV, "::")
1142+
.run_async(async {
1143+
let listener = crate::callback::bind_callback_listener()
1144+
.await
1145+
.expect("wildcard ipv6 bind");
1146+
let addr = listener.local_addr().expect("local addr");
1147+
1148+
assert_eq!(addr.ip(), IpAddr::V6(Ipv6Addr::UNSPECIFIED));
1149+
let uri = callback_redirect_uri(&listener).expect("redirect uri");
1150+
assert_eq!(uri, format!("http://[::1]:{}/callback", addr.port()));
1151+
})
1152+
.await;
1153+
}
1154+
11141155
#[tokio::test]
11151156
async fn browser_flow_rejects_invalid_callback_port() {
11161157
Environment::new()

0 commit comments

Comments
 (0)