[PATCH 3 of 3] Win32: fixed build on Windows with OpenSSL 3.0.x (ticket #2379)

[Maxim Dounin]

Hello!

On Wed, Sep 07, 2022 at 07:33:23PM +0400, Sergey Kandaurov wrote:

> > On 6 Sep 2022, at 07:49, Maxim Dounin <mdounin at mdounin.ru> wrote:
> > 
> > On Mon, Sep 05, 2022 at 10:44:06PM +0400, Sergey Kandaurov wrote:
> > 
> >> I see that it's convention in nginx to test external macros using #ifdef.
> >> In certain cases we use an exception there if it does or even does not
> >> make sense, such as when testing SSL_CTRL_SET_ECDH_AUTO (though that's
> >> rather a typo there).  Using #if BIO_get_ktls_send looks reasonable to me.
> > 
> > Sure, "#if SSL_CTRL_SET_ECDH_AUTO" looks like a typo (patch 
> > below), but it used to work, since SSL_CTRL_SET_ECDH_AUTO is a 
> > non-zero numeric constant when defined.  This is not the case with 
> > BIO_get_ktls_send.
> > 
> > [..]
> > # HG changeset patch
> > # User Maxim Dounin <mdounin at mdounin.ru>
> > # Date 1662432499 -10800
> > #      Tue Sep 06 05:48:19 2022 +0300
> > # Node ID de9fe82b0c7789474bb6284f13ebd3f903b129b0
> > # Parent  9cf231508a8dbc2492e0d1cd06d7b1258eb5f435
> > SSL: fixed incorrect usage of #if instead of #ifdef.
> > 
> > In 2014ed60f17f, "#if SSL_CTRL_SET_ECDH_AUTO" test was incorrectly used
> > instead of "#ifdef SSL_CTRL_SET_ECDH_AUTO".  There is no practical
> > difference, since SSL_CTRL_SET_ECDH_AUTO evaluates to a non-zero numeric
> > value when defined, but anyway it's better to correctly test if the value
> > is defined.
> > 
> > diff --git a/src/event/ngx_event_openssl.c b/src/event/ngx_event_openssl.c
> > --- a/src/event/ngx_event_openssl.c
> > +++ b/src/event/ngx_event_openssl.c
> > @@ -1426,7 +1426,7 @@ ngx_ssl_ecdh_curve(ngx_conf_t *cf, ngx_s
> > 
> >     SSL_CTX_set_options(ssl->ctx, SSL_OP_SINGLE_ECDH_USE);
> > 
> > -#if SSL_CTRL_SET_ECDH_AUTO
> > +#ifdef SSL_CTRL_SET_ECDH_AUTO
> >     /* not needed in OpenSSL 1.1.0+ */
> >     SSL_CTX_set_ecdh_auto(ssl->ctx, 1);
> > #endif
> > 
> 
> Just noted that BoringSSL uses stubs, which makes GCC unhappy.
> 
> // SSL_CTX_set_ecdh_auto returns one.
> #define SSL_CTX_set_ecdh_auto(ctx, onoff) 1
> #define SSL_CTRL_SET_ECDH_AUTO doesnt_exist
> 
> Patch to address this below:
> 
> # HG changeset patch
> # User Sergey Kandaurov <pluknet at nginx.com>
> # Date 1662564729 -14400
> #      Wed Sep 07 19:32:09 2022 +0400
> # Node ID e7997671018932c6cc97e1debf95b515e4d39e3c
> # Parent  a423e314c22fe99fe9faf28f033c266426993105
> SSL: silenced GCC warnings when building with BoringSSL.
> 
> BoringSSL uses macro stub for SSL_CTX_set_ecdh_auto that expands to 1,
> which triggers -Wunused-value "statement with no effect" warnings.
> 
> diff -r a423e314c22f -r e79976710189 src/event/ngx_event_openssl.c
> --- a/src/event/ngx_event_openssl.c	Wed Sep 07 00:47:31 2022 +0300
> +++ b/src/event/ngx_event_openssl.c	Wed Sep 07 19:32:09 2022 +0400
> @@ -1428,7 +1428,7 @@ ngx_ssl_ecdh_curve(ngx_conf_t *cf, ngx_s
>  
>  #ifdef SSL_CTRL_SET_ECDH_AUTO
>      /* not needed in OpenSSL 1.1.0+ */
> -    SSL_CTX_set_ecdh_auto(ssl->ctx, 1);
> +    (void) SSL_CTX_set_ecdh_auto(ssl->ctx, 1);
>  #endif
>  
>      if (ngx_strcmp(name->data, "auto") == 0) {

Looks good.

-- 
Maxim Dounin
http://mdounin.ru/