KMS: fix RSA PSS signing issue for salt length #12467
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
LocalStack Community integration with Pro 2 files ± 0 2 suites ±0 4m 3s ⏱️ - 1h 47m 36s Results for commit aa692da. ± Comparison against base commit 9383d50. This pull request removes 3636 and adds 8 tests. Note that renamed tests count towards both. |
silv-io
approved these changes
Apr 2, 2025
There was a problem hiding this comment.
LGTM! First I got a bit confused because Section 3.3 in the RFC doesn't mention the recommended salt length. However, 3.1 does:
saltLength
The saltLength field is the octet length of the salt. For a
given hashAlgorithm, the recommended value of saltLength is the
number of octets in the hash value. Unlike the other fields of
type RSASSA-PSS-params, saltLength does not need to be fixed
for a given RSA key pair; a different value could be used for
each RSASSA-PSS signature generated.
So using the digest (hash) length here is indeed more correct.
Awesome work on the test as well! :)
k-a-il
requested changes
Apr 3, 2025
Comment on lines
+749
to
+760
| @pytest.mark.parametrize( | ||
| "key_spec,sign_algo", | ||
| [ | ||
| ("RSA_2048", "RSASSA_PSS_SHA_256"), | ||
| ("RSA_2048", "RSASSA_PSS_SHA_384"), | ||
| ("RSA_2048", "RSASSA_PSS_SHA_512"), | ||
| ("RSA_4096", "RSASSA_PKCS1_V1_5_SHA_256"), | ||
| ("RSA_4096", "RSASSA_PKCS1_V1_5_SHA_512"), | ||
| ("ECC_NIST_P256", "ECDSA_SHA_256"), | ||
| ("ECC_NIST_P384", "ECDSA_SHA_384"), | ||
| ("ECC_SECG_P256K1", "ECDSA_SHA_256"), | ||
| ], |
k-a-il
approved these changes
Apr 4, 2025
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.
Motivation
When using an RSA key to sign with PSS algorithm, currently the salt length is set to be
MAX_LENGTH, which is not in accordance with the ones required by RFC 4055 making making it difficult to verify such signatures across platforms: https://datatracker.ietf.org/doc/html/rfc4055#section-3.3.Closes #9602
Changes
This PR:
cryptographylibrary.