From 9b811c7a16be20ea6daf197cf1b1b43bb7492330 Mon Sep 17 00:00:00 2001 From: Riatre Date: Wed, 8 May 2024 23:09:07 +0800 Subject: [PATCH] Wire up agent forward for libssh backend (#5345) * Wire up agent forward for libssh backend TSIA. There's a drive-by change in sftpwrap.rs for bumping to new libssh-rs. --- Cargo.lock | 6 +- wezterm-ssh/Cargo.toml | 3 +- wezterm-ssh/src/channelwrap.rs | 15 +++++ wezterm-ssh/src/pty.rs | 12 ++-- wezterm-ssh/src/sessioninner.rs | 83 +++++++++++++++++++++++++- wezterm-ssh/src/sessionwrap.rs | 12 ++++ wezterm-ssh/src/sftpwrap.rs | 8 ++- wezterm-ssh/tests/e2e/agent_forward.rs | 53 ++++++++++++++++ wezterm-ssh/tests/e2e/mod.rs | 1 + wezterm-ssh/tests/sshd.rs | 3 +- 10 files changed, 179 insertions(+), 17 deletions(-) create mode 100644 wezterm-ssh/tests/e2e/agent_forward.rs diff --git a/Cargo.lock b/Cargo.lock index 483dfe87d29..fa58dd8db4e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3022,11 +3022,12 @@ dependencies = [ [[package]] name = "libssh-rs" -version = "0.2.2" +version = "0.3.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "eb3fe324fb06b71d28abb81382ac547f25b4895e853a9968482dc5002fb3db08" +checksum = "c13fbf28a79bb77f11a44716baca6327655198455b7c6f6189784279f3e331db" dependencies = [ "bitflags 1.3.2", + "libc", "libssh-rs-sys", "openssl-sys", "thiserror", @@ -6795,6 +6796,7 @@ dependencies = [ "ssh2", "termwiz", "thiserror", + "uds_windows", "whoami", ] diff --git a/wezterm-ssh/Cargo.toml b/wezterm-ssh/Cargo.toml index dcf14b296e0..5dd513a32c0 100644 --- a/wezterm-ssh/Cargo.toml +++ b/wezterm-ssh/Cargo.toml @@ -31,10 +31,11 @@ portable-pty = { version="0.8", path = "../pty" } regex = "1" smol = "1.2" ssh2 = {version="0.9.3", features=["openssl-on-win32"], optional = true} -libssh-rs = {version="0.2.1", features=["vendored"], optional = true} +libssh-rs = {version="0.3.1", features=["vendored"], optional = true} #libssh-rs = {path="../../libssh-rs/libssh-rs", features=["vendored"], optional = true} thiserror = "1.0" socket2 = "0.5" +uds_windows = "1.1.0" # Not used directly, but is used to centralize the openssl vendor feature selection async_ossl = { path = "../async_ossl" } diff --git a/wezterm-ssh/src/channelwrap.rs b/wezterm-ssh/src/channelwrap.rs index f7409cb52f6..db79dbcdcde 100644 --- a/wezterm-ssh/src/channelwrap.rs +++ b/wezterm-ssh/src/channelwrap.rs @@ -146,6 +146,21 @@ impl ChannelWrap { } } + pub fn request_auth_agent_forwarding(&mut self) -> anyhow::Result<()> { + match self { + /* libssh2 doesn't properly support agent forwarding + * at this time: + * */ + #[cfg(feature = "ssh2")] + Self::Ssh2(_chan) => Err(anyhow::anyhow!( + "ssh2 does not support request_auth_agent_forwarding" + )), + + #[cfg(feature = "libssh-rs")] + Self::LibSsh(chan) => Ok(chan.request_auth_agent()?), + } + } + pub fn resize_pty(&mut self, resize: &ResizePty) -> anyhow::Result<()> { match self { #[cfg(feature = "ssh2")] diff --git a/wezterm-ssh/src/pty.rs b/wezterm-ssh/src/pty.rs index 9136b29d8c3..175a3a311a2 100644 --- a/wezterm-ssh/src/pty.rs +++ b/wezterm-ssh/src/pty.rs @@ -218,17 +218,13 @@ impl crate::sessioninner::SessionInner { let mut channel = sess.open_session()?; - /* libssh2 doesn't properly support agent forwarding - * at this time: - * if let Some("yes") = self.config.get("forwardagent").map(|s| s.as_str()) { - log::info!("requesting agent forwarding"); - if let Err(err) = channel.request_auth_agent_forwarding() { - log::error!("Failed to establish agent forwarding: {:#}", err); + if self.identity_agent().is_some() { + if let Err(err) = channel.request_auth_agent_forwarding() { + log::error!("Failed to request agent forwarding: {:#}", err); + } } - log::info!("agent forwarding OK!"); } - */ channel.request_pty(&newpty)?; diff --git a/wezterm-ssh/src/sessioninner.rs b/wezterm-ssh/src/sessioninner.rs index a0f30d6461e..7e5d50d1021 100644 --- a/wezterm-ssh/src/sessioninner.rs +++ b/wezterm-ssh/src/sessioninner.rs @@ -243,6 +243,13 @@ impl SessionInner { .try_send(SessionEvent::Authenticated) .context("notifying user that session is authenticated")?; + if let Some("yes") = self.config.get("forwardagent").map(|s| s.as_str()) { + if self.identity_agent().is_some() { + sess.enable_accept_agent_forward(true); + } else { + log::error!("ForwardAgent is set to yes, but IdentityAgent is not set"); + } + } sess.set_blocking(false); let mut sess = SessionWrap::with_libssh(sess); self.request_loop(&mut sess) @@ -405,6 +412,7 @@ impl SessionInner { self.tick_io()?; self.drain_request_pipe(); self.dispatch_pending_requests(sess)?; + self.connect_pending_agent_forward_channels(sess); if self.channels.is_empty() && self.session_was_dropped { log::trace!( @@ -517,8 +525,16 @@ impl SessionInner { let stdin = &mut chan.descriptors[0]; if stdin.fd.is_some() && !stdin.buf.is_empty() { - write_from_buf(&mut chan.channel.writer(), &mut stdin.buf) - .context("writing to channel")?; + if let Err(err) = write_from_buf(&mut chan.channel.writer(), &mut stdin.buf) + .context("writing to channel") + { + log::trace!( + "Failed to write data to channel {} stdin: {:#}, closing pipe", + id, + err + ); + stdin.fd.take(); + } } for (idx, out) in chan @@ -805,6 +821,62 @@ impl SessionInner { } } + fn connect_pending_agent_forward_channels(&mut self, sess: &mut SessionWrap) { + fn process_one(sess: &mut SessionInner, channel: ChannelWrap) -> anyhow::Result<()> { + let identity_agent = sess + .identity_agent() + .ok_or_else(|| anyhow!("no identity agent in config"))?; + let mut fd = { + #[cfg(unix)] + { + use std::os::unix::net::UnixStream; + FileDescriptor::new(UnixStream::connect(&identity_agent)?) + } + #[cfg(windows)] + unsafe { + use std::os::windows::io::{FromRawSocket, IntoRawSocket}; + use uds_windows::UnixStream; + FileDescriptor::from_raw_socket( + UnixStream::connect(&identity_agent)?.into_raw_socket(), + ) + } + }; + fd.set_non_blocking(true)?; + + let read_from_agent = fd; + let write_to_agent = read_from_agent.try_clone()?; + let channel_id = sess.next_channel_id; + sess.next_channel_id += 1; + let info = ChannelInfo { + channel_id, + channel, + exit: None, + exited: false, + descriptors: [ + DescriptorState { + fd: Some(read_from_agent), + buf: VecDeque::with_capacity(8192), + }, + DescriptorState { + fd: Some(write_to_agent), + buf: VecDeque::with_capacity(8192), + }, + DescriptorState { + fd: None, + buf: VecDeque::with_capacity(8192), + }, + ], + }; + sess.channels.insert(channel_id, info); + Ok(()) + } + while let Some(channel) = sess.accept_agent_forward() { + if let Err(err) = process_one(self, channel) { + log::error!("error connecting agent forward: {:#}", err); + } + } + } + pub fn signal_channel(&mut self, info: &SignalChannel) -> anyhow::Result<()> { let chan_info = self .channels @@ -944,6 +1016,13 @@ impl SessionInner { } } } + + pub fn identity_agent(&self) -> Option { + self.config + .get("identityagent") + .map(|s| s.to_owned()) + .or_else(|| std::env::var("SSH_AUTH_SOCK").ok()) + } } fn write_from_buf(w: &mut W, buf: &mut VecDeque) -> std::io::Result<()> { diff --git a/wezterm-ssh/src/sessionwrap.rs b/wezterm-ssh/src/sessionwrap.rs index 2a3d9449dd9..72578f21939 100644 --- a/wezterm-ssh/src/sessionwrap.rs +++ b/wezterm-ssh/src/sessionwrap.rs @@ -92,4 +92,16 @@ impl SessionWrap { } } } + + pub fn accept_agent_forward(&mut self) -> Option { + match self { + // Unimplemented for now, an error message was printed earlier when the user tries to + // enable agent forwarding so just return nothing here. + #[cfg(feature = "ssh2")] + Self::Ssh2(_sess) => None, + + #[cfg(feature = "libssh-rs")] + Self::LibSsh(sess) => sess.sess.accept_agent_forward().map(ChannelWrap::LibSsh), + } + } } diff --git a/wezterm-ssh/src/sftpwrap.rs b/wezterm-ssh/src/sftpwrap.rs index 11939ddecbb..4335479e026 100644 --- a/wezterm-ssh/src/sftpwrap.rs +++ b/wezterm-ssh/src/sftpwrap.rs @@ -38,6 +38,7 @@ impl SftpWrap { Self::LibSsh(sftp) => { use crate::sftp::types::WriteMode; use libc::{O_APPEND, O_RDONLY, O_RDWR, O_WRONLY}; + use libssh_rs::OpenFlags; use std::convert::TryInto; let accesstype = match (opts.write, opts.read) { (Some(WriteMode::Append), true) => O_RDWR | O_APPEND, @@ -47,8 +48,11 @@ impl SftpWrap { (None, true) => O_RDONLY, (None, false) => 0, }; - let file = - sftp.open(filename.as_str(), accesstype, opts.mode.try_into().unwrap())?; + let file = sftp.open( + filename.as_str(), + OpenFlags::from_bits_truncate(accesstype), + opts.mode.try_into().unwrap(), + )?; Ok(FileWrap::LibSsh(file)) } } diff --git a/wezterm-ssh/tests/e2e/agent_forward.rs b/wezterm-ssh/tests/e2e/agent_forward.rs new file mode 100644 index 00000000000..2af6cfd021b --- /dev/null +++ b/wezterm-ssh/tests/e2e/agent_forward.rs @@ -0,0 +1,53 @@ +use crate::sshd::*; +use portable_pty::{MasterPty, PtySize}; +use rstest::*; +use std::io::Read; +use wezterm_ssh::Config; + +#[fixture] +async fn session_with_agent_forward( + #[future] + #[with({ let mut config = Config::new(); config.set_option("forwardagent", "yes"); config })] + session: SessionWithSshd, +) -> SessionWithSshd { + session.await +} + +#[rstest] +#[smol_potat::test] +#[cfg_attr(not(any(target_os = "macos", target_os = "linux")), ignore)] +#[cfg_attr(not(feature = "libssh-rs"), ignore)] +async fn ssh_add_should_be_able_to_list_identities_with_agent_forward( + #[future] session_with_agent_forward: SessionWithSshd, +) { + let session: SessionWithSshd = session_with_agent_forward.await; + + let (pty, _child_process) = session + .request_pty("dumb", PtySize::default(), Some("ssh-add -l"), None) + .await + .unwrap(); + let mut reader = pty.try_clone_reader().unwrap(); + let mut output: String = String::new(); + reader.read_to_string(&mut output).unwrap(); + assert_eq!(output, "The agent has no identities.\r\n"); +} + +#[rstest] +#[smol_potat::test] +#[cfg_attr(not(any(target_os = "macos", target_os = "linux")), ignore)] +#[cfg_attr(not(feature = "libssh-rs"), ignore)] +async fn no_agent_forward_should_happen_when_disabled(#[future] session: SessionWithSshd) { + let session: SessionWithSshd = session.await; + + let (pty, _child_process) = session + .request_pty("dumb", PtySize::default(), Some("ssh-add -l"), None) + .await + .unwrap(); + let mut reader = pty.try_clone_reader().unwrap(); + let mut output: String = String::new(); + reader.read_to_string(&mut output).unwrap(); + assert_eq!( + output, + "Could not open a connection to your authentication agent.\r\n" + ); +} diff --git a/wezterm-ssh/tests/e2e/mod.rs b/wezterm-ssh/tests/e2e/mod.rs index 4ce980ac525..fc3e9b2487e 100644 --- a/wezterm-ssh/tests/e2e/mod.rs +++ b/wezterm-ssh/tests/e2e/mod.rs @@ -1 +1,2 @@ +mod agent_forward; mod sftp; diff --git a/wezterm-ssh/tests/sshd.rs b/wezterm-ssh/tests/sshd.rs index e485c6736a6..619f78417c4 100644 --- a/wezterm-ssh/tests/sshd.rs +++ b/wezterm-ssh/tests/sshd.rs @@ -431,10 +431,9 @@ impl std::ops::DerefMut for SessionWithSshd { #[fixture] /// Stand up an sshd instance and then connect to it and perform authentication -pub async fn session(sshd: Sshd) -> SessionWithSshd { +pub async fn session(#[default(Config::new())] mut config: Config, sshd: Sshd) -> SessionWithSshd { let port = sshd.port; - let mut config = Config::new(); config.add_default_config_files(); // Load our config to point to ourselves, using current sshd instance's port,