ssh-key: simplified ECDSA private key decoding logic (#356)

Followup to #351

Also includes a fix and test to ensure that short keys zero-pad MSB
rather than LSB, since they're encoded as big endian.

Author: tarcieri

ssh-key/src/private/ecdsa.rs

@@ -39,27 +39,31 @@ impl<const SIZE: usize> Decode for EcdsaPrivateKey<SIZE> {
 
     fn decode(reader: &mut impl Reader) -> Result<Self> {
         reader.read_prefixed(|reader| {
-            let len = reader.remaining_len();
+            let mut len = reader.remaining_len();
+
+            // Strip leading zero if necessary:
+            // `mpint` is signed and may need a leading zero for unsigned integers
             if len == SIZE.checked_add(1).ok_or(encoding::Error::Length)? {
-                // Strip leading zero
                 // TODO(tarcieri): make sure leading zero was necessary
                 if u8::decode(reader)? != 0 {
                     return Err(Error::FormatEncoding);
                 }
+
+                len = SIZE;
             }
 
-            let mut bytes = [0u8; SIZE];
-            if SIZE == 66 {
-                // https://stackoverflow.com/questions/50002149/why-p-521-public-key-x-y-some-time-is-65-bytes-some-time-is-66-bytes
-                // although lower keys than 64 are vanishingly possible, but lets stop here
-                if len > 63 {
-                    reader.read(&mut bytes[..core::cmp::min(len, SIZE)])?;
-                } else {
-                    return Err(encoding::Error::Length.into());
-                }
-            } else {
-                reader.read(&mut bytes)?;
+            // Minimum allowed key size: may be smaller than modulus size
+            const MIN_SIZE: usize = 32;
+            if len < MIN_SIZE || len > SIZE {
+                return Err(encoding::Error::Length.into());
             }
+
+            // Add leading zeros if the encoded key is smaller than `SIZE`.
+            // The resulting value is big endian and needs leading zero padding.
+            let leading_zeros = SIZE.checked_sub(len).ok_or(encoding::Error::Length)?;
+
+            let mut bytes = [0u8; SIZE];
+            reader.read(&mut bytes[leading_zeros..])?;
             Ok(Self { bytes })
         })
     }

ssh-key/tests/private_key.rs

@@ -737,7 +737,15 @@ phY3prb3dzQGwtemFjemtvd3M=
     assert_eq!(
         key.algorithm(),
         Algorithm::Ecdsa {
-            curve: ssh_key::EcdsaCurve::NistP521
+            curve: EcdsaCurve::NistP521
         }
     );
+
+    let keypair_data = key.key_data().ecdsa().unwrap();
+    assert_eq!(
+        keypair_data.private_key_bytes(),
+        hex!(
+            "000f65a3a88314a64da9c750b55639b5e532dae02163c8049b8b5a7085a9e24f8f2cfbb3ee5edf2def95f49819e83df6ce8695b24a207093972b0e5f745d03ed50d4"
+        )
+    );
 }