From a32e36be7edb7244ee355ecde65959ef6ffa7261 Mon Sep 17 00:00:00 2001 From: Joseph Montanaro Date: Thu, 4 Jul 2024 21:57:27 -0400 Subject: [PATCH] fix RSA key signatures --- package.json | 2 +- src-tauri/Cargo.lock | 67 ++++++++++++++++++++++++-- src-tauri/Cargo.toml | 6 ++- src-tauri/src/credentials/ssh.rs | 37 +++++++++++++- src-tauri/src/errors.rs | 4 ++ src-tauri/src/srv/agent.rs | 34 ++++++------- src-tauri/tauri.conf.json | 2 +- src/views/credentials/NewSshKey.svelte | 4 +- 8 files changed, 124 insertions(+), 32 deletions(-) diff --git a/package.json b/package.json index 18a654a..486cc17 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "creddy", - "version": "0.5.1", + "version": "0.5.2", "scripts": { "dev": "vite", "build": "vite build", diff --git a/src-tauri/Cargo.lock b/src-tauri/Cargo.lock index ac81551..5a2c599 100644 --- a/src-tauri/Cargo.lock +++ b/src-tauri/Cargo.lock @@ -1071,7 +1071,7 @@ dependencies = [ "cocoa-foundation", "core-foundation", "core-graphics", - "foreign-types", + "foreign-types 0.5.0", "libc", "objc", ] @@ -1146,7 +1146,7 @@ dependencies = [ "bitflags 1.3.2", "core-foundation", "core-graphics-types", - "foreign-types", + "foreign-types 0.5.0", "libc", ] @@ -1196,7 +1196,7 @@ dependencies = [ [[package]] name = "creddy" -version = "0.5.0" +version = "0.5.2" dependencies = [ "argon2", "auto-launch", @@ -1211,13 +1211,17 @@ dependencies = [ "futures", "is-terminal", "once_cell", + "openssl", "rfd 0.13.0", + "rsa", "serde", "serde_json", + "sha2", "signature 2.2.0", "sodiumoxide", "sqlx", "ssh-agent-lib", + "ssh-encoding", "ssh-key", "strum", "strum_macros", @@ -1841,6 +1845,15 @@ version = "1.0.7" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3f9eec918d3f24069decb9af1554cad7c880e2da24a9afd88aca000531ab82c1" +[[package]] +name = "foreign-types" +version = "0.3.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f6f339eb8adc052cd2ca78910fda869aefa38d22d5cb648e6485e4d3fc06f3b1" +dependencies = [ + "foreign-types-shared 0.1.1", +] + [[package]] name = "foreign-types" version = "0.5.0" @@ -1848,7 +1861,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d737d9aa519fb7b749cbc3b962edcf310a8dd1f4b67c91c4f83975dbdd17d965" dependencies = [ "foreign-types-macros", - "foreign-types-shared", + "foreign-types-shared 0.3.1", ] [[package]] @@ -1862,6 +1875,12 @@ dependencies = [ "syn 2.0.68", ] +[[package]] +name = "foreign-types-shared" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "00b0228411908ca8685dba7fc2cdd70ec9990a6e753e89b6ac91a84c40fbaf4b" + [[package]] name = "foreign-types-shared" version = "0.3.1" @@ -3426,12 +3445,50 @@ version = "0.3.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c08d65885ee38876c4f86fa503fb49d7b507c2b62552df7c70b2fce627e06381" +[[package]] +name = "openssl" +version = "0.10.64" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "95a0481286a310808298130d22dd1fef0fa571e05a8f44ec801801e84b216b1f" +dependencies = [ + "bitflags 2.6.0", + "cfg-if", + "foreign-types 0.3.2", + "libc", + "once_cell", + "openssl-macros", + "openssl-sys", +] + +[[package]] +name = "openssl-macros" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a948666b637a0f465e8564c73e89d4dde00d72d4d473cc972f390fc3dcee7d9c" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.68", +] + [[package]] name = "openssl-probe" version = "0.1.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ff011a302c396a5197692431fc1948019154afc178baf7d8e37367442a4601cf" +[[package]] +name = "openssl-sys" +version = "0.9.102" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c597637d56fbc83893a35eb0dd04b2b8e7a50c91e64e9493e398b5df4fb45fa2" +dependencies = [ + "cc", + "libc", + "pkg-config", + "vcpkg", +] + [[package]] name = "option-ext" version = "0.2.0" @@ -4788,7 +4845,7 @@ dependencies = [ "bytemuck", "cfg_aliases", "core-graphics", - "foreign-types", + "foreign-types 0.5.0", "js-sys", "log", "objc2", diff --git a/src-tauri/Cargo.toml b/src-tauri/Cargo.toml index 31fb91f..5948385 100644 --- a/src-tauri/Cargo.toml +++ b/src-tauri/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "creddy" -version = "0.5.1" +version = "0.5.2" description = "A friendly AWS credentials manager" authors = ["Joseph Montanaro"] license = "" @@ -58,6 +58,10 @@ tokio-stream = "0.1.15" sqlx = { version = "0.7.4", features = ["sqlite", "runtime-tokio", "uuid"] } tokio-util = { version = "0.7.11", features = ["codec"] } futures = "0.3.30" +openssl = "0.10.64" +rsa = "0.9.6" +sha2 = "0.10.8" +ssh-encoding = "0.2.0" [features] # by default Tauri runs in production mode diff --git a/src-tauri/src/credentials/ssh.rs b/src-tauri/src/credentials/ssh.rs index d6c5bbc..46781cb 100644 --- a/src-tauri/src/credentials/ssh.rs +++ b/src-tauri/src/credentials/ssh.rs @@ -12,6 +12,8 @@ use serde::ser::{ SerializeStruct, }; use serde::de::{self, Visitor}; +use sha2::{Sha256, Sha512}; +use signature::{Signer, SignatureEncoding}; use sqlx::{ FromRow, Sqlite, @@ -19,11 +21,15 @@ use sqlx::{ Transaction, types::Uuid, }; -use ssh_agent_lib::proto::message::Identity; +use ssh_agent_lib::proto::message::{ + Identity, + SignRequest, +}; +use ssh_encoding::Encode; use ssh_key::{ Algorithm, LineEnding, - private::PrivateKey, + private::{PrivateKey, KeypairData}, public::PublicKey, }; use tokio_stream::StreamExt; @@ -119,6 +125,33 @@ impl SshKey { Ok(identities) } + + pub fn sign_request(&self, req: &SignRequest) -> Result, HandlerError> { + let mut sig = Vec::new(); + match self.private_key.key_data() { + KeypairData::Rsa(keypair) => { + // 2 is the flag value for `SSH_AGENT_RSA_SHA2_256` + if req.flags & 2 > 0 { + let signer = rsa::pkcs1v15::SigningKey::::try_from(keypair)?; + let sig_data = signer.try_sign(&req.data)?.to_vec(); + "rsa-sha-256".encode(&mut sig)?; + sig_data.encode(&mut sig)?; + } + else { + let signer = rsa::pkcs1v15::SigningKey::::try_from(keypair)?; + let sig_data = signer.try_sign(&req.data)?.to_vec(); + "rsa-sha2-512".encode(&mut sig)?; + sig_data.encode(&mut sig)?; + } + }, + _ => { + let sig_data = self.private_key.try_sign(&req.data)?; + self.algorithm.as_str().encode(&mut sig)?; + sig_data.as_bytes().encode(&mut sig)?; + }, + } + Ok(sig) + } } diff --git a/src-tauri/src/errors.rs b/src-tauri/src/errors.rs index 9f4407b..e30ec28 100644 --- a/src-tauri/src/errors.rs +++ b/src-tauri/src/errors.rs @@ -195,6 +195,10 @@ pub enum HandlerError { SshAgent(#[from] ssh_agent_lib::error::AgentError), #[error(transparent)] SshKey(#[from] ssh_key::Error), + #[error(transparent)] + Signature(#[from] signature::Error), + #[error(transparent)] + Encoding(#[from] ssh_encoding::Error), } diff --git a/src-tauri/src/srv/agent.rs b/src-tauri/src/srv/agent.rs index 70f67a7..d046027 100644 --- a/src-tauri/src/srv/agent.rs +++ b/src-tauri/src/srv/agent.rs @@ -1,5 +1,4 @@ use futures::SinkExt; -use signature::Signer; use ssh_agent_lib::agent::MessageCodec; use ssh_agent_lib::proto::message::{ Message, @@ -36,12 +35,21 @@ async fn handle( adapter.send(resp).await?; }, Message::SignRequest(req) => { - // CloseWaiter could corrupt the framing, but this doesn't matter - // since we don't plan to pull any more frames out of the stream + // Note: If the client writes more data to the stream *while* at the + // same time waiting for a resopnse to a previous request, this will + // corrupt the framing. Clients don't seem to behave that way though? let waiter = CloseWaiter { stream: adapter.get_mut() }; let resp = sign_request(req, app_handle.clone(), client_pid, waiter).await?; + + // have to do this before we send since we can't inspect the message after + let is_failure = matches!(resp, Message::Failure); adapter.send(resp).await?; - break; + + if is_failure { + // this way we don't get spammed with requests for other keys + // after denying the first + break + } }, _ => adapter.send(Message::Failure).await?, }; @@ -93,15 +101,8 @@ async fn sign_request( } let key = state.sshkey_by_name(&key_name).await?; - let sig = Signer::sign(&key.private_key, &req.data); - let key_type = key.algorithm.as_str().as_bytes(); - - let payload_len = key_type.len() + sig.as_bytes().len() + 8; - let mut payload = Vec::with_capacity(payload_len); - encode_string(&mut payload, key.algorithm.as_str().as_bytes()); - encode_string(&mut payload, sig.as_bytes()); - - Ok(Message::SignResponse(payload)) + let sig = key.sign_request(&req)?; + Ok(Message::SignResponse(sig)) }; let res = proceed.await; @@ -112,10 +113,3 @@ async fn sign_request( lease.release(); res } - - -fn encode_string(buf: &mut Vec, s: &[u8]) { - let len = s.len() as u32; - buf.extend(len.to_be_bytes()); - buf.extend(s); -} diff --git a/src-tauri/tauri.conf.json b/src-tauri/tauri.conf.json index 025b835..caa8b94 100644 --- a/src-tauri/tauri.conf.json +++ b/src-tauri/tauri.conf.json @@ -50,7 +50,7 @@ } }, "productName": "creddy", - "version": "0.5.1", + "version": "0.5.2", "identifier": "creddy", "plugins": {}, "app": { diff --git a/src/views/credentials/NewSshKey.svelte b/src/views/credentials/NewSshKey.svelte index 8854c4c..cfbcee1 100644 --- a/src/views/credentials/NewSshKey.svelte +++ b/src/views/credentials/NewSshKey.svelte @@ -60,7 +60,7 @@