Repository navigation
fix(pss): reject unrepresentable salt lengths in the AlgorithmIdentifier - #714
Open
choksi2212 wants to merge 1 commit into
Open
choksi2212 wants to merge 1 commit into
choksi2212 wants to merge 1 commit into
Conversation
`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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
SigningKey::signature_algorithm_identifier()andBlindedSigningKey::signature_algorithm_identifier()truncatedsalt_lenfromusizetou8with anascast before encoding it into the RSASSA-PSSAlgorithmIdentifier. Forsalt_len >= 256the encodedRsaPssParamstherefore carried a wrong salt length (e.g.256became0), so any verifier that reads theAlgorithmIdentifier— as the issue's PoC does — used the wrong salt length and valid signatures failed verification.RsaPssParams::salt_lenis au8, so a salt length of 256 or more cannot be represented in the parameters at all. Rather than silently truncating,get_pss_signature_algo_idnow returnsErrorKind::Value { tag: Tag::Integer }for out-of-range lengths.Fixes #703
Changes
get_pss_signature_algo_idtakessalt_len: usizeand converts it withu8::try_from, mapping overflow toder::ErrorKind::Value { tag: Tag::Integer }instead ofas u8.SigningKey/BlindedSigningKeypassself.salt_lenthrough unchanged (no cast).get_default_pss_signature_algo_idpasses the digest output size asusize.new_with_salt_len(both key types) that salt lengths of 256+ are fine for signing butsignature_algorithm_identifier()errors for them.Verification
as u8(tests kept), the new out-of-range tests FAIL —expect_errpanics because pristine code returns a truncated-wrongAlgorithmIdentifier(256encodes as0, per the issue's PoC). With the fix, all pass.src/pss.rs:test_pss_algo_id_salt_len_out_of_range—256,300,usize::MAXerror for bothSigningKeyandBlindedSigningKey.test_pss_algo_id_salt_len_representable—255(exactu8boundary),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 encodingwithRUSTFLAGS="-Dwarnings"— clean.cargo fmt --check— clean.cargo teston 1.85.0 — all pss tests pass (only APIs available since well before the MSRV are used).cargo build --no-default-features— clean (change isencoding-gated).Note: signing itself still supports any RFC 8017 salt length (
emLen - hLen - 2); only theAlgorithmIdentifierencoding is constrained by theu8inRsaPssParams. Making the parameters carry larger lengths would require widening the type inpkcs1(an upstream breaking change), so failing loudly is the correct behavior at this layer.