SSLEngine weird behavior in 11+21?

[Xuelei Fan]

Hi Simone,

Thank you very much for the debug logs.

Read more in-lines, please.


On 7/12/2018 2:03 AM, Simone Bordet wrote:
> Hi,
> 
> On Thu, Jul 12, 2018 at 12:06 AM Simone Bordet <simone.bordet at gmail.com> wrote:
>>
>> Hi,
>>
>> I can see this (weird) behavior in SSLEngine for TLS 1.3 in JDK 11+21.
>> It's a simple new connection (no resumption) that performs a TLS 1.3 handshake.
>> The bytes numbers are those that I get, they may be different for
>> others depending on certificate size, etc.
>>
>> 1. Client wraps 394 bytes then goes into NEED_UNWRAP.
>> 2. Server unwraps 394 bytes then goes into NEED_TASK and then into NEED_WRAP.
>> 3. Server wraps (in 3 wraps) 160, 6 and 907 bytes then goes into NEED_UNWRAP.
>> 4. Client unwraps 160 bytes then goes into NEED_TASK and then into NEED_WRAP (?)
>> 5. Client wraps 6 bytes, then goes into NEED_UNWRAP.
>> 6. Client unwraps (in 2 unwraps) 6 and 907 bytes, then goes into
>> NEED_TASK and then into NEED_WRAP.
>> 7. Client wraps 58 bytes and goes into FINISHED (?)
>> 8. Server unwraps (in 2 unwraps) 6 and 58 bytes then goes into NEED_WRAP.
>> 9. Server wraps 72 bytes then goes into FINISHED.
>> 10. Client MUST unwrap those 72 bytes going again into FINISHED (which
>> already happened at 7).
>>
>> There are 2 things that I find weird:
>>
>> A) That at 4, the client goes into NEED_WRAP, even if it has not
>> finished to unwrap what the server sent. Apparently, it only goes into
>> NEED_WRAP to generate a CHANGE_CIPHER_SPEC (I am guessing from the
>> number of bytes generated), but then goes back into NEED_UNWRAP to
>> finish reading what the server sent. This is also not optimal as it
>> forces applications to do something with those 6 bytes: either put
>> them aside (additional data structures that may not be needed) or -
>> worse - write them to the server causing an additional write (after
>> all the effort TLS 1.3 put in to have a 1 RTT handshake).
>> I think that step 4 should go into NEED_UNWRAP, and that step 5 and
>> step 6 should be switched, so that the client would unwrap the 160, 6
>> and 907 sent by the server, and only after wrapping the 6 and the 58.
>> To be clear, the current behavior is (u==unwrap, w=wrap): u160, w6,
>> u6, u907, w58.
>> I think it should be: u160, u6, u907, w6, w58.
>>
>> Is there any reason that the 6 bytes needs to be generated in-between
>> the processing of the frames sent by the server?
>>
In order to mitigate the compatibility impact, the TLS 1.3 is 
implemented in the middlebox compatibility mode (See Section D.4, TLS 
1.3 draft 28) in JDK.  The 6 bytes message is the dummy 
change_cipher_spec record.  More details, please refer to the "Middlebox 
Compatibility Mode" section in TLS 1.3 draft 28:
     https://tools.ietf.org/html/draft-ietf-tls-tls13-28#appendix-D.4

>> B)That at 7 the client goes into FINISHED, but it is not finished
>> really: in fact it needs to perform step 10, but there is no
>> indication from the SSLEngine that it must do so.
>> Currently, step 7 says the client is finished and there is no clue
>> that it must unwrap those last 72 bytes.
>> I think step 7 should go into NEED_UNWRAP instead, and only at 10 go
>> into FINISHED.
TLS 1.3 allows handshake message delivered after the main handshake. 
The concept is called post-handshake message.  Please refer to the 
"Post-Handshake Messages" for more details:
     https://tools.ietf.org/html/draft-ietf-tls-tls13-28#section-4.6

In JDK 11, two post-handshake messages are supported, new session ticket 
and key/iv update.  The 72 bytes in #9 and #10 are for the new session 
ticket.  In JDK, the new session ticket post-handshake message is 
delivered immediately after the main handshake in server side.  While 
for client side, it should be ready to accept the message at any time 
the main handshake complete.  So, in #7, the FINISHED status means the 
main handshake complete.  While in #10, the FINISHED status means that 
the post-handshake message get processed.

I really concern about the compatibility impact.  Did you run into 
problems with the new handshake message flow?

> 
> In addition to what reported above, I would like to report also the
> weird behavior during the close handshake (i.e. when one side decides
> to close the connection).
> 
> 1. client.closeOutbound() then goes into NEED_WRAP.
> 2. Client wraps 24 bytes, result is CLOSED, then goes into NOT_HANDSHAKING (?)
> 3. Server unwraps 24 bytes, result is CLOSED, then goes into NEED_WRAP.
> 4. Server wraps 24 bytes, result is CLOSED, then goes into NOT_HANDSHAKING.
> 5. Client unwraps 0 bytes (?)
> 
> I think at step 2 the client should go into NEED_UNWRAP to read (at
> step 5) the server response to the close_notify.
In TLS 1.3, half-close policy is used.  The server may not response with 
the close_notify for client close_notify request.  See the "Closure 
Alerts" section for more details:
    https://tools.ietf.org/html/draft-ietf-tls-tls13-28#section-6.1

It is a little bit complicated when moving from the duplex-close to 
half-close policy.  In order to mitigate the compatibility impact, in 
JDK implementation, if the local send the close_notify, we choose to 
close the underlying socket as well if needed.  But, if the peer send 
the close_notify, the unwrap() should be able to consume the bytes.  It 
looks like a implementation bug to me.

Would you please let us know if there are compatibility issues with the 
new half-close policy?

Thanks,
Xuelei

> Instead, at step 5 the client unwraps 0 bytes so we are left with
> those 24 bytes from the server that applications need to discard.
> 
> Also, I am not sure that the wrap result at step 2 and 3 should be
> CLOSED, perhaps OK is better?
> The server is actually closed at step 4, and the client at step 5.
> However, this is a minor issue.
> 
> Attached the debug logs as you requested.
> 
> Thanks!
>