[openssl-users] when should client stop calling SSL_read to get TLS1.3 session tickets after the close_notify?

[Sam Roberts]

On Fri, Feb 15, 2019 at 3:35 PM Matt Caswell <matt at openssl.org> wrote:
> On 15/02/2019 20:32, Viktor Dukhovni wrote:
> >> On Feb 15, 2019, at 12:11 PM, Sam Roberts <vieuxtech at gmail.com> wrote:
> > OpenSSL could delay the actual shutdown until we're about to return
> > from the SSL_accept() that invoked the callback.  That is SSL_shutdown()
> > called from callbacks could be deferred until a more favourable time.

In this case, it's an SSL_read() that invoked the callback, though
probably not relevant.

And actually, no deferal would be necessary, I looks to me that the
info callback for handshake done is coming too early. Particularly,
the writing of the NewSessionTickets into the BIO should occur before
the info callback. I'll check later, but I'm pretty sure with TLS1.2
the session tickets are written and then the HANDSHAKE_DONE info
callback occurs, so the timing here is incompatible with TLS1.2.

Though the deferal mechanism might be there already. It looks like
doing an SSL_write(); SSL_shutdown() in the info callback works fine,
on the client side new ticket callbacks are fired by the SSL_read()
before the SSL_read() sees the close_notify and returns 0. I haven't
looked at the packet/API trace for this, because the tests all pass
for this case, but I do see that in the javascript called from the
HANDSHAKE_DONE callback, that calling .end("x") (write + shutdown)
causes the client to get tickets, but .end() causes it to miss them
because they are after close_notify.

> Oh - right. I missed this detail. Calling SSL_shutdown() from inside a callback
> is definitely a bad idea. Don't do that.

The info callback, or ANY callbacks? What about the new ticket
callback, for example? Is it expected that no SSL_ calls are made in
ANY callbacks?

This code has been working a fair number of years now, I can move it
(and review every other callback where we callout to javascript code)
to a model where callbacks just save data, set global state, and
return into SSL, and we check after returning from SSL_read() what has
happened, and callback into javascript then, but its a bit of work,
and this fringe case of TLS servers that shutdown immediately after
handshake is not likely to be that important (at least in the short
term, though if our users scream about the API change we'll have to
decide whether we can enable TLS1.3 on stable branches, or if this
difference counts as semver-major for code that didn't explicitly
opt-in to 1.3).

Cheers,
Sam