> 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

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.

But anyway, I don't see an obvious win over the existing code:
the certificate chain is reconstructed if SSL_get0_verified_chain()
is (detected to be) not present, which should be fine in most cases.

That said, it doesn't seem to deserve introducing 3-line macro test,
or (see OTOH note) breaking old LibreSSL support for no apparent reason.

--
Sergey Kandaurov
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel