Hi Dimitri, On Tue, 07 May 2019 at 15:46:25 +0100, Dimitri John Ledkov wrote: > On Tue, 7 May 2019 14:16:43 +0100 Dimitri John Ledkov <x...@ubuntu.com> wrote: >> This issue concerns me a lot at the moment. I am currently trying to >> upgrade OpenSSL from 1.1.0 to 1.1.1 in Ubuntu 18.04 LTS (bionic). And >> as far as I understand all the comment on this debian bug report, >> current application are potentially broken and brokeness happens more >> often with TLSv1.3 and the new OpenSSL 1.1.1 defaults >> (SSL_MODE_AUTO_RETRY). >> >> As far as I understand we do not have a fixed LWP that works correctly >> in blocking, non-blocking, tls 1.2 and tls 1.3. To prevent regressing >> existing users further, does it make sense for me to make updates in >> bionic that: >> >> 1) limit SSL_new and SSL_CTX_new to TLS v1.2 max >> and >> 2) disable SSL_MODE_AUTO_RETRY by default for TLS v1.2 connections? >> >> My goal is to keep existing breakages as is, without introducing new >> ones, whilst getting OpenSSL 1.1.1 into bionic. Granted this will not >> get TLS v1.3 enabled for perl server/clients without code changes, but >> oh well. Those who want it, will be able to force / start using it.
It's IMHO unfortunate to change the default in Net::SSLeay, as TLSv1.3 brings quite a few improvements (in terms of security as well as performance). OpenSSL 1.1.1 was released on 11 Sep 2018 and uploaded to sid the day after, breaking programs using select()/poll() with blocking I/O; this is not specific to Perl bindings — other languages are also affected — yet no one is suggesting to disable TLSv1.3 globally in libssl :-) If TLSv1.3 should be disabled (and the SSL_MODE_AUTO_RETRY flag cleared) then IMHO I think it should be done as close at possible to the leaf application (LWP in this case), not in the library itself. After all we have only one RC bug about this, so I guess other programs (in any language) 1/ have workarounds; 2/ aren't using select()/poll() with blocking I/O; or 3/ aren't affected because they never used SSL_read() as documented. IHMO the libssl change shouldn't be reason to penalize all applications, given most seems unaffected. I still think the right fix is to make SSL_MODE_AUTO_RETRY (or even the whole mode bitmask mode?) configurable in IO::Socket::SSL, and clear it in programs and libraries using select()/poll() with blocking I/O, such as LWP in this case. AFAICT that follows the intention of OpenSSL's development team, unlike global library changes. AFAICT the attached patch (to sid's IO::Socket::SSL and LWP::Protocol::https, respectively 2.060-3 and 6.36-1) fixes the problem for me, while preserving TLSv1.3 support and default. > I proposed the following patch upstream / request for comments > https://github.com/radiator-software/p5-net-ssleay/pull/139 I personally don't like this change as I hope Buster's Net::SSLeay and other SSL libraries will default to TLSv1.3 on capable servers :-) 2 comments anyway: * OpenSSL <1.1.0 has no SSL_CTX_set_max_proto_version(), so an OpenSSL version test is lacking (nothing more as <1.1.0 has no TLSv1.3 support and SSL_MODE_AUTO_RETRY is unset by default). * Disabling TLSv1.3 won't always prevent hangs, you also need to clear SSL_MODE_AUTO_RETRY to revert to the pre-1.1.1 defaults. With TLSv1.2, SSL_read() returns SSL_ERROR_WANT_READ upon renegotiation, causing applications using select()/poll() with blocking I/O to hang if SSL_MODE_AUTO_RETRY is set. Cheers, -- Guilhem, who isn't not in the Debian Perl team, but who would be quite sad to have to wait one full release cycle for out-of-the-box TLSv1.3 support.
--- a/IO/Socket/SSL.pm +++ b/IO/Socket/SSL.pm @@ -260,6 +260,14 @@ INIT { init() } init(); } + + if (!defined &Net::SSLeay::CTX_clear_mode) { + # assume SSL_CTRL_CLEAR_MODE being 78 since it was always this way + *Net::SSLeay::CTX_clear_mode = sub { + my ($ctx,$opt) = @_; + Net::SSLeay::CTX_ctrl($ctx,78,$opt,0); + }; + } } # global defaults which can be changed using set_defaults @@ -2433,6 +2441,11 @@ # cannot guarantee, that the location of the buffer stays constant Net::SSLeay::CTX_set_mode( $ctx, SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER|SSL_MODE_ENABLE_PARTIAL_WRITE); + # Clear SSL_MODE_AUTO_RETRY on request; useful for applications using select/poll + # with blocking I/O. (Since OpenSSL 1.1.1 SSL_MODE_AUTO_RETRY is enabled by + # default, making such applications hang.) + Net::SSLeay::CTX_clear_mode( $ctx, Net::SSLeay::MODE_AUTO_RETRY() ) + if $arg_hash->{SSL_mode_auto_no_retry}; if ( my $proto_list = $arg_hash->{SSL_npn_protocols} ) { return IO::Socket::SSL->_internal_error("NPN not supported in Net::SSLeay",9) --- a/LWP/Protocol/https.pm +++ b/LWP/Protocol/https.pm @@ -32,6 +32,11 @@ $ssl_opts{SSL_ca_file} = '/etc/ssl/certs/ca-certificates.crt'; } } + # Clear SSL_MODE_AUTO_RETRY as LWP uses select with blocking I/O. + # (Since OpenSSL 1.1.1 SSL_MODE_AUTO_RETRY is enabled by default.) + # https://wiki.openssl.org/index.php/TLS1.3#Non-application_data_records + $ssl_opts{SSL_mode_auto_no_retry} = 1 + unless exists $ssl_opts{SSL_mode_auto_no_retry}; $self->{ssl_opts} = \%ssl_opts; return (%ssl_opts, $self->SUPER::_extra_sock_opts); }
signature.asc
Description: PGP signature