diff --git a/Cargo.lock b/Cargo.lock index 7b7ffde83d8072556207195c0d86a3e3e093c9c7..f4f5a49232218d54b9a24f712129c673610fe982 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -808,7 +808,6 @@ dependencies = [ "gpui", "log", "net", - "proto", "smol", "tempfile", "windows 0.61.1", diff --git a/crates/askpass/Cargo.toml b/crates/askpass/Cargo.toml index 5c7367b9dbf0fe72fb97b0904bd6570f93270299..6aec7e6d7e011c626a57c478fa65e161f43b2bdd 100644 --- a/crates/askpass/Cargo.toml +++ b/crates/askpass/Cargo.toml @@ -16,7 +16,6 @@ anyhow.workspace = true futures.workspace = true gpui.workspace = true net.workspace = true -proto.workspace = true smol.workspace = true log.workspace = true tempfile.workspace = true diff --git a/crates/askpass/src/askpass.rs b/crates/askpass/src/askpass.rs index 2dfdd792d94b67d2b6e39305b804fcbf1957f41e..dfe8a96ee6f19510df06948f94af48d621515747 100644 --- a/crates/askpass/src/askpass.rs +++ b/crates/askpass/src/askpass.rs @@ -1,10 +1,16 @@ mod encrypted_password; -pub use encrypted_password::{EncryptedPassword, ProcessExt}; -use util::paths::PathExt; +pub use encrypted_password::{EncryptedPassword, IKnowWhatIAmDoingAndIHaveReadTheDocs}; +use net::async_net::UnixListener; +use smol::lock::Mutex; +use util::fs::make_file_executable; + +use std::ffi::OsStr; +use std::ops::ControlFlow; +use std::sync::Arc; use std::sync::OnceLock; -use std::{ffi::OsStr, time::Duration}; +use std::time::Duration; use anyhow::{Context as _, Result}; use futures::channel::{mpsc, oneshot}; @@ -14,9 +20,7 @@ use futures::{ }; use gpui::{AsyncApp, BackgroundExecutor, Task}; use smol::fs; -use util::{ResultExt as _, debug_panic}; - -use crate::encrypted_password::decrypt; +use util::{ResultExt as _, debug_panic, maybe, paths::PathExt}; /// Path to the program used for askpass /// @@ -32,6 +36,7 @@ pub enum AskPassResult { pub struct AskPassDelegate { tx: mpsc::UnboundedSender<(String, oneshot::Sender)>, + executor: BackgroundExecutor, _task: Task<()>, } @@ -49,24 +54,27 @@ impl AskPassDelegate { password_prompt(prompt, channel, cx); } }); - Self { tx, _task: task } + Self { + tx, + _task: task, + executor: cx.background_executor().clone(), + } } - pub async fn ask_password(&mut self, prompt: String) -> Option { - let (tx, rx) = oneshot::channel(); - self.tx.send((prompt, tx)).await.ok()?; - rx.await.ok() + pub fn ask_password(&mut self, prompt: String) -> Task> { + let mut this_tx = self.tx.clone(); + self.executor.spawn(async move { + let (tx, rx) = oneshot::channel(); + this_tx.send((prompt, tx)).await.ok()?; + rx.await.ok() + }) } } pub struct AskPassSession { - #[cfg(not(target_os = "windows"))] - script_path: std::path::PathBuf, - #[cfg(target_os = "windows")] - askpass_helper: String, #[cfg(target_os = "windows")] secret: std::sync::Arc>, - _askpass_task: Task<()>, + askpass_task: PasswordProxy, askpass_opened_rx: Option>, askpass_kill_master_rx: Option>, } @@ -81,103 +89,57 @@ impl AskPassSession { /// You must retain this session until the master process exits. #[must_use] pub async fn new(executor: &BackgroundExecutor, mut delegate: AskPassDelegate) -> Result { - use net::async_net::UnixListener; - use util::fs::make_file_executable; - #[cfg(target_os = "windows")] let secret = std::sync::Arc::new(OnceLock::new()); - let temp_dir = tempfile::Builder::new().prefix("zed-askpass").tempdir()?; - let askpass_socket = temp_dir.path().join("askpass.sock"); - let askpass_script_path = temp_dir.path().join(ASKPASS_SCRIPT_NAME); let (askpass_opened_tx, askpass_opened_rx) = oneshot::channel::<()>(); - let listener = UnixListener::bind(&askpass_socket).context("creating askpass socket")?; - - let current_exec = - std::env::current_exe().context("Failed to determine current zed executable path.")?; - let askpass_program = ASKPASS_PROGRAM - .get_or_init(|| current_exec) - .try_shell_safe() - .context("Failed to shell-escape Askpass program path.")? - .to_string(); + let askpass_opened_tx = Arc::new(Mutex::new(Some(askpass_opened_tx))); let (askpass_kill_master_tx, askpass_kill_master_rx) = oneshot::channel::<()>(); - let mut kill_tx = Some(askpass_kill_master_tx); + let kill_tx = Arc::new(Mutex::new(Some(askpass_kill_master_tx))); #[cfg(target_os = "windows")] let askpass_secret = secret.clone(); - let askpass_task = executor.spawn(async move { - let mut askpass_opened_tx = Some(askpass_opened_tx); - - while let Ok((mut stream, _)) = listener.accept().await { - if let Some(askpass_opened_tx) = askpass_opened_tx.take() { - askpass_opened_tx.send(()).ok(); - } - let mut buffer = Vec::new(); - let mut reader = BufReader::new(&mut stream); - if reader.read_until(b'\0', &mut buffer).await.is_err() { - buffer.clear(); - } - let prompt = String::from_utf8_lossy(&buffer); - if let Some(password) = delegate.ask_password(prompt.into_owned()).await { - #[cfg(target_os = "windows")] - { - askpass_secret.get_or_init(|| password.clone()); - } - if let Ok(decrypted) = decrypt(password) { - stream.write_all(decrypted.as_bytes()).await.log_err(); + let get_password = { + let executor = executor.clone(); + + move |prompt| { + let prompt = delegate.ask_password(prompt); + let kill_tx = kill_tx.clone(); + let askpass_opened_tx = askpass_opened_tx.clone(); + #[cfg(target_os = "windows")] + let askpass_secret = askpass_secret.clone(); + executor.spawn(async move { + if let Some(askpass_opened_tx) = askpass_opened_tx.lock().await.take() { + askpass_opened_tx.send(()).ok(); } - } else { - if let Some(kill_tx) = kill_tx.take() { - kill_tx.send(()).log_err(); + if let Some(password) = prompt.await { + #[cfg(target_os = "windows")] + { + _ = askpass_secret.set(password.clone()); + } + ControlFlow::Continue(Ok(password)) + } else { + if let Some(kill_tx) = kill_tx.lock().await.take() { + kill_tx.send(()).log_err(); + } + ControlFlow::Break(()) } - // note: we expect the caller to drop this task when it's done. - // We need to keep the stream open until the caller is done to avoid - // spurious errors from ssh. - std::future::pending::<()>().await; - drop(stream); - } + }) } - drop(temp_dir) - }); - - // Create an askpass script that communicates back to this process. - let askpass_script = generate_askpass_script(&askpass_program, &askpass_socket); - fs::write(&askpass_script_path, askpass_script) - .await - .with_context(|| format!("creating askpass script at {askpass_script_path:?}"))?; - make_file_executable(&askpass_script_path).await?; - #[cfg(target_os = "windows")] - let askpass_helper = format!( - "powershell.exe -ExecutionPolicy Bypass -File {}", - askpass_script_path.display() - ); + }; + let askpass_task = PasswordProxy::new(get_password, executor.clone()).await?; Ok(Self { - #[cfg(not(target_os = "windows"))] - script_path: askpass_script_path, - #[cfg(target_os = "windows")] secret, - #[cfg(target_os = "windows")] - askpass_helper, - _askpass_task: askpass_task, + askpass_task, askpass_kill_master_rx: Some(askpass_kill_master_rx), askpass_opened_rx: Some(askpass_opened_rx), }) } - #[cfg(not(target_os = "windows"))] - pub fn script_path(&self) -> impl AsRef { - &self.script_path - } - - #[cfg(target_os = "windows")] - pub fn script_path(&self) -> impl AsRef { - &self.askpass_helper - } - // This will run the askpass task forever, resolving as many authentication requests as needed. // The caller is responsible for examining the result of their own commands and cancelling this // future when this is no longer needed. Note that this can only be called once, but due to the @@ -209,8 +171,109 @@ impl AskPassSession { pub fn get_password(&self) -> Option { self.secret.get().cloned() } + + pub fn script_path(&self) -> impl AsRef { + self.askpass_task.script_path() + } +} + +pub struct PasswordProxy { + _task: Task<()>, + #[cfg(not(target_os = "windows"))] + askpass_script_path: std::path::PathBuf, + #[cfg(target_os = "windows")] + askpass_helper: String, } +impl PasswordProxy { + pub async fn new( + mut get_password: impl FnMut(String) -> Task>> + + 'static + + Send + + Sync, + executor: BackgroundExecutor, + ) -> Result { + let temp_dir = tempfile::Builder::new().prefix("zed-askpass").tempdir()?; + let askpass_socket = temp_dir.path().join("askpass.sock"); + let askpass_script_path = temp_dir.path().join(ASKPASS_SCRIPT_NAME); + let current_exec = + std::env::current_exe().context("Failed to determine current zed executable path.")?; + + let askpass_program = ASKPASS_PROGRAM + .get_or_init(|| current_exec) + .try_shell_safe() + .context("Failed to shell-escape Askpass program path.")? + .to_string(); + // Create an askpass script that communicates back to this process. + let askpass_script = generate_askpass_script(&askpass_program, &askpass_socket); + let _task = executor.spawn(async move { + maybe!(async move { + let listener = + UnixListener::bind(&askpass_socket).context("creating askpass socket")?; + + while let Ok((mut stream, _)) = listener.accept().await { + let mut buffer = Vec::new(); + let mut reader = BufReader::new(&mut stream); + if reader.read_until(b'\0', &mut buffer).await.is_err() { + buffer.clear(); + } + let prompt = String::from_utf8_lossy(&buffer).into_owned(); + let password = get_password(prompt).await; + match password { + ControlFlow::Continue(password) => { + if let Ok(password) = password + && let Ok(decrypted) = + password.decrypt(IKnowWhatIAmDoingAndIHaveReadTheDocs) + { + stream.write_all(decrypted.as_bytes()).await.log_err(); + } + } + ControlFlow::Break(()) => { + // note: we expect the caller to drop this task when it's done. + // We need to keep the stream open until the caller is done to avoid + // spurious errors from ssh. + std::future::pending::<()>().await; + drop(stream); + } + } + } + drop(temp_dir); + Result::<_, anyhow::Error>::Ok(()) + }) + .await + .log_err(); + }); + + fs::write(&askpass_script_path, askpass_script) + .await + .with_context(|| format!("creating askpass script at {askpass_script_path:?}"))?; + make_file_executable(&askpass_script_path).await?; + #[cfg(target_os = "windows")] + let askpass_helper = format!( + "powershell.exe -ExecutionPolicy Bypass -File {}", + askpass_script_path.display() + ); + + Ok(Self { + _task, + #[cfg(not(target_os = "windows"))] + askpass_script_path, + #[cfg(target_os = "windows")] + askpass_helper, + }) + } + + pub fn script_path(&self) -> impl AsRef { + #[cfg(not(target_os = "windows"))] + { + &self.askpass_script_path + } + #[cfg(target_os = "windows")] + { + &self.askpass_helper + } + } +} /// The main function for when Zed is running in netcat mode for use in askpass. /// Called from both the remote server binary and the zed binary in their respective main functions. pub fn main(socket: &str) { diff --git a/crates/askpass/src/encrypted_password.rs b/crates/askpass/src/encrypted_password.rs index b5495df6e43cab8efe7a3e505602df11d03a1092..b5d54c18a1c608fa21a9a16c4e27c04530950c3a 100644 --- a/crates/askpass/src/encrypted_password.rs +++ b/crates/askpass/src/encrypted_password.rs @@ -21,27 +21,6 @@ type LengthWithoutPadding = u32; #[derive(Clone)] pub struct EncryptedPassword(Vec, LengthWithoutPadding); -pub trait ProcessExt { - fn encrypted_env(&mut self, name: &str, value: EncryptedPassword) -> &mut Self; -} - -impl ProcessExt for smol::process::Command { - fn encrypted_env(&mut self, name: &str, value: EncryptedPassword) -> &mut Self { - if let Ok(password) = decrypt(value) { - self.env(name, password); - } - self - } -} - -impl TryFrom for proto::AskPassResponse { - type Error = anyhow::Error; - fn try_from(pw: EncryptedPassword) -> Result { - let pw = decrypt(pw)?; - Ok(Self { response: pw }) - } -} - impl Drop for EncryptedPassword { fn drop(&mut self) { self.0.zeroize(); @@ -79,38 +58,45 @@ impl TryFrom<&str> for EncryptedPassword { } } -pub(crate) fn decrypt(mut password: EncryptedPassword) -> Result { - #[cfg(windows)] - { - use anyhow::Context; - use windows::Win32::Security::Cryptography::{ - CRYPTPROTECTMEMORY_BLOCK_SIZE, CRYPTPROTECTMEMORY_SAME_PROCESS, CryptUnprotectMemory, - }; - assert_eq!( - password.0.len() % CRYPTPROTECTMEMORY_BLOCK_SIZE as usize, - 0, - "Violated pre-condition (buffer size <{}> must be a multiple of CRYPTPROTECTMEMORY_BLOCK_SIZE <{}>) for CryptUnprotectMemory.", - password.0.len(), - CRYPTPROTECTMEMORY_BLOCK_SIZE - ); - if password.1 != 0 { - unsafe { - CryptUnprotectMemory( - password.0.as_mut_ptr() as _, - password.0.len().try_into()?, - CRYPTPROTECTMEMORY_SAME_PROCESS, - ) - .context("while decrypting a SSH password")? +/// Read the docs for [EncryptedPassword]; please take care of not storing the plaintext string in memory for extended +/// periods of time. +pub struct IKnowWhatIAmDoingAndIHaveReadTheDocs; + +impl EncryptedPassword { + pub fn decrypt(mut self, _: IKnowWhatIAmDoingAndIHaveReadTheDocs) -> Result { + #[cfg(windows)] + { + use anyhow::Context; + use windows::Win32::Security::Cryptography::{ + CRYPTPROTECTMEMORY_BLOCK_SIZE, CRYPTPROTECTMEMORY_SAME_PROCESS, + CryptUnprotectMemory, }; + assert_eq!( + self.0.len() % CRYPTPROTECTMEMORY_BLOCK_SIZE as usize, + 0, + "Violated pre-condition (buffer size <{}> must be a multiple of CRYPTPROTECTMEMORY_BLOCK_SIZE <{}>) for CryptUnprotectMemory.", + self.0.len(), + CRYPTPROTECTMEMORY_BLOCK_SIZE + ); + if self.1 != 0 { + unsafe { + CryptUnprotectMemory( + self.0.as_mut_ptr() as _, + self.0.len().try_into()?, + CRYPTPROTECTMEMORY_SAME_PROCESS, + ) + .context("while decrypting a SSH password")? + }; - { - // Remove padding - _ = password.0.drain(password.1 as usize..); + { + // Remove padding + _ = self.0.drain(self.1 as usize..); + } } - } - Ok(String::from_utf8(std::mem::take(&mut password.0))?) + Ok(String::from_utf8(std::mem::take(&mut self.0))?) + } + #[cfg(not(windows))] + Ok(String::from_utf8(std::mem::take(&mut self.0))?) } - #[cfg(not(windows))] - Ok(String::from_utf8(std::mem::take(&mut password.0))?) } diff --git a/crates/project/src/git_store.rs b/crates/project/src/git_store.rs index eb0ae8e5d53b943b2e1a81c5a7b720b2b61fa550..4a31d5810a57bc25d388533e3f4905156e2b8522 100644 --- a/crates/project/src/git_store.rs +++ b/crates/project/src/git_store.rs @@ -7,7 +7,7 @@ use crate::{ worktree_store::{WorktreeStore, WorktreeStoreEvent}, }; use anyhow::{Context as _, Result, anyhow, bail}; -use askpass::{AskPassDelegate, EncryptedPassword}; +use askpass::{AskPassDelegate, EncryptedPassword, IKnowWhatIAmDoingAndIHaveReadTheDocs}; use buffer_diff::{BufferDiff, BufferDiffEvent}; use client::ProjectId; use collections::HashMap; @@ -2120,7 +2120,10 @@ impl GitStore { .lock() .insert(envelope.payload.askpass_id, askpass); - response.try_into() + // In fact, we don't quite know what we're doing here, as we're sending askpass password unencrypted, but.. + Ok(proto::AskPassResponse { + response: response.decrypt(IKnowWhatIAmDoingAndIHaveReadTheDocs)?, + }) } async fn handle_check_for_pushed_commits( diff --git a/crates/remote/src/transport/ssh.rs b/crates/remote/src/transport/ssh.rs index 4f806e3556c1b458b420cfefcd03f3585fb222be..cb95aaea507671ec655f731fc48b6d79a856e17e 100644 --- a/crates/remote/src/transport/ssh.rs +++ b/crates/remote/src/transport/ssh.rs @@ -70,14 +70,13 @@ impl From for SshConnectionOptions { } } -#[derive(Clone)] struct SshSocket { connection_options: SshConnectionOptions, #[cfg(not(target_os = "windows"))] - socket_path: PathBuf, + socket_path: std::path::PathBuf, envs: HashMap, #[cfg(target_os = "windows")] - password: askpass::EncryptedPassword, + _proxy: askpass::PasswordProxy, } macro_rules! shell_script { @@ -343,16 +342,17 @@ impl SshRemoteConnection { } #[cfg(not(target_os = "windows"))] - let socket = SshSocket::new(connection_options, socket_path)?; + let socket = SshSocket::new(connection_options, socket_path).await?; #[cfg(target_os = "windows")] let socket = SshSocket::new( connection_options, - &temp_dir, askpass .get_password() .or_else(|| askpass::EncryptedPassword::try_from("").ok()) .context("Failed to fetch askpass password")?, - )?; + cx.background_executor().clone(), + ) + .await?; drop(askpass); let ssh_platform = socket.platform().await?; @@ -659,7 +659,7 @@ impl SshRemoteConnection { impl SshSocket { #[cfg(not(target_os = "windows"))] - fn new(options: SshConnectionOptions, socket_path: PathBuf) -> Result { + async fn new(options: SshConnectionOptions, socket_path: PathBuf) -> Result { Ok(Self { connection_options: options, envs: HashMap::default(), @@ -668,21 +668,26 @@ impl SshSocket { } #[cfg(target_os = "windows")] - fn new( + async fn new( options: SshConnectionOptions, - temp_dir: &TempDir, password: askpass::EncryptedPassword, + executor: gpui::BackgroundExecutor, ) -> Result { - let askpass_script = temp_dir.path().join("askpass.bat"); - std::fs::write(&askpass_script, "@ECHO OFF\necho %ZED_SSH_ASKPASS%")?; let mut envs = HashMap::default(); + let get_password = + move |_| Task::ready(std::ops::ControlFlow::Continue(Ok(password.clone()))); + + let _proxy = askpass::PasswordProxy::new(get_password, executor).await?; envs.insert("SSH_ASKPASS_REQUIRE".into(), "force".into()); - envs.insert("SSH_ASKPASS".into(), askpass_script.display().to_string()); + envs.insert( + "SSH_ASKPASS".into(), + _proxy.script_path().as_ref().display().to_string(), + ); Ok(Self { connection_options: options, envs, - password, + _proxy, }) } @@ -736,14 +741,12 @@ impl SshSocket { #[cfg(target_os = "windows")] fn ssh_options<'a>(&self, command: &'a mut process::Command) -> &'a mut process::Command { - use askpass::ProcessExt; command .stdin(Stdio::piped()) .stdout(Stdio::piped()) .stderr(Stdio::piped()) .args(self.connection_options.additional_args()) .envs(self.envs.clone()) - .encrypted_env("ZED_SSH_ASKPASS", self.password.clone()) } // On Windows, we need to use `SSH_ASKPASS` to provide the password to ssh.