Welcome! Log In Create A New Profile

Advanced

Re: [PATCH 03 of 11] Tests: added has_feature() tests for IO::Socket::SSL

Maxim Dounin
May 03, 2023 11:58PM
Hello!

On Wed, May 03, 2023 at 08:20:02PM +0400, Sergey Kandaurov wrote:

> > On 17 Apr 2023, at 07:31, Maxim Dounin <mdounin@mdounin.ru> wrote:
> >
> > # HG changeset patch
> > # User Maxim Dounin <mdounin@mdounin.ru>
> > # Date 1681702252 -10800
> > # Mon Apr 17 06:30:52 2023 +0300
> > # Node ID f704912ed09f3494a815709710c3744b0adca50b
> > # Parent 6f0148ef1991d92a003c8529c8cce9a8dd49e706
> > Tests: added has_feature() tests for IO::Socket::SSL.
> >
> > The following distinct features supported:
> >
> > - "socket_ssl", which requires IO::Socket::SSL and also implies
> > existance of the IO::Socket::SSL::SSL_VERIFY_NONE() symbol.
> > It is used by most of the tests.
> >
>
> SSL_VERIFY_NONE was added in IO::Socket::SSL 1.31 (2009.09.25).
> The check was added primarily for then supported CentOS 5.
> Now CentOS 5 is long obsolete, SSL_VERIFY_NONE can be dropped.

As implemented, with has(socket_ssl) at least version 1.31 of
IO::Socket::SSL is required to run SSL tests. It still works
correctly with older version though, and just skips SSL tests, so
the test suite can be run on old platforms if needed.

> Most popular modern distributions have something of IO::Socket::SSL 2.0xx,
> the oldest still supported CentOS 7 has IO::Socket::SSL 1.94.
>
> > - "socket_ssl_sni", which requires IO::Socket::SSL with the can_client_sni()
> > function (1.84), and SNI support available in Net::SSLeay and the OpenSSL
> > library being used. Used by ssl_sni.t, ssl_sni_sessions.t,
> > stream_ssl_preread.t. Additional Net::SSLeay testing is believed to be
> > unneeded and was removed.
> >
>
> I agree that Net::SSLeay is believed to be redundant: properly implemented
> IO::Socket::SSL should detect Net::SSLeay is too old and behave accordingly.
>
> Note that you removed can_client_sni() from h2_ssl_verify_client.t.
> In fact, it is replaced with "socket_ssl_alpn". ALPN support was added
> to IO::Socket::SSL after SNI, so it should work in practice, although not
> evident. Formally, the "socket_ssl_sni" prerequisite needs to be added.

It was dropped intentionally, since ALPN support basically implies
SNI support. I don't think trying to catch clients with ALPN but
without SNI support worth the effort - there are no such clients.

> On the other way, can_client_sni() was added in 1.83, while SNI support
> factually was added noticeably earlier - in version 1.56, via SSL_hostname.
> Using IO::Socket::SSL 1.82 allowed me to run the following tests
> that would be otherwise skipped due to socket_ssl_sni:
>
> ssl_sni.t - replacing with "socket_ssl" is enough
> stream_ssl_preread.t - replacing with "socket_ssl" is enough
> ssl_sni_sessions.t - also needs SSL_VERIFY_NONE in get_ssl_socket()
> to stop whining on old IO::Socket::SSL that presumably doesn't get it
> from $ctx. It won't run anyway though due to IO::Socket::SSL version
> is too old to support TLSv1.3 sessions.
>
> If we tend to drop anything older than 1.94 (CentOS 7), then we can freely
> drop "socket_ssl_sni" from the patch. SSL_hostname is already there.

I'm fine with skipping tests with older versions, but failing
tests without clear reasoning looks wrong to me.

That is, we can probably merge "socket_ssl_sni" requirements into
"socket_ssl", but simply dropping them will result in failures on
systems with old IO::Socket::SSL versions and/or systems with old
OpenSSL. And this is something I would like to avoid - even if
that's not something tested on a regular basis.

Either way, current distinction with separate SNI tests looks good
enough for me, and I would rather keep it as is. Especially given
that it's basically effortless with this patch series.

[...]

> > - "socket_ssl_alpn", which requires IO::Socket::SSL with ALPN support (2.009),
> > and ALPN support in Net::SSLeay and the OpenSSL library being used.
> > Used by h2_ssl.t, h2_ssl_verify_client.t, stream_ssl_alpn.t,
> > stream_ssl_preread_alpn.t.
> >
> > - "socket_ssl_sslversion", which requires IO::Socket::SSL with
> > the get_sslversion() and get_sslversion_int() methods (1.964).
> > Used by mail_imap_ssl.t.
>
> I don't like that the whole mail_imap_ssl.t is skipped on < 1.964
> (even though it is not quite new and most distributions have it now),
> because of a minor optional feature that could not be tested.
> In principle, we can rewrite the test similar to $ssl_protocol
> to avoid dependency on 1.964:

In no particular order:

- It's a skip, not a failure.

- IO::Socket::SSL 1.964 is rather old, and if only an older
version of IO::Socket::SSL is available - this likely means that
SSL isn't a specific feature being considered.

- That's not only IMAP SSL test we have, and skipping it shouldn't
be a problem: we still test at least some IMAP SSL functionality
even on platforms with such outdated IO::Socket::SSL.

- We still need "socket_ssl_sslversion" for other tests, notably
mail_ssl_session_reuse.t and stream_ssl_session_reuse.t, see the
following patches.

Overall, I don't think it worth the effort. If you think it worth
it, consider submitting a patch once this patch series settles.

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

[PATCH 00 of 11] SSL tests simplified

Maxim Dounin 589 April 16, 2023 11:46PM

[PATCH 01 of 11] Tests: SIGPIPE handling in mail tests

Maxim Dounin 106 April 16, 2023 11:46PM

[PATCH 03 of 11] Tests: added has_feature() tests for IO::Socket::SSL

Maxim Dounin 112 April 16, 2023 11:46PM

Re: [PATCH 03 of 11] Tests: added has_feature() tests for IO::Socket::SSL

Sergey Kandaurov 115 May 03, 2023 12:22PM

Re: [PATCH 03 of 11] Tests: added has_feature() tests for IO::Socket::SSL

Maxim Dounin 116 May 03, 2023 11:58PM

[PATCH 04 of 11] Tests: fixed server_tokens tests for build names with spaces

Maxim Dounin 112 April 16, 2023 11:46PM

Re: [PATCH 04 of 11] Tests: fixed server_tokens tests for build names with spaces

Sergey Kandaurov 80 May 11, 2023 07:50AM

Re: [PATCH 04 of 11] Tests: fixed server_tokens tests for build names with spaces

Maxim Dounin 102 May 14, 2023 02:54PM

[PATCH 05 of 11] Tests: added has_feature() test for SSL libraries

Maxim Dounin 108 April 16, 2023 11:46PM

[PATCH 09 of 11] Tests: simplified stream SSL tests with IO::Socket::SSL

Maxim Dounin 111 April 16, 2023 11:46PM

[PATCH 06 of 11] Tests: reworked mail SSL tests to use IO::Socket::SSL

Maxim Dounin 124 April 16, 2023 11:46PM

Re: [PATCH 06 of 11] Tests: reworked mail SSL tests to use IO::Socket::SSL

Sergey Kandaurov 173 May 11, 2023 10:40AM

Re: [PATCH 06 of 11] Tests: reworked mail SSL tests to use IO::Socket::SSL

Maxim Dounin 104 May 14, 2023 05:12PM

[PATCH 08 of 11] Tests: reworked stream SSL tests to use IO::Socket::SSL

Maxim Dounin 114 April 16, 2023 11:46PM

[PATCH 07 of 11] Tests: simplified mail_imap_ssl.t

Maxim Dounin 108 April 16, 2023 11:46PM

[PATCH 10 of 11] Tests: reworked http SSL tests to use IO::Socket::SSL

Maxim Dounin 112 April 16, 2023 11:46PM

Re: [PATCH 10 of 11] Tests: reworked http SSL tests to use IO::Socket::SSL

Sergey Kandaurov 107 May 11, 2023 10:28AM

Re: [PATCH 10 of 11] Tests: reworked http SSL tests to use IO::Socket::SSL

Maxim Dounin 125 May 18, 2023 11:18AM

[PATCH 11 of 11] Tests: simplified http SSL tests with IO::Socket::SSL

Maxim Dounin 111 April 16, 2023 11:46PM

[PATCH 0 of 6] SSL tests refactoring fixes

Sergey Kandaurov 89 May 22, 2023 03:58PM

[PATCH 1 of 6] Tests: unbreak ssl_stapling.t after IO::Socket::SSL refactoring

Sergey Kandaurov 105 May 22, 2023 03:58PM

Re: [PATCH 1 of 6] Tests: unbreak ssl_stapling.t after IO::Socket::SSL refactoring

Maxim Dounin 79 May 22, 2023 04:38PM

[PATCH 2 of 6] Tests: unbreak tests with IO::Socket:SSL lacking SSL_session_key

Sergey Kandaurov 89 May 22, 2023 03:58PM

Re: [PATCH 2 of 6] Tests: unbreak tests with IO::Socket:SSL lacking SSL_session_key

Maxim Dounin 79 May 22, 2023 07:44PM

Re: [PATCH 2 of 6] Tests: unbreak tests with IO::Socket:SSL lacking SSL_session_key

Sergey Kandaurov 99 May 23, 2023 06:36AM

Re: [PATCH 2 of 6] Tests: unbreak tests with IO::Socket:SSL lacking SSL_session_key

Maxim Dounin 144 May 23, 2023 09:32AM

[PATCH 3 of 6] Tests: unbreak stream_ssl_variables.t with old IO::Socket::SSL

Sergey Kandaurov 86 May 22, 2023 04:00PM

Re: [PATCH 3 of 6] Tests: unbreak stream_ssl_variables.t with old IO::Socket::SSL

Maxim Dounin 84 May 22, 2023 07:52PM

[PATCH 4 of 6] Tests: avoid specifying PSS in sigalgs unless in TLSv1.3

Sergey Kandaurov 88 May 22, 2023 04:00PM

Re: [PATCH 4 of 6] Tests: avoid specifying PSS in sigalgs unless in TLSv1.3

Maxim Dounin 82 May 22, 2023 09:08PM

Re: [PATCH 4 of 6] Tests: avoid specifying PSS in sigalgs unless in TLSv1.3

Sergey Kandaurov 158 May 23, 2023 08:24AM

[PATCH 5 of 6] Tests: added missing socket_ssl_alpn guard in mail_ssl.t

Sergey Kandaurov 92 May 22, 2023 04:00PM

Re: [PATCH 5 of 6] Tests: added missing socket_ssl_alpn guard in mail_ssl.t

Maxim Dounin 82 May 22, 2023 10:18PM

Re: [PATCH 5 of 6] Tests: added missing socket_ssl_alpn guard in mail_ssl.t

Sergey Kandaurov 149 May 23, 2023 08:56AM

[PATCH 6 of 6] Tests: added missing socket_ssl_reused prerequisites

Sergey Kandaurov 88 May 22, 2023 04:00PM

Re: [PATCH 6 of 6] Tests: added missing socket_ssl_reused prerequisites

Maxim Dounin 76 May 22, 2023 10:40PM



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

Online Users

Guests: 221
Record Number of Users: 8 on April 13, 2023
Record Number of Guests: 421 on December 02, 2018
Powered by nginx      Powered by FreeBSD      PHP Powered      Powered by MariaDB      ipv6 ready