ssh-key: fix ssh-dss signature encoding on 64-bit targets

Since switching from `BigUint::to_bytes_be()` to `BoxedUint::to_be_bytes()`
in 07f295e, `Signer<Signature> for DsaKeypair::try_sign` produces a
48-byte blob instead of the 40 bytes required by RFC 4253 §6.6, panicking
the debug-assert at `signature.rs:345` and breaking ssh-dss publickey
client auth in release builds.

Root cause: `dsa::Signature::r()`/`s()` are constructed at
`KeySize::n_aligned() == n.div_ceil(Limb::BITS) * Limb::BITS` precision.
For ssh-dss (n = 160) on 64-bit targets that's 192 bits, so
`to_be_bytes()` returns 24 bytes per component.

Trim the limb-alignment leading zeros down to `DSA_SIGNATURE_SIZE / 2`
before left-padding so values shorter than 160 bits still get padded.

Also bump dsa to 0.7.0-rc.14 to pick up the verify-side overflow fix
(`v1.concatenating_mul(&v2)` in `verifying_key.rs`); without it,
`DsaPublicKey::verify` panics with "attempted to multiply with overflow"
on 1024-bit p.

Reworked the existing `try_sign_and_verify_dsa` test so it asserts on the
encoded `signature.data.len() == DSA_SIGNATURE_SIZE` rather than on the
underlying `dsa` component byte lengths -- the old assertion (`r.len() ==
20`) couldn't hold on 64-bit given the alignment above.

Refs Eugeny/russh#705

Author: cwoolum

Cargo.lock

@@ -28,7 +28,7 @@ crypto-bigint = { version = "0.7.0-rc.28", features = ["alloc"] }
 argon2 = { version = "0.5", optional = true, default-features = false, features = ["alloc"] }
 bcrypt-pbkdf = { version = "0.10", optional = true, default-features = false, features = ["alloc"] }
 bigint = { package = "num-bigint-dig", version = "0.8.4", optional = true, default-features = false, features = ["std"] }
-dsa = { version = "0.7.0-rc.13", optional = true, default-features = false, features = ["hazmat"] }
+dsa = { version = "0.7.0-rc.14", optional = true, default-features = false, features = ["hazmat"] }
 ecdsa = { version = "0.17.0-rc.16", optional = true, default-features = false } # only need this explicit dependency due to a minimal-versions fail @ https://github.com/RustCrypto/elliptic-curves/issues/1038
 ed25519-dalek = { version = "3.0.0-pre.6", optional = true, default-features = false }
 hex = { version = "0.4", optional = true }

ssh-key/Cargo.toml

@@ -333,13 +333,19 @@ impl Signer<Signature> for DsaKeypair {
             .map_err(|_| signature::Error::new())?;
 
         // Encode the format specified in RFC4253 section 6.6: two raw 80-bit integers concatenated
-        let mut data = Vec::new();
+        let mut data = Vec::with_capacity(DSA_SIGNATURE_SIZE);
+        let half = DSA_SIGNATURE_SIZE / 2;
 
         for component in [signature.r(), signature.s()] {
-            let mut bytes = component.to_be_bytes();
-            let pad_len = (DSA_SIGNATURE_SIZE / 2).saturating_sub(bytes.len());
-            data.extend(iter::repeat(0).take(pad_len));
-            data.extend(&bytes);
+            // `BoxedUint::to_be_bytes()` is sized to `bits_precision`, which
+            // `dsa` rounds up to a multiple of `Limb::BITS` (24 bytes on 64-bit
+            // for n = 160). Since `dsa` enforces `r < q` and `s < q`, any bytes
+            // above the trailing `half` are zero and can be sliced off.
+            let bytes = component.to_be_bytes();
+            let bytes = &bytes[bytes.len().saturating_sub(half)..];
+            let pad_len = half.saturating_sub(bytes.len());
+            data.resize(data.len() + pad_len, 0);
+            data.extend_from_slice(bytes);
         }
 
         debug_assert_eq!(data.len(), DSA_SIGNATURE_SIZE);
@@ -887,65 +893,24 @@ mod tests {
         use encoding::Decode as _;
         use signature::{Signer as _, Verifier as _};
 
-        fn check_signature_component_lens(
-            keypair: &DsaKeypair,
-            data: &[u8],
-            r_len: usize,
-            s_len: usize,
-        ) {
-            use sha1::{Digest as _, Sha1};
-            use signature::DigestSigner as _;
-
-            let signature = dsa::SigningKey::try_from(keypair)
-                .expect("valid DSA signing key")
-                .try_sign_digest(|d: &mut Sha1| Ok(d.update(data)))
-                .expect("valid DSA signature");
-
-            let r = signature.r().to_be_bytes();
-            assert_eq!(
-                r.len(),
-                r_len,
-                "dsa signature component `r` has len {} != {}",
-                r.len(),
-                r_len
-            );
-            let s = signature.s().to_be_bytes();
-            assert_eq!(
-                s.len(),
-                s_len,
-                "dsa signature component `s` has len {} != {}",
-                s.len(),
-                s_len
-            );
-        }
-
         let keypair = hex!("0000008100c161fb30c9e4e3602c8510f93bbd48d813da845dfcc75f3696e440cd019d609809608cd592b8430db901d7b43740740045b547c60fb035d69f9c64d3dfbfb13bb3edd8ccfdd44705739a639eb70f4aed16b0b8355de1b21cd9d442eff250895573a8af7ce2fb71fb062e887482dab5c68139845fb8afafc5f3819dc782920d510000001500f3fb6762430332bd5950edc5cd1ae6f17b88514f0000008061ef1394d864905e8efec3b610b7288a6522893af2a475f910796e0de47c8b065d365e942e80e471d1e6d4abdee1d3d3ede7103c6996432f1a9f9a671a31388672d63555077911fc69e641a997087260d22cdbf4965aa64bb382204f88987890ec225a5a7723a977dc1ecc5e04cf678f994692b20470adbf697489f800817b920000008100a9a6f1b65fc724d65df7441908b34af66489a4a3872cbbba25ea1bcfc83f25c4af1a62e339eefc814907cfaf0cb6d2d16996212a32a27a63013f01c57d0630f0be16c8c69d16fc25438e613b904b98aeb3e7c356fa8e75ee1d474c9f82f1280c5a6c18e9e607fcf7586eefb75ea9399da893b807375ac1396fd586bf277161980000001500ced95f1c7bbb39be4987837ad1f71be31bb7b0d9");
         let keypair = DsaKeypair::decode(&mut &keypair[..]).expect("properly encoded DSA keypair");
 
         let data = hex!("F0000040713d5f6fffe0000e6421ab0b3a69774d3da02fd72b107d6b32b6dad7c1660bbf507bf3eac3304cc5058f7e6f81b04239b8471459b1f3b387e2626f7eb8f6bcdd3200000006626c616465320000000e7373682d636f6e6e656374696f6e00000009686f73746261736564000000077373682d647373000001b2000000077373682d6473730000008100c161fb30c9e4e3602c8510f93bbd48d813da845dfcc75f3696e440cd019d609809608cd592b8430db901d7b43740740045b547c60fb035d69f9c64d3dfbfb13bb3edd8ccfdd44705739a639eb70f4aed16b0b8355de1b21cd9d442eff250895573a8af7ce2fb71fb062e887482dab5c68139845fb8afafc5f3819dc782920d510000001500f3fb6762430332bd5950edc5cd1ae6f17b88514f0000008061ef1394d864905e8efec3b610b7288a6522893af2a475f910796e0de47c8b065d365e942e80e471d1e6d4abdee1d3d3ede7103c6996432f1a9f9a671a31388672d63555077911fc69e641a997087260d22cdbf4965aa64bb382204f88987890ec225a5a7723a977dc1ecc5e04cf678f994692b20470adbf697489f800817b920000008100a9a6f1b65fc724d65df7441908b34af66489a4a3872cbbba25ea1bcfc83f25c4af1a62e339eefc814907cfaf0cb6d2d16996212a32a27a63013f01c57d0630f0be16c8c69d16fc25438e613b904b98aeb3e7c356fa8e75ee1d474c9f82f1280c5a6c18e9e607fcf7586eefb75ea9399da893b807375ac1396fd586bf2771619800000015746f6d61746f7373682e6c6f63616c646f6d61696e00000009746f6d61746f737368");
-        check_signature_component_lens(
-            &keypair,
-            &data,
-            DSA_SIGNATURE_SIZE / 2,
-            DSA_SIGNATURE_SIZE / 2,
-        );
         let signature = keypair.try_sign(&data[..]).expect("dsa try_sign is ok");
+        assert_eq!(signature.as_bytes().len(), DSA_SIGNATURE_SIZE);
         keypair
             .public
             .verify(&data[..], &signature)
             .expect("dsa verify is ok");
 
+        // This input produces an `r` shorter than 160 bits, exercising the
+        // left-pad path for naturally short signature components.
         let data = hex!("00000040713d5f6fffe0000e6421ab0b3a69774d3da02fd72b107d6b32b6dad7c1660bbf507bf3eac3304cc5058f7e6f81b04239b8471459b1f3b387e2626f7eb8f6bcdd3200000006626c616465320000000e7373682d636f6e6e656374696f6e00000009686f73746261736564000000077373682d647373000001b2000000077373682d6473730000008100c161fb30c9e4e3602c8510f93bbd48d813da845dfcc75f3696e440cd019d609809608cd592b8430db901d7b43740740045b547c60fb035d69f9c64d3dfbfb13bb3edd8ccfdd44705739a639eb70f4aed16b0b8355de1b21cd9d442eff250895573a8af7ce2fb71fb062e887482dab5c68139845fb8afafc5f3819dc782920d510000001500f3fb6762430332bd5950edc5cd1ae6f17b88514f0000008061ef1394d864905e8efec3b610b7288a6522893af2a475f910796e0de47c8b065d365e942e80e471d1e6d4abdee1d3d3ede7103c6996432f1a9f9a671a31388672d63555077911fc69e641a997087260d22cdbf4965aa64bb382204f88987890ec225a5a7723a977dc1ecc5e04cf678f994692b20470adbf697489f800817b920000008100a9a6f1b65fc724d65df7441908b34af66489a4a3872cbbba25ea1bcfc83f25c4af1a62e339eefc814907cfaf0cb6d2d16996212a32a27a63013f01c57d0630f0be16c8c69d16fc25438e613b904b98aeb3e7c356fa8e75ee1d474c9f82f1280c5a6c18e9e607fcf7586eefb75ea9399da893b807375ac1396fd586bf2771619800000015746f6d61746f7373682e6c6f63616c646f6d61696e00000009746f6d61746f737368");
-        // verify that this data produces signature with `r` integer component that is less than 160 bits/20 bytes.
-        check_signature_component_lens(
-            &keypair,
-            &data,
-            DSA_SIGNATURE_SIZE / 2 - 1,
-            DSA_SIGNATURE_SIZE / 2,
-        );
         let signature = keypair
             .try_sign(&data[..])
-            .expect("dsa try_sign for r.len() == 19 is ok");
+            .expect("dsa try_sign for short `r` is ok");
+        assert_eq!(signature.as_bytes().len(), DSA_SIGNATURE_SIZE);
         keypair
             .public
             .verify(&data[..], &signature)