ssl_do_handshake can hang with small buffer

[kroeckx]

From #7948 by @njsmith:

My test suite literally started deadlocking when I upgraded to 1.1.1. It's a test where the client does SSL_do_handshake → SSL_write, while the server does SSL_do_handshake → SSL_read, and the transport layer between them has minimal buffering. This is the same communication pattern used by e.g. HTTP/1.1, and it worked fine with previous versions of openssl.

The deadlock happens because the server's SSL_do_handshake doesn't return until it finishes writing out the session tickets, but by the time the server starts trying to write the tickets, the client's SSL_do_handshake has already completed and the client has switched to SSL_write. Neither write can complete because neither side is reading → deadlock.

I could make a standalone reproducer (in Python, say), if that would be helpful, but the problem is very straightforward. As long as the server's SSL_do_handshake sends session tickets but the client's SSL_do_handshake doesn't read them, you have a deadlock hazard here.

I assume that this is with both sides having blocking sockets. Both sides doing a write and blocking on it is always going to deadlock.

One difference between TLS 1.2 and 1.3 is that in TLS 1.2 the server sends the last message of the handshake, while in TLS 1.3 the client does it. In TLS 1.3 the client can directly start sending after it has sent the finish message, while in TLS 1.2 it needs to wait for finish message from the server.

OpenSSL seems to currently hang in SSL_do_handshake() to sent the session tickets. I guess we could make SSL_do_handshake() return when we received the finish message from the client, but I don't think this solves anything. We would then just have to send them in SSL_read(), and we'd hang in SSL_read() instead of SSL_do_handshake().

I currently don't see how we can support TLS 1.3 with blocking sockets on both the client and the server where the first thing the client wants to do is write and the server wants to send session tickets.

[mattcaswell]

Wouldn't this impact non-blocking sockets too? I've not tried it, but I'm not sure what is special about blocking sockets in this scenario - except of course for non-blocking sockets SSL_do_handshake() would return - but always give SSL_ERROR_WANT_WRITE until the session tickets were written.

I wonder whether this ever actually occurs in a real world scenario, i.e. not in some "test" application? I just did a quick test and observed an s_client <-> s_server interaction. Two session ticket TCP packets were sent* each of length 341 bytes, i.e. the client would need to have a buffer of less than 642 bytes before this becomes a problem. Would we ever reasonably expect clients with buffers that small?

Possible fixes and/or workarounds might be:

• We could delay sending the tickets until SSL_write() is called on the server (which might be never for some applications). If we went this route I'd suggest making it a non-default option
• We could introduce an option to enable sending of the tickets "on demand" rather than automatically. Again I'd think this would be a non-default scenario.
• The application could configure itself to send no tickets (obviously at the cost of not being able to do resumption)
• We could introduce some kind of "give up" capability when writing the session tickets, e.g. if we've tried X times and we still can't write then don't bother. This wouldn't help with blocking sockets though.
• We just document this as a restriction

* aside: I wonder this should be optimised so that no "flush" occurs when we know there are more session tickets to write out, in order that all session tickets go into a single TCP packet where possible?

[kroeckx]

Getting an SSL_ERROR_WANT_WRITE should not prevent us from calling SSL_read() if we know we can read data. I'm not sure if we currently will then process that data. Don't we have some other open issue about this?

[mattcaswell]

Getting an SSL_ERROR_WANT_WRITE should not prevent us from calling
SSL_read() if we know we can read data. I'm not sure if we
currently will then process that data. Don't we have some other
open issue about this?

If we are writing out session tickets then we are in the "init" state. If you call SSL_read() while in the "init" state then we immediately go back into the state machine code and start trying to write the tickets out again.

[njsmith]

Wouldn't this impact non-blocking sockets too?

Yeah, the code where I hit this actually uses memory BIOs and handles the I/O itself (to make it easy to run over arbitrary transports, not just OS sockets).

I wonder whether this ever actually occurs in a real world scenario, i.e. not in some "test" application? I just did a quick test and observed an s_client <-> s_server interaction. Two session ticket TCP packets were sent* each of length 341 bytes, i.e. the client would need to have a buffer of less than 642 bytes before this becomes a problem. Would we ever reasonably expect clients with buffers that small?

Yeah, this is why #7948 emphasized the way automatic session tickets can cause data loss, not the way it can deadlock with small buffers :-). The two issues have the same cause, though, and I just posted there about why I think this is important, even though it seems like it should only matter in rare edge cases.

We could delay sending the tickets until SSL_write() is called on the server (which might be never for some applications). If we went this route I'd suggest making it a non-default option
We could introduce an option to enable sending of the tickets "on demand" rather than automatically. Again I'd think this would be a non-default scenario.
The application could configure itself to send no tickets (obviously at the cost of not being able to do resumption)

I guess if these were all implemented, then that would be sufficient to let me work around it in my library for my users – I could unconditionally suppress the automatic sending, and then either let SSL_write take care of it or implement my own tracking and auto-sending in my write wrapper. Unfortunately, right now the only workaround seems to be to disable session tickets entirely for all users, with no way to recover, which is really not great :-(. (And even doing that requires some pretty arcane tweaks at the C level.)

A SSL_write_ticket function seems like a reasonable thing to have in any case, just to expose the TLS 1.3 functionality that a ticket can be sent at any time?

In the long run I suspect the only fully-satisfactory solution would be to add a TLS extension that lets the client request tickets as part of the opening handshake. This would help because right now, the problems all happen because clients don't have any way to know whether session tickets are incoming at session startup. If this extension existed, then openssl could make the client-side SSL_do_handshake request tickets unconditionally, and if the ServerHello confirmed that the extension was understood and session tickets were coming, the client-side SSL_do_handshake would wait until the tickets were received before returning. But I have no idea how hard it would be to get such an extension standardized, and obviously it would still be nice to have some plan in the mean time.

[kroeckx]

You can argue that sending after the finished message from the client is received and processed we are no longer in init, just trying to write (non-application) data.

[davidben]

We could delay sending the tickets until SSL_write() is called on the server (which might be never for some applications). If we went this route I'd suggest making it a non-default option

This is an interesting nuisance! We're still mulling things over on our end, but I think we're largely leaning towards this option, perhaps even by default. It's the most straightforward. For application protocols where the server never writes, the client may also never call SSL_read, so it wouldn't pick up the ticket anyway. (Of course, the client code could be 1.3-aware and pump SSL_read to pick up tickets, but then the server code could also be 1.3-aware and pump an empty SSL_write or some dedicated API to flush the post-handshake bits.)

[kroeckx]

One thing I'm not clear about is why the client would hang if it's not blocking on the write.

[mattcaswell]

For application protocols where the server never writes, the client may also never call SSL_read, so it wouldn't pick up the ticket anyway.

Well, that's not entirely true - at least not in OpenSSL (don't know about BoringSSL). If a bi-di shutdown occurs then we recently changed things to process the tickets at that point.

[davidben]

We discard SSL_shutdown tickets to avoid confusing callers with unexpected callback calls. The only code I've ever seen do bidi shutdown (it's kinda pointless and the API is awful) does it on accident in teardown code, at a point where getting a ticket callback is questionable. Given, in practice, servers only really send tickets at the beginning, and the ticket will have been picked up already, we thought picking up tickets in that weird edge case wasn't worth the breakage risk.

(Usually the sequence of events is the programmer doesn't call SSL_shutdown at all, notices session resumption is bust due to OpenSSL junking the sessions on SSL_free, finds out they need to call SSL_shutdown, and calls it twice because they found mention of that somewhere. Setting quiet shutdown by default is probably more compatible with what existing OpenSSL code wants than the actual behavior.)

But, yeah, one cannot guarantee TLS 1.3 will always behave like TLS 1.2 w.r.t. session resumption all the time. That was already hopeless with post-handshake tickets. (SSL_do_handshake used to make the session available at the client. Now the client needs to pump SSL_read a bit.) The best we can do is preserve resumption most of the time. Otherwise, connections will still work, but we may not resume in weird cases unless you incorporate a few more things into your program.

[kaduk]

In the long run I suspect the only fully-satisfactory solution would be to add a TLS extension that lets the client request tickets as part of the opening handshake.

Sounds an awful lot like https://tools.ietf.org/html/draft-wood-tls-ticketrequests-01

[davidben]

I'm not sure that quite does it since the server still needs to know the client will be blocking its write on that read. But that turns our nice 1-RTT handshake into a 2-RTT one, so we don't really want that.

[njsmith]

@davidben that proposal as currently written doesn't quite solve things, but I think a variant would. Suppose we made it so servers who received a ticket_request extension in the ClientHello confirmed that by echoing it back in EncryptedExtensions, and that when they did this echo that was a guarantee that they would send the specified number of tickets immediately after they sent Finished.

No matter which handshake mode we're in, the client's SSL_do_handshake always reads until it sees the server's Finished. With the extension above, the client could instead keep reading until it sees the expected number of session tickets. And since the client receives EncryptedExtensions before it receives Finished, this can all be done reliably without any extra round trips.