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

Fix OPENSSL_cleanup() detection without using our own atexit() handler #551

Merged
merged 1 commit into from Sep 11, 2020

Conversation

dwmw2
Copy link
Contributor

@dwmw2 dwmw2 commented May 13, 2020

We can't register our own atexit() or OPENSSL_atexit() handler because
there's no way to unregister it when the SoftHSM DSO is unloaded. This
causes the crash reported at https://bugzilla.redhat.com/1831086#c8

Instead of using that method to set a flag showing that OPENSSL_cleanup()
has occurred, instead test directly by calling OPENSSL_init_crypto() for
something that would do nothing, but will fail if OPENSSL_cleanup()
has indeed been run already.

Fixes: c2cc065 "Issue #548: Don't clean up engines after OpenSSL
has already shut down"

@dwmw2
Copy link
Contributor Author

dwmw2 commented May 13, 2020

Note outstanding request for testing, at https://bugzilla.redhat.com/show_bug.cgi?id=1831086#c11

@dwmw2
Copy link
Contributor Author

dwmw2 commented May 13, 2020

Those CI failures don't look like they're mine...

@dwmw2
Copy link
Contributor Author

dwmw2 commented May 13, 2020

Note outstanding request for testing, at https://bugzilla.redhat.com/show_bug.cgi?id=1831086#c11

Note confirmation of fix from user, at https://bugzilla.redhat.com/show_bug.cgi?id=1831086#c12

We can't register our own atexit() or OPENSSL_atexit() handler because
there's no way to unregister it when the SoftHSM DSO is unloaded. This
causes the crash reported at https://bugzilla.redhat.com/1831086#c8

Instead of using that method to set a flag showing that OPENSSL_cleanup()
has occurred, instead test directly by calling OPENSSL_init_crypto() for
something that *would* do nothing, but will fail if OPENSSL_cleanup()
has indeed been run already.

Fixes: c2cc065 "Issue opendnssec#548: Don't clean up engines after OpenSSL
                   has already shut down"
@dwmw2
Copy link
Contributor Author

dwmw2 commented May 21, 2020

No change other than commit date; just trying to prod the CI as some of those failures look transient.

@dwmw2
Copy link
Contributor Author

dwmw2 commented May 21, 2020

OK, the travis-ci build is now OK, and appveyor has been broken in master for a while. Ignoring that.

@rijswijk
Copy link
Contributor

Will ask @halderen for another review and to merge it if OK.

// Detect that situation because reinitialisation will fail
// after OPENSSL_cleanup() has run.
(void)ERR_set_mark();
ossl_shutdown = !OPENSSL_init_crypto(OPENSSL_INIT_ENGINE_RDRAND, NULL);
Copy link
Contributor

Choose a reason for hiding this comment

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

It's possible to compile OpenSSL without RDRAND support.

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.

None yet

4 participants