diff --git a/apps/desktop/desktop_native/Cargo.lock b/apps/desktop/desktop_native/Cargo.lock index 82264617339..16a5a48cd06 100644 --- a/apps/desktop/desktop_native/Cargo.lock +++ b/apps/desktop/desktop_native/Cargo.lock @@ -307,7 +307,7 @@ checksum = "b048fb63fd8b5923fc5aa7b340d8e156aec7ec02f0c78fa8a6ddc2613f6f71de" [[package]] name = "bitwarden-russh" version = "0.1.0" -source = "git+https://github.com/bitwarden/bitwarden-russh.git?branch=km/pm-10098/clean-russh-implementation#86ff1bf2f4620a3ae5684adee31abdbee33c6f07" +source = "git+https://github.com/bitwarden/bitwarden-russh.git?rev=b4e7f2fedbe3df8c35545feb000176d3e7b2bc32#b4e7f2fedbe3df8c35545feb000176d3e7b2bc32" dependencies = [ "anyhow", "byteorder", diff --git a/apps/desktop/desktop_native/core/Cargo.toml b/apps/desktop/desktop_native/core/Cargo.toml index 6f333c480e6..6abf7fbb0a7 100644 --- a/apps/desktop/desktop_native/core/Cargo.toml +++ b/apps/desktop/desktop_native/core/Cargo.toml @@ -10,22 +10,22 @@ default = ["sys"] manual_test = [] sys = [ - "dep:widestring", - "dep:windows", - "dep:core-foundation", - "dep:security-framework", - "dep:security-framework-sys", - "dep:gio", - "dep:libsecret", - "dep:zbus", - "dep:zbus_polkit", + "dep:widestring", + "dep:windows", + "dep:core-foundation", + "dep:security-framework", + "dep:security-framework-sys", + "dep:gio", + "dep:libsecret", + "dep:zbus", + "dep:zbus_polkit", ] [dependencies] aes = "=0.8.4" anyhow = "=1.0.93" arboard = { version = "=3.4.1", default-features = false, features = [ - "wayland-data-control", + "wayland-data-control", ] } async-stream = "=0.3.6" base64 = "=0.22.1" @@ -50,7 +50,7 @@ ssh-key = { version = "=0.6.7", default-features = false, features = [ "rsa", "getrandom", ] } -bitwarden-russh = { git = "https://github.com/bitwarden/bitwarden-russh.git", branch = "km/pm-10098/clean-russh-implementation" } +bitwarden-russh = { git = "https://github.com/bitwarden/bitwarden-russh.git", rev = "b4e7f2fedbe3df8c35545feb000176d3e7b2bc32" } tokio = { version = "=1.41.1", features = ["io-util", "sync", "macros", "net"] } tokio-stream = { version = "=0.1.15", features = ["net"] } tokio-util = { version = "=0.7.12", features = ["codec"] } diff --git a/apps/desktop/desktop_native/core/src/ssh_agent/mod.rs b/apps/desktop/desktop_native/core/src/ssh_agent/mod.rs index 8cbcd97d91e..9d04ea87ccb 100644 --- a/apps/desktop/desktop_native/core/src/ssh_agent/mod.rs +++ b/apps/desktop/desktop_native/core/src/ssh_agent/mod.rs @@ -17,9 +17,11 @@ pub mod importer; pub struct BitwardenDesktopAgent { keystore: ssh_agent::KeyStore, cancellation_token: CancellationToken, - show_ui_request_tx: tokio::sync::mpsc::Sender<(u32, String)>, + show_ui_request_tx: tokio::sync::mpsc::Sender<(u32, (String, bool))>, get_ui_response_rx: Arc>>, request_id: Arc>, + /// before first unlock, or after account switching, listing keys should require an unlock to get a list of public keys + needs_unlock: Arc>, is_running: Arc>, } @@ -33,8 +35,30 @@ impl ssh_agent::Agent for BitwardenDesktopAgent { let request_id = self.get_request_id().await; let mut rx_channel = self.get_ui_response_rx.lock().await.resubscribe(); + let message = (request_id, (ssh_key.cipher_uuid.clone(), false)); self.show_ui_request_tx - .send((request_id, ssh_key.cipher_uuid.clone())) + .send(message) + .await + .expect("Should send request to ui"); + while let Ok((id, response)) = rx_channel.recv().await { + if id == request_id { + return response; + } + } + false + } + + async fn can_list(&self) -> bool { + if !*self.needs_unlock.lock().await{ + return true; + } + + let request_id = self.get_request_id().await; + + let mut rx_channel = self.get_ui_response_rx.lock().await.resubscribe(); + let message = (request_id, ("".to_string(), true)); + self.show_ui_request_tx + .send(message) .await .expect("Should send request to ui"); while let Ok((id, response)) = rx_channel.recv().await { @@ -75,6 +99,8 @@ impl BitwardenDesktopAgent { let keystore = &mut self.keystore; keystore.0.write().expect("RwLock is not poisoned").clear(); + *self.needs_unlock.blocking_lock() = false; + for (key, name, cipher_id) in new_keys.iter() { match parse_key_safe(&key) { Ok(private_key) => { @@ -119,6 +145,14 @@ impl BitwardenDesktopAgent { Ok(()) } + pub fn clear_keys(&mut self) -> Result<(), anyhow::Error> { + let keystore = &mut self.keystore; + keystore.0.write().expect("RwLock is not poisoned").clear(); + *self.needs_unlock.blocking_lock() = true; + + Ok(()) + } + async fn get_request_id(&self) -> u32 { if !*self.is_running.lock().await { println!("[BitwardenDesktopAgent] Agent is not running, but tried to get request id"); diff --git a/apps/desktop/desktop_native/core/src/ssh_agent/unix.rs b/apps/desktop/desktop_native/core/src/ssh_agent/unix.rs index 27f27998ce9..ed2fe9ffab1 100644 --- a/apps/desktop/desktop_native/core/src/ssh_agent/unix.rs +++ b/apps/desktop/desktop_native/core/src/ssh_agent/unix.rs @@ -14,7 +14,7 @@ use super::BitwardenDesktopAgent; impl BitwardenDesktopAgent { pub async fn start_server( - auth_request_tx: tokio::sync::mpsc::Sender<(u32, String)>, + auth_request_tx: tokio::sync::mpsc::Sender<(u32, (String, bool))>, auth_response_rx: Arc>>, ) -> Result { let agent = BitwardenDesktopAgent { @@ -23,6 +23,7 @@ impl BitwardenDesktopAgent { show_ui_request_tx: auth_request_tx, get_ui_response_rx: auth_response_rx, request_id: Arc::new(tokio::sync::Mutex::new(0)), + needs_unlock: Arc::new(tokio::sync::Mutex::new(true)), is_running: Arc::new(tokio::sync::Mutex::new(false)), }; let cloned_agent_state = agent.clone(); diff --git a/apps/desktop/desktop_native/core/src/ssh_agent/windows.rs b/apps/desktop/desktop_native/core/src/ssh_agent/windows.rs index 9ccae7af675..6a99b7cfb00 100644 --- a/apps/desktop/desktop_native/core/src/ssh_agent/windows.rs +++ b/apps/desktop/desktop_native/core/src/ssh_agent/windows.rs @@ -12,7 +12,7 @@ use super::BitwardenDesktopAgent; impl BitwardenDesktopAgent { pub async fn start_server( - auth_request_tx: tokio::sync::mpsc::Sender<(u32, String)>, + auth_request_tx: tokio::sync::mpsc::Sender<(u32, (String, bool))>, auth_response_rx: Arc>>, ) -> Result { let agent_state = BitwardenDesktopAgent { @@ -21,6 +21,7 @@ impl BitwardenDesktopAgent { get_ui_response_rx: auth_response_rx, cancellation_token: CancellationToken::new(), request_id: Arc::new(tokio::sync::Mutex::new(0)), + needs_unlock: Arc::new(tokio::sync::Mutex::new(true)), is_running: Arc::new(tokio::sync::Mutex::new(true)), }; let stream = named_pipe_listener_stream::NamedPipeServerStream::new( diff --git a/apps/desktop/desktop_native/napi/index.d.ts b/apps/desktop/desktop_native/napi/index.d.ts index 62e448d4800..956b2d726da 100644 --- a/apps/desktop/desktop_native/napi/index.d.ts +++ b/apps/desktop/desktop_native/napi/index.d.ts @@ -69,12 +69,13 @@ export declare namespace sshagent { status: SshKeyImportStatus sshKey?: SshKey } - export function serve(callback: (err: Error | null, arg: string) => any): Promise + export function serve(callback: (err: Error | null, arg0: string, arg1: boolean) => any): Promise export function stop(agentState: SshAgentState): void export function isRunning(agentState: SshAgentState): boolean export function setKeys(agentState: SshAgentState, newKeys: Array): void export function lock(agentState: SshAgentState): void export function importKey(encodedKey: string, password: string): SshKeyImportResult + export function clearKeys(agentState: SshAgentState): void export function generateKeypair(keyAlgorithm: string): Promise export class SshAgentState { } } diff --git a/apps/desktop/desktop_native/napi/src/lib.rs b/apps/desktop/desktop_native/napi/src/lib.rs index 8e156eb3efa..4c7bc8eaa93 100644 --- a/apps/desktop/desktop_native/napi/src/lib.rs +++ b/apps/desktop/desktop_native/napi/src/lib.rs @@ -247,15 +247,15 @@ pub mod sshagent { #[napi] pub async fn serve( - callback: ThreadsafeFunction, + callback: ThreadsafeFunction<(String, bool), CalleeHandled>, ) -> napi::Result { - let (auth_request_tx, mut auth_request_rx) = tokio::sync::mpsc::channel::<(u32, String)>(32); + let (auth_request_tx, mut auth_request_rx) = tokio::sync::mpsc::channel::<(u32, (String, bool))>(32); let (auth_response_tx, auth_response_rx) = tokio::sync::broadcast::channel::<(u32, bool)>(32); let auth_response_tx_arc = Arc::new(Mutex::new(auth_response_tx)); tokio::spawn(async move { let _ = auth_response_rx; - while let Some((request_id, cipher_uuid)) = auth_request_rx.recv().await { + while let Some((request_id, (cipher_uuid, is_list_request))) = auth_request_rx.recv().await { let cloned_request_id = request_id.clone(); let cloned_cipher_uuid = cipher_uuid.clone(); let cloned_response_tx_arc = auth_response_tx_arc.clone(); @@ -266,7 +266,7 @@ pub mod sshagent { let auth_response_tx_arc = cloned_response_tx_arc; let callback = cloned_callback; let promise_result: Result, napi::Error> = - callback.call_async(Ok(cipher_uuid)).await; + callback.call_async(Ok((cipher_uuid, is_list_request))).await; match promise_result { Ok(promise_result) => match promise_result.await { Ok(result) => { @@ -345,6 +345,12 @@ pub mod sshagent { Ok(result.into()) } + #[napi] + pub fn clear_keys(agent_state: &mut SshAgentState) -> napi::Result<()> { + let bitwarden_agent_state = &mut agent_state.state; + bitwarden_agent_state.clear_keys().map_err(|e| napi::Error::from_reason(e.to_string())) + } + #[napi] pub async fn generate_keypair(key_algorithm: String) -> napi::Result { desktop_core::ssh_agent::generator::generate_keypair(key_algorithm) diff --git a/apps/desktop/src/platform/main/main-ssh-agent.service.ts b/apps/desktop/src/platform/main/main-ssh-agent.service.ts index 60487aae4da..5284029c698 100644 --- a/apps/desktop/src/platform/main/main-ssh-agent.service.ts +++ b/apps/desktop/src/platform/main/main-ssh-agent.service.ts @@ -27,7 +27,7 @@ export class MainSshAgentService { init() { // handle sign request passing to UI sshagent - .serve(async (err: Error, cipherId: string) => { + .serve(async (err: Error, cipherId: string, isListRequest: boolean) => { // clear all old (> SIGN_TIMEOUT) requests this.requestResponses = this.requestResponses.filter( (response) => response.timestamp > new Date(Date.now() - this.SIGN_TIMEOUT), @@ -37,6 +37,7 @@ export class MainSshAgentService { const id_for_this_request = this.request_id; this.messagingService.send("sshagent.signrequest", { cipherId, + isListRequest, requestId: id_for_this_request, }); @@ -111,5 +112,11 @@ export class MainSshAgentService { sshagent.lock(this.agentState); } }); + + ipcMain.handle("sshagent.clearkeys", async (event: any) => { + if (this.agentState != null) { + sshagent.clearKeys(this.agentState); + } + }); } } diff --git a/apps/desktop/src/platform/preload.ts b/apps/desktop/src/platform/preload.ts index 171e83bbef0..519ed684320 100644 --- a/apps/desktop/src/platform/preload.ts +++ b/apps/desktop/src/platform/preload.ts @@ -56,6 +56,9 @@ const sshAgent = { lock: async () => { return await ipcRenderer.invoke("sshagent.lock"); }, + clearKeys: async () => { + return await ipcRenderer.invoke("sshagent.clearkeys"); + }, importKey: async (key: string, password: string): Promise => { const res = await ipcRenderer.invoke("sshagent.importkey", { privateKey: key, diff --git a/apps/desktop/src/platform/services/ssh-agent.service.ts b/apps/desktop/src/platform/services/ssh-agent.service.ts index fdd86788ed5..969300fcc5f 100644 --- a/apps/desktop/src/platform/services/ssh-agent.service.ts +++ b/apps/desktop/src/platform/services/ssh-agent.service.ts @@ -5,9 +5,11 @@ import { concatMap, EMPTY, filter, + firstValueFrom, from, map, of, + skip, Subject, switchMap, takeUntil, @@ -17,6 +19,7 @@ import { withLatestFrom, } from "rxjs"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; @@ -52,6 +55,7 @@ export class SshAgentService implements OnDestroy { private i18nService: I18nService, private desktopSettingsService: DesktopSettingsService, private configService: ConfigService, + private accountService: AccountService, ) {} async init() { @@ -112,19 +116,34 @@ export class SshAgentService implements OnDestroy { ), ), // This concatMap handles showing the dialog to approve the request. - concatMap(([message, decryptedCiphers]) => { + concatMap(async ([message, ciphers]) => { const cipherId = message.cipherId as string; + const isListRequest = message.isListRequest as boolean; const requestId = message.requestId as number; - if (decryptedCiphers === undefined) { - return of(false).pipe( - switchMap((result) => - ipc.platform.sshAgent.signRequestResponse(requestId, Boolean(result)), - ), + if (isListRequest) { + const sshCiphers = ciphers.filter( + (cipher) => cipher.type === CipherType.SshKey && !cipher.isDeleted, ); + const keys = sshCiphers.map((cipher) => { + return { + name: cipher.name, + privateKey: cipher.sshKey.privateKey, + cipherId: cipher.id, + }; + }); + await ipc.platform.sshAgent.setKeys(keys); + await ipc.platform.sshAgent.signRequestResponse(requestId, true); + return; + } + + if (ciphers === undefined) { + ipc.platform.sshAgent + .signRequestResponse(requestId, false) + .catch((e) => this.logService.error("Failed to respond to SSH request", e)); } - const cipher = decryptedCiphers.find((cipher) => cipher.id == cipherId); + const cipher = ciphers.find((cipher) => cipher.id == cipherId); ipc.platform.focusWindow(); const dialogRef = ApproveSshRequestComponent.open( @@ -133,16 +152,34 @@ export class SshAgentService implements OnDestroy { this.i18nService.t("unknownApplication"), ); - return dialogRef.closed.pipe( - switchMap((result) => { - return ipc.platform.sshAgent.signRequestResponse(requestId, Boolean(result)); - }), - ); + const result = await firstValueFrom(dialogRef.closed); + return ipc.platform.sshAgent.signRequestResponse(requestId, result); }), takeUntil(this.destroy$), ) .subscribe(); + this.accountService.activeAccount$.pipe(skip(1), takeUntil(this.destroy$)).subscribe({ + next: (account) => { + this.logService.info("Active account changed, clearing SSH keys"); + ipc.platform.sshAgent + .clearKeys() + .catch((e) => this.logService.error("Failed to clear SSH keys", e)); + }, + error: (e: unknown) => { + this.logService.error("Error in active account observable", e); + ipc.platform.sshAgent + .clearKeys() + .catch((e) => this.logService.error("Failed to clear SSH keys", e)); + }, + complete: () => { + this.logService.info("Active account observable completed, clearing SSH keys"); + ipc.platform.sshAgent + .clearKeys() + .catch((e) => this.logService.error("Failed to clear SSH keys", e)); + }, + }); + combineLatest([ timer(0, this.SSH_REFRESH_INTERVAL), this.desktopSettingsService.sshAgentEnabled$, @@ -150,7 +187,7 @@ export class SshAgentService implements OnDestroy { .pipe( concatMap(async ([, enabled]) => { if (!enabled) { - await ipc.platform.sshAgent.setKeys([]); + await ipc.platform.sshAgent.clearKeys(); return; }