Unexpected EOF handling

[Dmitry Belyavsky]

Hello,

I want to draw everybody's attention to the position (and argumentation) of
Nginx developers:
===========

As already mentioned by Dmitry, we here at nginx don't think the change was
necessary. As Matt already said above in the comments to SSL_CONF_cmd.pod
change, the error was always reported. The only issue is that
SSL_ERROR_SYSCALL with a 0 errno is not properly documented. On the other
hand, the behaviour was present since ancient OpenSSL versions, and
actually tested in various software using OpenSSL library, including nginx.
A better solution, in our opinion, would be to document the error instead.

Right now the situation in OpenSSL 3.0 is that the error reporting
behaviour was changed, and, if we are going to support OpenSSL 3.0, we have
to introduce specific error testing for OpenSSL 3.0. And at the same time
we have to support previous error reporting, since we support OpenSSL
versions starting from OpenSSL 0.9.8, as well as other libraries such as
BoringSSL and LibreSSL, which still report connection close with
SSL_ERROR_SYSCALL with a 0 errno.

For obvious reasons we don't want to support multiple code paths to test
for the same error. Especially keeping in mind that due to BoringSSL and
LibreSSL we probably have to support these multiple code paths forever.

It would be really helpful if the change in question was reverted and the
existing behaviour (that is, SSL_ERROR_SYSCALL with a 0 errno) was
documented instead.
===========

>From my point of view, if we don't revert the change for the sake of API
clarity, we need to provide an option restoring old behaviour at least for
test purposes.

On Thu, May 7, 2020 at 2:52 PM Tomas Mraz <tmraz at redhat.com> wrote:

> On Thu, 2020-05-07 at 13:22 +0200, Kurt Roeckx wrote:
> > Hi,
> >
> > We introduced a change in the 1.1.1e release that changed how we
> > handled an unexpected EOF. This broke various software which
> > resulted in the release of 1.1.1f that reverted that change.
> > In master we still have the 1.1.1e behaviour.
> >
> > The change itself is small, it just calls SSLfatal() with a new
> > error code. This has at least 2 effects:
> > - The error code was changed from SSL_ERROR_SYSCALL with errno 0
> >   to SSL_ERROR_SSL with reason code
> >   SSL_R_UNEXPECTED_EOF_WHILE_READING
> > - The session is marked as in error, and so can't be used to
> >   resume.
> >
> > There is code written that now checks for the SSL_ERROR_SYSCALL
> > with errno 0 case, while we never documented that behaviour. The
> > 1.1.1 manpage of SSL_get_error() currently reports this as a bug
> > and that it will change to SSL_ERROR_SSL /
> > SSL_R_UNEXPECTED_EOF_WHILE_READING.
> >
> > Code that checks the SSL_ERROR_SYSCALL with errno 0 seem to fall
> > in at least 2 categories:
> > - Ignore the error
> > - Check for the error, for instance in a test suite
> >
> > Not sending the close notify alert is a violation of the TLS
> > specifications, but there is software out there doesn't send it,
> > so there is a need to be able to deal with peers that don't send
> > it.
> >
> > When the close notify alert wasn't received, but we did get an
> > EOF, there are 2 cases:
> > - It's a fatal error, the protocol needs the close notify alert to
> >   prevent a truncation attack
> > - The protocol running on top of SSL can detect a truncation
> >   attack itself, and does so. It doesn't need the close notify
> >   alert. It's not a fatal error. They just want to check that that
> >   is what happened.
> >
> > The problem is that we can't make a distiction between the 2
> > cases, so the only thing we can do currently is to look at
> > it as a fatal error. So the documentation was changed to say
> > that if you're in the 2nd cases, and need to talk to a peer
> > that doesn't send the close notify alert, you should not wait
> > for the close notify alert, so that you don't get an error.
> >
> > The alternative is that we add an option that does tell us if we
> > should look at it as a fatal error or not.
> >
> > There is at least a request that we keep returning the old error code
> > (SSL_ERROR_SYSCALL with errno 0). I think that from an API point
> > of view this is very confusing. We'd then have SSL_ERROR_SYSCALL
> > as documented that it's fatal, except when errno is 0. If errno is
> > 0, it can either be fatal or not, and we're not telling you which
> > one it is. I think we also can't guarantee that SSL_ERROR_SYSCALL
> > with errno 0 is always the unexpected EOF, we returned that
> > combination because of a bug in OpenSSL.
> >
> > So I think we need at least to agree on:
> > - Do we want an option that makes the unexpected EOF either a fatal
> >   error or a non-fatal error?
> > - Which error should we return?
>
> From application developer side of view for protocols that do not
> depend on SSL detecting the truncation I think inventing a different
> behavior for this condition than the existing one as of 1.1.1f would be
> clearly wrong. Switching on a SSL_OP if that newly provided OP is a
> trivial change to an application that needs to accommodate various
> versions of OpenSSL (and I am not talking about forks), however
> switching on a SSL_OP and also add another special error case is much
> more complicated change and has potential for bringing regressions in
> the applications that need to do it.
>
> It is also an API break, however we would do it anyway unless we make
> the legacy behavior the default one, so that is not really relevant
> here.
>
> Is there really another situation where SSL_ERROR_SYSCALL with errno 0
> could be returned apart from the unclean EOF condition?
>
> I can't really think of any.
>
> So I would be just for properly documenting the condition and keeping
> it as is if the SSL_OP to ignore unclean EOF is in effect.
>
> --
> Tomáš Mráz
> No matter how far down the wrong road you've gone, turn back.
>                                               Turkish proverb
> [You'll know whether the road is wrong if you carefully listen to your
> conscience.]
>
>
>

-- 
SY, Dmitry Belyavsky
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mta.openssl.org/pipermail/openssl-project/attachments/20200507/e867cd38/attachment.html>