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.

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

Avoid accessing RSA struct's internals directly #370

Merged
merged 1 commit into from
Mar 27, 2024

Conversation

amirhosv
Copy link
Contributor

Description of changes:

AWS-LC has made the RSA struct opaque. This PR replaces direct accesses to internals of RSA struct with API calls. The FIPS builds do not have an API to disable blinding, therefore, for FIPS builds we still keep accessing the internals until a similar API is provided.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@amirhosv amirhosv marked this pull request as ready for review March 20, 2024 19:47
@amirhosv amirhosv requested a review from a team as a code owner March 20, 2024 19:47
// RSA blinding can't be performed without |e|; 0 indicates |e|'s absence.
rsa->flags |= RSA_FLAG_NO_BLINDING;
res = RSA_set0_key(rsa, modulus, NULL, privExp);
// RSA blinding can't be performed without |e|.
Copy link
Contributor

@WillChilds-Klein WillChilds-Klein Mar 26, 2024

Choose a reason for hiding this comment

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

nit: suggest moving this comment to doc comment for RSA_new_private_key_no_e

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll fix that in the next PR.

Comment on lines +191 to +193
// We need to change this API once AWS-LC provides a method similar to the following:
// https://github.com/google/boringssl/blob/master/include/openssl/rsa.h#L630
RSA_blinding_off_temp_for_accp_compatibility(rsa);
Copy link
Contributor

Choose a reason for hiding this comment

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

should we file an Issue against AWS-LC for a stable API to accomplish this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

AWS-LC has this on their radar.

@amirhosv amirhosv merged commit 4ec7ff6 into corretto:main Mar 27, 2024
10 checks passed
@amirhosv amirhosv deleted the rsa branch March 27, 2024 18:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants