Hello!

On Tue, Dec 19, 2023 at 12:16:58PM +0100, Илья Шипицин wrote:

> вт, 19 дек. 2023 г. в 09:58, Maxim Dounin <mdounin@mdounin.ru>:
>
> > Hello!
> >
> > On Tue, Dec 19, 2023 at 02:09:10AM +0400, Sergey Kandaurov wrote:
> >
> > > > On 24 Nov 2023, at 00:29, Ilya Shipitsin <chipitsine@gmail.com> wrote:
> > > >
> > > > # HG changeset patch
> > > > # User Ilya Shipitsin <chipitsine@gmail.com>
> > > > # Date 1700769135 -3600
> > > > # Thu Nov 23 20:52:15 2023 +0100
> > > > # Node ID 2001e73ce136d5bfc9bde27d338865b14b8ad436
> > > > # Parent 7ec761f0365f418511e30b82e9adf80bc56681df
> > > > ssl: SSL_get0_verified_chain is available for LibreSSL >= 3.3.6
> > >
> > > style: SSL prefix should be uppercase.
> > >
> > > >
> > > > diff -r 7ec761f0365f -r 2001e73ce136
> > src/event/ngx_event_openssl_stapling.c
> > > > --- a/src/event/ngx_event_openssl_stapling.c Thu Oct 26
> > 23:35:09 2023 +0300
> > > > +++ b/src/event/ngx_event_openssl_stapling.c Thu Nov 23
> > 20:52:15 2023 +0100
> > > > @@ -893,7 +893,8 @@
> > > > ocsp->cert_status = V_OCSP_CERTSTATUS_GOOD;
> > > > ocsp->conf = ocf;
> > > >
> > > > -#if (OPENSSL_VERSION_NUMBER >= 0x10100000L && !defined
> > LIBRESSL_VERSION_NUMBER)
> > > > +/* minimum OpenSSL 1.1.1 & LibreSSL 3.3.6 */
> > > > +#if (OPENSSL_VERSION_NUMBER >= 0x10100000L && !defined
> > LIBRESSL_VERSION_NUMBER) || (defined(LIBRESSL_VERSION_NUMBER) &&
> > (LIBRESSL_VERSION_NUMBER >= 0x3030600L))
> > > >
> > > > ocsp->certs = SSL_get0_verified_chain(c->ssl->connection);
> > > >
> > >
> > > Testing "defined(LIBRESSL_VERSION_NUMBER)" is superfluous.
> > > The macro test suffers from a very long line.
> > >
> > > The correct version test seems to be against LibreSSL 3.5.0, see
> > > https://ftp.openbsd.org/pub/OpenBSD/LibreSSL/libressl-3.5.0-relnotes.txt
> > >
> > > So, the resulting change would be as follows:
> > >
> > > diff --git a/src/event/ngx_event_openssl_stapling.c
> > b/src/event/ngx_event_openssl_stapling.c
> > > --- a/src/event/ngx_event_openssl_stapling.c
> > > +++ b/src/event/ngx_event_openssl_stapling.c
> > > @@ -893,7 +893,9 @@ ngx_ssl_ocsp_validate(ngx_connection_t *
> > > ocsp->cert_status = V_OCSP_CERTSTATUS_GOOD;
> > > ocsp->conf = ocf;
> > >
> > > -#if (OPENSSL_VERSION_NUMBER >= 0x10100000L && !defined
> > LIBRESSL_VERSION_NUMBER)
> > > +#if (OPENSSL_VERSION_NUMBER >= 0x10100000L \
> > > + && !defined LIBRESSL_VERSION_NUMBER) \
> > > + || LIBRESSL_VERSION_NUMBER >= 0x3050000fL
> >
> > Rather,
> >
> > +#if (OPENSSL_VERSION_NUMBER >= 0x10100000L
> > \
> > + && (!defined LIBRESSL_VERSION_NUMBER
> > \
> > + || LIBRESSL_VERSION_NUMBER >= 0x3050000fL))
> >
> > >
> > > ocsp->certs = SSL_get0_verified_chain(c->ssl->connection);
> > >
> > >
> > > On the other hand, I don't like the resulting style mudness.
> > > It may have sense just to drop old LibreSSL versions support:
> > > maintaining one or two most recent stable branches should be enough.
> >
> > +1 on this.
> >
>
> if we want to keep code clean, we can move "if LibreSSL >= 3.7" to
> "configure" level

I wouldn't expect such a change to be accepted. We generally
don't try to test various SSL library features in configure -
there are a lot of things to check, and testing appropriate
defines (or version numbers, if no defines are available) is
believed to work much better.

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