[8u] RFR : TLSv1.3 protocol support

Wed Mar 25 19:16:07 UTC 2020

Hi Andrew, Aleksey,

it seems we have little "deadlock" here and I can somehow understand
both positions. Aleksey/Azul already did the whole backport and
already have the code running in their 8u version. On the other hand I
also understand Andrew because reviewing such a huge change is really
very hard if not impossible and I can understand that he as a
maintainer is reluctant to merge in a change which can not be
reasonably reviewed and understood.

The question is now how we can solve this problem. I think the list of
included bug fixes you've provided is definitely helpful Aleksey. I'm
just wondering how you managed to get to the big patch you are now
proposing for inclusion? When doing this downport internally at Azul,
haven' you also applied all the listed changes one by one and if yes,
maybe it is not that hard to provide the corresponding patches? Or
have you just taken the complete 11u implementation, adapted it for 8u
and just found which changes have contributed to the 11u code later?

Maybe describing your original downport process can help us to better
understand your patch?

Thank you and best regards,
Volker

On Wed, Mar 25, 2020 at 3:36 AM Andrew Hughes <gnu.andrew at redhat.com> wrote:
>
> On 23/03/2020 09:12, Alexey Bakhtin wrote:
> > Hello All,
> >
> > Could you please review the updated patch for TLSv1.3 protocol support in JDK8u
> > This version includes support of TLS_KRB5 cipher suites according to RFC2712.
> > Kerberos cipher suites are enabled for TLSv1.2 protocol and below only
> >
> > Webrev: http://cr.openjdk.java.net/~dcherepanov/tls1.3/webrev.v2/
> >
> > Regards
> > Alexey
> >
> > On 6 Mar 2020, at 13:19, Alexey Bakhtin <alexey at azul.com<mailto:alexey at azul.com>> wrote:
> >
> > Hello Andrew,
> >
> > This webrev covers bug fixes from the initial TLSv1.3 implementation (JDK-8196584) till OpenJDK 11.0.6-ga repository state :
> >
> > 8196584: TLS 1.3 Implementation
> > 8206355: SSLSessionImpl.getLocalPrincipal() throws NPE
> > 8207029: Unable to use custom SSLEngine with default TrustManagerFactory after updating to JDK 11 b21
> > 8206929: Check session context for TLS 1.3 session resumption
> > 8207250: setUseClientMode post handshake with the same value as before does not throw IAE
> > 8207223: SSL Handshake failures are reported with more generic SSLException
> > 8207237: SSLSocket#setEnabledCipherSuites is accepting empty string
> > 8208166: Still unable to use custom SSLEngine with default TrustManagerFactory after JDK-8207029
> > 8207009: TLS 1.3 half-close and synchronization issues
> > 8206176: Remove the temporary tls13VN field
> > 8207317: SSLEngine negotiation fail exception behavior changed from fail-fast to fail-lazy
> > 8209916: NPE in SupportedGroupsExtension
> > 8210846: TLSv.1.3 interop problems with OpenSSL 1.1.1 when used on the client side with mutual auth
> > 8208209: Improve TLS connection stability again
> > 8208350: Disable all DES cipher suites
> > 8209916: NPE in SupportedGroupsExtension
> > 8210846: TLSv.1.3 interop problems with OpenSSL 1.1.1 when used on the client side with mutual auth
> > 8210334: TLS 1.3 server fails if ClientHello doesn't have pre_shared_key and psk_key_exchange_modes
> > 8211806: TLS 1.3 handshake server name indication is missing on a session resume
> > 8211883: Disable anon and NULL cipher suites
> > 8209965: The "supported_groups" extension in ServerHellos
> > 8210974: No extensions debug log for ClientHello
> > 8212885: TLS 1.3 resumed session does not retain peer certificate chain
> > 8213202: Possible race condition in TLS 1.3 session resumption
> > 8214688: TLS 1.3 session resumption with hello retry request failed with “illegal_parameter"
> > 8217579: TLS_EMPTY_RENEGOTIATION_INFO_SCSV is disabled after 8211883
> > 8214129: SSL session resumption/SNI with TLS1.2 causes StackOverflowError
> > 8213782: NullPointerException in sun.security.ssl.OutputRecord.changeWriteCiphers
> > 8214339: SSLSocketImpl erroneously wraps SocketException
> > 8210989: RSASSA-PSS certificate cannot be selected for client auth on TLSv1.2
> > 8215443: The use of TransportContext.fatal() leads to bad coding style
> > 8215790: Delegated task created by SSLEngine throws java.nio.BufferUnderflowException
> > 8219389: Delegated task created by SSLEngine throws BufferUnderflowException
> > 8216045: The size of key_exchange may be wrong on FFDHE
> > 8210985: Update the default SSL session cache size to 20480
> > 8221253: TLSv1.3 may generate TLSInnerPlainText longer than 2^14+1 bytes
> > 8148188: Enhance the security libraries to record events of interest
> > 8215281: Use String.isEmpty() when applicable in java.base
> > 8216326: SSLSocket stream close() does not close the associated socket
> > 8218863: Better endpoint checks
> > 8218873: Improve JSSE endpoint checking
> > 8222678: Improve TLS negotiation
> > 8216039: TLS with BC and RSASSA-PSS breaks ECDHServerKeyExchange
> > 8223482: Unsupported ciphersuites may be offered by a TLS client
> > 8214098: sun.security.ssl.HandshakeHash.T12HandshakeHash constructor check backwards.
> > 8228825: Enhance ECDSA operations
> > 8218580: endpoint identification algorithm should be case-insensitive
> > 8200381: Typos in javadoc - missing verb "be" and alike
> > 8211866: TLS 1.3 CertificateRequest message sometimes offers disallowed signature algorithms
> > 8212738: Incorrectly named signature scheme ecdsa_secp512r1_sha512
> > 8212752: Typo in SSL log message related to inactive/disabled signature scheme
> > 8214321: Misleading code in SSLCipher
> > 8215524: Finished message validation failure should be decrypt_error alert
> > 8217610: TLSv1.3 fail with ClassException when EC keys are stored in PKCS11
> > 8233954: UnsatisfiedLinkError or NoSuchAlgorithmException after removing sunec.dll
> > 8225766: Curve in certificate should not affect signature scheme when using TLSv1.3
> > 8229728: Implement negotiation parameters
> > 8231780: Better TLS messaging support
> > 8227758: More valid PKIX processing
> > 8236039: JSSE Client does not accept status_request extension in CertificateRequest messages for TLS 1.3
> >
> > Regards
> > Alexey
> >
> > On 6 Mar 2020, at 09:49, Andrew Hughes <gnu.andrew at redhat.com<mailto:gnu.andrew at redhat.com>> wrote:
> >
> > On 05/03/2020 20:08, Alexey Bakhtin wrote:
> > Hello All,
> >
> > Could you please review the initial implementation of TLSv1.3 protocol support in JDK8u.
> > Most of these changes are direct copies of TLS implementation from OpenJDK 11.0.6-ga, but adjusted for JDK 8u.
> > This is a single patch for all functionality including tests, so it could be easily applied to the current state of jdk8u and tested. If required, the patch can be divided into smaller reviews for different functional parts or features.
> >
> > There is a difference between JDK11 and JDK8 public API, so it is not possible to implement all TLS related features available in JDK11.
> > Below is a list of features included/excluded from the proposed 8u TLS 1.3 backport:
> >
> > 1. DTLS support (JEP-219) [2] [3]
> >    - JDK8 does not have public API required for DTLS implementation
> >    - All DTLS related code is removed from the 8u TLS 1.3 backport
> >
> > 2. OCSP Stapling implementation (JEP-249) [4] [5]
> >    - This feature introduces new public API in the  javax.net.ssl.ExtendedSSLSession class
> >    -  OCSP Stapling can be supported in 8u TLS 1.3 without additional API.
> >    -  Proposed patch includes OCSP Stapling implementation but it is disabled by default. It can be enabled by “jdk.tls.client.enableStatusRequestExtension” System Property
> >
> > 3. ChaCha20 and Poly1305 CipherSuite support (JEP-329) [6] [7] [8]
> >    - Chacha20-Poly1305 AEAD cipher suites can be supported in 8u TLS 1.3 without additional public API
> >    - Chacha20-Poly1305 cipher suite support is not backported to JDK11 so it is not included in the 8u TLS 1.3 backport
> >    - Support of Chacha20-Poly1305 cipher suites could be added later by separate patch
> >
> > 4. Authenticator on a HttpURLConnection (JDK-8169495) [9]
> >    - This feature can not be supported in 8u TLS 1.3 backport without additional public API
> >    - The code for this feature was removed from the 8u TLS 1.3 backport
> >
> > 5. TLS_KRB5 cipher suites (RFC 2712) [10]
> >    - JDK8 includes support for TLS_KRB5 cipher suites according to RFC2712
> >    - OpenJDK 11.0.6-ga does not support TLS_KRB5 cipher suites
> >    - Proposed 8u TLS 1.3 backport DOES NOT support TLS_KRB5 cipher suites
> >    - Support for TLS_KRB5 cipher suites will be added later as part of this or separate review
> >
> > Webrev :  http://cr.openjdk.java.net/~dcherepanov/tls1.3/webrev.v1/
> >
> > [1] https://openjdk.java.net/jeps/332
> > [2] https://openjdk.java.net/jeps/219
> > [3] https://bugs.openjdk.java.net/browse/JDK-8043758
> > [4] https://openjdk.java.net/jeps/249
> > [5] https://bugs.openjdk.java.net/browse/JDK-8046321
> > [6] https://openjdk.java.net/jeps/329
> > [7] https://bugs.openjdk.java.net/browse/JDK-8153028
> > [8] https://bugs.openjdk.java.net/browse/JDK-8140466
> > [9] https://bugs.openjdk.java.net/browse/JDK-8169495
> > [10] https://tools.ietf.org/html/rfc2712
> >
> >
> > Regards,
> > Alexey
> >
> >
> > Which bugs are being backported by this webrev?
> >
> > Thanks,
> > --
> > Andrew :)
> >
> > Senior Free Java Software Engineer
> > Red Hat, Inc. (http://www.redhat.com<http://www.redhat.com/>)
> >
> > PGP Key: ed25519/0xCFDA0F9B35964222 (hkp://keys.gnupg.net)
> > Fingerprint = 5132 579D D154 0ED2 3E04  C5A0 CFDA 0F9B 3596 4222
> >
> >
> >
>
> This list of bugs doesn't look accurate to me. I've not been through
> them all, but a number stand out to me as I've included them in 8u for
> security updates.  At least 8208350, 8210985 & 8228825 are already present.
>
> I think it would be preferable to focus on backporting the first of
> these bugs (which seems to be the main TLS 1.3 implementation), and then
> work from there. It seems unreasonable to expect someone to review such
> a mammoth number of fixes in one patch
>
> Backporting one or two fixes in one patch makes it much easier to
> compare with the original and see what changes need to be made for 8u.
> It also makes it easier to exclude changes we don't want.
>
> I expect me trying to untangle all the bugs in this patch would involve
> nearly as much work as just backporting them myself.
>
> Thanks,
> --
> Andrew :)
>
> Senior Free Java Software Engineer
> Red Hat, Inc. (http://www.redhat.com)
>
> PGP Key: ed25519/0xCFDA0F9B35964222 (hkp://keys.gnupg.net)
> Fingerprint = 5132 579D D154 0ED2 3E04  C5A0 CFDA 0F9B 3596 4222
>