[PATCH] SSL: Add ENGINE_init() calls before using engines.

Anderson Sasaki ansasaki at redhat.com
Thu May 10 16:42:58 UTC 2018


Hello,

Thanks again for the feedback.

> 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"

The proposed changes were applied in the new patch version.

> > 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.

Added ENGINE_finish() calls.

> 
> > +            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.

Logging the engine name instead.

The patch follows:

# HG changeset patch
# User Anderson Toshiyuki Sasaki <ansasaki at redhat.com>
# Date 1525954250 -7200
#      Thu May 10 14:10:50 2018 +0200
# Node ID e7d0387ad229ca47514f346c8b692e6f388d72e1
# Parent  ceab908790c4eb7cd01c40bd16581ef794222ca5
SSL: added 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 uninitialized.

diff -r ceab908790c4 -r e7d0387ad229 src/event/ngx_event_openssl.c
--- a/src/event/ngx_event_openssl.c	Tue Apr 24 14:04:59 2018 +0300
+++ b/src/event/ngx_event_openssl.c	Thu May 10 14:10:50 2018 +0200
@@ -527,6 +527,13 @@
             return NGX_ERROR;
         }
 
+        if (!ENGINE_init(engine)) {
+            ngx_ssl_error(NGX_LOG_EMERG, ssl->log, 0,
+                          "ENGINE_init(\"%s\") failed", p);
+            ENGINE_free(engine);
+            return NGX_ERROR;
+        }
+
         *last++ = ':';
 
         pkey = ENGINE_load_private_key(engine, (char *) last, 0, 0);
@@ -534,10 +541,12 @@
         if (pkey == NULL) {
             ngx_ssl_error(NGX_LOG_EMERG, ssl->log, 0,
                           "ENGINE_load_private_key(\"%s\") failed", last);
+            ENGINE_finish(engine);
             ENGINE_free(engine);
             return NGX_ERROR;
         }
 
+        ENGINE_finish(engine);
         ENGINE_free(engine);
 
         if (SSL_CTX_use_PrivateKey(ssl->ctx, pkey) == 0) {


More information about the nginx-devel mailing list