Skip to content

Commit

Permalink
Fix improper RSA key conversion from ssh_key crate (#400)
Browse files Browse the repository at this point in the history
Co-authored-by: Eugene <[email protected]>
  • Loading branch information
EpicEric and Eugeny authored Dec 1, 2024
1 parent 0bb5938 commit 2a4f451
Show file tree
Hide file tree
Showing 7 changed files with 45 additions and 14 deletions.
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ futures = "0.3"
hmac = "0.12"
log = "0.4"
rand = "0.8"
rsa = "0.9"
rsa = "=0.9.6"
sha1 = { version = "0.10", features = ["oid"] }
sha2 = { version = "0.10", features = ["oid"] }
signature = "2.2"
Expand Down
6 changes: 2 additions & 4 deletions cryptovec/src/platform/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,18 +19,16 @@ pub use windows::{memset, mlock, munlock};

#[cfg(test)]
mod tests {
use wasm_bindgen_test::wasm_bindgen_test;

use super::*;

#[wasm_bindgen_test]
#[test]
fn test_memset() {
let mut buf = vec![0u8; 10];
memset(buf.as_mut_ptr(), 0xff, buf.len());
assert_eq!(buf, vec![0xff; 10]);
}

#[wasm_bindgen_test]
#[test]
fn test_memset_partial() {
let mut buf = vec![0u8; 10];
memset(buf.as_mut_ptr(), 0xff, 5);
Expand Down
4 changes: 2 additions & 2 deletions russh-keys/src/agent/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use tokio::time::sleep;
use {std, tokio};

use super::{msg, Constraint};
use crate::helpers::EncodedExt;
use crate::helpers::{sign_workaround, EncodedExt};
use crate::Error;

#[derive(Clone)]
Expand Down Expand Up @@ -342,7 +342,7 @@ impl<S: AsyncRead + AsyncWrite + Send + Unpin + 'static, A: Agent + Send + Sync
writebuf.push(msg::SIGN_RESPONSE);
let data = Bytes::decode(r)?;

let signature = signature::Signer::try_sign(&*key, &data)?;
let signature = sign_workaround(&key, &data)?;
signature.encoded()?.encode(writebuf)?;

let len = writebuf.len();
Expand Down
33 changes: 33 additions & 0 deletions russh-keys/src/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,3 +41,36 @@ macro_rules! map_err {
}

pub use map_err;
use ssh_key::PrivateKey;

// TODO only needed until https://github.com/RustCrypto/SSH/pull/318 is released
#[doc(hidden)]
pub fn sign_workaround(
key: &PrivateKey,
data: &[u8],
) -> Result<ssh_key::Signature, signature::Error> {
Ok(match key.key_data() {
ssh_key::private::KeypairData::Rsa(rsa_keypair) => {
let pk = rsa::RsaPrivateKey::from_components(
<rsa::BigUint as std::convert::TryFrom<_>>::try_from(&rsa_keypair.public.n)?,
<rsa::BigUint as std::convert::TryFrom<_>>::try_from(&rsa_keypair.public.e)?,
<rsa::BigUint as std::convert::TryFrom<_>>::try_from(&rsa_keypair.private.d)?,
vec![
<rsa::BigUint as std::convert::TryFrom<_>>::try_from(&rsa_keypair.private.p)?,
<rsa::BigUint as std::convert::TryFrom<_>>::try_from(&rsa_keypair.private.q)?,
],
)?;
let signature = signature::Signer::try_sign(
&rsa::pkcs1v15::SigningKey::<sha2::Sha512>::new(pk),
data,
)?;
ssh_key::Signature::new(
ssh_key::Algorithm::Rsa {
hash: Some(ssh_key::HashAlg::Sha512),
},
<rsa::pkcs1v15::Signature as signature::SignatureEncoding>::to_vec(&signature),
)?
}
keypair => signature::Signer::try_sign(keypair, data)?,
})
}
4 changes: 2 additions & 2 deletions russh-keys/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -857,7 +857,7 @@ Cog3JMeTrb3LiPHgN6gU2P30MRp6L1j1J/MtlOAr5rux
client.request_identities().await?;
let buf = russh_cryptovec::CryptoVec::from_slice(b"blabla");
let len = buf.len();
let buf = client.sign_request(&public, buf).await.unwrap();
let buf = client.sign_request(public, buf).await.unwrap();
let (a, b) = buf.split_at(len);

match key.public_key().key_data() {
Expand Down Expand Up @@ -943,7 +943,7 @@ Cog3JMeTrb3LiPHgN6gU2P30MRp6L1j1J/MtlOAr5rux
client.request_identities().await.unwrap();
let buf = russh_cryptovec::CryptoVec::from_slice(b"blabla");
let len = buf.len();
let buf = client.sign_request(&public, buf).await.unwrap();
let buf = client.sign_request(public, buf).await.unwrap();
let (a, b) = buf.split_at(len);
if let ssh_key::public::KeyData::Ed25519 { .. } = public.key_data() {
let sig = &b[b.len() - 64..];
Expand Down
6 changes: 3 additions & 3 deletions russh/src/client/encrypted.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use std::num::Wrapping;

use bytes::Bytes;
use log::{debug, error, info, trace, warn};
use russh_keys::helpers::{map_err, EncodedExt};
use russh_keys::helpers::{map_err, sign_workaround, EncodedExt};
use ssh_encoding::{Decode, Encode};

use crate::cert::PublicKeyOrCertificate;
Expand Down Expand Up @@ -1049,7 +1049,7 @@ impl Encrypted {
)?;

// Extend with self-signature.
let signature = signature::Signer::try_sign(&**key, buffer)?;
let signature = sign_workaround(key, buffer)?;
signature.encoded()?.encode(&mut *buffer)?;

push_packet!(self.write, {
Expand All @@ -1065,7 +1065,7 @@ impl Encrypted {
)?;

// Extend with self-signature.
let signature = signature::Signer::try_sign(&**key, buffer)?;
let signature = sign_workaround(key, buffer)?;
signature.encoded()?.encode(&mut *buffer)?;

push_packet!(self.write, {
Expand Down
4 changes: 2 additions & 2 deletions russh/src/server/kex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use std::cell::RefCell;
use std::ops::DerefMut;

use log::debug;
use russh_keys::helpers::EncodedExt;
use russh_keys::helpers::{sign_workaround, EncodedExt};
use ssh_encoding::Encode;

use super::*;
Expand Down Expand Up @@ -145,7 +145,7 @@ impl KexDh {
debug!("hash: {:?}", hash);
debug!("key: {:?}", config.keys[kexdhdone.key]);

let signature = signature::Signer::try_sign(&config.keys[kexdhdone.key], &hash)?;
let signature = sign_workaround(&config.keys[kexdhdone.key], &hash)?;
signature.encoded()?.encode(&mut *buffer)?;

cipher.write(&buffer, write_buffer);
Expand Down

0 comments on commit 2a4f451

Please sign in to comment.