Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Check RSA-OAEP mechanims when decrypting #671

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

Check RSA-OAEP mechanims when decrypting #671

wants to merge 1 commit into from

Conversation

Jakuje
Copy link
Contributor

@Jakuje Jakuje commented Mar 14, 2022

The same check is in all the other methods handling the RSA-OAEP
encryption, wrapping and unwrapping, but for some reason, it was missing
in the decryption operation.

Related: OpenSC/libp11#440

@Jakuje Jakuje mentioned this pull request Mar 14, 2022
Merged
@jschlyter jschlyter marked this pull request as draft November 29, 2024 16:21
@jschlyter
Copy link
Contributor

Please rebase on develop and mark as ready when ready.

@Jakuje Jakuje marked this pull request as ready for review November 29, 2024 16:23
@Jakuje Jakuje requested a review from a team as a code owner November 29, 2024 16:23
@Jakuje
Copy link
Contributor Author

Jakuje commented Nov 29, 2024

Please rebase on develop and mark as ready when ready.

done

@Jakuje
Check RSA-OAEP mechanims when decrypting
3e46092 
The same check is in all the other methods handling the RSA-OAEP
encryption, wrapping and unwrapping, but for some reason, it was missing
in the decryption operation.

Signed-off-by: Jakub Jelen <jjelen@redhat.com>
@jschlyter jschlyter requested a review from bjosv November 29, 2024 18:15
bjosv
bjosv approved these changes Nov 29, 2024
View reviewed changes
Copy link
Contributor

@bjosv bjosv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good finding, we will check that params->source == CKZ_DATA_SPECIFIED now.
I wonder if we could improve our tests in the near future to cover this, any idea?

@Jakuje
Copy link
Contributor Author

Jakuje commented Nov 29, 2024

We discussed the CKZ_DATA_SPECIFIED issue and the specs is not exactly clear if it should be always 1 ( CKZ_DATA_SPECIFIED) or it could be also 0 in cases there are no additional data. But this is mostly on a side and should not affect this PR.

Indeed, more tests could help for all the operations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bjosv bjosv bjosv approved these changes

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Jakuje @jschlyter @bjosv