<div dir="ltr"><div dir="ltr"><div dir="ltr">On Mon, Feb 18, 2019 at 2:18 PM Jakob Bohm via openssl-users <<a href="mailto:openssl-users@openssl.org">openssl-users@openssl.org</a>> wrote:<br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On 17/02/2019 14:26, Matt Caswell wrote:<br>
> On 16/02/2019 05:04, Sam Roberts wrote:<br>
>> On Fri, Feb 15, 2019 at 3:35 PM Matt Caswell <<a href="mailto:matt@openssl.org" target="_blank">matt@openssl.org</a>> wrote:<br>
>>> On 15/02/2019 20:32, Viktor Dukhovni wrote:<br>
>>>>> On Feb 15, 2019, at 12:11 PM, Sam Roberts <<a href="mailto:vieuxtech@gmail.com" target="_blank">vieuxtech@gmail.com</a>> wrote:<br>
>>>> OpenSSL could delay the actual shutdown until we're about to return<br>
>>>> from the SSL_accept() that invoked the callback.  That is SSL_shutdown()<br>
>>>> called from callbacks could be deferred until a more favourable time.<br>
>> In this case, it's an SSL_read() that invoked the callback, though<br>
>> probably not relevant.<br>
>><br>
>> And actually, no deferal would be necessary, I looks to me that the<br>
>> info callback for handshake done is coming too early. Particularly,<br>
>> the writing of the NewSessionTickets into the BIO should occur before<br>
>> the info callback. I'll check later, but I'm pretty sure with TLS1.2<br>
>> the session tickets are written and then the HANDSHAKE_DONE info<br>
>> callback occurs, so the timing here is incompatible with TLS1.2.<br>
> In TLSv1.2 New session tickets are written as part of the handshake. In TLSv1.3<br>
> session tickets are sent after the handshake has completed. It sounds to me like<br>
> the info callback is doing the right thing.<br>
That seems to be a major theme in many reported OpenSSL 1.1.1<br>
problems.  It seems that you guys have gotten too hung up on how<br>
the TLS 1.3 RFC uses words like handshake differently than the<br>
TLS 1.2 RFC, rather than by the higher level semantics of what<br>
would be considered the API visible meta-operations.<br>
<br>
 From an API user perspective, the messages that are exchanged<br>
right after the RFC-handshake in order to complete the connection<br>
set up should be considered part of the API-handshake.<br>
<br>
This made little difference for the "change cipher spec" TLS 1.2<br>
record, but makes a lot more difference for TLS 1.3 where various<br>
things like certificate checks and session tickets fall into that<br>
gray area.<br>
<br>
Any opportunity to send data earlier than that should be handled<br>
in a way that doesn't break the API for applications that aren't<br>
doing so.<br>
>> Though the deferal mechanism might be there already. It looks like<br>
>> doing an SSL_write(); SSL_shutdown() in the info callback works fine,<br>
>> on the client side new ticket callbacks are fired by the SSL_read()<br>
>> before the SSL_read() sees the close_notify and returns 0. I haven't<br>
>> looked at the packet/API trace for this, because the tests all pass<br>
>> for this case, but I do see that in the javascript called from the<br>
>> HANDSHAKE_DONE callback, that calling .end("x") (write + shutdown)<br>
>> causes the client to get tickets, but .end() causes it to miss them<br>
>> because they are after close_notify.<br>
>><br>
>>> Oh - right. I missed this detail. Calling SSL_shutdown() from inside a callback<br>
>>> is definitely a bad idea. Don't do that.<br>
>> The info callback, or ANY callbacks? What about the new ticket<br>
>> callback, for example? Is it expected that no SSL_ calls are made in<br>
>> ANY callbacks?<br>
> I wouldn't go that far. Callbacks occur during the processing of an IO<br>
> operation. Callbacks are generally expected to be small and quick. I wouldn't<br>
> call anything that might invoke a new IO operation from within a callback, so<br>
> SSL_read, SSL_write, SSL_do_handshake, SSL_shutdown etc.<br>
><br>
Callbacks are often an opportunity for applications to detect<br>
violations of security policy.  It thus makes a lot of sense for<br>
callbacks to request that the connection is ended as soon as<br>
allowed by the risk of creating an attack side channel.<br>
<br>
Other OpenSSL callbacks represent the one place to do certain<br>
complex tasks, such as choosing among different certificates,<br>
checking against outside (networked!) revocation systems etc.><br><br></blockquote><div><br></div><div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"></blockquote>All of that makes me question; so in migrating to 1.3, does the basic flow change?<br><blockquote class="gmail_quote" style="margin:0px 0.8ex;border-left:1px solid rgb(204,204,204);border-right:1px solid rgb(204,204,204);padding-left:1ex;padding-right:1ex"></blockquote><br><blockquote class="gmail_quote" style="margin:0px 0.8ex;border-left:1px solid rgb(204,204,204);border-right:1px solid rgb(204,204,204);padding-left:1ex;padding-right:1ex"></blockquote><a href="https://github.com/d3x0r/SACK/blob/master/src/netlib/ssl_layer.c#L178">https://github.com/d3x0r/SACK/blob/master/src/netlib/ssl_layer.c#L178</a>  (handshake... hmm that's long tedious debug optioned code)</div><div class="gmail_quote"><br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0.8ex;border-left:1px solid rgb(204,204,204);border-right:1px solid rgb(204,204,204);padding-left:1ex;padding-right:1ex"></blockquote>summary is pretty short...</div><div class="gmail_quote"><br><blockquote class="gmail_quote" style="margin:0px 0.8ex;border-left:1px solid rgb(204,204,204);border-right:1px solid rgb(204,204,204);padding-left:1ex;padding-right:1ex"></blockquote> 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;<br><blockquote class="gmail_quote" style="margin:0px 0.8ex;border-left:1px solid rgb(204,204,204);border-right:1px solid rgb(204,204,204);padding-left:1ex;padding-right:1ex"></blockquote><br><blockquote class="gmail_quote" style="margin:0px 0.8ex;border-left:1px solid rgb(204,204,204);border-right:1px solid rgb(204,204,204);padding-left:1ex;padding-right:1ex"></blockquote>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...<br><blockquote class="gmail_quote" style="margin:0px 0.8ex;border-left:1px solid rgb(204,204,204);border-right:1px solid rgb(204,204,204);padding-left:1ex;padding-right:1ex"></blockquote><br><blockquote class="gmail_quote" style="margin:0px 0.8ex;border-left:1px solid rgb(204,204,204);border-right:1px solid rgb(204,204,204);padding-left:1ex;padding-right:1ex"></blockquote>then kinda...<br><blockquote class="gmail_quote" style="margin:0px 0.8ex;border-left:1px solid rgb(204,204,204);border-right:1px solid rgb(204,204,204);padding-left:1ex;padding-right:1ex"></blockquote>onread() { /* recv got data */<br><blockquote class="gmail_quote" style="margin:0px 0.8ex;border-left:1px solid rgb(204,204,204);border-right:1px solid rgb(204,204,204);padding-left:1ex;padding-right:1ex"></blockquote>    handshake(); <br><blockquote class="gmail_quote" style="margin:0px 0.8ex;border-left:1px solid rgb(204,204,204);border-right:1px solid rgb(204,204,204);padding-left:1ex;padding-right:1ex"></blockquote>      -1 ; close<br><blockquote class="gmail_quote" style="margin:0px 0.8ex;border-left:1px solid rgb(204,204,204);border-right:1px solid rgb(204,204,204);padding-left:1ex;padding-right:1ex"></blockquote>       0 - return wait for more data<br><blockquote class="gmail_quote" style="margin:0px 0.8ex;border-left:1px solid rgb(204,204,204);border-right:1px solid rgb(204,204,204);padding-left:1ex;padding-right:1ex"></blockquote>       2 - verify handshaken certs<br><blockquote class="gmail_quote" style="margin:0px 0.8ex;border-left:1px solid rgb(204,204,204);border-right:1px solid rgb(204,204,204);padding-left:1ex;padding-right:1ex"></blockquote>       1 - continue as normal.<br><blockquote class="gmail_quote" style="margin:0px 0.8ex;border-left:1px solid rgb(204,204,204);border-right:1px solid rgb(204,204,204);padding-left:1ex;padding-right:1ex"></blockquote>    read data (if any) (post to app)<br><blockquote class="gmail_quote" style="margin:0px 0.8ex;border-left:1px solid rgb(204,204,204);border-right:1px solid rgb(204,204,204);padding-left:1ex;padding-right:1ex"></blockquote>    read if any control data/send control data to remote<br><blockquote class="gmail_quote" style="margin:0px 0.8ex;border-left:1px solid rgb(204,204,204);border-right:1px solid rgb(204,204,204);padding-left:1ex;padding-right:1ex"></blockquote>}<br><blockquote class="gmail_quote" style="margin:0px 0.8ex;border-left:1px solid rgb(204,204,204);border-right:1px solid rgb(204,204,204);padding-left:1ex;padding-right:1ex"></blockquote><br>and I could optionally? register verification callbacks and remove the == 2 check inline?<br></div><div class="gmail_quote"><br class="gmail-Apple-interchange-newline"></div></div><div> </div></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
Enjoy<br>
<br>
Jakob<br>
-- <br>
Jakob Bohm, CIO, Partner, WiseMo A/S.  <a href="https://www.wisemo.com" rel="noreferrer" target="_blank">https://www.wisemo.com</a><br>
Transformervej 29, 2860 Søborg, Denmark.  Direct +45 31 13 16 10<br>
This public discussion message is non-binding and may contain errors.<br>
WiseMo - Remote Service Management for PCs, Phones and Embedded<br>
<br>
</blockquote></div></div></div>