Skip to content

fix(pss): reject unrepresentable salt lengths in the AlgorithmIdentifier - #714

Open
choksi2212 wants to merge 1 commit into
RustCrypto:masterfrom
choksi2212:fix/pss-salt-len-truncation-703
Open

choksi2212 wants to merge 1 commit into
RustCrypto:masterfrom
choksi2212:fix/pss-salt-len-truncation-703

Conversation

@choksi2212

Copy link
Copy Markdown

Summary

SigningKey::signature_algorithm_identifier() and BlindedSigningKey::signature_algorithm_identifier() truncated salt_len from usize to u8 with an as cast before encoding it into the RSASSA-PSS AlgorithmIdentifier. For salt_len >= 256 the encoded RsaPssParams therefore carried a wrong salt length (e.g. 256 became 0), so any verifier that reads the AlgorithmIdentifier — as the issue's PoC does — used the wrong salt length and valid signatures failed verification.

RsaPssParams::salt_len is a u8, so a salt length of 256 or more cannot be represented in the parameters at all. Rather than silently truncating, get_pss_signature_algo_id now returns ErrorKind::Value { tag: Tag::Integer } for out-of-range lengths.

Fixes #703

Changes

  • get_pss_signature_algo_id takes salt_len: usize and converts it with u8::try_from, mapping overflow to der::ErrorKind::Value { tag: Tag::Integer } instead of as u8.
  • SigningKey / BlindedSigningKey pass self.salt_len through unchanged (no cast).
  • get_default_pss_signature_algo_id passes the digest output size as usize.
  • Doc notes on new_with_salt_len (both key types) that salt lengths of 256+ are fine for signing but signature_algorithm_identifier() errors for them.

Verification

  • Negative control, both directions: with the production change reverted to as u8 (tests kept), the new out-of-range tests FAIL — expect_err panics because pristine code returns a truncated-wrong AlgorithmIdentifier (256 encodes as 0, per the issue's PoC). With the fix, all pass.
  • New regression tests in src/pss.rs:
    • test_pss_algo_id_salt_len_out_of_range — 256, 300, usize::MAX error for both SigningKey and BlindedSigningKey.
    • test_pss_algo_id_salt_len_representable — 255 (exact u8 boundary), 128: encoding succeeds and a DER decode of the parameters round-trips the exact salt length, for both key types.
    • test_pss_algo_id_default_salt_len_unchanged — the default digest-size path is unaffected.
  • cargo test --release — 126 passed / 0 failed across all targets (including wycheproof, with submodules initialized).
  • cargo clippy --lib --tests --features encoding with RUSTFLAGS="-Dwarnings" — clean.
  • cargo fmt --check — clean.
  • MSRV: cargo test on 1.85.0 — all pss tests pass (only APIs available since well before the MSRV are used).
  • cargo build --no-default-features — clean (change is encoding-gated).

Note: signing itself still supports any RFC 8017 salt length (emLen - hLen - 2); only the AlgorithmIdentifier encoding is constrained by the u8 in RsaPssParams. Making the parameters carry larger lengths would require widening the type in pkcs1 (an upstream breaking change), so failing loudly is the correct behavior at this layer.

`signature_algorithm_identifier()` truncated `salt_len` from `usize` to `u8`
with an `as` cast. For `salt_len >= 256` the encoded `RsaPssParams` carried
a wrong salt length (e.g. 256 -> 0), so any verifier reading the
`AlgorithmIdentifier` used the wrong salt length and valid signatures failed
verification.

`RsaPssParams::salt_len` is a `u8`, so lengths of 256 or more cannot be
represented at all; return `ErrorKind::Value` instead of silently truncating.

Fixes RustCrypto#703

Signed-off-by: choksi2212 <choksi2212@users.noreply.github.com>

This branch has not been deployed

No deployments
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.

Silent salt_len truncation in signature_algorithm_identifier() causes signature verification failures

1 participant