Support for wrapping EC keys + RSA wrapping fixes/comments/questions #5
Conversation
This file contains 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
simo5
reviewed
Feb 28, 2024
simo5
requested changes
Feb 29, 2024
Jakuje
force-pushed
the
ecc-wrap
branch
2 times, most recently
from
c738707 to
2982d52
Compare
simo5
changed the title
Signed-off-by: Jakub Jelen <[email protected]>
Signed-off-by: Jakub Jelen <[email protected]>
Signed-off-by: Jakub Jelen <[email protected]>
Signed-off-by: Jakub Jelen <[email protected]>
Signed-off-by: Jakub Jelen <[email protected]>
Signed-off-by: Jakub Jelen <[email protected]>
Signed-off-by: Jakub Jelen <[email protected]>
Signed-off-by: Jakub Jelen <[email protected]>
simo5
approved these changes
Mar 1, 2024
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.
Fixes also unwrapping for RSA keys, but we might need better solution.
I think the
DerEncBigUintensures the BN is unsigned and starts with zero byte, which is something we do not want when we import the BN as a modulus as you are using the modulus size to enforce the buffer size. Without stripping the null byte from the modulus, the operation requires 257 B buffer for signature and verification of this signature does not work.The RSA wrapping is also using the
RSAPrivateKeyASN1 directly, but from my reading of the PKCS#11 specification, we should usePrivateKeyInfo. This not obvious for RSA keys as everything needed is in the inner object, but EC keys needs theprivateKeyAlgorithmfield to learn what curve is used, if I read it right.Keeping it as a draft as there is more quesitons to resolve before merging.