Hello! On Fri, Apr 27, 2018 at 11:27:57AM -0400, Anderson Sasaki wrote:
> Hello, > > > > > In my opinion it would be better to have nginx working with engines in > > > > both scenarios. > > > > And is not a problem to call ENGINE_init() from multiple places, since > > > > the API takes care of this case. > > > > > > I'll check these statements in your next patch, but for now it > > > seems an odd functionality to me, because we have openssl config > > > and even nginx ssl_engine directive for that. > > > > Note that the "ssl_engine" directive is not to initialize engines, > > but rather to register an engine as the default one for operations it > > supports. It is designed to make it possible to work with various > > SSL accelerators. > > I believe this patch does not affect the "ssl_engine" directive anymore. > I removed everything I could to make the patch minimal. And changed the log > message to be more specific. > I will send the other changes in a new thread. > > The patch follows: > > # HG changeset patch > # User Anderson Toshiyuki Sasaki <ansas...@redhat.com> > # Date 1524841096 -7200 > # Fri Apr 27 16:58:16 2018 +0200 > # Node ID f5b0a791092224ff5d3eaf4da9a95e6018e7235f > # Parent 46c0c7ef4913011f3f1e073f9ac880b07b1a8154 > SSL: Add ENGINE_init() call before loading key. > It is necessary to call ENGINE_init() before using an OpenSSL engine > to get the engine functional reference. Without this, when > ENGINE_load_private_key() is called, the engine is still unitialized. In no particular order: - Should be "SSL: added ..." (no capital letter after a semicolon, prefer past tense). - An empty line after the summary. - Please prefer double spacing. - "uniNItialized" > diff -r 46c0c7ef4913 -r f5b0a7910922 src/event/ngx_event_openssl.c > --- a/src/event/ngx_event_openssl.c Wed Apr 25 14:57:24 2018 +0300 > +++ b/src/event/ngx_event_openssl.c Fri Apr 27 16:58:16 2018 +0200 > @@ -527,6 +527,13 @@ > return NGX_ERROR; > } > > + if (!ENGINE_init(engine)) { As previously noted at [1], this may need ENGINE_finish() to avoid leaking loaded engines. > + ngx_ssl_error(NGX_LOG_EMERG, ssl->log, 0, > + "ENGINE_init(engine) failed"); There is no reason to log static string "engine" here. Consider using engine name here. > + ENGINE_free(engine); > + return NGX_ERROR; > + } > + > *last++ = ':'; > > pkey = ENGINE_load_private_key(engine, (char *) last, 0, 0); [1] http://mailman.nginx.org/pipermail/nginx-devel/2015-June/007081.html -- Maxim Dounin http://mdounin.ru/ _______________________________________________ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel