ssh-key: make all DSA key fields private (#275)

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.
RustCrypto · Aug 13, 2024 · c0f531d · c0f531d
1 parent 1be857b
commit c0f531d
Show file tree

Hide file tree

Showing 7 changed files with 90 additions and 29 deletions.
diff --git 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<Self> {
+        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<Self> {
-        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<Self> {
+        // 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<Self> {
         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
@@ -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
@@ -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<Self> {
+        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
@@ -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
@@ -74,27 +74,27 @@ 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!(
             "4739b3908a8415466dc7b156fb98ecb71552a170ba0b3b7aa81bd81391de0a7ae7a1b45002dfeadc9225fbc
              520a713fe4104a74bed53fd5915da736365afd3f09777bbccfbadf7ac2b087b7f4d95fabe47d72a46e95088
              f9cd2a9fbf236b58a6982647f3c00430ad7352d47a25ebbe9477f0c3127da86ad7448644b76de5875c"
         ),
-        dsa_key.g.as_bytes(),
+        dsa_key.g().as_bytes(),
     );
     assert_eq!(
         &hex!(
             "6042a6b3fd861344cb21ccccd8719e25aa0be0980e79cbabf4877f5ef071f6039770352eac3d4c368f29daf
              a57b475c78d44989f16577527e598334be6aae4abd750c36af80489d392697c1f32f3cf3c9a8b99bcddb53d
              7a37e1a28fd53d4934131cf41c437c6734d1e04004adcd925b84b3956c30c3a3904eecb31400b0df48"
         ),
-        dsa_key.y.as_bytes(),
+        dsa_key.y().as_bytes(),
     );
 
     assert_eq!("[email protected]", cert.comment());

diff --git a/ssh-key/tests/private_key.rs b/ssh-key/tests/private_key.rs
@@ -76,31 +76,31 @@ 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!(
             "4739b3908a8415466dc7b156fb98ecb71552a170ba0b3b7aa81bd81391de0a7ae7a1b45002dfeadc9225fbc
              520a713fe4104a74bed53fd5915da736365afd3f09777bbccfbadf7ac2b087b7f4d95fabe47d72a46e95088
              f9cd2a9fbf236b58a6982647f3c00430ad7352d47a25ebbe9477f0c3127da86ad7448644b76de5875c"
         ),
-        dsa_keypair.public.g.as_bytes(),
+        dsa_keypair.public().g().as_bytes(),
     );
     assert_eq!(
         &hex!(
             "6042a6b3fd861344cb21ccccd8719e25aa0be0980e79cbabf4877f5ef071f6039770352eac3d4c368f29daf
              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!("[email protected]", key.comment());
 }

diff --git a/ssh-key/tests/public_key.rs b/ssh-key/tests/public_key.rs
@@ -71,27 +71,27 @@ 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!(
             "4739b3908a8415466dc7b156fb98ecb71552a170ba0b3b7aa81bd81391de0a7ae7a1b45002dfeadc9225fbc
              520a713fe4104a74bed53fd5915da736365afd3f09777bbccfbadf7ac2b087b7f4d95fabe47d72a46e95088
              f9cd2a9fbf236b58a6982647f3c00430ad7352d47a25ebbe9477f0c3127da86ad7448644b76de5875c"
         ),
-        dsa_key.g.as_bytes(),
+        dsa_key.g().as_bytes(),
     );
     assert_eq!(
         &hex!(
             "6042a6b3fd861344cb21ccccd8719e25aa0be0980e79cbabf4877f5ef071f6039770352eac3d4c368f29daf
              a57b475c78d44989f16577527e598334be6aae4abd750c36af80489d392697c1f32f3cf3c9a8b99bcddb53d
              7a37e1a28fd53d4934131cf41c437c6734d1e04004adcd925b84b3956c30c3a3904eecb31400b0df48"
         ),
-        dsa_key.y.as_bytes(),
+        dsa_key.y().as_bytes(),
     );
 
     assert_eq!("[email protected]", key.comment());