SSLEngine weird behavior in 11+21?

[Simone Bordet]

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

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

-- 
Simone Bordet
---
Finally, no matter how good the architecture and design are,
to deliver bug-free software with optimal performance and reliability,
the implementation technique must be flawless.   Victoria Livschitz
-------------- next part --------------
A non-text attachment was scrubbed...
Name: sslengine.log.gz
Type: application/gzip
Size: 15056 bytes
Desc: not available
URL: <https://mail.openjdk.org/pipermail/security-dev/attachments/20180712/e0c87483/sslengine.log.gz>