From b0ce34b9f8fd188cdcba60eb98c607116c91fb89 Mon Sep 17 00:00:00 2001 From: ChristianVisintin Date: Sat, 5 Dec 2020 17:37:19 +0100 Subject: [PATCH] Scp: use oneshot channels instead of ptys (more stable; more reliable and overall works) --- src/filetransfer/scp_transfer.rs | 323 +++++++++++++++---------------- 1 file changed, 155 insertions(+), 168 deletions(-) diff --git a/src/filetransfer/scp_transfer.rs b/src/filetransfer/scp_transfer.rs index bbfbb50..347ea2e 100644 --- a/src/filetransfer/scp_transfer.rs +++ b/src/filetransfer/scp_transfer.rs @@ -1,6 +1,6 @@ -//! ## SFTP_Transfer +//! ## SCP_Transfer //! -//! `sftp_transfer` is the module which provides the implementation for the SFTP file transfer +//! `scps_transfer` is the module which provides the implementation for the SCP file transfer /* * @@ -42,10 +42,10 @@ use std::time::SystemTime; /// ## ScpFileTransfer /// -/// SFTP file transfer structure +/// SCP file transfer structure pub struct ScpFileTransfer { session: Option, - pty: Option, + wrkdir: PathBuf, } impl ScpFileTransfer { @@ -55,7 +55,7 @@ impl ScpFileTransfer { pub fn new() -> ScpFileTransfer { ScpFileTransfer { session: None, - pty: None, + wrkdir: PathBuf::from("~"), } } @@ -230,44 +230,54 @@ impl ScpFileTransfer { (filename, symlink) } + /// ### perform_shell_cmd_with + /// + /// Perform a shell command, but change directory to specified path first + fn perform_shell_cmd_with_path( + &mut self, + path: &Path, + cmd: &str, + ) -> Result { + self.perform_shell_cmd(format!("cd \"{}\"; {}", path.display(), cmd).as_str()) + } + /// ### perform_shell_cmd /// /// Perform a shell command and read the output from shell /// This operation is, obviously, blocking. fn perform_shell_cmd(&mut self, cmd: &str) -> Result { - match self.pty.as_mut() { - Some(pty) => { - // Send command - let command: String = match cmd.ends_with("\n") { - true => String::from(cmd), - false => format!("{}\n", cmd), + match self.session.as_mut() { + Some(session) => { + // Create channel + let mut channel: Channel = match session.channel_session() { + Ok(ch) => ch, + Err(err) => { + return Err(FileTransferError::new_ex( + FileTransferErrorType::ProtocolError, + format!("Could not open channel: {}", err), + )) + } }; - if let Err(err) = pty.write(command.as_str().as_bytes()) { + // Execute command + if let Err(err) = channel.exec(cmd) { return Err(FileTransferError::new_ex( - FileTransferErrorType::BadAddress, - format!("{}", err), + FileTransferErrorType::ProtocolError, + format!("Could not execute command \"{}\": {}", cmd, err), )); } - // Read + // Read output let mut output: String = String::new(); - let mut buffer: [u8; 8192] = [0; 8192]; - loop { - match pty.read(&mut buffer) { - Ok(bytes_read) => { - if bytes_read == 0 { - break; - } - output.push_str(std::str::from_utf8(&buffer[0..bytes_read]).unwrap()); - } - Err(err) => { - return Err(FileTransferError::new_ex( - FileTransferErrorType::BadAddress, - format!("{}", err), - )) - } + match channel.read_to_string(&mut output) { + Ok(_) => { + // Wait close + let _ = channel.wait_close(); + Ok(output) } + Err(err) => Err(FileTransferError::new_ex( + FileTransferErrorType::ProtocolError, + format!("Could not read output: {}", err), + )), } - Ok(output) } None => Err(FileTransferError::new( FileTransferErrorType::UninitializedSession, @@ -316,70 +326,23 @@ impl FileTransfer for ScpFileTransfer { format!("{}", err), )); } - // Try authentication - if let Err(err) = session.userauth_password( - username.unwrap_or(String::from("")).as_str(), - password.unwrap_or(String::from("")).as_str(), - ) { - return Err(FileTransferError::new_ex( - FileTransferErrorType::AuthenticationFailed, - format!("{}", err), - )); - } - // Request pty - let mut mode = ssh2::PtyModes::new(); - mode.set_character(ssh2::PtyModeOpcode::VINTR, Some(3 as char)); - let mut channel: Channel = match session.channel_session() { - Ok(ch) => ch, - Err(err) => { - return Err(FileTransferError::new_ex( - FileTransferErrorType::ProtocolError, - format!("Could not get channel: {}", err), - )) - } + let username: String = match username { + Some(u) => u.clone(), + None => String::from(""), }; - // Configure channel - if let Err(err) = channel.request_pty("vt100", Some(mode), None) { - return Err(FileTransferError::new_ex( - FileTransferErrorType::ProtocolError, - format!("Could not get pty: {}", err), - )); - } - // Start shell - if let Err(err) = channel.shell() { - return Err(FileTransferError::new_ex( - FileTransferErrorType::ProtocolError, - format!("Could not start shell: {}", err), - )); - } - // Set blocking to true - session.set_blocking(true); - // @! Prevent any strange shell such as fish (which I use actually); go for a basic sh - if let Err(err) = self.perform_shell_cmd("sh") { - return Err(FileTransferError::new_ex( - FileTransferErrorType::ProtocolError, - format!("Could not run sh: {}", err), - )); - } - // @! Remove prompt line (for outputs) - if let Err(err) = self.perform_shell_cmd("unset PS1") { - return Err(FileTransferError::new_ex( - FileTransferErrorType::ProtocolError, - format!("Could not unset PS1: {}", err), - )); - } - // Get working directory - /* - self.wrkdir = match self.perform_shell_cmd("realpath .") { - Ok(output) => PathBuf::from(output.as_str().trim()), - Err(err) => { + // Try authenticating with user agent + if let Err(_) = session.userauth_agent(username.as_str()) { + // Try authentication with password then + if let Err(err) = session.userauth_password( + username.as_str(), + password.unwrap_or(String::from("")).as_str(), + ) { return Err(FileTransferError::new_ex( - FileTransferErrorType::ProtocolError, - format!("Could not get wrkdir: {}", err), - )) + FileTransferErrorType::AuthenticationFailed, + format!("{}", err), + )); } - }; - */ + } // Get banner let banner: Option = match session.banner() { Some(s) => Some(String::from(s)), @@ -387,8 +350,11 @@ impl FileTransfer for ScpFileTransfer { }; // Set session self.session = Some(session); - // Set pty channel - self.pty = Some(channel); + // Get working directory + match self.perform_shell_cmd("pwd") { + Ok(output) => self.wrkdir = PathBuf::from(output.as_str().trim()), + Err(err) => return Err(err), + } Ok(banner) } @@ -396,18 +362,13 @@ impl FileTransfer for ScpFileTransfer { /// /// Disconnect from the remote server fn disconnect(&mut self) -> Result<(), FileTransferError> { - if self.pty.is_some() { - // Send exit, but don't give a f about the output - let _ = self.perform_shell_cmd("exit 0"); - } match self.session.as_ref() { Some(session) => { // Disconnect (greet server with 'Mandi' as they do in Friuli) match session.disconnect(None, "Mandi!", None) { Ok(()) => { - // Set session and sftp to none + // Set session to none self.session = None; - self.pty = None; Ok(()) } Err(err) => Err(FileTransferError::new_ex( @@ -426,8 +387,8 @@ impl FileTransfer for ScpFileTransfer { /// /// Indicates whether the client is connected to remote fn is_connected(&self) -> bool { - match self.pty.as_ref() { - Some(pty) => pty.eof(), + match self.session.as_ref() { + Some(_) => true, None => false, } } @@ -438,13 +399,7 @@ impl FileTransfer for ScpFileTransfer { fn pwd(&mut self) -> Result { match self.is_connected() { - true => match self.perform_shell_cmd("pwd") { - Ok(wrkdir) => Ok(PathBuf::from(wrkdir.as_str().trim())), - Err(err) => Err(FileTransferError::new_ex( - FileTransferErrorType::ProtocolError, - format!("{}", err), - )), - }, + true => Ok(self.wrkdir.clone()), false => Err(FileTransferError::new( FileTransferErrorType::UninitializedSession, )), @@ -458,13 +413,30 @@ impl FileTransfer for ScpFileTransfer { fn change_dir(&mut self, dir: &Path) -> Result { match self.is_connected() { true => { + let p: PathBuf = self.wrkdir.clone(); + let remote_path: PathBuf = match dir.is_absolute() { + true => PathBuf::from(dir), + false => { + let mut p: PathBuf = PathBuf::from("."); + p.push(dir); + p + } + }; // Change directory - match self.perform_shell_cmd(format!("cd \"{}\" && echo 0", dir.display()).as_str()) - { + match self.perform_shell_cmd_with_path( + p.as_path(), + format!("cd \"{}\"; echo $?; pwd", remote_path.display()).as_str(), + ) { Ok(output) => { - // Check if output is 0 - match output.as_str().trim() == "0" { - true => self.pwd(), // Return working directory + // Trim + let output: String = String::from(output.as_str().trim()); + // Check if output starts with 0; should be 0{PWD} + match output.as_str().starts_with("0") { + true => { + // Set working directory + self.wrkdir = PathBuf::from(&output.as_str()[1..].trim()); + Ok(self.wrkdir.clone()) + } false => Err(FileTransferError::new_ex( // No such file or directory FileTransferErrorType::NoSuchFileOrDirectory, @@ -492,12 +464,16 @@ impl FileTransfer for ScpFileTransfer { match self.is_connected() { true => { // Send ls -l to path - match self.perform_shell_cmd(format!("ls -l \"{}\"", path.display()).as_str()) { + let p: PathBuf = self.wrkdir.clone(); + match self.perform_shell_cmd_with_path( + p.as_path(), + format!("ls -l \"{}\"", path.display()).as_str(), + ) { Ok(output) => { // Split output by (\r)\n let lines: Vec<&str> = output.as_str().lines().collect(); let mut entries: Vec = Vec::with_capacity(lines.len()); - for line in lines[1..].iter() { + for line in lines.iter() { // First line must always be ignored // Parse row, if ok push to entries if let Ok(entry) = self.parse_ls_output(path, line) { @@ -525,10 +501,12 @@ impl FileTransfer for ScpFileTransfer { fn mkdir(&mut self, dir: &Path) -> Result<(), FileTransferError> { match self.is_connected() { true => { + let p: PathBuf = self.wrkdir.clone(); // Mkdir dir && echo 0 - match self - .perform_shell_cmd(format!("mkdir \"{}\" && echo 0", dir.display()).as_str()) - { + match self.perform_shell_cmd_with_path( + p.as_path(), + format!("mkdir \"{}\"; echo $?", dir.display()).as_str(), + ) { Ok(output) => { // Check if output is 0 match output.as_str().trim() == "0" { @@ -564,9 +542,11 @@ impl FileTransfer for ScpFileTransfer { FsEntry::Directory(dir) => dir.abs_path.clone(), FsEntry::File(file) => file.abs_path.clone(), }; - match self - .perform_shell_cmd(format!("rm -rf \"{}\" && echo 0", path.display()).as_str()) - { + let p: PathBuf = self.wrkdir.clone(); + match self.perform_shell_cmd_with_path( + p.as_path(), + format!("rm -rf \"{}\"; echo $?", path.display()).as_str(), + ) { Ok(output) => { // Check if output is 0 match output.as_str().trim() == "0" { @@ -601,8 +581,10 @@ impl FileTransfer for ScpFileTransfer { FsEntry::Directory(dir) => dir.abs_path.clone(), FsEntry::File(file) => file.abs_path.clone(), }; - match self.perform_shell_cmd( - format!("mv -f \"{}\" {}\" && echo 0", path.display(), dst.display()).as_str(), + let p: PathBuf = self.wrkdir.clone(); + match self.perform_shell_cmd_with_path( + p.as_path(), + format!("mv -f \"{}\" {}\"; echo $?", path.display(), dst.display()).as_str(), ) { Ok(output) => { // Check if output is 0 @@ -639,7 +621,11 @@ impl FileTransfer for ScpFileTransfer { } match self.is_connected() { true => { - match self.perform_shell_cmd(format!("ls -l \"{}\"", path.display()).as_str()) { + let p: PathBuf = self.wrkdir.clone(); + match self.perform_shell_cmd_with_path( + p.as_path(), + format!("ls -l \"{}\"", path.display()).as_str(), + ) { Ok(line) => { // Parse ls line let parent: PathBuf = match path.parent() { @@ -683,6 +669,8 @@ impl FileTransfer for ScpFileTransfer { ) -> Result, FileTransferError> { match self.session.as_ref() { Some(session) => { + // Set blocking to true + session.set_blocking(true); // Calculate file mode let mode: i32 = match local.unix_pex { None => 0o644, @@ -726,13 +714,17 @@ impl FileTransfer for ScpFileTransfer { /// Returns file and its size fn recv_file(&mut self, file: &FsFile) -> Result, FileTransferError> { match self.session.as_ref() { - Some(session) => match session.scp_recv(file.abs_path.as_path()) { - Ok(reader) => Ok(Box::new(reader.0)), - Err(err) => Err(FileTransferError::new_ex( - FileTransferErrorType::ProtocolError, - format!("{}", err), - )), - }, + Some(session) => { + // Set blocking to true + session.set_blocking(true); + match session.scp_recv(file.abs_path.as_path()) { + Ok(reader) => Ok(Box::new(reader.0)), + Err(err) => Err(FileTransferError::new_ex( + FileTransferErrorType::ProtocolError, + format!("{}", err), + )), + } + } None => Err(FileTransferError::new( FileTransferErrorType::UninitializedSession, )), @@ -747,7 +739,10 @@ impl FileTransfer for ScpFileTransfer { /// This is necessary for some protocols such as FTP. /// You must call this method each time you want to finalize the write of the remote file. fn on_sent(&mut self, _writable: Box) -> Result<(), FileTransferError> { - // Nothing to do + if let Some(session) = self.session.as_ref() { + // Set blocking to false + session.set_blocking(false); + } Ok(()) } @@ -759,7 +754,10 @@ impl FileTransfer for ScpFileTransfer { /// This mighe be necessary for some protocols. /// You must call this method each time you want to finalize the read of the remote file. fn on_recv(&mut self, _readable: Box) -> Result<(), FileTransferError> { - // Nothing to do + if let Some(session) = self.session.as_ref() { + // Set blocking to false + session.set_blocking(false); + } Ok(()) } } @@ -770,15 +768,14 @@ mod tests { use super::*; #[test] - fn test_filetransfer_sftp_new() { + fn test_filetransfer_scp_new() { let client: ScpFileTransfer = ScpFileTransfer::new(); assert!(client.session.is_none()); - assert!(client.pty.is_none()); assert_eq!(client.is_connected(), false); } #[test] - fn test_filetransfer_sftp_connect() { + fn test_filetransfer_scp_connect() { let mut client: ScpFileTransfer = ScpFileTransfer::new(); assert_eq!(client.is_connected(), false); assert!(client @@ -789,17 +786,15 @@ mod tests { Some(String::from("password")) ) .is_ok()); - // Check session and sftp + // Check session and scp assert!(client.session.is_some()); - assert!(client.pty.is_some()); assert_eq!(client.is_connected(), true); // Disconnect assert!(client.disconnect().is_ok()); assert_eq!(client.is_connected(), false); } - #[test] - fn test_filetransfer_sftp_bad_auth() { + fn test_filetransfer_scp_bad_auth() { let mut client: ScpFileTransfer = ScpFileTransfer::new(); assert!(client .connect( @@ -812,7 +807,7 @@ mod tests { } #[test] - fn test_filetransfer_sftp_no_credentials() { + fn test_filetransfer_scp_no_credentials() { let mut client: ScpFileTransfer = ScpFileTransfer::new(); assert!(client .connect(String::from("test.rebex.net"), 22, None, None) @@ -820,7 +815,7 @@ mod tests { } #[test] - fn test_filetransfer_sftp_bad_server() { + fn test_filetransfer_scp_bad_server() { let mut client: ScpFileTransfer = ScpFileTransfer::new(); assert!(client .connect( @@ -831,9 +826,8 @@ mod tests { ) .is_err()); } - #[test] - fn test_filetransfer_sftp_pwd() { + fn test_filetransfer_scp_pwd() { let mut client: ScpFileTransfer = ScpFileTransfer::new(); assert!(client .connect( @@ -843,9 +837,8 @@ mod tests { Some(String::from("password")) ) .is_ok()); - // Check session and sftp + // Check session and scp assert!(client.session.is_some()); - assert!(client.pty.is_some()); // Pwd assert_eq!(client.pwd().ok().unwrap(), PathBuf::from("/")); // Disconnect @@ -853,7 +846,7 @@ mod tests { } #[test] - fn test_filetransfer_sftp_cwd() { + fn test_filetransfer_scp_cwd() { let mut client: ScpFileTransfer = ScpFileTransfer::new(); assert!(client .connect( @@ -863,19 +856,18 @@ mod tests { Some(String::from("password")) ) .is_ok()); - // Check session and sftp + // Check session and scp assert!(client.session.is_some()); - assert!(client.pty.is_some()); // Cwd (relative) assert!(client.change_dir(PathBuf::from("pub/").as_path()).is_ok()); // Cwd (absolute) - assert!(client.change_dir(PathBuf::from("/").as_path()).is_ok()); + assert!(client.change_dir(PathBuf::from("/pub").as_path()).is_ok()); // Disconnect assert!(client.disconnect().is_ok()); } #[test] - fn test_filetransfer_sftp_cwd_error() { + fn test_filetransfer_scp_cwd_error() { let mut client: ScpFileTransfer = ScpFileTransfer::new(); assert!(client .connect( @@ -898,7 +890,7 @@ mod tests { } #[test] - fn test_filetransfer_sftp_ls() { + fn test_filetransfer_scp_ls() { let mut client: ScpFileTransfer = ScpFileTransfer::new(); assert!(client .connect( @@ -908,19 +900,18 @@ mod tests { Some(String::from("password")) ) .is_ok()); - // Check session and sftp + // Check session and scp assert!(client.session.is_some()); - assert!(client.pty.is_some()); // List dir let pwd: PathBuf = client.pwd().ok().unwrap(); let files: Vec = client.list_dir(pwd.as_path()).ok().unwrap(); - assert_eq!(files.len(), 3); // There are 3 files + assert_eq!(files.len(), 5); // There are 5 files // Disconnect assert!(client.disconnect().is_ok()); } #[test] - fn test_filetransfer_sftp_stat() { + fn test_filetransfer_scp_stat() { let mut client: ScpFileTransfer = ScpFileTransfer::new(); assert!(client .connect( @@ -930,9 +921,8 @@ mod tests { Some(String::from("password")) ) .is_ok()); - // Check session and sftp + // Check session and scp assert!(client.session.is_some()); - assert!(client.pty.is_some()); let file: FsEntry = client .stat(PathBuf::from("readme.txt").as_path()) .ok() @@ -945,7 +935,7 @@ mod tests { } #[test] - fn test_filetransfer_sftp_recv() { + fn test_filetransfer_scp_recv() { let mut client: ScpFileTransfer = ScpFileTransfer::new(); assert!(client .connect( @@ -955,9 +945,8 @@ mod tests { Some(String::from("password")) ) .is_ok()); - // Check session and sftp + // Check session and scp assert!(client.session.is_some()); - assert!(client.pty.is_some()); let file: FsFile = FsFile { name: String::from("readme.txt"), abs_path: PathBuf::from("/readme.txt"), @@ -978,7 +967,7 @@ mod tests { assert!(client.disconnect().is_ok()); } #[test] - fn test_filetransfer_sftp_recv_failed_nosuchfile() { + fn test_filetransfer_scp_recv_failed_nosuchfile() { let mut client: ScpFileTransfer = ScpFileTransfer::new(); assert!(client .connect( @@ -988,9 +977,8 @@ mod tests { Some(String::from("password")) ) .is_ok()); - // Check session and sftp + // Check session and scp assert!(client.session.is_some()); - assert!(client.pty.is_some()); // Receive file let file: FsFile = FsFile { name: String::from("omar.txt"), @@ -1010,12 +998,11 @@ mod tests { // Disconnect assert!(client.disconnect().is_ok()); } - - // NOTE: other functions doesn't work with this test SFTP server + // NOTE: other functions doesn't work with this test scp server /* NOTE: the server doesn't allow you to create directories #[test] - fn test_filetransfer_sftp_mkdir() { + fn test_filetransfer_scp_mkdir() { let mut client: ScpFileTransfer = ScpFileTransfer::new(); assert!(client.connect(String::from("test.rebex.net"), 22, Some(String::from("demo")), Some(String::from("password"))).is_ok()); let dir: String = String::from("foo");