From 601fe5a11995f93ed8692764b4d4aba5fce7e2b4 Mon Sep 17 00:00:00 2001 From: Arthur Gautier Date: Thu, 4 Apr 2024 22:43:16 -0700 Subject: [PATCH] fixup cert parsing --- Cargo.lock | 10 +- Cargo.toml | 5 + examples/key_storage.rs | 30 +++--- src/proto.rs | 2 + src/proto/message.rs | 202 +++++++++++++++++++++++++++++++++++----- src/proto/privatekey.rs | 174 ++++++++++++++++++++++++++++++++++ 6 files changed, 380 insertions(+), 43 deletions(-) create mode 100644 src/proto/privatekey.rs diff --git a/Cargo.lock b/Cargo.lock index 02b0b9e..dfe2175 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -973,6 +973,7 @@ dependencies = [ "sha1", "ssh-encoding", "ssh-key", + "subtle", "testresult", "tokio", "tokio-util", @@ -981,8 +982,7 @@ dependencies = [ [[package]] name = "ssh-cipher" version = "0.2.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "caac132742f0d33c3af65bfcde7f6aa8f62f0e991d80db99149eb9d44708784f" +source = "git+https://github.com/baloo/SSH.git?branch=baloo/ssh-key/fixes#4d941202b45c0e96a3f816b39bc13a08f10dfcfc" dependencies = [ "cipher", "ssh-encoding", @@ -991,8 +991,7 @@ dependencies = [ [[package]] name = "ssh-encoding" version = "0.2.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "eb9242b9ef4108a78e8cd1a2c98e193ef372437f8c22be363075233321dd4a15" +source = "git+https://github.com/baloo/SSH.git?branch=baloo/ssh-key/fixes#4d941202b45c0e96a3f816b39bc13a08f10dfcfc" dependencies = [ "base64ct", "pem-rfc7468", @@ -1002,8 +1001,7 @@ dependencies = [ [[package]] name = "ssh-key" version = "0.6.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3b71299a724c8d84956caaf8fc3b3ea57c3587fe2d0b800cd0dc1f3599905d7e" +source = "git+https://github.com/baloo/SSH.git?branch=baloo/ssh-key/fixes#4d941202b45c0e96a3f816b39bc13a08f10dfcfc" dependencies = [ "num-bigint-dig", "p256", diff --git a/Cargo.toml b/Cargo.toml index 276f9e4..b37a391 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -25,6 +25,7 @@ service-binding = { version = "^2" } ssh-encoding = { version = "0.2.0" } ssh-key = { version = "0.6.4", features = ["rsa", "alloc"] } #uuid = { version = "1.8.0", features = ["v4"] } +subtle = { version = "2", default-features = false } [features] default = ["agent"] @@ -46,3 +47,7 @@ ssh-key = { version = "0.6.4", features = ["p256"] } p256 = { version = "0.13.2" } const-str = "0.5.7" rstest = "0.18.2" + +[patch.crates-io] +ssh-encoding = { git = "https://github.com/baloo/SSH.git", branch = "baloo/ssh-key/fixes" } +ssh-key = { git = "https://github.com/baloo/SSH.git", branch = "baloo/ssh-key/fixes" } diff --git a/examples/key_storage.rs b/examples/key_storage.rs index 25ea65e..e6454d7 100644 --- a/examples/key_storage.rs +++ b/examples/key_storage.rs @@ -7,7 +7,7 @@ use ssh_agent_lib::proto::extension::SessionBind; use tokio::net::UnixListener; use ssh_agent_lib::agent::{Agent, Session}; -use ssh_agent_lib::proto::message::{self, Message, SignRequest}; +use ssh_agent_lib::proto::message::{self, Credential, Message, SignRequest}; use ssh_agent_lib::proto::{signature, AddIdentityConstrained, KeyConstraint}; use ssh_key::{ private::{KeypairData, PrivateKey}, @@ -128,12 +128,14 @@ impl KeyStorage { Ok(Message::Success) } Message::AddIdentity(identity) => { - let privkey = PrivateKey::try_from(identity.privkey).unwrap(); - self.identity_add(Identity { - pubkey: PublicKey::from(&privkey), - privkey, - comment: identity.comment, - }); + if let Credential::Key { privkey, comment } = identity.credential { + let privkey = PrivateKey::try_from(privkey).unwrap(); + self.identity_add(Identity { + pubkey: PublicKey::from(&privkey), + privkey, + comment: comment, + }); + } Ok(Message::Success) } Message::AddIdConstrained(AddIdentityConstrained { @@ -150,12 +152,14 @@ impl KeyStorage { } } } - let privkey = PrivateKey::try_from(identity.privkey).unwrap(); - self.identity_add(Identity { - pubkey: PublicKey::from(&privkey), - privkey, - comment: identity.comment, - }); + if let Credential::Key { privkey, comment } = identity.credential { + let privkey = PrivateKey::try_from(privkey).unwrap(); + self.identity_add(Identity { + pubkey: PublicKey::from(&privkey), + privkey, + comment: comment, + }); + } Ok(Message::Success) } Message::SignRequest(request) => { diff --git a/src/proto.rs b/src/proto.rs index 95257f6..e96241f 100644 --- a/src/proto.rs +++ b/src/proto.rs @@ -1,10 +1,12 @@ pub mod error; pub mod extension; pub mod message; +pub mod privatekey; pub mod signature; pub use self::error::*; pub use self::message::*; +pub use self::privatekey::*; pub use self::signature::*; pub type MpInt = Vec; diff --git a/src/proto/message.rs b/src/proto/message.rs index 312dbb5..0dcaa39 100644 --- a/src/proto/message.rs +++ b/src/proto/message.rs @@ -1,7 +1,10 @@ +use core::str::FromStr; use ssh_encoding::{CheckedSum, Decode, Encode, Error as EncodingError, Reader, Writer}; -use ssh_key::{private::KeypairData, public::KeyData, Error, Signature}; +use ssh_key::{ + certificate::Certificate, private::KeypairData, public::KeyData, Algorithm, Error, Signature, +}; -use super::ProtoError; +use super::{PrivateKeyData, ProtoError}; type Result = core::result::Result; @@ -94,35 +97,105 @@ impl Encode for SignRequest { } } +#[derive(Clone, PartialEq, Debug)] +pub enum Credential { + Key { + privkey: KeypairData, + comment: String, + }, + Cert { + algorithm: Algorithm, + certificate: Certificate, + privkey: PrivateKeyData, + comment: String, + }, +} + +impl Encode for Credential { + fn encoded_len(&self) -> ssh_encoding::Result { + match self { + Self::Key { privkey, comment } => { + [privkey.encoded_len()?, comment.encoded_len()?].checked_sum() + } + Self::Cert { + algorithm, + certificate, + privkey, + comment, + } => [ + algorithm.to_certificate_type().encoded_len()?, + certificate.encoded_len_prefixed()?, + privkey.encoded_len()?, + comment.encoded_len()?, + ] + .checked_sum(), + } + } + + fn encode(&self, writer: &mut impl Writer) -> ssh_encoding::Result<()> { + match self { + Self::Key { privkey, comment } => { + privkey.encode(writer)?; + comment.encode(writer) + } + Self::Cert { + algorithm, + certificate, + privkey, + comment, + } => { + algorithm.to_certificate_type().encode(writer)?; + certificate.encode_prefixed(writer)?; + privkey.encode(writer)?; + comment.encode(writer) + } + } + } +} + #[derive(Clone, PartialEq, Debug)] pub struct AddIdentity { - pub privkey: KeypairData, - pub comment: String, + pub credential: Credential, } impl Decode for AddIdentity { type Error = ProtoError; fn decode(reader: &mut impl Reader) -> Result { - eprintln!("before add identity decode"); - let privkey = KeypairData::decode(reader)?; - eprintln!("after add identity decode: {privkey:?}"); - let comment = String::decode(reader)?; + let alg = String::decode(reader)?; + let cert_alg = Algorithm::new_certificate(&alg); + let credential = if let Ok(algorithm) = cert_alg { + let certificate = reader.read_prefixed(|reader| { + let cert = ssh_key::certificate::Certificate::decode(reader)?; + Ok::<_, ProtoError>(cert) + })?; + let privkey = PrivateKeyData::decode_as(reader, algorithm.clone())?; + let comment = String::decode(reader)?; + + Credential::Cert { + algorithm, + certificate, + privkey, + comment, + } + } else { + let algorithm = Algorithm::from_str(&alg).map_err(EncodingError::from)?; + let privkey = KeypairData::decode_as(reader, algorithm)?; + let comment = String::decode(reader)?; + Credential::Key { privkey, comment } + }; - eprintln!("after comment {comment:?}"); - Ok(Self { privkey, comment }) + Ok(Self { credential }) } } impl Encode for AddIdentity { fn encoded_len(&self) -> ssh_encoding::Result { - [self.privkey.encoded_len()?, self.comment.encoded_len()?].checked_sum() + self.credential.encoded_len() } fn encode(&self, writer: &mut impl Writer) -> ssh_encoding::Result<()> { - self.privkey.encode(writer)?; - self.comment.encode(writer)?; - Ok(()) + self.credential.encode(writer) } } @@ -136,14 +209,10 @@ impl Decode for AddIdentityConstrained { type Error = ProtoError; fn decode(reader: &mut impl Reader) -> Result { - eprintln!("XX"); - let identity = AddIdentity::decode(reader)?; - eprintln!("found identity: {identity:?}"); let mut constraints = vec![]; while !reader.is_finished() { - eprintln!("constraint"); constraints.push(KeyConstraint::decode(reader)?); } @@ -588,8 +657,10 @@ mod tests { let expected = AddIdentityConstrained { identity: AddIdentity { - privkey: KeypairData::Ecdsa(demo_key()), - comment: "baloo@angela".to_string(), + credential: Credential::Key { + privkey: KeypairData::Ecdsa(demo_key()), + comment: "baloo@angela".to_string(), + }, }, constraints: vec![KeyConstraint::Lifetime(2)], }; @@ -661,8 +732,10 @@ mod tests { let expected = AddIdentityConstrained { identity: AddIdentity { - privkey: KeypairData::Ecdsa(demo_key()), - comment: "baloo@angela".to_string(), + credential: Credential::Key { + privkey: KeypairData::Ecdsa(demo_key()), + comment: "baloo@angela".to_string(), + }, }, constraints: vec![KeyConstraint::Extension( "restrict-destination-v00@openssh.com".to_string(), @@ -744,8 +817,10 @@ mod tests { let out = AddIdentity::decode(&mut reader).expect("parse message"); let expected = AddIdentity { - privkey: KeypairData::Ecdsa(demo_key()), - comment: "baloo@angela".to_string(), + credential: Credential::Key { + privkey: KeypairData::Ecdsa(demo_key()), + comment: "baloo@angela".to_string(), + }, }; assert_eq!(out, expected); @@ -782,4 +857,83 @@ mod tests { expected.encode(&mut buf).expect("serialize message"); assert_eq!(buf, msg); } + + #[test] + fn test_parse_certificates() { + let msg: &[u8] = &hex!( + " + 190000001c7373682d7273612d + 636572742d763031406f70656e7373682e636f6d000003200000001c7373682d + 7273612d636572742d763031406f70656e7373682e636f6d00000020c551bbbb + 4b7a8cd1f0e5f01689926b0253d51cd230aec837b6439f86ad4f9b9a00000003 + 0100010000018100e0419157579956319a7f810b747b25187f5ff26556f7ff03 + 7b57fa7d5911d55abd59438d98a2205a87def0805ea6d8881f9790a010cbe0a2 + 0d6145abac98de4fa3fc0f2b53b8241db205b79e64e0a7ccd33f9f2cd34ae9d2 + ce791bc6aabc8fe1951e37a7af04b3fa0b029710e7e958403c7bf6d40c13b264 + 834f37402ec6630c486014b68413793db3340bceb6aa4c703170048b59c944c5 + 2678f91f872d169619eb39066bc78021925efd226113f2523ecbefdaf5caa853 + 36b760e7e458f7abd1af48917a778805535dcf45345b2ed4c4aab2286bd12f38 + 1173e856e95929ac27515608606f07ff8514188e2e9b14c822cfd8ce12946f2b + 562c3f51b4a86317ebce585a832af467f8ea27fd3ed1aa59d187825e9e771ad8 + c383f6fdef2853ed22579bc00a7fcf52d9906d25dcd5e80ae35115aeb4bcba67 + 1fa865c26bde46272806c4991fc9d548878d2b99ba522083b8863d7c434c21bd + 42da838ed0355ad2fde62e8d0684bcc194f2911f235c85ffd3b2b4870e95460a + 2d3422130ccecf610000000000000001000000010000000664617272656e0000 + 000a0000000664617272656e00000000660f5cc400000000660f6b3c00000000 + 00000082000000157065726d69742d5831312d666f7277617264696e67000000 + 00000000177065726d69742d6167656e742d666f7277617264696e6700000000 + 000000167065726d69742d706f72742d666f7277617264696e67000000000000 + 000a7065726d69742d707479000000000000000e7065726d69742d757365722d + 72630000000000000000000000330000000b7373682d65643235353139000000 + 20dc83ccfc6ef8488b329f736057286325de5905237e55d7711e0a0a8d792ce2 + cb000000530000000b7373682d656432353531390000004001f88ec5a9f1cbd5 + 4c1668b3e33ac6f52c32dff0c51207fbb55a55b88b8809c369e9ac008e3228dd + 90978ff2d6bebd9bbb392883bcb56d9f81f6afc200ce270300000180063980b0 + 5c8b42329056de1f025eb78d68fdf1b2631811302c75913b86e81b288c975e6b + ff04cf464705a2ce23de7085c2ff79e75cfefd393f4b0420253b55269f9307cc + 627b8ac6579c5fb3dbf9c5c39658a28557e83132419a98491ef0aae35a785937 + f0785e5ae430c83edb0a91b95efa6b840851a8c4c025b00752330dd153be15be + 190f79b0d31548877e5fcecd498c8206488dc0f8c25216db63850e86a82194aa + a94dc3585f35cf73bb8f464566d6821de52f18d5ee37a7e718e228adf314668d + b1285eea7e34fa71e9ff787eeac0bf3f97d038a5dd9ecf6a9782a6d1354f5a74 + be42c6cd15aaf6efa77e06018e0a8d90dcaffac60972a58e39e2773269ab3ac3 + 0d352d66586cf8e19a821b29016b0f75aaaad7caf17ed4913665999fe491e0bd + 2c08141dafeeb08bfe5bedea52ab46e33851def2204462b59fa83f853d1e3645 + c6b7e4d8e4d95fe3b74e34fe3e37c53d026be9c19643ab4014bb82ef922208af + 68435bdc89bdbe0518655bb3ea28078bebb7bde88ff44970181bd381000000c1 + 00e0dd19b95c563d9198f0f4e4b19677fd17465875757da008b93c0138fd89d7 + 1a1f5669d967b69814462530642a5595de4ee39a838ac8d38136cc2c20f7a7e6 + 2bbba10146a35a2b8fba51b70a0b1a43b43fd26b84ae5a7d1ef7857eab7b2301 + 0c1d35c3cc1c781407f45875684a63a25a3f71fd32f0984dab7b70febadb1fe4 + 4395f80a228f46f3f7dd05205d453c404d88712d2051cfac3a33e888a6fea26b + 332f5ac58edfad6a64cb16e39280aacc607d32f90fb6fe45b21bd288fe9d4fc6 + b2000000c100faba9137f37dc9ab8b2821ce0c444b03f5ea6ea5059488214ecc + cc02417c601e32e923710d2dc1417bfe293502aed390eb93e544a51fd4686b4b + 520e49f559e259b9cd1c2e08e41cfb36b4979bd5f4f6917d73aeb4a47d7cfc71 + 14ec7773aec5a54b0cdc4244cdd1db8acc8c98955bf1abbe35db3dc7f540ff8a + 858a61399001f0f9c4c440de7a50ab1a55ff1bb24f3ecdba42ca8a34a83bc76f + fc5687d9093ba4eba91723b9ae5acdcfc650d8d95b5e8fda85ce957075079d2a + 134f4ed9b181000000c100e4f88607532262eaf1db3f11d02535c32a7506acb9 + bcd2b3e9b852a71fea134921015399be8830db4000b7f33ec3af71b56448178b + d4d3310ad322855c80aff5bf29fbeebdbbb09a3f09cd5fc017f0d004c08c3f56 + 9e4efc15c5fa9474e0bae15e7b416ca5bd0f053d869f3908bc042bd111af7fc5 + 97ef541f70140ccdbae1d5bc781d3dc14b3a113f939f1da21d2031d4f37805d3 + 6fc420a728ffbeed8e1e1ddb8d4d232df1e02a152965694139f38b5a60b9198c + 513ac733f51f2c04164de10000000c62616c6f6f40616e67656c610100000002 + " + ); + let mut reader = msg; + + let out = Message::decode(&mut reader).expect("parse message"); + + //let expected = Message::IdentitiesAnswer(vec![Identity { + // pubkey: KeyData::Ecdsa(demo_key().into()), + // comment: "baloo@angela".to_string(), + //}]); + //assert_eq!(out, expected); + + //let mut buf = vec![]; + //expected.encode(&mut buf).expect("serialize message"); + //assert_eq!(buf, msg); + } } diff --git a/src/proto/privatekey.rs b/src/proto/privatekey.rs new file mode 100644 index 0000000..b8750f9 --- /dev/null +++ b/src/proto/privatekey.rs @@ -0,0 +1,174 @@ +// TODO(baloo): this should move to ssh-key/src/private + +use core::fmt; + +use ssh_encoding::{Decode, Encode, Reader, Writer}; +use ssh_key::{ + private::{ + self, + DsaPrivateKey, //Ed25519PrivateKey, + RsaPrivateKey, + }, + Algorithm, EcdsaCurve, Error, Result, +}; +use subtle::{Choice, ConstantTimeEq}; + +/// Elliptic Curve Digital Signature Algorithm (ECDSA) private/public keypair. +#[derive(Clone, Debug)] +pub enum EcdsaPrivateKey { + /// NIST P-256 ECDSA keypair. + NistP256(private::EcdsaPrivateKey<32>), + + /// NIST P-384 ECDSA keypair. + NistP384(private::EcdsaPrivateKey<48>), + + /// NIST P-521 ECDSA keypair. + NistP521(private::EcdsaPrivateKey<66>), +} + +impl ConstantTimeEq for EcdsaPrivateKey { + fn ct_eq(&self, other: &Self) -> Choice { + let private_key_a = match self { + Self::NistP256(private) => private.as_slice(), + Self::NistP384(private) => private.as_slice(), + Self::NistP521(private) => private.as_slice(), + }; + + let private_key_b = match other { + Self::NistP256(private) => private.as_slice(), + Self::NistP384(private) => private.as_slice(), + Self::NistP521(private) => private.as_slice(), + }; + + private_key_a.ct_eq(private_key_b) + } +} + +impl Eq for EcdsaPrivateKey {} + +impl PartialEq for EcdsaPrivateKey { + fn eq(&self, other: &Self) -> bool { + self.ct_eq(other).into() + } +} + +impl EcdsaPrivateKey { + fn decode_as(reader: &mut impl Reader, curve: EcdsaCurve) -> Result { + match curve { + EcdsaCurve::NistP256 => { + private::EcdsaPrivateKey::<32>::decode(reader).map(Self::NistP256) + } + EcdsaCurve::NistP384 => { + private::EcdsaPrivateKey::<48>::decode(reader).map(Self::NistP384) + } + EcdsaCurve::NistP521 => { + private::EcdsaPrivateKey::<66>::decode(reader).map(Self::NistP521) + } + } + } +} + +impl Encode for EcdsaPrivateKey { + fn encoded_len(&self) -> ssh_encoding::Result { + match self { + Self::NistP256(private) => private.encoded_len(), + Self::NistP384(private) => private.encoded_len(), + Self::NistP521(private) => private.encoded_len(), + } + } + + fn encode(&self, writer: &mut impl Writer) -> ssh_encoding::Result<()> { + match self { + Self::NistP256(private) => private.encode(writer), + Self::NistP384(private) => private.encode(writer), + Self::NistP521(private) => private.encode(writer), + } + } +} + +#[derive(Clone)] +#[non_exhaustive] +pub enum PrivateKeyData { + /// Digital Signature Algorithm (DSA) private key. + Dsa(DsaPrivateKey), + + /// ECDSA private key. + Ecdsa(EcdsaPrivateKey), + + // TODO(baloo): Ed25519PrivateKey does not implement [`Decode`] + ///// Ed25519 private key. + //Ed25519(Ed25519PrivateKey), + /// RSA private key. + Rsa(RsaPrivateKey), +} + +impl PrivateKeyData { + /// Decode [`PrivateKeyData`] for the specified algorithm. + pub fn decode_as(reader: &mut impl Reader, algorithm: Algorithm) -> Result { + match algorithm { + Algorithm::Dsa => DsaPrivateKey::decode(reader).map(Self::Dsa), + Algorithm::Ecdsa { curve } => { + EcdsaPrivateKey::decode_as(reader, curve).map(Self::Ecdsa) + } + //Algorithm::Ed25519 => Ed25519PrivateKey::decode(reader).map(Self::Ed25519), + Algorithm::Rsa { .. } => RsaPrivateKey::decode(reader).map(Self::Rsa), + #[allow(unreachable_patterns)] + _ => Err(Error::AlgorithmUnknown), + } + } +} + +impl fmt::Debug for PrivateKeyData { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::Dsa(_) => write!(f, "PrivateKeyData::Dsa"), + Self::Ecdsa(_) => write!(f, "PrivateKeyData::Ecdsa"), + //Self::Ed25519(_) => write!(f, "PrivateKeyData::Ed25519"), + Self::Rsa(_) => write!(f, "PrivateKeyData::Rsa"), + } + } +} + +impl Encode for PrivateKeyData { + fn encoded_len(&self) -> ssh_encoding::Result { + match self { + Self::Dsa(key) => key.encoded_len(), + Self::Ecdsa(key) => key.encoded_len(), + //Self::Ed25519(key) => key.encoded_len(), + Self::Rsa(key) => key.encoded_len(), + } + } + + fn encode(&self, writer: &mut impl Writer) -> ssh_encoding::Result<()> { + match self { + Self::Dsa(key) => key.encode(writer)?, + Self::Ecdsa(key) => key.encode(writer)?, + //Self::Ed25519(key) => key.encode(writer)?, + Self::Rsa(key) => key.encode(writer)?, + } + + Ok(()) + } +} + +impl ConstantTimeEq for PrivateKeyData { + fn ct_eq(&self, other: &Self) -> Choice { + // Note: constant-time with respect to key *data* comparisons, not algorithms + match (self, other) { + (Self::Dsa(a), Self::Dsa(b)) => a.ct_eq(b), + (Self::Ecdsa(a), Self::Ecdsa(b)) => a.ct_eq(b), + //(Self::Ed25519(a), Self::Ed25519(b)) => a.ct_eq(b), + (Self::Rsa(a), Self::Rsa(b)) => a.ct_eq(b), + #[allow(unreachable_patterns)] + _ => Choice::from(0), + } + } +} + +impl Eq for PrivateKeyData {} + +impl PartialEq for PrivateKeyData { + fn eq(&self, other: &Self) -> bool { + self.ct_eq(other).into() + } +}