Remove 'unsalted' PSS handling #294
Merged
Conversation
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
tarcieri
approved these changes
Apr 15, 2023
Member
tarcieri
left a comment
tarcieri
left a comment
There was a problem hiding this comment.
Looks good to me!
I don't understand the potential real-world usages of unsalted PSS, but from a theoretical perspective it makes no sense: the purpose of having a salt is to increase the tightness of the security proof. I don't understand why one would want to use PSS in a way which defeats the security proof.
So that said, I'm all for removing the unsalted API.
I think we could further improve the API by having one where the default salt length equals the output length of the digest function, but I'm happy to open a followup PR to add that.
Contributor
Author
|
@tarcieri sounds good to me. |
Member
|
@lumag alternatively, you could implement ^^^ proposed API in this PR, which would avoid the need to deprecate anything. Instead, change (Or I'm still happy to do that as a followup, which would simplify the overall changes and help organize discussion perhaps) |
Contributor
Author
|
Sure, I'll do that in a minute |
Current new() and random() functions cause confusion. There is the default from ASN.1 encoding of RSAPSS parameters (20). There is also another default of (mod_size - 2 - hash_size). And there is a recommendation to use salt_len of hash_size. Drop old defaults and always use digest output size as the salt_len. Clearly document new default. Signed-off-by: Dmitry Baryshkov <[email protected]>
All RSA PSS standards (e.g. RFC 8017) clearly specify that RSA PSS verification has an explicit salt length parameter (rather than determining it from the message). Drop our 'automagic' code and pass salt length when verifying the message. Old functions now default to digest output size as a hash length. Signed-off-by: Dmitry Baryshkov <[email protected]>
The emsa_pss_get_salt() is possibly non-constant-time op. Change it to be a contant-time operation. Signed-off-by: Dmitry Baryshkov <[email protected]>
Contributor
Author
|
done |
tarcieri
approved these changes
Apr 15, 2023
Merged
tarcieri
pushed a commit
that referenced
this pull request
Aug 25, 2025
…n during verification (#546) This PR adds an optional method to automatically detect the salt length during RSA-PSS signature verification. Should Fix: #361 and similar situations. ### Problem The crate currently requires a fixed salt length for PSS verification. This prevents verification of signatures where the salt length is not known beforehand, a situation not uncommon in interoperability contexts. This capability was previously available but was removed in PR #294. ### Solution This change re-introduces salt length auto-detection as an explicit, opt-in feature. A new constructor, `VerifyingKey::new_with_auto_salt_len`, creates a verifier that performs this detection during verification. * The change is opt-in and does not affect default behavior. * It improves interoperability by handling signatures with variable / unknown salt lengths. * The salt detection logic is implemented in constant(-ish) time to resist timing attacks. ### Commit Structure This PR consists of three commits: 1. **test:** Adds a failing unit test to demonstrate the issue. 2. **feat:** Implements `new_with_auto_salt_len` and re-introduces the previous detection logic. 3. **fix:** Updates the detection logic to be less prone to timing attacks. Because salt length auto-detection is OpenSSL's default, it's likely that many systems were built without a mechanism to enforce or communicate a fixed salt length. Without this PR, it was impossible to reliably use this crate in my current project, and I suspect others face the same blocker. I've lost quite a bit of time dealing with seemingly random signature verification failures until I realized the crux of the problem.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
The 'unsalted' PSS handling is not behaving like one would expect. It doesn't use salt of 0 length, but insteaad it uses some defaults which are not properly documented. Make the
salt_lenmandatatory for both singing and verification.