SSLEngine weird behavior in 11+21?

Thu Jul 12 17:47:18 UTC 2018

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

> 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 :(

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()".

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().

Now at step 7 it returns FINISHED, and SSLEngine.getHandshakeStatus()
returns NOT_HANDSHAKING, so in principles there is nothing more to do.
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.

Would returning OK instead of FINISHED at step #10 be doable?
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.
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".

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

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.

Thanks!

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