Bug#914034: Bug#911938: libhttp-daemon-ssl-perl FTBFS: tests fail: Connection refused

[Steffen Ullrich]

On Mon, May 13, 2019 at 03:18:14PM +0200, Guilhem Moulin <guilhem at debian.org> wrote:
> On Mon, 13 May 2019 at 06:31:26 +0200, Steffen Ullrich wrote:
> > Applications which relied on blocking I/O in connection with select could
> > also hang before,
>
> Uh, what?  “Before” meaning with ≤TLSv1.2, or with OpenSSL <1.1.1a's
> default flags?  libssl mentions no such thing beside the new default
> mode.  And in fact the s_client() program, *from OpenSSL upstream
> itself*, does precisely that: a select loop in blocking I/O mode (unless
> the ‘-nbio’ flag is set).

"Before" meaning already with previous OpenSSL versions. But the effect was
likely hard to notice: an incomplete SSL record would not cause a permanent
hang but only a blocking read until the rest of the data was received, i.e.
only some slow down in some situations.

>
>     https://github.com/openssl/openssl/blob/OpenSSL_1_1_1/apps/s_client.c#L2817
>
> s_client() is roughly speaking a C version the ‘netcat.pl’ prototype I
> attached earlier.  Unsurprisingly, since 1.1.1a the code clears
> SSL_MODE_AUTO_RETRY from the bitmask mode of the newly created SSL_CTX
> object.  That change was even done in the very same commit that enabled
> SSL_MODE_AUTO_RETRY by default :-)
>
>     https://github.com/openssl/openssl/commit/693cf80c6ff54ae276a44d305d4ad07168ec6895#diff-7f3b79983f6d53c047c90a62813cc11f

s_client is just a better test program and not production code which
actually cares about edge cases. And the change was likely needed since
s_client now had serious problems and this was the easiest way to address
these.

> IMHO using polling sockets in blocking I/O is fairly common.  Granted,
> that itself itself doesn't say much about code quality, but the fact that
> the OpenSSL project *does* use it (and explicitly mentions it in the
> docs) gives me confidence that it's a fine use which should still be
> supported.

From my experience with OpenSSL and its documentation I don't agree with
this. Documentation is sometimes wrong or misleading and more often
incomplete (but its actually improving).
And while using blocking I/O with polling sockets might be fine with plain
sockets where each byte is part of application data it is not fine with SSL
since the unit in SSL is not a byte but a record and some records might
contain application data and some not.

> The documentation (libssl's SSL_read()'s [0], Net::SSLeay::read() [1],
> as well as IO::Socket::SSL::sysread() [2]) all warn that reading from an
> SSL socket has a different behavior than usual read(2) system calls, in
> that read failures should be treated with care, as there are both
> retryable (eg. when no application data was received) and non-retryable
> errors.

As I was the one who wrote the documentation for IO::Socket::SSL: the part
about sysread failing was supposed to be about non-blocking sockets. A
blocking socket was not expected to return nothing just because the handshake
was not done.

>
> So from my perspective, the expectation that IO::Socket::SSL::sysread()
> behaves like a “normal” sysread doesn't hold, and never did.  (There is
> even an entry for “Expecting exactly the same behavior as plain
> sockets” in the “Common errors” section of IO::Socket::SSL's manpage!)

This part talks about specific cases and how to deal with these but says
nothing about blocking sockets temporary failing.

> For what it's worth I think it's a shame that SSL_MODE_AUTO_RETRY was
> not the default earlier, as it's convenient to be able to write
>
>     $sock->sysread($buf, $len) // die;
>
> and not bother about inspecting $SSL_ERROR.  But programs have been

I agree with that. But in the past SSL_MODE_AUTO_RETRY was only relevant
with unexpected renegotiations which nearly never happened. So nobody
actually noticed.

> written with the old default in mind, and the fact that SSL_read()
> doesn't behave like a normal read() system call.  For these programs to
> keep working, there needs to be a way to switch SSL_MODE_AUTO_RETRY off.
> libssl provides SSL_CTX_clear_mode(), Net::SSLeay has
> CTX_ctrl(,78,SSL_CTRL_CLEAR_MODE,0), and I'd like to have something
> similar in IO::Socket::SSL, too.

I'm not convinced that this is the way to go, i.e. I'd rather see the
applications use non-blocking sockets.  But I'm also not stubbornly
against it.
Do you know a relevant module or actually used application which has
problems because of the default mode of SSL_MODE_AUTO_RETRY the ability to
change it would fix the related problems and not introduce new ones, i.e.
where such a fix would be the right thing to do?
LWP of course does not count since the right thing to do in LWP was to
remove the obsolete patch which kept the socket blocking.

I have also some problems to provide a consistent API for this which can
actually be understood. This would in my opinion mean that I have a clear
default for the value, i.e. that I enable SSL_MODE_AUTO_RETRY by default for
older OpenSSL versions the same way it is done in OpenSSL 1.1.1. But that
might break other things :(

Another option might be to have an option SSL_mode_auto_retry which by
default is not set but might be set to either 0 or 1 to get a consistent
behavior across different OpenSSL versions.

Regards,
Steffen