Re: [TLS] TLS 1.3 and TCP interactions

[David Benjamin <[e-mail]>]

On Fri, May 29, 2020 at 8:06 PM Jeremy Harris <[e-mail]> wrote:

> On 29/05/2020 22:52, David Benjamin wrote:
> > On Fri, May 29, 2020 at 5:36 PM Jeremy Harris <[e-mail]> wrote:
> >
> >> On 29/05/2020 21:59, David Benjamin wrote:
> >>> Moreover, in a client-speaks-first protocol, the error now comes after
> >> the
> >>> client has already sent its request. This is not only a behavior change
> >> but
> >>> makes it unreliable over TCP. TCP sees:
> >>>
> >>>
> >>>    1.
> >>>
> >>>    Client: write(ClientHello);
> >>>    2.
> >>>
> >>>    Server: read(ClientHello); write(ServerHello..Finished);
> >>>    3.
> >>>
> >>>    Client: read(ServerHello..Finished); write(Certificate..Finished);
> >>>    4.
> >>>
> >>>    Server: read(Certificate..Finished); write(bad_certificate);
> close();
> >>
> >> Rather than close(), the server should shutdown(SHUT_WR)
> >>
> >>>    5.
> >>>
> >>>    Client: write(“GET / ...”); read(???);
> >>>
> >>>
> >>> Note (4) and (5) happen in parallel. Ideally ??? would be a
> >> bad_certificate
> >>> alert, but it is sometimes a TCP reset. I’m not a TCP expert, but I
> >> believe
> >>> this is because the client writes data (“GET / ...”) the server never
> >>> consumes.
> >>
> >> ... because, having solely notified that it will send no more, the
> >> server is free to continue to read from the socket.
> >>
> >
> > The server's application logic has no reason to continue to read from the
> > socket.
>
> Perhaps not, though I suggest it is valuable to it for debug and
> development to be able to see the client's reaction to the shutdown.\
>

I have never seen a production server do this.


> However, you ignored the value of the shutdown() action not resulting
> in a RST.  That means that the client view becomes deterministic;
> your "read(???)" in the close/RST scenario becomes
> "read(error, ALERT:bad cert)".
>

Yes, I said exactly this twice now. :-P Except the shutdown() isn't the
deciding factor. It's the lack of close(). shutdown(SHUT_WR) doesn't hurt,
but the alert already signals to the client that the connection is done.

> It seems the only fix is for the server to keep the connection alive for
some time after the failure, maybe draining some bytes from the
application, with some limit before giving up and resetting if the client
seems to be writing a lot of data without ever reading. This would need to
be quite up the stack. We have not implemented this.

However, there are two problems here. First, the server needs to call
close() eventually to release resources. This is a failed connection. The
natural thing to do is to tear down the connection state which, ultimately,
will destroy the socket in the natural implementation. In
particular, connection management is often outside the low-level TLS
library. The HTTP server is simply doing something like:

   if (tls_server_handshake(connection->tls) != TLS_HANDSHAKE_SUCCESS) {
      // Release all resources associated with connection. It's not needed
anymore.
      // This, among other work, needs to call close(connection->fd). The
caller here
      // is not a TLS expert and reasonably assumes that the TLS library
has sent
      // any data it needs to send. But the TLS library doesn't own the fd
and can't
      // prevent the close.
      delete_connection(connection);
  }

If the HTTP server software is aware of this mess, it can certainly get the
right behavior by leaving the socket open for longer, releasing it later
after a timeout or so. This is not obvious, hence the note here, so folks
would be aware of this interaction. Moreover, simply closing the socket on
error worked just fine in TLS 1.2, so most existing HTTP server software
are not going to keep their connections alive after error for the sake of
it.

Additionally, as noted in the original email, leaving the socket open is
insufficient if the client's first write exceeds the transport buffers.
Either the client needs to eagerly read (not an obvious expectation, nor
something that all clients do, hence the email), or the server must drain
the socket (an *especially* non-obvious expectation).

David