Skip to content

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

Open
cwoolum wants to merge 1 commit into
Eugeny:russh-current-0.6.7from
cwoolum:fix/dsa-sign-leading-zero-trim
Open

ssh-key: fix ssh-dss signature encoding on 64-bit targets#4
cwoolum wants to merge 1 commit into
Eugeny:russh-current-0.6.7from
cwoolum:fix/dsa-sign-leading-zero-trim

Conversation

@cwoolum
Copy link
Copy Markdown

@cwoolum cwoolum commented May 12, 2026

Refs Eugeny/russh#705

Problem

Since 07f295e8 ("bump crypto packages"), Signer<Signature> for DsaKeypair::try_sign produces a 48-byte signature blob instead of the 40 bytes required by RFC 4253 §6.6 on 64-bit targets. The debug_assert_eq!(data.len(), DSA_SIGNATURE_SIZE) at signature.rs:345 panics in debug, and release builds emit an invalid signature that the peer rejects. This breaks ssh-dss publickey client auth in any russh 0.60.x consumer (russh 0.60 pins =0.6.18).

Cause

That commit migrated r/s encoding from BigUint::to_bytes_be() (minimum-length) to BoxedUint::to_be_bytes() (sized to bits_precision). dsa constructs r/s at KeySize::n_aligned() precision:

// dsa/src/size.rs
pub(crate) fn n_aligned(&self) -> u32 {
    self.n.div_ceil(Limb::BITS) * Limb::BITS
}

For an ssh-dss key (n = 160) on a 64-bit target (Limb::BITS = 64), this rounds up to 192 bits → 24 bytes per component, 48 bytes total.

Fix

dsa enforces r < q and s < q, so the bytes above the trailing 20 are always zero — slice them off, and left-pad the rare case where the natural representation is shorter than 20 bytes:

for component in [signature.r(), signature.s()] {
    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);
}

Also bumps dsa to 0.7.0-rc.14, which fixes a verify-side attempted to multiply with overflow panic in verifying_key.rs:105 for 1024-bit p (rc.14 uses concatenating_mul instead of *). Without that bump, DsaPublicKey::verify still panics even with the encoding fix.

Test

Reworked try_sign_and_verify_dsa to assert on signature.as_bytes().len() == DSA_SIGNATURE_SIZE and roundtrip through verify, instead of asserting on the underlying dsa component byte lengths (the prior r.len() == 20 assertion can't hold on 64-bit given the alignment above).

Both data points from the original test (one where r is naturally 160 bits, one where it's shorter and needs left-padding) are kept.

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
@cwoolum cwoolum force-pushed the fix/dsa-sign-leading-zero-trim branch from 86ce6fd to 864a197 Compare May 12, 2026 01:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant