SSLEngine weird behavior in 11+21?

[Xuelei Fan]

On 7/12/2018 1:17 PM, Simone Bordet wrote:
> Hi,
> 
> On Thu, Jul 12, 2018 at 9:29 PM Xuelei Fan <xuelei.fan at oracle.com> wrote:
>> Per the TLS 1.3 specification:
>>     -  The server sends a dummy change_cipher_spec record immediately
>>        after its first handshake message.  This may either be after a
>>        ServerHello or a HelloRetryRequest.
>>
>> and
>>     -  If not offering early data, the client sends a dummy
>>        change_cipher_spec record (see the third paragraph of Section 5.1)
>>        immediately before its second flight.  This may either be before
>>        its second ClientHello or before its encrypted handshake flight.
>>        If offering early data, the record is placed immediately after the
>>        first ClientHello.
>>
>> My read of the spec is that the dummy change_cipher_spec record is
>> generated immediately after the ServerHello in server side, and before
>> the 2nd flight in client side.
> 
> The spec is about *sending*, while I'm about *generating*.
> 
> Currently:
> - Server wraps 160, 6 and 907 in 3 wraps.
> - Server *sends* the 1073 bytes in 1 TCP write
> - Client unwraps 160, then goes into NEED_WRAP
> - Client wraps 6, then goes again into NEED_UNWRAP to finish with the
> 6 and 907 received by the server.
> 
> What I'm suggesting:
> - Server wraps 160, 6 and 907 in 3 wraps.
> - Server *sends* the 1073 (160+6+907) bytes in 1 TCP write
> - Client unwraps 160, stays in NEED_UNWRAP, unwraps the 6, unwraps the
> 907, then goes into NEED_WRAP
> - Client wraps 6 and 58
> - Client *sends* the 64 (6+58) bytes in 1 TCP write.
> 
> The 6 bytes of the dummy change_cipher_spec are *sent* just after
> receiving the ServerHello in both cases, they are just *generated* at
> different times.
> 
> By having all the unwraps() consecutive and all the wraps()
> consecutive you can enable a number of optimizations I described
> earlier.
> 
> Imagine a client that would perform a TCP *send* every time the state
> moves away from NEED_WRAP.
> Currently it would:
> 1. TCP send for ClientHello
> 2. Reads 1073 from server
> 3. Unwrap 160, then NEED_WRAP
> 4. Generate 6, then NEED_UNWRAP
> 5. TCP send the 6 bytes
> 6. Unwrap 6 and 907, then NEED_WRAP
> 7. Generate 58 then FINISHED
> 8 TCP send the 58 bytes.
> 
> You can see that the client issues 3 TCP sends.
> 
> With what I am suggesting, the client would only issue 2 TCP sends,
> which seems more in line with the efforts in TLS 1.3 to be more
> efficient.
> 
> It would just be a matter of *generating* those 6 bytes a bit later,
> *after* having fully unwrapped the ServerHello.
> 
It's a good idea!

>> We may consider a option to turn off the middlebox compatibility mode if
>> it helps Jetty.
> 
> No need to.
> 
>> But the implementation code and the application should
>> be ready to accept the fact that third party's implementation may be
>> implemented in middlebox compatibility mode, and the change_cipher_spec
>> record may still come in.
> 
> See above, it's not about processing the dummy change_cipher_spec,
> it's just about generating it a little later.
> 
>> After #7 (FINISHED), if the client only send application data, but never
>> call read again, it still works, right?  The application don't have to
>> read something, I think.
> 
> Sure, it just can't take advantage of session resumption though.
> The client never knows that there are post-handshake messages to read
> because SSLEgine did not tell.
> SSLEngine just said FINISHED and NOT_HANDSHAKING.
> 
> However, I see your point. Handshake messages could come at any time,
> just that this one seems more part of the initial handshake,
> especially if it's about session resumption.
> 
Yes.  Applications using SSLEngine may want to monitor the network 
incoming, and call unwrap() when there is something comes.  Otherwise, 
if the SSLEngine client does not try to read any more, there is no 
chance to get the post-handshake message.

I know some application might never read again after the full handshake, 
but it may not be able to work for some user cases (session resumption, 
etc), even in the TLS 1.2 context.  As SSLEngine knows nothing about the 
underlying transportation, there is not much it can do here.

>> in #10, you said, "Client MUST unwrap ...".  Do you mean that the client
>> cannot send application data without read/unwrap something?
> 
> It can send data, but won't be good to leave unprocessed bytes around.
> The application would not know that there is stuff to read from the
> network and to process as post-handshake messages.
> It may never read from the network again.
> 
See above.

Thank you very much, Simone.  I find at least two improvements we can 
take.  It's really good!

Regards,
Xuelei

>> Half-close means that the server may not send the close_notify when it
>> receive the client close_notify.  It's a very tricky policy.  The client
>> close_notify only means the client want to close its writing side.  The
>> server may not send the close_notify if it doesn't want to close.  If we
>> have the client goes into NEED_UNWRAP, there is a potential dead waiting.
> 
> Good point.
> 
> Thanks!
>