> On 6 Sep 2022, at 07:49, Maxim Dounin <mdounin@mdounin.ru> wrote:
>
> Hello!
>
> 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@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@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) {
--
Sergey Kandaurov
_______________________________________________
nginx-devel mailing list -- nginx-devel@nginx.org
To unsubscribe send an email to nginx-devel-leave@nginx.org
