Hello!

On Wed, Apr 25, 2018 at 11:52:45AM -0400, Anderson Sasaki wrote:

> # HG changeset patch
> # User Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
> # Date 1524670310 -7200
> # Wed Apr 25 17:31:50 2018 +0200
> # Node ID f916a804d526c1acb493c7c4e5c114d947e0eed1
> # Parent 46c0c7ef4913011f3f1e073f9ac880b07b1a8154
> SSL: Add ENGINE_init() calls before using engines.
> It is necessary to call ENGINE_init() before using a OpenSSL engine
> to get the engine functional reference.
>
> diff -r 46c0c7ef4913 -r f916a804d526 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 Wed Apr 25 17:31:50 2018 +0200
> @@ -527,27 +527,44 @@
> 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);
>
> if (pkey == NULL) {
> ngx_ssl_error(NGX_LOG_EMERG, ssl->log, 0,
> - "ENGINE_load_private_key(\"%s\") failed", last);
> + "ENGINE_load_private_key(\"%s\", %s, %d, %d) failed",
> + p, last, 0, 0);
> ENGINE_free(engine);
> return NGX_ERROR;
> }
>
> - ENGINE_free(engine);
> + if (!ENGINE_set_default(engine, ENGINE_METHOD_PKEY_METHS)) {
> + ngx_ssl_error(NGX_LOG_EMERG, ssl->log, 0,
> + "ENGINE_set_default(\"%s\", %s) failed",
> + p, "ENGINE_METHOD_PKEY_METHS");
> + EVP_PKEY_free(pkey);
> + ENGINE_free(engine);
> + return NGX_ERROR;
> + }

Apart from the ENGINE_init() discussion you are having with
Dmirty, the patch seems to contain various unrelated changes,
including logging changes and the ENGINE_set_default() call quoted
above.

If you really think these changes are needed, you may want to
either submit these changes separately (or document why these
changes should be in the ENGINE_init() patch in the commit log, if
you really think these changes should be an integral part of the
ENGINE_init() patch).

[...]

> @@ -4215,13 +4232,18 @@
> return NGX_CONF_ERROR;
> }
>
> - if (ENGINE_set_default(engine, ENGINE_METHOD_ALL) == 0) {
> - ngx_ssl_error(NGX_LOG_EMERG, cf->log, 0,
> - "ENGINE_set_default(\"%V\", ENGINE_METHOD_ALL) failed",
> + if (!ENGINE_init(engine)) {
> + ngx_ssl_error(NGX_LOG_EMERG, cf->log, 0, "ENGINE_init(\"%V\") failed",
> &value[1]);
> -
> ENGINE_free(engine);
> -
> + return NGX_CONF_ERROR;
> + }
> +
> + if (ENGINE_set_default(engine, ENGINE_METHOD_PKEY_METHS) == 0) {
> + ngx_ssl_error(NGX_LOG_EMERG, cf->log, 0,
> + "ENGINE_set_default(\"%V\", %s) failed",
> + &value[1], "ENGINE_METHOD_PKEY_METHS");
> + ENGINE_free(engine);
> return NGX_CONF_ERROR;
> }

Note well that the change in the ENGINE_set_default() arguments
here seems to be simply wrong.

--
Maxim Dounin
http://mdounin.ru/
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel