when should client stop calling SSL_read to get TLS1.3 session tickets after the close_notify?

classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|

when should client stop calling SSL_read to get TLS1.3 session tickets after the close_notify?

Sam Roberts
In particular, I'm getting a close_notify alert, followed by two
NewSessionTickets from the server.

The does SSL_read()/SSL_get_error(), it is returning
SSL_ERROR_ZERO_RETURN, so I stop calling SSL_read().

However, that means that the NewSessionTickets aren't seen, so I don't
get the callbacks from SSL_CTX_sess_set_new_cb().

Should we be  calling SSL_read() until some other return value occurs?

Note that no data is written by the server, and SSL_shutdown() is
called from inside the server's SSL_CB_HANDSHAKE_DONE info callback.
The node test suite is rife with this pracitce, where a connection is
established to prove its possible, but then just ended without data
transfer. For TLS1.2 we get the session callbacks, but TLS1.3 we do
not.

This is the trace, edited to reduce SSL_trace verbosity:

server TLSWrap::SSLInfoCallback(where SSL_CB_HANDSHAKE_DONE, alert U)
established? 0
    state 0x21 TWST: SSLv3/TLS write session ticket TLSv1.3
server TLSWrap::DoShutdown() established? 1 ssl? 1
Sent Record
  Inner Content Type = Alert (21)
  Level=warning(1), description=close notify(0)
Sent Record
    NewSessionTicket, Length=245
Sent Record
    NewSessionTicket, Length=245


client TLSWrap::OnStreamRead(nread 566) established? 1 ssl? 1 parsing?
0 eof? 0
Received Record
    Level=warning(1), description=close notify(0)

    SSL_read() => 0
    SSL_get_shutdown() => SSL_RECEIVED_SHUTDOWN
    SSL_get_error() => SSL_ERROR_ZERO_RETURN

At this point, we consider the connection closed... not sure what else to do.

Thanks,
Sam
--
openssl-users mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-users
Reply | Threaded
Open this post in threaded view
|

Re: when should client stop calling SSL_read to get TLS1.3 session tickets after the close_notify?

Matt Caswell-2


On 14/02/2019 22:51, Sam Roberts wrote:
> In particular, I'm getting a close_notify alert, followed by two
> NewSessionTickets from the server.

This sounds like a bug somewhere. Once you have close_notify you shouldn't
expect anything else. Is that an OpenSSL server?

Matt
--
openssl-users mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-users
Reply | Threaded
Open this post in threaded view
|

Re: when should client stop calling SSL_read to get TLS1.3 session tickets after the close_notify?

Sam Roberts
In reply to this post by Sam Roberts
Resending, I just got banned for "bounces", though why gmail would be
bouncing I don't know.

On Thu, Feb 14, 2019 at 2:51 PM Sam Roberts <[hidden email]> wrote:
 In particular, I'm getting a close_notify alert, followed by two
 NewSessionTickets from the server.

 The does SSL_read()/SSL_get_error(), it is returning
 SSL_ERROR_ZERO_RETURN, so I stop calling SSL_read().

 However, that means that the NewSessionTickets aren't seen, so I don't
 get the callbacks from SSL_CTX_sess_set_new_cb().

 Should we be  calling SSL_read() until some other return value occurs?

 Note that no data is written by the server, and SSL_shutdown() is
 called from inside the server's SSL_CB_HANDSHAKE_DONE info callback.
 The node test suite is rife with this pracitce, where a connection is
 established to prove its possible, but then just ended without data
 transfer. For TLS1.2 we get the session callbacks, but TLS1.3 we do
 not.

 This is the trace, edited to reduce SSL_trace verbosity:

 server TLSWrap::SSLInfoCallback(where SSL_CB_HANDSHAKE_DONE, alert U)
 established? 0
     state 0x21 TWST: SSLv3/TLS write session ticket TLSv1.3
 server TLSWrap::DoShutdown() established? 1 ssl? 1
 Sent Record
   Inner Content Type = Alert (21)
   Level=warning(1), description=close notify(0)
 Sent Record
     NewSessionTicket, Length=245
 Sent Record
     NewSessionTicket, Length=245


 client TLSWrap::OnStreamRead(nread 566)
 Received Record
     Level=warning(1), description=close notify(0)

     SSL_read() => 0
     SSL_get_shutdown() => SSL_RECEIVED_SHUTDOWN
     SSL_get_error() => SSL_ERROR_ZERO_RETURN

 At this point, we consider the connection closed... not sure what else to do.

 Thanks,
 Sam
Reply | Threaded
Open this post in threaded view
|

Re: when should client stop calling SSL_read to get TLS1.3 session tickets after the close_notify?

Matt Caswell-2
Resending my answer, because I guess you didn't get it:

On 15/02/2019 17:11, Sam Roberts wrote:
> Resending, I just got banned for "bounces", though why gmail would be
> bouncing I don't know.
>
> On Thu, Feb 14, 2019 at 2:51 PM Sam Roberts <[hidden email]> wrote:
>  In particular, I'm getting a close_notify alert, followed by two
>  NewSessionTickets from the server.

This sounds like a bug somewhere. Once you have close_notify you shouldn't
expect anything else. Is that an OpenSSL server?

Matt



>
>  The does SSL_read()/SSL_get_error(), it is returning
>  SSL_ERROR_ZERO_RETURN, so I stop calling SSL_read().
>
>  However, that means that the NewSessionTickets aren't seen, so I don't
>  get the callbacks from SSL_CTX_sess_set_new_cb().
>
>  Should we be  calling SSL_read() until some other return value occurs?
>
>  Note that no data is written by the server, and SSL_shutdown() is
>  called from inside the server's SSL_CB_HANDSHAKE_DONE info callback.
>  The node test suite is rife with this pracitce, where a connection is
>  established to prove its possible, but then just ended without data
>  transfer. For TLS1.2 we get the session callbacks, but TLS1.3 we do
>  not.
>
>  This is the trace, edited to reduce SSL_trace verbosity:
>
>  server TLSWrap::SSLInfoCallback(where SSL_CB_HANDSHAKE_DONE, alert U)
>  established? 0
>      state 0x21 TWST: SSLv3/TLS write session ticket TLSv1.3
>  server TLSWrap::DoShutdown() established? 1 ssl? 1
>  Sent Record
>    Inner Content Type = Alert (21)
>    Level=warning(1), description=close notify(0)
>  Sent Record
>      NewSessionTicket, Length=245
>  Sent Record
>      NewSessionTicket, Length=245
>
>
>  client TLSWrap::OnStreamRead(nread 566)
>  Received Record
>      Level=warning(1), description=close notify(0)
>
>      SSL_read() => 0
>      SSL_get_shutdown() => SSL_RECEIVED_SHUTDOWN
>      SSL_get_error() => SSL_ERROR_ZERO_RETURN
>
>  At this point, we consider the connection closed... not sure what else to do.
>
>  Thanks,
>  Sam
>
Reply | Threaded
Open this post in threaded view
|

Re: when should client stop calling SSL_read to get TLS1.3 session tickets after the close_notify?

Viktor Dukhovni
In reply to this post by Sam Roberts
> On Feb 15, 2019, at 12:11 PM, Sam Roberts <[hidden email]> wrote:
>
> In particular, I'm getting a close_notify alert, followed by two
> NewSessionTickets from the server.
>
> The does SSL_read()/SSL_get_error(), it is returning
> SSL_ERROR_ZERO_RETURN, so I stop calling SSL_read().
>
> However, that means that the NewSessionTickets aren't seen, so I don't
> get the callbacks from SSL_CTX_sess_set_new_cb().
>
> Should we be  calling SSL_read() until some other return value occurs?
>
> Note that no data is written by the server, and SSL_shutdown() is
> called from inside the server's SSL_CB_HANDSHAKE_DONE info callback.
> The node test suite is rife with this pracitce, where a connection is
> established to prove its possible, but then just ended without data
> transfer. For TLS1.2 we get the session callbacks, but TLS1.3 we do

The code that's calling SSL_shutdown from the middle of the callback
is too clever by half.  It well and truly *deserves* to break.

Which is not to say that everything that's deserved should necessarily
happen, sometimes reality is more forgiving than just.  Perhaps that
should also the case here, but maybe not.

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.

Not sure whether the complexity of doing this is warranted.  Perhaps
the all too clever code should get its just deserts after all.

--
        Viktor.

Reply | Threaded
Open this post in threaded view
|

Re: when should client stop calling SSL_read to get TLS1.3 session tickets after the close_notify?

Matt Caswell-2


On 15/02/2019 20:32, Viktor Dukhovni wrote:

>> On Feb 15, 2019, at 12:11 PM, Sam Roberts <[hidden email]> wrote:
>>
>> In particular, I'm getting a close_notify alert, followed by two
>> NewSessionTickets from the server.
>>
>> The does SSL_read()/SSL_get_error(), it is returning
>> SSL_ERROR_ZERO_RETURN, so I stop calling SSL_read().
>>
>> However, that means that the NewSessionTickets aren't seen, so I don't
>> get the callbacks from SSL_CTX_sess_set_new_cb().
>>
>> Should we be  calling SSL_read() until some other return value occurs?
>>
>> Note that no data is written by the server, and SSL_shutdown() is
>> called from inside the server's SSL_CB_HANDSHAKE_DONE info callback.
>> The node test suite is rife with this pracitce, where a connection is
>> established to prove its possible, but then just ended without data
>> transfer. For TLS1.2 we get the session callbacks, but TLS1.3 we do
>
> The code that's calling SSL_shutdown from the middle of the callback
> is too clever by half.  It well and truly *deserves* to break.
>
> Which is not to say that everything that's deserved should necessarily
> happen, sometimes reality is more forgiving than just.  Perhaps that
> should also the case here, but maybe not.
>
> 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.
>
> Not sure whether the complexity of doing this is warranted.  Perhaps
> the all too clever code should get its just deserts after all.
>

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

Matt
Reply | Threaded
Open this post in threaded view
|

Re: 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 <[hidden email]> wrote:
> On 15/02/2019 20:32, Viktor Dukhovni wrote:
> >> On Feb 15, 2019, at 12:11 PM, Sam Roberts <[hidden email]> 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
Reply | Threaded
Open this post in threaded view
|

Re: when should client stop calling SSL_read to get TLS1.3 session tickets after the close_notify?

Matt Caswell-2


On 16/02/2019 05:04, Sam Roberts wrote:

> On Fri, Feb 15, 2019 at 3:35 PM Matt Caswell <[hidden email]> wrote:
>> On 15/02/2019 20:32, Viktor Dukhovni wrote:
>>>> On Feb 15, 2019, at 12:11 PM, Sam Roberts <[hidden email]> 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.

>
> 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.

Matt

Reply | Threaded
Open this post in threaded view
|

Re: when should client stop calling SSL_read to get TLS1.3 session tickets after the close_notify?

OpenSSL - User mailing list
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 <[hidden email]> wrote:
>>> On 15/02/2019 20:32, Viktor Dukhovni wrote:
>>>>> On Feb 15, 2019, at 12:11 PM, Sam Roberts <[hidden email]> 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

Reply | Threaded
Open this post in threaded view
|

Re: when should client stop calling SSL_read to get TLS1.3 session tickets after the close_notify?

d3x0r
On Mon, Feb 18, 2019 at 2:18 PM Jakob Bohm via openssl-users <[hidden email]> 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 <[hidden email]> wrote:
>>> On 15/02/2019 20:32, Viktor Dukhovni wrote:
>>>>> On Feb 15, 2019, at 12:11 PM, Sam Roberts <[hidden email]> 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