ssh-key: make RsaPublicKey fields private; add key_size (#269)

Additionally: - Adds `Mpint::is_positive` - Ensures `e` and `n` values of `RsaPublicKey` are always positive Closes #261
RustCrypto · Aug 13, 2024 · 1c24d90 · 1c24d90
1 parent e0d423b
commit 1c24d90
Show file tree

Hide file tree

Showing 6 changed files with 77 additions and 24 deletions.
diff --git a/ssh-key/src/mpint.rs b/ssh-key/src/mpint.rs
@@ -96,6 +96,11 @@ impl Mpint {
             _ => None,
         }
     }
+
+    /// Is this [`Mpint`] positive?
+    pub fn is_positive(&self) -> bool {
+        self.as_positive_bytes().is_some()
+    }
 }
 
 impl AsRef<[u8]> for Mpint {

diff --git a/ssh-key/src/private/rsa.rs b/ssh-key/src/private/rsa.rs
@@ -136,7 +136,7 @@ impl Decode for RsaKeypair {
     fn decode(reader: &mut impl Reader) -> Result<Self> {
         let n = Mpint::decode(reader)?;
         let e = Mpint::decode(reader)?;
-        let public = RsaPublicKey { n, e };
+        let public = RsaPublicKey::new(e, n)?;
         let private = RsaPrivateKey::decode(reader)?;
         Ok(RsaKeypair { public, private })
     }
@@ -145,16 +145,16 @@ impl Decode for RsaKeypair {
 impl Encode for RsaKeypair {
     fn encoded_len(&self) -> encoding::Result<usize> {
         [
-            self.public.n.encoded_len()?,
-            self.public.e.encoded_len()?,
+            self.public.n().encoded_len()?,
+            self.public.e().encoded_len()?,
             self.private.encoded_len()?,
         ]
         .checked_sum()
     }
 
     fn encode(&self, writer: &mut impl Writer) -> encoding::Result<()> {
-        self.public.n.encode(writer)?;
-        self.public.e.encode(writer)?;
+        self.public.n().encode(writer)?;
+        self.public.e().encode(writer)?;
         self.private.encode(writer)
     }
 }
@@ -194,8 +194,8 @@ impl TryFrom<&RsaKeypair> for rsa::RsaPrivateKey {
 
     fn try_from(key: &RsaKeypair) -> Result<rsa::RsaPrivateKey> {
         let ret = rsa::RsaPrivateKey::from_components(
-            rsa::BigUint::try_from(&key.public.n)?,
-            rsa::BigUint::try_from(&key.public.e)?,
+            rsa::BigUint::try_from(key.public.n())?,
+            rsa::BigUint::try_from(key.public.e())?,
             rsa::BigUint::try_from(&key.private.d)?,
             vec![
                 rsa::BigUint::try_from(&key.private.p)?,

diff --git a/ssh-key/src/public/rsa.rs b/ssh-key/src/public/rsa.rs
@@ -17,16 +17,55 @@ use {
 #[derive(Clone, Debug, Eq, Ord, PartialEq, PartialOrd)]
 pub struct RsaPublicKey {
     /// RSA public exponent.
-    pub e: Mpint,
+    e: Mpint,
 
     /// RSA modulus.
-    pub n: Mpint,
+    n: Mpint,
+
+    /// Length of this key in bits.
+    bits: u32,
 }
 
 impl RsaPublicKey {
     /// Minimum allowed RSA key size.
     #[cfg(feature = "rsa")]
     pub(crate) const MIN_KEY_SIZE: usize = RsaKeypair::MIN_KEY_SIZE;
+
+    /// Create a new [`RsaPublicKey`] with the given components:
+    ///
+    /// - `e`: RSA public exponent.
+    /// - `n`: RSA modulus.
+    pub fn new(e: Mpint, n: Mpint) -> Result<Self> {
+        if !e.is_positive() {
+            return Err(Error::FormatEncoding);
+        }
+
+        let bits = match n.as_positive_bytes() {
+            Some(bytes) => bytes
+                .len()
+                .checked_mul(8)
+                .and_then(|bits| u32::try_from(bits).ok())
+                .ok_or(Error::FormatEncoding)?,
+            None => return Err(Error::FormatEncoding),
+        };
+
+        Ok(Self { e, n, bits })
+    }
+
+    /// Get the RSA public exponent.
+    pub fn e(&self) -> &Mpint {
+        &self.e
+    }
+
+    /// Get the RSA modulus.
+    pub fn n(&self) -> &Mpint {
+        &self.n
+    }
+
+    /// Get the size of the RSA modulus in bits.
+    pub fn key_size(&self) -> u32 {
+        self.bits
+    }
 }
 
 impl Decode for RsaPublicKey {
@@ -35,7 +74,7 @@ impl Decode for RsaPublicKey {
     fn decode(reader: &mut impl Reader) -> Result<Self> {
         let e = Mpint::decode(reader)?;
         let n = Mpint::decode(reader)?;
-        Ok(Self { e, n })
+        Self::new(e, n)
     }
 }
 
@@ -100,10 +139,9 @@ impl TryFrom<&rsa::RsaPublicKey> for RsaPublicKey {
     type Error = Error;
 
     fn try_from(key: &rsa::RsaPublicKey) -> Result<RsaPublicKey> {
-        Ok(RsaPublicKey {
-            e: key.e().try_into()?,
-            n: key.n().try_into()?,
-        })
+        let e = Mpint::try_from(key.e())?;
+        let n = Mpint::try_from(key.n())?;
+        RsaPublicKey::new(e, n)
     }
 }
 

diff --git a/ssh-key/tests/certificate.rs b/ssh-key/tests/certificate.rs
@@ -165,7 +165,13 @@ fn decode_rsa_4096_openssh() {
     assert_eq!(Algorithm::Rsa { hash: None }, cert.public_key().algorithm());
 
     let rsa_key = cert.public_key().rsa().unwrap();
-    assert_eq!(&hex!("010001"), rsa_key.e.as_bytes());
+    dbg!(rsa_key.n().as_bytes());
+    dbg!(
+        rsa_key.n().as_positive_bytes(),
+        rsa_key.n().as_positive_bytes().unwrap().len()
+    );
+    assert_eq!(4096, rsa_key.key_size());
+    assert_eq!(&hex!("010001"), rsa_key.e().as_bytes());
     assert_eq!(
         &hex!(
             "00b45911edc6ec5e7d2261a48c46ab889b1858306271123e6f02dc914cf3c0352492e8a6b7a7925added527
@@ -181,7 +187,7 @@ fn decode_rsa_4096_openssh() {
              4814140f75cac08079431043222fb91f075d76be55cbe138e3b99a605c561c49dea50e253c8306c4f4f77d9
              96f898db64c5d8a0a15c6efa28b0934bf0b6f2b01950d877230fe4401078420fd6dd3"
         ),
-        rsa_key.n.as_bytes(),
+        rsa_key.n().as_bytes(),
     );
 
     assert_eq!("[email protected]", cert.comment());

diff --git a/ssh-key/tests/private_key.rs b/ssh-key/tests/private_key.rs
@@ -268,7 +268,8 @@ fn decode_rsa_3072_openssh() {
     assert!(key.kdf().is_none());
 
     let rsa_keypair = key.key_data().rsa().unwrap();
-    assert_eq!(&hex!("010001"), rsa_keypair.public.e.as_bytes());
+    assert_eq!(3072, rsa_keypair.public.key_size());
+    assert_eq!(&hex!("010001"), rsa_keypair.public.e().as_bytes());
     assert_eq!(
         &hex!(
             "00a68e478c9bc93726436b7f5e9e6f9a46e1b73bec1e8cb7754de2c6a5b6c455f2f012a7259afcf94181d69
@@ -281,7 +282,7 @@ fn decode_rsa_3072_openssh() {
              0549d3174b85bd7f6624c3753cf235b650d0e4228f32be7b54a590d869fb7786559bb7a4d66f9d3a69c085e
              fdf083a915d47a1d9161a08756b263b06e739d99f2890362abc96ade42cce8f939a40daff9"
         ),
-        rsa_keypair.public.n.as_bytes(),
+        rsa_keypair.public.n().as_bytes(),
     );
     assert_eq!(
         &hex!(
@@ -340,7 +341,8 @@ fn decode_rsa_4096_openssh() {
     assert!(key.kdf().is_none());
 
     let rsa_keypair = key.key_data().rsa().unwrap();
-    assert_eq!(&hex!("010001"), rsa_keypair.public.e.as_bytes());
+    assert_eq!(4096, rsa_keypair.public.key_size());
+    assert_eq!(&hex!("010001"), rsa_keypair.public.e().as_bytes());
     assert_eq!(
         &hex!(
             "00b45911edc6ec5e7d2261a48c46ab889b1858306271123e6f02dc914cf3c0352492e8a6b7a7925added527
@@ -356,7 +358,7 @@ fn decode_rsa_4096_openssh() {
              4814140f75cac08079431043222fb91f075d76be55cbe138e3b99a605c561c49dea50e253c8306c4f4f77d9
              96f898db64c5d8a0a15c6efa28b0934bf0b6f2b01950d877230fe4401078420fd6dd3"
         ),
-        rsa_keypair.public.n.as_bytes(),
+        rsa_keypair.public.n().as_bytes(),
     );
     assert_eq!(
         &hex!(

diff --git a/ssh-key/tests/public_key.rs b/ssh-key/tests/public_key.rs
@@ -220,7 +220,8 @@ fn decode_rsa_3072_openssh() {
     assert_eq!(Algorithm::Rsa { hash: None }, key.key_data().algorithm());
 
     let rsa_key = key.key_data().rsa().unwrap();
-    assert_eq!(&hex!("010001"), rsa_key.e.as_bytes());
+    assert_eq!(rsa_key.key_size(), 3072);
+    assert_eq!(&hex!("010001"), rsa_key.e().as_bytes());
     assert_eq!(
         &hex!(
             "00a68e478c9bc93726436b7f5e9e6f9a46e1b73bec1e8cb7754de2c6a5b6c455f2f012a7259afcf94181d69
@@ -233,7 +234,7 @@ fn decode_rsa_3072_openssh() {
              0549d3174b85bd7f6624c3753cf235b650d0e4228f32be7b54a590d869fb7786559bb7a4d66f9d3a69c085e
              fdf083a915d47a1d9161a08756b263b06e739d99f2890362abc96ade42cce8f939a40daff9"
         ),
-        rsa_key.n.as_bytes(),
+        rsa_key.n().as_bytes(),
     );
     assert_eq!("[email protected]", key.comment());
     assert_eq!(
@@ -249,7 +250,8 @@ fn decode_rsa_4096_openssh() {
     assert_eq!(Algorithm::Rsa { hash: None }, key.key_data().algorithm());
 
     let rsa_key = key.key_data().rsa().unwrap();
-    assert_eq!(&hex!("010001"), rsa_key.e.as_bytes());
+    assert_eq!(rsa_key.key_size(), 4096);
+    assert_eq!(&hex!("010001"), rsa_key.e().as_bytes());
     assert_eq!(
         &hex!(
             "00b45911edc6ec5e7d2261a48c46ab889b1858306271123e6f02dc914cf3c0352492e8a6b7a7925added527
@@ -265,7 +267,7 @@ fn decode_rsa_4096_openssh() {
              4814140f75cac08079431043222fb91f075d76be55cbe138e3b99a605c561c49dea50e253c8306c4f4f77d9
              96f898db64c5d8a0a15c6efa28b0934bf0b6f2b01950d877230fe4401078420fd6dd3"
         ),
-        rsa_key.n.as_bytes(),
+        rsa_key.n().as_bytes(),
     );
     assert_eq!("[email protected]", key.comment());
     assert_eq!(