On Mon, Feb 18, 2019 at 2:18 PM Jakob Bohm via openssl-users <
openssl-users@openssl.org> wrote:
> 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 <m...@openssl.org> wrote:
> >>> On 15/02/2019 20:32, Viktor Dukhovni wrote:
> >>>>> On Feb 15, 2019, at 12:11 PM, Sam Roberts <vieuxt...@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.
> > 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.>
>
>
All of that makes me question; so in migrating to 1.3, does the basic flow
change?
>
https://github.com/d3x0r/SACK/blob/master/src/netlib/ssl_layer.c#L178
(handshake... hmm that's long tedious debug optioned code)
summary is pretty short...
if (!SSL_is_init_finished(ses->ssl)) {r = SSL_do_handshake(ses->ssl); if(
r == 0 )/*error/incomplete */ else /* handle errors; usually WANT_READ;
read for any control data pending, and send data*/ } else return 2/1;
>
until is_init_finished which is handshake() returns 2 on the first
is_init_finished... and 1 after that; so the first callback does
certificate verification...
>
then kinda...
> onread() { /* recv got data */
> handshake();
> -1 ; close
> 0 - return wait for more data
> 2 - verify handshaken certs
> 1 - continue as normal.
> read data (if any) (post to app)
> read if any control data/send control data to remote
> }
>
and I could optionally? register verification callbacks and remove the == 2
check inline?
> 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
>
>
