Repository navigation
pss: pin salt length bounds in the AlgorithmIdentifier tests - #730
Open
choksi2212 wants to merge 1 commit into
Open
choksi2212 wants to merge 1 commit into
choksi2212 wants to merge 1 commit into
Conversation
Complements the rejection added in RustCrypto#704 with the tests from RustCrypto#714: - representable salt lengths (255 boundary, 128) DER-round-trip through the encoded AlgorithmIdentifier parameters back to the configured salt_len, for both SigningKey and BlindedSigningKey (pins that the advertised parameters carry the right value, not just that encoding succeeds) - the out-of-range matrix (256, 300, usize::MAX) errors on both key types instead of truncating to u8 (verified: all three fail when the u8::try_from guard is removed) - the digest-size default path still encodes with the digest's output size Refs RustCrypto#703 Signed-off-by: Manas Choksi <12302040701105@mbit.edu.in>
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.
Tests-only follow-up to #704, carrying over the additional
psssalt length tests from #714, as invited in #714 (comment). No production code is touched.What the tests pin
RsaPssParams::salt_lenis au8, so asalt_lenof 256 or more cannot be represented in the PSSAlgorithmIdentifier. Before #704, theas u8cast silently truncated such lengths (256 wrapped to 0), advertising a wrong salt length to verifiers and making valid signatures fail verification (issue #703). #704 added theu8::try_fromrejection; this PR pins it down:test_pss_algo_id_salt_len_representable(255, 128): the encodedAlgorithmIdentifierparameters DER-round-trip back to the configuredsalt_len, for bothSigningKeyandBlindedSigningKey. This is stronger than anis_ok()check: it catches any future encoding change that succeeds but advertises a wrong value.test_pss_algo_id_salt_len_out_of_range(256, 300,usize::MAX): both key types error instead of truncating.test_pss_algo_id_default_salt_len_unchanged: the digest-size default (get_default_pss_signature_algo_id) still encodes and carries the digest's output size, i.e. the validation does not alter the default path.Verification
u8::try_fromguard removed (truncation restored), the three out-of-range cases fail (encoding wrongly succeeds) while the representable and default tests keep passing, so each test fails exactly when its target behavior regresses.cargo test(default features): all suites green (87 lib + integration).cargo test --all-features: all suites green (102 lib + integration).cargo clippy --all -- -D warnings(the repo's CI invocation): clean.cargo fmt --check: clean.Refs #703