From 9d78e316ef5c6197c5e02945881c0f3fabd78f12 Mon Sep 17 00:00:00 2001 From: Tony Arcieri Date: Tue, 13 Aug 2024 15:15:35 -0600 Subject: [PATCH] ssh-key: make all DSA key fields private Like we did for RSA in #274, this makes all fields of DSA keys private, and also validates that their inner `Mpint`s are positive. --- ssh-key/src/private/dsa.rs | 35 +++++++++++++++++++---- ssh-key/src/private/keypair.rs | 2 +- ssh-key/src/public/dsa.rs | 52 +++++++++++++++++++++++++++++----- ssh-key/src/signature.rs | 4 +-- ssh-key/tests/certificate.rs | 8 +++--- ssh-key/tests/private_key.rs | 10 +++---- ssh-key/tests/public_key.rs | 8 +++--- 7 files changed, 90 insertions(+), 29 deletions(-) diff --git a/ssh-key/src/private/dsa.rs b/ssh-key/src/private/dsa.rs index 6f111d6..bbd0bbd 100644 --- a/ssh-key/src/private/dsa.rs +++ b/ssh-key/src/private/dsa.rs @@ -22,6 +22,15 @@ pub struct DsaPrivateKey { } impl DsaPrivateKey { + /// Create a new DSA private key given the value `x`. + pub fn new(x: Mpint) -> Result { + if x.is_positive() { + Ok(Self { inner: x }) + } else { + Err(Error::FormatEncoding) + } + } + /// Get the serialized private key as bytes. pub fn as_bytes(&self) -> &[u8] { self.inner.as_bytes() @@ -57,9 +66,7 @@ impl Decode for DsaPrivateKey { type Error = Error; fn decode(reader: &mut impl Reader) -> Result { - Ok(Self { - inner: Mpint::decode(reader)?, - }) + Self::new(Mpint::decode(reader)?) } } @@ -127,10 +134,10 @@ impl TryFrom<&dsa::SigningKey> for DsaPrivateKey { #[derive(Clone)] pub struct DsaKeypair { /// Public key. - pub public: DsaPublicKey, + public: DsaPublicKey, /// Private key. - pub private: DsaPrivateKey, + private: DsaPrivateKey, } impl DsaKeypair { @@ -145,6 +152,22 @@ impl DsaKeypair { let components = dsa::Components::generate(rng, Self::KEY_SIZE); dsa::SigningKey::generate(rng, components).try_into() } + + /// Create a new [`DsaKeypair`] with the given `public` and `private` components. + pub fn new(public: DsaPublicKey, private: DsaPrivateKey) -> Result { + // TODO(tarcieri): validate the `public` and `private` components match + Ok(Self { public, private }) + } + + /// Get the public component of this key. + pub fn public(&self) -> &DsaPublicKey { + &self.public + } + + /// Get the private component of this key. + pub fn private(&self) -> &DsaPrivateKey { + &self.private + } } impl ConstantTimeEq for DsaKeypair { @@ -167,7 +190,7 @@ impl Decode for DsaKeypair { fn decode(reader: &mut impl Reader) -> Result { let public = DsaPublicKey::decode(reader)?; let private = DsaPrivateKey::decode(reader)?; - Ok(DsaKeypair { public, private }) + DsaKeypair::new(public, private) } } diff --git a/ssh-key/src/private/keypair.rs b/ssh-key/src/private/keypair.rs index 5281955..ee8d6b1 100644 --- a/ssh-key/src/private/keypair.rs +++ b/ssh-key/src/private/keypair.rs @@ -215,7 +215,7 @@ impl KeypairData { pub(super) fn checkint(&self) -> u32 { let bytes = match self { #[cfg(feature = "alloc")] - Self::Dsa(dsa) => dsa.private.as_bytes(), + Self::Dsa(dsa) => dsa.private().as_bytes(), #[cfg(feature = "ecdsa")] Self::Ecdsa(ecdsa) => ecdsa.private_key_bytes(), Self::Ed25519(ed25519) => ed25519.private.as_ref(), diff --git a/ssh-key/src/public/dsa.rs b/ssh-key/src/public/dsa.rs index 189aa6a..a25c117 100644 --- a/ssh-key/src/public/dsa.rs +++ b/ssh-key/src/public/dsa.rs @@ -10,17 +10,55 @@ use encoding::{CheckedSum, Decode, Encode, Reader, Writer}; #[derive(Clone, Debug, Eq, Ord, PartialEq, PartialOrd)] pub struct DsaPublicKey { /// Prime modulus. - pub p: Mpint, + p: Mpint, /// Prime divisor of `p - 1`. - pub q: Mpint, + q: Mpint, - /// Generator of a subgroup of order `q` in the multiplicative group - /// `GF(p)`, such that `1 < g < p`. - pub g: Mpint, + /// Generator of a subgroup of order `q` in the multiplicative group `GF(p)`, such that + /// `1 < g < p`. + g: Mpint, /// The public key, where `y = gˣ mod p`. - pub y: Mpint, + y: Mpint, +} + +impl DsaPublicKey { + /// Create a new [`DsaPublicKey`] with the following components: + /// + /// - `p`: prime modulus. + /// - `q`: prime divisor of `p - 1`. + /// - `g`: generator of a subgroup of order `q` in the multiplicative group `GF(p)`, such + /// that `1 < g < p`. + /// - `y`: the public key, where `y = gˣ mod p`. + pub fn new(p: Mpint, q: Mpint, g: Mpint, y: Mpint) -> Result { + if p.is_positive() && q.is_positive() && g.is_positive() && y.is_positive() { + Ok(Self { p, q, g, y }) + } else { + Err(Error::FormatEncoding) + } + } + + /// Prime modulus. + pub fn p(&self) -> &Mpint { + &self.p + } + + /// Prime divisor of `p - 1`. + pub fn q(&self) -> &Mpint { + &self.q + } + + /// Generator of a subgroup of order `q` in the multiplicative group `GF(p)`, such that + /// `1 < g < p`. + pub fn g(&self) -> &Mpint { + &self.g + } + + /// The public key, where `y = gˣ mod p`. + pub fn y(&self) -> &Mpint { + &self.y + } } impl Decode for DsaPublicKey { @@ -31,7 +69,7 @@ impl Decode for DsaPublicKey { let q = Mpint::decode(reader)?; let g = Mpint::decode(reader)?; let y = Mpint::decode(reader)?; - Ok(Self { p, q, g, y }) + Self::new(p, q, g, y) } } diff --git a/ssh-key/src/signature.rs b/ssh-key/src/signature.rs index b0be10c..68e6c59 100644 --- a/ssh-key/src/signature.rs +++ b/ssh-key/src/signature.rs @@ -845,7 +845,7 @@ mod tests { ); let signature = keypair.try_sign(&data[..]).expect("dsa try_sign is ok"); keypair - .public + .public() .verify(&data[..], &signature) .expect("dsa verify is ok"); @@ -861,7 +861,7 @@ mod tests { .try_sign(&data[..]) .expect("dsa try_sign for r.len() == 19 is ok"); keypair - .public + .public() .verify(&data[..], &signature) .expect("dsa verify is ok"); } diff --git a/ssh-key/tests/certificate.rs b/ssh-key/tests/certificate.rs index 44c5572..e57daec 100644 --- a/ssh-key/tests/certificate.rs +++ b/ssh-key/tests/certificate.rs @@ -74,11 +74,11 @@ fn decode_dsa_openssh() { e3c48e2ccbafd2170f69e8e5c8b6ab69b9c5f45d95e1d9293e965227eee5b879b1123371c21b1db60f14b5e 5c05a4782ceb43a32f449647703063621e7a286bec95b16726c18b5e52383d00b297a6b03489b06068a5" ), - dsa_key.p.as_bytes(), + dsa_key.p().as_bytes(), ); assert_eq!( &hex!("00891815378597fe42d3fd261fe76df365845bbb87"), - dsa_key.q.as_bytes(), + dsa_key.q().as_bytes(), ); assert_eq!( &hex!( @@ -86,7 +86,7 @@ fn decode_dsa_openssh() { 520a713fe4104a74bed53fd5915da736365afd3f09777bbccfbadf7ac2b087b7f4d95fabe47d72a46e95088 f9cd2a9fbf236b58a6982647f3c00430ad7352d47a25ebbe9477f0c3127da86ad7448644b76de5875c" ), - dsa_key.g.as_bytes(), + dsa_key.g().as_bytes(), ); assert_eq!( &hex!( @@ -94,7 +94,7 @@ fn decode_dsa_openssh() { a57b475c78d44989f16577527e598334be6aae4abd750c36af80489d392697c1f32f3cf3c9a8b99bcddb53d 7a37e1a28fd53d4934131cf41c437c6734d1e04004adcd925b84b3956c30c3a3904eecb31400b0df48" ), - dsa_key.y.as_bytes(), + dsa_key.y().as_bytes(), ); assert_eq!("user@example.com", cert.comment()); diff --git a/ssh-key/tests/private_key.rs b/ssh-key/tests/private_key.rs index 5356a74..a6b3eb2 100644 --- a/ssh-key/tests/private_key.rs +++ b/ssh-key/tests/private_key.rs @@ -76,11 +76,11 @@ fn decode_dsa_openssh() { e3c48e2ccbafd2170f69e8e5c8b6ab69b9c5f45d95e1d9293e965227eee5b879b1123371c21b1db60f14b5e 5c05a4782ceb43a32f449647703063621e7a286bec95b16726c18b5e52383d00b297a6b03489b06068a5" ), - dsa_keypair.public.p.as_bytes(), + dsa_keypair.public().p().as_bytes(), ); assert_eq!( &hex!("00891815378597fe42d3fd261fe76df365845bbb87"), - dsa_keypair.public.q.as_bytes(), + dsa_keypair.public().q().as_bytes(), ); assert_eq!( &hex!( @@ -88,7 +88,7 @@ fn decode_dsa_openssh() { 520a713fe4104a74bed53fd5915da736365afd3f09777bbccfbadf7ac2b087b7f4d95fabe47d72a46e95088 f9cd2a9fbf236b58a6982647f3c00430ad7352d47a25ebbe9477f0c3127da86ad7448644b76de5875c" ), - dsa_keypair.public.g.as_bytes(), + dsa_keypair.public().g().as_bytes(), ); assert_eq!( &hex!( @@ -96,11 +96,11 @@ fn decode_dsa_openssh() { a57b475c78d44989f16577527e598334be6aae4abd750c36af80489d392697c1f32f3cf3c9a8b99bcddb53d 7a37e1a28fd53d4934131cf41c437c6734d1e04004adcd925b84b3956c30c3a3904eecb31400b0df48" ), - dsa_keypair.public.y.as_bytes(), + dsa_keypair.public().y().as_bytes(), ); assert_eq!( &hex!("0c377ac449e770d89a3557743cbd050396114b62"), - dsa_keypair.private.as_bytes() + dsa_keypair.private().as_bytes() ); assert_eq!("user@example.com", key.comment()); } diff --git a/ssh-key/tests/public_key.rs b/ssh-key/tests/public_key.rs index 4c8f2da..3cffb45 100644 --- a/ssh-key/tests/public_key.rs +++ b/ssh-key/tests/public_key.rs @@ -71,11 +71,11 @@ fn decode_dsa_openssh() { e3c48e2ccbafd2170f69e8e5c8b6ab69b9c5f45d95e1d9293e965227eee5b879b1123371c21b1db60f14b5e 5c05a4782ceb43a32f449647703063621e7a286bec95b16726c18b5e52383d00b297a6b03489b06068a5" ), - dsa_key.p.as_bytes(), + dsa_key.p().as_bytes(), ); assert_eq!( &hex!("00891815378597fe42d3fd261fe76df365845bbb87"), - dsa_key.q.as_bytes(), + dsa_key.q().as_bytes(), ); assert_eq!( &hex!( @@ -83,7 +83,7 @@ fn decode_dsa_openssh() { 520a713fe4104a74bed53fd5915da736365afd3f09777bbccfbadf7ac2b087b7f4d95fabe47d72a46e95088 f9cd2a9fbf236b58a6982647f3c00430ad7352d47a25ebbe9477f0c3127da86ad7448644b76de5875c" ), - dsa_key.g.as_bytes(), + dsa_key.g().as_bytes(), ); assert_eq!( &hex!( @@ -91,7 +91,7 @@ fn decode_dsa_openssh() { a57b475c78d44989f16577527e598334be6aae4abd750c36af80489d392697c1f32f3cf3c9a8b99bcddb53d 7a37e1a28fd53d4934131cf41c437c6734d1e04004adcd925b84b3956c30c3a3904eecb31400b0df48" ), - dsa_key.y.as_bytes(), + dsa_key.y().as_bytes(), ); assert_eq!("user@example.com", key.comment());