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

openssl3 integration: cleanup providers #3481

Merged
merged 3 commits into from
Sep 9, 2022
Merged

Conversation

toidiu
Copy link
Contributor

@toidiu toidiu commented Sep 9, 2022

Description of changes:

OSSL providers need to be cleaned up after load. This PR iterates over all providers via OSSL_PROVIDER_do_all and calls OSSL_PROVIDER_unload on each. The cleanup function is called before process end in s2n_cleanup_atexit_impl

Testing:

Manual runs of OSSL3 in CI:

a50ee3dbe417b8993e90da662590a891334c6b38

How is this change tested (unit tests, fuzz tests, etc.)? Are there any testing steps to be verified by the reviewer?

Is this a refactor change? If so, how have you proved that the intended behavior hasn't changed?

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@toidiu toidiu requested a review from a team as a code owner September 9, 2022 06:43
@github-actions github-actions bot added the s2n-core team label Sep 9, 2022
@toidiu toidiu mentioned this pull request Sep 9, 2022
10 tasks
@toidiu toidiu changed the title cleanup providers openssl3 integration: cleanup providers Sep 9, 2022
@toidiu toidiu requested a review from lrstewart September 9, 2022 19:56
Comment on lines +163 to +164

return S2N_RESULT_OK;
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit, kind of silly spacing

Suggested change
return S2N_RESULT_OK;
return S2N_RESULT_OK;

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 think its fine. It separates the important code from the return bit. We do this quite often actualy:

RESULT_ENSURE(s2n_constant_time_equals((const uint8_t *) expected_version_name, (const uint8_t *) s2n_libcrypto_get_version_name(), (const uint32_t) strlen(expected_version_name)), S2N_ERR_LIBCRYPTO_VERSION_NAME_MISMATCH);
return S2N_RESULT_OK;

Copy link
Contributor

Choose a reason for hiding this comment

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

We usually don't do it for single line functions :)

@toidiu toidiu merged commit 2b19314 into aws:main Sep 9, 2022
@toidiu toidiu deleted the ak-openssl3_gc branch September 9, 2022 21:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants