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

[Maxim Dounin]

Hello!

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

> > On 17 Apr 2023, at 07:31, Maxim Dounin <mdounin at mdounin.ru> wrote:
> > 
> > # HG changeset patch
> > # User Maxim Dounin <mdounin at 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/