Unexpected EOF handling

[Matt Caswell]

On 07/05/2020 20:28, Dmitry Belyavsky wrote:
> 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.

Presumably nginx can already handle the situation where a close_notify
*is* received. So if we have an option to behave as if that had occurred
in the event of eof then nginx should be able to handle it without
requiring special codepaths?? If so then we don't necessarily have to
have an option to restore the old behaviour - just an option to treat
eof like a close_notify.

Matt


> 
> On Thu, May 7, 2020 at 2:52 PM Tomas Mraz <tmraz at redhat.com
> <mailto: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