Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
cwoolum wants to merge 1 commit into
base: russh-current-0.6.7
Choose a base branch
from
+19 −54
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion ssh-key/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
67 changes: 16 additions & 51 deletions ssh-key/src/signature.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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)
Expand Down