Welcome! Log In Create A New Profile

Advanced

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

Maxim Dounin
September 06, 2022 08:12PM
Hello!

On Tue, Sep 06, 2022 at 03:35:20PM +0400, Sergey Kandaurov wrote:

> > 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:
> >
> >>> On 1 Sep 2022, at 20:49, Maxim Dounin <mdounin@mdounin.ru> wrote:
> >>>
> >>> # HG changeset patch
> >>> # User Maxim Dounin <mdounin@mdounin.ru>
> >>> # Date 1662050858 -10800
> >>> # Thu Sep 01 19:47:38 2022 +0300
> >>> # Node ID d73286c43b44f3161ca4de1d9d1cbb070c6da4a7
> >>> # Parent 63a4b5ffd440c526bc96c6879dc1b6b489975d98
> >>> Win32: fixed build on Windows with OpenSSL 3.0.x (ticket #2379).
> >>
> >> BTW, win32 build on Windows XP with OpenSSL 3.0.x is currently broken
> >> for another reason: due to a missing InterlockedOr64 implementation.
> >> See the related fix, expected to appear in upcoming OpenSSL 3.0.6:
> >> ce3951fc30c7bc7c3dbacba19d87c79d9af9da0d
> >
> > Well, that's exactly the reason why I've fixed 64-bit build with
> > MSVC 2010, the 2nd patch in this series. But see ticket's
> > comment:1, the patch committed into OpenSSL is incomplete and only
> > fixes MSVC 2008 and older, but won't fix Windows XP builds in newer
> > compilers.
>
> So, the checks for _MSC_VER in the committed fix
> seem to be not enough if older WIN32_WINNT requested.

Yes, exactly. Either way, this should be irrelevant to nginx
builds with "no-threads".

[...]

> >>> -#ifdef BIO_get_ktls_send
> >>> +#if (defined BIO_get_ktls_send && !NGX_WIN32)
> >>>
> >>> int sslerr, flags;
> >>> ssize_t n;
> >>>
> >>
> >> This could be simplified if replaced #ifdef with #if.
> >> BIO_get_ktls_send is documented to be a macro (and so tested here).
> >> When OpenSSL isn't configured with KTLS, the macro is explanded to 0.
> >> Replacement allows optimize ngx_ssl_sendfile() at compile time, as well.
> >
> > While BIO_get_ktls_send is documented to be a macro, it's not
> > required to expand to "(0)" if KTLS is not supported.
>
> Well, this is sort of documented in SSL_write.pod:
> SSL_sendfile() is available only when
> Kernel TLS is enabled, which can be checked by calling BIO_get_ktls_send().
>
> Then in BIO_ctrl.pod:
> BIO_get_ktls_send() returns 1 if the BIO is using the Kernel TLS data-path for
> sending. Otherwise, it returns zero.

It still not required to expand to "(0)", and can instead expand
to a function which returns 0, making it completely
indistinguishable from the case when KTLS can be supported.

Either way, this is irrelevant, given that #if also breaks
positive tests.

> > Further, it's not going to actually work for positive tests, since
> > non-numeric macro values will evaluate to 0, effectively disabling
> > SSL_sendfile() for all builds.
>
> Missed that point, thnx.

[...]

> > # HG changeset patch
> > # User Maxim Dounin <mdounin@mdounin.ru>
> > # Date 1662434808 -10800
> > # Tue Sep 06 06:26:48 2022 +0300
> > # Node ID 5e493208769dd754253f567405c05b3cf5f05c8a
> > # Parent 2642d97294f0e09c1fd67e434aae759768805617
> > Win32: disabled threads support in OpenSSL builds.
> >
> > Threads are disabled during UNIX builds (see b329c0ab1a48), and also not
> > needed for Windows builds.
> >
> > This used to be the default before OpenSSL 1.1.0.
> >
> > diff -r 2642d97294f0 -r 5e493208769d auto/lib/openssl/makefile.msvc
> > --- a/auto/lib/openssl/makefile.msvc Tue Sep 06 05:48:19 2022 +0300
> > +++ b/auto/lib/openssl/makefile.msvc Tue Sep 06 06:26:48 2022 +0300
> > @@ -6,7 +6,7 @@
> > all:
> > cd $(OPENSSL)
> >
> > - perl Configure VC-WIN32 no-shared \
> > + perl Configure VC-WIN32 no-shared no-threads \
> > --prefix="%cd%/openssl" \
> > --openssldir="%cd%/openssl/ssl" \
> > $(OPENSSL_OPT)
> >
> >
>
> Looks good.
> Does it make sense to apply this to makefile.bcc for consistency?

I've considered it, but decided to don't touch makefile.bcc, given
that OpenSSL 1.1.0 removed "the aged BC-32 config and all its
supporting scripts" (quote from CHANGES), so "no-threads" for BCC
is the default for all existing OpenSSL versions which do support
building with BCC.

Pushed to http://mdounin.ru/hg/nginx, thanks.

--
Maxim Dounin
http://mdounin.ru/
_______________________________________________
nginx-devel mailing list -- nginx-devel@nginx.org
To unsubscribe send an email to nginx-devel-leave@nginx.org
Subject Author Views Posted

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

Sergey Kandaurov 285 September 05, 2022 02:46PM

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

Maxim Dounin 60 September 05, 2022 11:52PM

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

Sergey Kandaurov 50 September 06, 2022 07:36AM

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

Maxim Dounin 55 September 06, 2022 08:12PM

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

Sergey Kandaurov 44 September 07, 2022 11:36AM

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

Maxim Dounin 62 September 07, 2022 05:14PM



Sorry, you do not have permission to post/reply in this forum.

Online Users

Guests: 83
Record Number of Users: 6 on February 13, 2018
Record Number of Guests: 421 on December 02, 2018
Powered by nginx      Powered by FreeBSD      PHP Powered      Powered by MariaDB      ipv6 ready