On Fri, May 29, 2020 at 8:06 PM Jeremy Harris <[email protected]> wrote:
> On 29/05/2020 22:52, David Benjamin wrote: > > On Fri, May 29, 2020 at 5:36 PM Jeremy Harris <[email protected]> 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
_______________________________________________ TLS mailing list [email protected] https://www.ietf.org/mailman/listinfo/tls
