Maxim Dounin
May 23, 2023 09:32AM
Hello!

On Tue, May 23, 2023 at 02:34:54PM +0400, Sergey Kandaurov wrote:

> > On 23 May 2023, at 03:43, Maxim Dounin <mdounin@mdounin.ru> wrote:
> >
> > Hello!
> >
> > On Mon, May 22, 2023 at 11:52:14PM +0400, Sergey Kandaurov wrote:
> >
> >> # HG changeset patch
> >> # User Sergey Kandaurov <pluknet@nginx.com>
> >> # Date 1684773874 -14400
> >> # Mon May 22 20:44:34 2023 +0400
> >> # Node ID 4a3a451716ba26f8fc4be1ccc88dd8596101a6b2
> >> # Parent 231b14e2041afed10f5c2e6f62aad298854a6379
> >> Tests: unbreak tests with IO::Socket:SSL lacking SSL_session_key.
> >>
> >> diff --git a/ssl_certificate.t b/ssl_certificate.t
> >> --- a/ssl_certificate.t
> >> +++ b/ssl_certificate.t
> >> @@ -171,6 +171,8 @@ local $TODO = 'no TLSv1.3 sessions, old
> >> if $Net::SSLeay::VERSION < 1.88 && test_tls13();
> >> local $TODO = 'no TLSv1.3 sessions, old IO::Socket::SSL'
> >> if $IO::Socket::SSL::VERSION < 2.061 && test_tls13();
> >> +local $TODO = 'no SSL_session_key, old IO::Socket::SSL'
> >> + if $IO::Socket::SSL::VERSION < 1.965;
> >>
> >> like(get('default', 8080, $s), qr/default:r/, 'session reused');
> >>
> >
> > Should be:
> >
> > diff -r a797d7428fa5 ssl_certificate.t
> > --- a/ssl_certificate.t Thu May 18 18:07:19 2023 +0300
> > +++ b/ssl_certificate.t Mon May 22 22:20:59 2023 +0000
> > @@ -177,6 +177,8 @@
> > TODO: {
> > # ticket key name mismatch prevents session resumption
> > local $TODO = 'not yet' unless $t->has_version('1.23.2');
> > +local $TODO = 'no SSL_session_key, old IO::Socket::SSL'
> > + if $IO::Socket::SSL::VERSION < 1.965;
> >
> > like(get('default', 8081, $s), qr/default:r/, 'session id context match');
> >
> > as the "session reused" is passed regardless of SSL_session_key
> > presence.
>
> I agree. The reason why it was done so is to cover an unrelated
> issue with TLSv1.2 session reuse that happens in ssl_certificate.t
> with IO::Socket::SSL < 1.963, which results in inability to restore
> the session. Probably that is because of improper behavior in stop_SSL.
> Well, it doesn't happen if remove http_end() to stop closing a client socket.
> Though, given that this bug doesn't appear on real distros that have
> IO::Socket::SSL [1.84..1.963] (inclusive), the range where it manifests,
> so that it doesn't annoy in real life, then we can simply ignore it.

We can use SKIP instead if there are unexpected issues with older
IO::Socket::SSL versions, or even introduce socket_ssl_session_key
feature and skip the entire test.

But if this does not happen on real distros, this probably don't
worth the effort. For sure there are IO::Socket::SSL (as well as
Net::SSLeay and OpenSSL) versions with bugs, but trying to work
around all possible bugs is certainly not the goal.

> >
> >> diff --git a/stream_ssl_certificate.t b/stream_ssl_certificate.t
> >> --- a/stream_ssl_certificate.t
> >> +++ b/stream_ssl_certificate.t
> >> @@ -148,6 +148,8 @@ local $TODO = 'no TLSv1.3 sessions, old
> >> if $Net::SSLeay::VERSION < 1.88 && test_tls13();
> >> local $TODO = 'no TLSv1.3 sessions, old IO::Socket::SSL'
> >> if $IO::Socket::SSL::VERSION < 2.061 && test_tls13();
> >> +local $TODO = 'no SSL_session_key, old IO::Socket::SSL'
> >> + if $IO::Socket::SSL::VERSION < 1.965;
> >>
> >> like(get('default', 8080, $s), qr/default:r/, 'session reused');
> >>
> >
> > The same here.
> >
> > Additionally, ssl_session_ticket_key.t also uses SSL_session_key,
> > but it currently happens to be skipped at least on CentOS 7 due to
> > old Net::SSLeay. Given IO::Socket::SSL changes, appropriate
> > version for it is probably 2.029.
> >
> > diff --git a/ssl_session_ticket_key.t b/ssl_session_ticket_key.t
> > --- a/ssl_session_ticket_key.t
> > +++ b/ssl_session_ticket_key.t
> > @@ -24,6 +24,8 @@ select STDOUT; $| = 1;
> >
> > eval { require Net::SSLeay; die if $Net::SSLeay::VERSION < 1.86; };
> > plan(skip_all => 'Net::SSLeay version => 1.86 required') if $@;
> > +eval { require IO::Socket::SSL; die if $IO::Socket::SSL::VERSION < 2.029; };
> > +plan(skip_all => 'IO::Socket::SSL version => 2.029 required') if $@;
> >
> > my $t = Test::Nginx->new()->has(qw/http http_ssl socket_ssl/)
> > ->has_daemon('openssl')->plan(2)
> >
>
> Indeed, it doesn't happen to work with old IO::Socket::SSL versions,
> thanks for catching this.
> By manual testing, it appears that 2.030 is required to work.
> The relevant git log:
> remove internal sub session_cache and access cache directly (faster)
> This also fixes a problem when SSL_session_key was used, which was
> introduced in 2.029
>
> Additionally, if TLSv1.3 was negotiated, for session reuse to work,
> 2.061 is required (same as for rest TLSv1.3 session reuse tests).
>
> Moved this part to the separate change:
>
> # HG changeset patch
> # User Sergey Kandaurov <pluknet@nginx.com>
> # Date 1684835827 -14400
> # Tue May 23 13:57:07 2023 +0400
> # Node ID 5dc8aa4446484456eb1310e63b092f1d08258574
> # Parent d570dbcad925499d968006ea509798234df09248
> Tests: unbreak ssl_session_ticket_key.t with old IO::Socket::SSL.
>
> Once ssl_session_ticket_key.t was rewritten using IO::Socket::SSL,
> it requires checks for SSL_session_key and TLSv1.3 session reuse support.
> While SSL_session_key has been introduced in IO::Socket::SSL 1.965,
> further improvements appeared in 2.030 are needed for tests to work.
>
> diff --git a/ssl_session_ticket_key.t b/ssl_session_ticket_key.t
> --- a/ssl_session_ticket_key.t
> +++ b/ssl_session_ticket_key.t
> @@ -24,6 +24,8 @@ select STDOUT; $| = 1;
>
> eval { require Net::SSLeay; die if $Net::SSLeay::VERSION < 1.86; };
> plan(skip_all => 'Net::SSLeay version => 1.86 required') if $@;
> +eval { require IO::Socket::SSL; die if $IO::Socket::SSL::VERSION < 2.030; };
> +plan(skip_all => 'IO::Socket::SSL version => 2.030 required') if $@;
>
> my $t = Test::Nginx->new()->has(qw/http http_ssl socket_ssl/)
> ->has_daemon('openssl')->plan(2)
> @@ -99,6 +101,8 @@ select undef, undef, undef, 2.5;
>
> local $TODO = 'no TLSv1.3 sessions in LibreSSL'
> if $t->has_module('LibreSSL') && test_tls13();
> +local $TODO = 'no TLSv1.3 sessions, old IO::Socket::SSL'
> + if $IO::Socket::SSL::VERSION < 2.061 && test_tls13();
>
> cmp_ok(get_ticket_key_name(), 'ne', $key, 'ticket key next');
>

This needs Net::SSLeay 1.88 also then?
The usual pattern is:

local $TODO = 'no TLSv1.3 sessions, old Net::SSLeay'
if $Net::SSLeay::VERSION < 1.88 && test_tls13();
local $TODO = 'no TLSv1.3 sessions, old IO::Socket::SSL'
if $IO::Socket::SSL::VERSION < 2.061 && test_tls13();
local $TODO = 'no TLSv1.3 sessions in LibreSSL'
if $t->has_module('LibreSSL') && test_tls13();

--
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 565 April 16, 2023 11:46PM

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

Maxim Dounin 96 April 16, 2023 11:46PM

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

Maxim Dounin 99 April 16, 2023 11:46PM

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

Sergey Kandaurov 106 May 03, 2023 12:22PM

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

Maxim Dounin 107 May 03, 2023 11:58PM

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

Maxim Dounin 100 April 16, 2023 11:46PM

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

Sergey Kandaurov 73 May 11, 2023 07:50AM

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

Maxim Dounin 87 May 14, 2023 02:54PM

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

Maxim Dounin 95 April 16, 2023 11:46PM

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

Maxim Dounin 101 April 16, 2023 11:46PM

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

Maxim Dounin 113 April 16, 2023 11:46PM

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

Sergey Kandaurov 100 May 11, 2023 10:40AM

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

Maxim Dounin 90 May 14, 2023 05:12PM

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

Maxim Dounin 103 April 16, 2023 11:46PM

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

Maxim Dounin 100 April 16, 2023 11:46PM

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

Maxim Dounin 100 April 16, 2023 11:46PM

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

Sergey Kandaurov 95 May 11, 2023 10:28AM

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

Maxim Dounin 116 May 18, 2023 11:18AM

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

Maxim Dounin 101 April 16, 2023 11:46PM

[PATCH 0 of 6] SSL tests refactoring fixes

Sergey Kandaurov 80 May 22, 2023 03:58PM

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

Sergey Kandaurov 91 May 22, 2023 03:58PM

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

Maxim Dounin 72 May 22, 2023 04:38PM

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

Sergey Kandaurov 79 May 22, 2023 03:58PM

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

Maxim Dounin 70 May 22, 2023 07:44PM

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

Sergey Kandaurov 90 May 23, 2023 06:36AM

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

Maxim Dounin 137 May 23, 2023 09:32AM

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

Sergey Kandaurov 77 May 22, 2023 04:00PM

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

Maxim Dounin 76 May 22, 2023 07:52PM

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

Sergey Kandaurov 78 May 22, 2023 04:00PM

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

Maxim Dounin 74 May 22, 2023 09:08PM

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

Sergey Kandaurov 91 May 23, 2023 08:24AM

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

Sergey Kandaurov 84 May 22, 2023 04:00PM

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

Maxim Dounin 74 May 22, 2023 10:18PM

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

Sergey Kandaurov 130 May 23, 2023 08:56AM

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

Sergey Kandaurov 82 May 22, 2023 04:00PM

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

Maxim Dounin 69 May 22, 2023 10:40PM



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

Online Users

Guests: 251
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