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

[Maxim Dounin]

Hello!

On Thu, May 11, 2023 at 06:39:32PM +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 1681702257 -10800
> > #      Mon Apr 17 06:30:57 2023 +0300
> > # Node ID 20d603cd3cbeab89127108fe9cb6dffd0e9469e8
> > # Parent  a8e22a3212da945e9060d4233905eb6de1399d34
> > Tests: reworked mail SSL tests to use IO::Socket::SSL.
> > 
> > Relevant infrastructure is provided in Test::Nginx::IMAP (and also POP3
> > and SMTP for completeness).  This also ensures that SSL handshake and
> > various read operations are guarded with timeouts.
> > 
> 
> [..]
> 
> > diff --git a/mail_ssl_conf_command.t b/mail_ssl_conf_command.t
> > --- a/mail_ssl_conf_command.t
> > +++ b/mail_ssl_conf_command.t
> > @@ -16,6 +16,7 @@ BEGIN { use FindBin; chdir($FindBin::Bin
> > 
> > use lib 'lib';
> > use Test::Nginx;
> > +use Test::Nginx::IMAP;
> > 
> > ###############################################################################
> > 
> > @@ -24,15 +25,8 @@ select STDOUT; $| = 1;
> > 
> > local $SIG{PIPE} = 'IGNORE';
> > 
> > -eval {
> > -	require Net::SSLeay;
> > -	Net::SSLeay::load_error_strings();
> > -	Net::SSLeay::SSLeay_add_ssl_algorithms();
> > -	Net::SSLeay::randomize();
> > -};
> > -plan(skip_all => 'Net::SSLeay not installed') if $@;
> > -
> > -my $t = Test::Nginx->new()->has(qw/mail mail_ssl imap openssl:1.0.2/)
> > +my $t = Test::Nginx->new()
> > +	->has(qw/mail mail_ssl imap openssl:1.0.2 socket_ssl_reused/)
> > 	->has_daemon('openssl');
> > 
> > plan(skip_all => 'no ssl_conf_command') if $t->has_module('BoringSSL');
> > @@ -50,7 +44,7 @@ mail {
> >     auth_http  http://127.0.0.1:8080;   # unused
> > 
> >     server {
> > -        listen       127.0.0.1:8443 ssl;
> > +        listen       127.0.0.1:8993 ssl;
> 
> Please avoid using an upper half of 8000 .. 8999 range for TCP tests.

Still, correct port for IMAPS (IMAP over SSL) is 993, and proper 
mapping to the 8xxx range would be 8993.

> The reason is a deficiency in automatic port selection used for
> parallel testing, see Test::Nginx::port().  Currently, ports are
> selected in the reversed order for TCP and UDP for a reason:
> another socket type for the same port is used as a lock.
> But that gives a race if you try to concurrently select the same
> port for both TCP and UDP.
> Currently, this is worked around by splitting the range:
> bottom half is used for TCP, upper half is used for UDP.
> 
> Luckily, nginx retries bind/listen on NGX_EADDRINUSE up to 5 times,
> but this doesn't always work.

I tend to think this shouldn't be a practical issue unless the 
same initial port number is used for both TCP and UDP tests.  Commits 
1236:93f749c1d5c5 and 1237:e4974af3fb12 suggests this was actually 
the case when this split was introduced.

That is, failure to use proper port numbers matching the protocol 
being tested was the real reason for the issue observed with 
automatic port selection.  And I would rather not use this issue 
as an excuse to not use proper port numbers.

If that's will be demonstrated to be a practical problem even if 
initial port numbers are different, a better solution might be to 
allocate UDP ports in the 9xxx range.  This will make it possible 
to use proper port numbers in all cases.

-- 
Maxim Dounin
http://mdounin.ru/