On 17/02/2019 14:26, Matt Caswell wrote:
On 16/02/2019 05:04, Sam Roberts wrote:
On Fri, Feb 15, 2019 at 3:35 PM Matt Caswell <matt@xxxxxxxxxxx> wrote:
On 15/02/2019 20:32, Viktor Dukhovni wrote:
On Feb 15, 2019, at 12:11 PM, Sam Roberts <vieuxtech@xxxxxxxxx> 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.
In TLSv1.2 New session tickets are written as part of the handshake. In TLSv1.3
session tickets are sent after the handshake has completed. It sounds to me like
the info callback is doing the right thing.
That seems to be a major theme in many reported OpenSSL 1.1.1
problems. It seems that you guys have gotten too hung up on how
the TLS 1.3 RFC uses words like handshake differently than the
TLS 1.2 RFC, rather than by the higher level semantics of what
would be considered the API visible meta-operations.
From an API user perspective, the messages that are exchanged
right after the RFC-handshake in order to complete the connection
set up should be considered part of the API-handshake.
This made little difference for the "change cipher spec" TLS 1.2
record, but makes a lot more difference for TLS 1.3 where various
things like certificate checks and session tickets fall into that
gray area.
Any opportunity to send data earlier than that should be handled
in a way that doesn't break the API for applications that aren't
doing so.
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?
I wouldn't go that far. Callbacks occur during the processing of an IO
operation. Callbacks are generally expected to be small and quick. I wouldn't
call anything that might invoke a new IO operation from within a callback, so
SSL_read, SSL_write, SSL_do_handshake, SSL_shutdown etc.
Callbacks are often an opportunity for applications to detect
violations of security policy. It thus makes a lot of sense for
callbacks to request that the connection is ended as soon as
allowed by the risk of creating an attack side channel.
Other OpenSSL callbacks represent the one place to do certain
complex tasks, such as choosing among different certificates,
checking against outside (networked!) revocation systems etc.
Enjoy
Jakob
--
Jakob Bohm, CIO, Partner, WiseMo A/S. https://www.wisemo.com
Transformervej 29, 2860 Søborg, Denmark. Direct +45 31 13 16 10
This public discussion message is non-binding and may contain errors.
WiseMo - Remote Service Management for PCs, Phones and Embedded