On 7/12/2018 10:47 AM, Simone Bordet wrote:
Hi,

On Thu, Jul 12, 2018 at 4:34 PM Xuelei Fan <xuelei....@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.

Reply via email to