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