Hello Maxim,

Ah, well that explains why a patch like this has never been upstreamed,
even though it exists in a bunch of places on teh interwebz. Sorry,
I didn't do enough archeology here.

I didn't realize the `init = 1` workaround existed, so thanks for the
pointer there. However, it would be ideal if users could use OpenSSL's
dynamic engine loading, and avoid authoring an OpenSSL config file.

From the description in the issue you linked, it looks like the patch was
removed for OpenSSL 1.0.x compatibility. Would you accept a patch that
supplies the init/finish directives only if the OpenSSL version >= 1.1.0?

At this point many distros have OpenSSL 1.1 and a fixed PKCS #11 engine;
for example, the patch I submitted worked smoothly with OpenSSL 1.1 and the
PKCS #11 engine available on Debian 11, and without any engine config.

Thanks a bunch,
Brad

On Tue, Dec 7, 2021 at 2:28 PM Maxim Dounin <mdounin@mdounin.ru> wrote:

> Hello!
>
> On Tue, Dec 07, 2021 at 11:45:01AM -0500, Bradley Hess wrote:
>
> > # HG changeset patch
> > # User Bradley Hess <bdhess@google.com>
> > # Date 1638894138 18000
> > # Tue Dec 07 11:22:18 2021 -0500
> > # Node ID c3ec7674556519a9068c4e7a9f6279bbff6c3d31
> > # Parent a7a77549265ef46f1f0fdb3897f4beabf9e09c40
> > Add ENGINE_init/finish directives around ENGINE_load_private_key.
> >
> > ENGINE_by_id creates a "structural reference" to an engine; for
> > actually performing cryptography, this ought to be a "functional
> > reference" per those two topics in the OpenSSL docs:
> > https://www.openssl.org/docs/man1.1.1/man3/ENGINE_load_private_key.html
> >
> > Unlike the default OpenSSL engine, the OpenSC PKCS #11 engine cannot
> > load a private key if it is not first initialized.
> >
> > diff -r a7a77549265e -r c3ec76745565 src/event/ngx_event_openssl.c
> > --- a/src/event/ngx_event_openssl.c Thu Nov 25 22:02:10 2021 +0300
> > +++ b/src/event/ngx_event_openssl.c Tue Dec 07 11:22:18 2021 -0500
> > @@ -734,16 +734,24 @@
> > return NULL;
> > }
> >
> > + if (!ENGINE_init(engine)) {
> > + *err = "ENGINE_init() failed";
> > + ENGINE_free(engine);
> > + return NULL;
> > + }
> > +
> > *last++ = ':';
> >
> > pkey = ENGINE_load_private_key(engine, (char *) last, 0, 0);
> >
> > if (pkey == NULL) {
> > *err = "ENGINE_load_private_key() failed";
> > + ENGINE_finish(engine);
> > ENGINE_free(engine);
> > return NULL;
> > }
> >
> > + ENGINE_finish(engine);
> > ENGINE_free(engine);
> >
> > return pkey;
>
> Previously, a similar patch was rejected, because it caused an
> immediate segfault for most affected users, see here:
>
> http://mailman.nginx.org/pipermail/nginx-devel/2018-May/011149.html
>
> If you think that things have changed since then, you may want to
> elaborate.
>
> --
> Maxim Dounin
> http://mdounin.ru/
> _______________________________________________
> nginx-devel mailing list
> nginx-devel@nginx.org
> http://mailman.nginx.org/mailman/listinfo/nginx-devel
>
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel