diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 3bc14e27..b952a232 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -773,17 +773,19 @@ as exec and ping. `ForwardingConfig` carries its own copy of the resolved address family for the forwarding-target filter, since forwarders run detached from the connect config. -**Scope.** The constraint is a hard filter where bssh opens the socket, and a -hint where the remote server does: +**Scope.** The constraint is a hard filter where bssh opens the socket. For +forwarding targets the unforced path preserves server-side resolution, while a +forced family switches to locally resolved numeric addresses so the family +request has an observable effect: | Path | Behavior | | --- | --- | | Direct connect (exec, interactive, ping, SFTP) | Hard filter in `connect_with_config_inner` | | First jump hop | Hard filter (shares the direct connect path) | | `-L` / `-D` listener | Selects the implicit bind address (`::1` / `::` under `-6`); an explicit bind address wins | -| `-L` / SOCKS5 `-D` target | Filters the `direct-tcpip` candidate list; the remote sshd still performs the connect, so this is advisory | +| `-L` / SOCKS5 `-D` target | With `Any`, sends the requested hostname in `direct-tcpip` and lets the remote sshd resolve it. With `V4` / `V6`, resolves locally, filters the candidate list, and sends the matching numeric address | | SOCKS4 `-D` target | Unfiltered; SOCKS4 carries a literal IPv4 destination by protocol definition | -| Jump hops past the first, and the destination behind a chain | Filters the `direct-tcpip` candidate list through the same advisory mechanism as `-L`/SOCKS5 `-D` targets. The family also selects the address recorded for host key verification, so known_hosts diagnostics stay consistent | +| Jump hops past the first, and the destination behind a chain | Uses the same `direct-tcpip` model as `-L`/SOCKS5 `-D` targets: hostname with `Any`, locally filtered numeric address with `V4` / `V6`. The family also selects the best-effort address recorded for host key verification diagnostics | | `-R` listener | Not constrained; the server binds it | | `bssh-server` | Out of scope; separate CLI | diff --git a/docs/man/bssh.1 b/docs/man/bssh.1 index 450395c8..cf9acaa5 100644 --- a/docs/man/bssh.1 +++ b/docs/man/bssh.1 @@ -1178,20 +1178,34 @@ a specification that does not name a bind address listens on ::1 instead of wildcard form listens on :: instead of 0.0.0.0. A specification that names a bind address explicitly always overrides the flag. -.SS What the constraint only hints at +.SS Forwarding target resolution .IP \[bu] 2 -Forwarding targets and tunneled jump-chain targets. bssh resolves the target of a +Forwarding targets and tunneled jump-chain targets. Without +.B \-4 +or +.BR \-6 , +bssh sends the target hostname from a .B \-L -forward or a SOCKS5 +forward, SOCKS5 .B \-D -request, later jump-chain hop, or final destination behind a jump chain locally -to decide which address to name in the +domain request, later jump-chain hop, or final destination behind a jump chain +as written in the +.I direct-tcpip +channel request. The remote SSH server resolves that name and performs the +actual connection, matching OpenSSH and allowing names that exist only from the +server's network position. +.IP \[bu] 2 +When a family is forced with +.B \-4 +or +.BR \-6 , +bssh resolves the forwarding target locally, filters the candidate list to the +requested family, and sends the matching numeric address in the .I direct-tcpip -channel request, and the address family filters that candidate list. The remote -SSH server performs the actual connection and may resolve the name differently, -so this is a best-effort hint rather than a guarantee. SOCKS4 requests carry a -literal IPv4 destination by protocol definition and are passed through -unfiltered. +request. This gives the family request a concrete effect on the server-side +connection, but it necessarily uses the client's resolver view for that forced +path. SOCKS4 requests carry a literal IPv4 destination by protocol definition and +are passed through unfiltered. .SS What the constraint does not cover The remote listener created by diff --git a/src/forwarding/dynamic/socks.rs b/src/forwarding/dynamic/socks.rs index c0020c3e..bda9131a 100644 --- a/src/forwarding/dynamic/socks.rs +++ b/src/forwarding/dynamic/socks.rs @@ -212,9 +212,9 @@ pub async fn handle_socks5_connection( debug!("SOCKS5 CONNECT to {} from {}", destination, peer_addr); - // Create SSH channel to destination. A SOCKS5 request may name a domain, - // so the forced address family narrows which resolved address is offered - // to the server. + // Create SSH channel to destination. Domain requests stay as names unless + // an address family is forced, in which case the channel manager resolves + // and sends a matching numeric address. let ssh_channel = match ssh_client .open_direct_tcpip_channel_with_family(destination.as_str(), None, address_family) .await diff --git a/src/ssh/tokio_client/channel_manager.rs b/src/ssh/tokio_client/channel_manager.rs index a43101e5..4c8ec916 100644 --- a/src/ssh/tokio_client/channel_manager.rs +++ b/src/ssh/tokio_client/channel_manager.rs @@ -63,6 +63,12 @@ const MAX_SUDO_PROMPT_BUFFER_SIZE: usize = 64 * 1024; /// Set to 10 to support reasonable multi-sudo command chains. const MAX_SUDO_PASSWORD_SENDS: u32 = 10; +#[derive(Debug, Clone, PartialEq, Eq)] +struct DirectTcpipRequestTarget { + host: String, + port: u32, +} + /// Command output variants for streaming #[derive(Debug, Clone)] pub enum CommandOutput { @@ -166,6 +172,29 @@ impl Client { Ok(targets) } + fn direct_tcpip_request_targets( + target: &T, + address_family: AddressFamily, + ) -> Result, super::Error> { + if !address_family.is_forced() { + let (host, port) = target.host_port().map_err(super::Error::AddressInvalid)?; + return Ok(vec![DirectTcpipRequestTarget { + host, + port: port.into(), + }]); + } + + Self::direct_tcpip_targets(target, address_family).map(|targets| { + targets + .into_iter() + .map(|target| DirectTcpipRequestTarget { + host: target.ip().to_string(), + port: target.port().into(), + }) + .collect() + }) + } + /// Get a new SSH channel for communication. pub async fn get_channel(&self) -> Result, super::Error> { self.connection_handle @@ -192,14 +221,15 @@ impl Client { .await } - /// Open a `direct-tcpip` channel, restricting the candidate target - /// addresses to `address_family`. + /// Open a `direct-tcpip` channel, optionally restricting the candidate + /// target addresses to `address_family`. /// - /// The target address is resolved locally only to pick which address to - /// name in the channel-open request; the remote sshd performs the actual - /// connect and may resolve the name differently. Filtering here is - /// therefore a best-effort hint, not a guarantee, and it is why port - /// forwarding documents `-4` / `-6` as advisory for the far end. + /// With [`AddressFamily::Any`] bssh sends the hostname exactly as supplied, + /// and the remote sshd resolves and connects it. With a forced family + /// (`-4`, `-6`, or ssh_config `AddressFamily inet|inet6`), bssh resolves + /// locally, filters to the requested family, and sends the matching numeric + /// address so the family request has a concrete effect on the server-side + /// connection. pub async fn open_direct_tcpip_channel_with_family< T: ToSocketAddrsWithHostname, S: Into>, @@ -209,7 +239,7 @@ impl Client { src: S, address_family: AddressFamily, ) -> Result, super::Error> { - let targets = Self::direct_tcpip_targets(&target, address_family)?; + let targets = Self::direct_tcpip_request_targets(&target, address_family)?; let src = src .into() @@ -220,15 +250,10 @@ impl Client { io::ErrorKind::InvalidInput, "could not resolve to any addresses", )); - for target in targets { + for DirectTcpipRequestTarget { host, port } in targets { match self .connection_handle - .channel_open_direct_tcpip( - target.ip().to_string(), - target.port().into(), - src.0.clone(), - src.1, - ) + .channel_open_direct_tcpip(host, port, src.0.clone(), src.1) .await { Ok(channel) => return Ok(channel), @@ -717,4 +742,80 @@ mod tests { vec![v6("[2001:db8::10]:22"), v6("[2001:db8::11]:22")] ); } + + #[test] + fn direct_tcpip_request_targets_send_unforced_hostname_without_resolution() { + let targets = + Client::direct_tcpip_request_targets(&"server-only.internal:5432", AddressFamily::Any) + .expect("unforced direct-tcpip targets must not require local DNS"); + + assert_eq!( + targets, + vec![DirectTcpipRequestTarget { + host: "server-only.internal".to_string(), + port: 5432, + }] + ); + } + + #[test] + fn direct_tcpip_request_targets_send_unforced_tuple_hostname_for_jump_hops() { + let targets = Client::direct_tcpip_request_targets( + &("jump-private.internal", 2222), + AddressFamily::Any, + ) + .expect("unforced jump-hop targets must not require local DNS"); + + assert_eq!( + targets, + vec![DirectTcpipRequestTarget { + host: "jump-private.internal".to_string(), + port: 2222, + }] + ); + } + + #[test] + fn direct_tcpip_request_targets_send_forced_ipv4_address() { + let candidates = multi_hop_candidates(); + let targets = + Client::direct_tcpip_request_targets(&candidates.as_slice(), AddressFamily::V4) + .expect("forced IPv4 direct-tcpip targets must resolve to numeric addresses"); + + assert_eq!( + targets, + vec![ + DirectTcpipRequestTarget { + host: "192.0.2.10".to_string(), + port: 22, + }, + DirectTcpipRequestTarget { + host: "192.0.2.11".to_string(), + port: 22, + }, + ] + ); + } + + #[test] + fn direct_tcpip_request_targets_send_forced_ipv6_address() { + let candidates = multi_hop_candidates(); + let targets = + Client::direct_tcpip_request_targets(&candidates.as_slice(), AddressFamily::V6) + .expect("forced IPv6 direct-tcpip targets must resolve to numeric addresses"); + + assert_eq!( + targets, + vec![ + DirectTcpipRequestTarget { + host: "2001:db8::10".to_string(), + port: 22, + }, + DirectTcpipRequestTarget { + host: "2001:db8::11".to_string(), + port: 22, + }, + ] + ); + } } diff --git a/src/ssh/tokio_client/to_socket_addrs_with_hostname.rs b/src/ssh/tokio_client/to_socket_addrs_with_hostname.rs index f1c97442..029c0a44 100644 --- a/src/ssh/tokio_client/to_socket_addrs_with_hostname.rs +++ b/src/ssh/tokio_client/to_socket_addrs_with_hostname.rs @@ -4,6 +4,9 @@ use std::net::{IpAddr, Ipv4Addr, Ipv6Addr, SocketAddr, SocketAddrV4, SocketAddrV pub trait ToSocketAddrsWithHostname { fn to_socket_addrs(&self) -> io::Result>; fn hostname(&self) -> String; + fn host_port(&self) -> io::Result<(String, u16)> { + parse_host_port(&self.hostname()) + } } impl ToSocketAddrsWithHostname for String { @@ -13,6 +16,9 @@ impl ToSocketAddrsWithHostname for String { fn hostname(&self) -> String { self.clone() } + fn host_port(&self) -> io::Result<(String, u16)> { + parse_host_port(self) + } } impl ToSocketAddrsWithHostname for &str { @@ -22,6 +28,9 @@ impl ToSocketAddrsWithHostname for &str { fn hostname(&self) -> String { self.to_string() } + fn host_port(&self) -> io::Result<(String, u16)> { + parse_host_port(self) + } } impl ToSocketAddrsWithHostname for (&str, u16) { @@ -31,6 +40,9 @@ impl ToSocketAddrsWithHostname for (&str, u16) { fn hostname(&self) -> String { self.0.to_string() } + fn host_port(&self) -> io::Result<(String, u16)> { + Ok((self.0.to_string(), self.1)) + } } impl ToSocketAddrsWithHostname for (String, u16) { @@ -40,6 +52,9 @@ impl ToSocketAddrsWithHostname for (String, u16) { fn hostname(&self) -> String { self.0.clone() } + fn host_port(&self) -> io::Result<(String, u16)> { + Ok((self.0.clone(), self.1)) + } } impl ToSocketAddrsWithHostname for (IpAddr, u16) { @@ -49,6 +64,9 @@ impl ToSocketAddrsWithHostname for (IpAddr, u16) { fn hostname(&self) -> String { format!("{}", self.0) } + fn host_port(&self) -> io::Result<(String, u16)> { + Ok((self.0.to_string(), self.1)) + } } impl ToSocketAddrsWithHostname for (Ipv4Addr, u16) { @@ -58,6 +76,9 @@ impl ToSocketAddrsWithHostname for (Ipv4Addr, u16) { fn hostname(&self) -> String { format!("{}", self.0) } + fn host_port(&self) -> io::Result<(String, u16)> { + Ok((self.0.to_string(), self.1)) + } } impl ToSocketAddrsWithHostname for (Ipv6Addr, u16) { @@ -67,6 +88,9 @@ impl ToSocketAddrsWithHostname for (Ipv6Addr, u16) { fn hostname(&self) -> String { format!("{}", self.0) } + fn host_port(&self) -> io::Result<(String, u16)> { + Ok((self.0.to_string(), self.1)) + } } impl ToSocketAddrsWithHostname for SocketAddr { @@ -76,6 +100,9 @@ impl ToSocketAddrsWithHostname for SocketAddr { fn hostname(&self) -> String { format!("{}", self.ip()) } + fn host_port(&self) -> io::Result<(String, u16)> { + Ok((self.ip().to_string(), self.port())) + } } impl ToSocketAddrsWithHostname for SocketAddrV4 { @@ -85,6 +112,9 @@ impl ToSocketAddrsWithHostname for SocketAddrV4 { fn hostname(&self) -> String { format!("{}", self.ip()) } + fn host_port(&self) -> io::Result<(String, u16)> { + Ok((self.ip().to_string(), self.port())) + } } impl ToSocketAddrsWithHostname for SocketAddrV6 { @@ -94,6 +124,9 @@ impl ToSocketAddrsWithHostname for SocketAddrV6 { fn hostname(&self) -> String { format!("{}", self.ip()) } + fn host_port(&self) -> io::Result<(String, u16)> { + Ok((self.ip().to_string(), self.port())) + } } impl ToSocketAddrsWithHostname for &[SocketAddr] { @@ -107,4 +140,58 @@ impl ToSocketAddrsWithHostname for &[SocketAddr] { .collect::>() .join(",") } + fn host_port(&self) -> io::Result<(String, u16)> { + self.first() + .map(|addr| (addr.ip().to_string(), addr.port())) + .ok_or_else(|| io::Error::new(io::ErrorKind::InvalidInput, "missing socket address")) + } +} + +fn parse_host_port(target: &str) -> io::Result<(String, u16)> { + let (host, port) = if let Some(rest) = target.strip_prefix('[') { + let (host, rest) = rest.split_once(']').ok_or_else(|| { + io::Error::new( + io::ErrorKind::InvalidInput, + "missing closing bracket in IPv6 host", + ) + })?; + let port = rest + .strip_prefix(':') + .ok_or_else(|| io::Error::new(io::ErrorKind::InvalidInput, "missing port separator"))?; + (host, port) + } else { + target + .rsplit_once(':') + .ok_or_else(|| io::Error::new(io::ErrorKind::InvalidInput, "missing port separator"))? + }; + + if host.is_empty() { + return Err(io::Error::new(io::ErrorKind::InvalidInput, "missing host")); + } + + let port = port + .parse::() + .map_err(|err| io::Error::new(io::ErrorKind::InvalidInput, err))?; + Ok((host.to_string(), port)) +} + +#[cfg(test)] +mod tests { + use super::{ToSocketAddrsWithHostname, parse_host_port}; + + #[test] + fn host_port_parses_domain_without_resolution() { + assert_eq!( + "server-only.internal:5432".host_port().unwrap(), + ("server-only.internal".to_string(), 5432) + ); + } + + #[test] + fn host_port_parses_bracketed_ipv6_literal() { + assert_eq!( + parse_host_port("[2001:db8::1]:443").unwrap(), + ("2001:db8::1".to_string(), 443) + ); + } }