Hello Ilya,


sorry about the delay ...


On Wed, 27 Nov 2019 at 07:11, Илья Шипицин <chipits...@gmail.com> wrote:
>
> -#if (HA_OPENSSL_VERSION_NUMBER >= 0x1010000fL)
> +#if (HA_OPENSSL_VERSION_NUMBER >= 0x1010000fL) || 
> defined(OPENSSL_NO_DEPRECATED)
> [...]
> -#if defined(USE_THREAD) && (HA_OPENSSL_VERSION_NUMBER < 0x10100000L)
> +#if defined(USE_THREAD) && (HA_OPENSSL_VERSION_NUMBER < 0x10100000L) && 
> !defined(OPENSSL_NO_DEPRECATED)
> [...]
> -#if defined(USE_THREAD) && (HA_OPENSSL_VERSION_NUMBER < 0x10100000L)
> +#if defined(USE_THREAD) && (HA_OPENSSL_VERSION_NUMBER < 0x10100000L) && 
> !defined(OPENSSL_NO_DEPRECATED)
> [...]

I'm confused. This is not required in my environment for the build to
succeed and I don't see any reason why HA_OPENSSL_VERSION_NUMBER would
be smaller here? Can you elaborate why the HA_OPENSSL_VERSION_NUMBER
comparison would fail to do its job in those comparisons?

The X509_getm_ issue has been fixed by Rosen's patch [1], which is
committed and backported.

SSL_CTX_set_ecdh_auto issue is fixed by your patch (additional guard
in ssl_sock.c) or by removing the existing guards and defining the
same compatibility macro openssl uses [2] (as per the input from Willy
and Emmanuel):

#ifndef SSL_CTX_set_ecdh_auto
#define SSL_CTX_set_ecdh_auto(dummy, onoff)      ((onoff) != 0)
#endif

I'd prefer the latter, which is what OpenSSL uses (when not using
no-deprecated) and does not pollute the ssl_sock.c.

Everything builds just fine after that for me (both master and 2.0),
without any warnings. I also tried with threading disabled
(USE_THREAD=).

I will be sending the single SSL_CTX_set_ecdh_auto() fix shortly. Let
me know what you think and if you believe something is missing for
no-deprecated compatibility.


FYI: to avoid rebuilding openssl each time with and without
no-deprecate option, the same can be achieved when building haproxy by
adding DEFINE="-DOPENSSL_API_COMPAT=0x10100000L
-DOPENSSL_NO_DEPRECATED" to the make command (maybe this can be useful
in CI - I don't know anything about that).

Once we agree on a fix and commit it, we should definitely add a CI
build testing this (with openssl 1.1.1). I disagree to test the build
against openssl master, because the API may continually change during
development (I mentioned this point in another conversation but I
don't recall whether it was on ML or GH).



thanks,
lukas


[1] 
https://github.com/haproxy/haproxy/commit/b3814c2ca8a8c28a890f8f50e0a35d5247222a12
[2] 
https://github.com/openssl/openssl/blob/bf4006a6f9be691ba6eef0e8629e63369a033ccf/include/openssl/ssl.h#L1480

Reply via email to