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 Dynamic engine loading so that the call to ENGINE_load_builtin_engines() is performed. #11543

Closed
wants to merge 1 commit into from

Conversation

slontis
Copy link
Member

@slontis slontis commented Apr 14, 2020

Fixes #11510

PR #11240 Added support for passing the libctx to the config loader.
As part of this work the call to OPENSSL_load_builtin_modules() + ENGINE_load_builtin_engines() was deferred until module_run() is called.
The call to ENGINE_load_builtin_engines() has been added to ENGINE_by_id().

Checklist
  • documentation is added or updated
  • tests are added or updated

…gines() is performed.

Fixes openssl#11510

PR openssl#11240 Added support for passing the libctx to the config loader.
As part of this work the call to OPENSSL_load_builtin_modules() + ENGINE_load_builtin_engines() was deferred until module_run() is called.
The call to ENGINE_load_builtin_engines() has been added to ENGINE_by_id().
@slontis slontis added approval: review pending This pull request needs review by a committer branch: master Merge to master branch labels Apr 14, 2020
@slontis slontis added this to In progress in 3.0 New Core + FIPS via automation Apr 14, 2020
@levitte
Copy link
Member

levitte commented Apr 14, 2020

Isn't this a spot that should use OPENSSL_init_crypto()?

@mattcaswell?

@slontis
Copy link
Member Author

slontis commented Apr 14, 2020

Isn't this a spot that should use OPENSSL_init_crypto()?

The function does this..

void ENGINE_load_builtin_engines(void)
{
    OPENSSL_init_crypto(OPENSSL_INIT_ENGINE_ALL_BUILTIN, NULL);
}

3.0 New Core + FIPS automation moved this from In progress to Reviewer approved Apr 14, 2020
@beldmit
Copy link
Member

beldmit commented Apr 14, 2020

Many thanks!

@beldmit beldmit added approval: done This pull request has the required number of approvals and removed approval: review pending This pull request needs review by a committer labels Apr 14, 2020
@levitte
Copy link
Member

levitte commented Apr 14, 2020

The function does this..

Ah! Ok then!

@openssl-machine
Copy link
Collaborator

24 hours has passed since 'approval: done' was set, but as this PR has been updated in that time the label 'approval: ready to merge' is not being automatically set. Please review the updates and set the label manually.

openssl-machine pushed a commit that referenced this pull request Apr 15, 2020
…gines() is performed.

Fixes #11510

PR #11240 Added support for passing the libctx to the config loader.
As part of this work the call to OPENSSL_load_builtin_modules() + ENGINE_load_builtin_engines() was deferred until module_run() is called.
The call to ENGINE_load_builtin_engines() has been added to ENGINE_by_id().

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Dmitry Belyavskiy <beldmit@gmail.com>
(Merged from #11543)
@beldmit
Copy link
Member

beldmit commented Apr 15, 2020

Merged. Thanks!

@beldmit beldmit closed this Apr 15, 2020
3.0 New Core + FIPS automation moved this from Reviewer approved to Done Apr 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approval: done This pull request has the required number of approvals branch: master Merge to master branch
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

Dynamic engines seem to be broken
5 participants