SSLEngine weird behavior in 11+21?

Thu Jul 12 19:28:45 UTC 2018

On 7/12/2018 10:47 AM, Simone Bordet wrote:
> Hi,
> 
> On Thu, Jul 12, 2018 at 4:34 PM Xuelei Fan <xuelei.fan at oracle.com> wrote:
>> 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
> 
> Okay. But then those 6 bytes don't need to be generated immediately, right?
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 SSLEngine can finish to unwrap everything received by the server,
> and then wrap everything it needs to wrap.
> This will make simpler for applications to allocate buffers, recycle
> them, and in general implement TLS via SSLEngine in a simpler way (and
> more efficiently with less writes and/or data copies).
> 
The call to wrap() and unwrap() should follow the instructions of the 
handshake status (NEED_UNWRAP or NEED_WRAP).  The sequence of 
wrap()/unwrap() is not expected to impact the usage.

We may consider a option to turn off the middlebox compatibility mode if 
it helps Jetty.  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.

>> 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?
> 
> Yes, the Jetty CI integration went from 0 test failures in JDK 11+18
> (and all previous JDK up to 8) to 500+ failures, all due to TLS 1.3.
> We will need to update our usage of SSLEngine heavily and disable TLS
> 1.3 in the meantime :(
> 
Please feel free to send us the compatibility problems in this email alias.

> For example, FINISHED state was only every happening once before.
> Based on that, we could detect how many times a client was requesting
> renegotiations, and impose a limit on that (otherwise it would be an
> attack to the server).
> With the double FINISHED state in #7 and #10 we are not able to tell
> the difference based on the SSLEngine state alone.
> We would need to put "if (tls > 1.2)
> cannot_be_renegotiation_do_something_else()".
> 
As the logic is related to TLS specification details, it might need to 
make some adjustment in Jetty as renegotiation has gone in TLS 1.3.

> What concerns me is that before SSLEngine was emitting instructions on
> what it needed an application to do in order to proceed the TLS
> handshake.
> You would call beginHandshake() on a client, and
> SSLEngine.getHandshakeStatus() would return NEED_WRAP, and the
> application would know that it needed to call wrap().
> 
It still works, I think.

Normally, call beginHandshake(), and then check the handshake status for 
further operations.  If the status if NEED_WRAP, call wrap(); if the 
status if NEED_UNWRAP(), call unwrap(); etc.

> Now at step 7 it returns FINISHED, and SSLEngine.getHandshakeStatus()
> returns NOT_HANDSHAKING, so in principles there is nothing more to do.
It is still the same for TLS 1.3, right?  The client can send 
application data now, I think.

> In a custom application protocol where a client opens a connection
> expecting to only send data to the server and never receiving data
> from the server, the client would never expect to read more bytes from
> the server for those post-handshake frames, because the SSLEngine does
> not tell.
> Having to put conditional code like "if (tls > 1.2)
> read_more_from_server_even_if_finished()" seems ugly.
> 
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.

in #10, you said, "Client MUST unwrap ...".  Do you mean that the client 
cannot send application data without read/unwrap something?

> Would returning OK instead of FINISHED at step #10 be doable?
Post-handshaking may update the states of the connection.  A signal is 
required if post-handshake happens.  If we silently process the 
post-handshake messages, there might be some other problems.  For 
example, the application is not aware the post-handshake authentication 
has completed if doing it silently.

> After all, encrypting an empty string was always possible and there
> really is no difference for SSLEngine users: unwrapping those 72 bytes
> for the session ticket or the empty string would produce in both cases
> 0 decrypted bytes (it only changes the internal state of SSLEngine
> with the session ticket).
> 
>> 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
> 
> In that section I read:
> "Each party MUST send a "close_notify" alert before closing its write
> side of the connection, unless it has already sent some error alert."
> 
> Given that, I expect that for close_notify, at step #2, the client
> goes into NEED_UNWRAP, as the server MUST send a close_notify too.
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.

Per the TLS 1.3 specification, the local can request to close its 
writing side, but it cannot to request to close the peer writing side. 
It means it is not defined about how to close the read side.  It could 
lead to issues in practice.  I had a question in the IETF TLS email 
list.  But unfortunately, I did not get a ideal solution:
     https://www.ietf.org/mail-archive/web/tls/current/msg25581.html

In order to countermeasure the issues, JDK will try to close the 
underlying socket if either side sends the close_notify message.  But we 
still need to take care of third party's implementation which may not 
behave the same way.

> Or at least that the server close_notify is consumed by the client at
> step #5, although this would again break the "SSLEngine state machine
> telling applications what to do".
I think the following issues should be addressed:
1. the consuming of server close_notify issue
2. the NEED_WRAP after the CLOSEED status.

I may miss something.  Except #2, is there any other issue that break 
the "SSLEngine state machine telling applications what to do" rule?

> 
>> 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.
> 
> Okay.
> 
>> Would you please let us know if there are compatibility issues with the
>> new half-close policy?
> 
> We've been massively struck by test failures, so I cannot tell which
> test had compatibility issues due to the close_notify differences.
> I suspect that most failures are due to the change of behavior at step #4.
> 
> At this point, my gut feeling is that having a single codebase
> handling TLS 1.2 and TLS 1.3 would be difficult.
> I expect a few "if (tls > 1.2)" spread out in the code because now we
> need additional information that we did not need before because
> SSLEngine and its state machine was telling the application what next
> step was required but now it's not always the case anymore.
> 
It is not expected to dig into the TLS protocol version details in 
applications code.  Otherwise, it would be a trouble once again if TLS 
1.4 comes in the future.  If an application depends on the protocol 
specification by itself, however, it may need some additional update 
when upgrade to new version.

Thank you very much for sharing the evaluation with us.  Let me see how 
we could have an improvement of the half-close issue (JDK-).

> I know that other SSLEngine users for widely used Java network servers
> are around in this list :)
> Would be great if they can feedback about whether JDK's SSLEngine with
> TLS 1.3 is working well for them.
> 
Yes, please feel free to share your ideas.

Thanks,
Xuelei

> Thanks!
> 
> --
> Simone Bordet
> ----
> http://cometd.org
> http://webtide.com
> Developer advice, training, services and support
> from the Jetty & CometD experts.
> 