[8u] TLSv1.3 RFR: 8245467: Remove 8u TLSv1.2 implementation files

Martin Balao mbalao at redhat.com
Thu May 21 16:51:49 UTC 2020 

On 5/21/20 10:08 AM, Alexey Bakhtin wrote:
> Please review changes required to backport TLSv1.3 protocol from JDK11.0.7 to JDK8u
> 

Hi,

For documentation purposes I'll explain here why we believe Step 0 is
indeed the first thing to do. Step 0 is the deletion of all the files
under sun/security/ssl directory that will be either file-replaced or
deleted by TLS 1.3 related patches. What we are accomplishing is to
clear the sun/security/ssl directory to place a brand-new implementation
of the TLS engine (SunJSSE provider, implementing the javax.net.ssl
API). No file out of sun/security/ssl is to be file-replaced, in the
sense of containing an unnoticed accumulation of changesets. Thus, for
any file out of sun/security/ssl we will go through each change
individually in following Steps.

We understand that:

1) the TLS engine (under sun.security.ssl namespace) can be considered a
coherent unit and accept the collapse of all TLS 1.3 related patches -up
to 11.0.7- as a starting point; and,

2) make it clear through separated and incremental patches what is
needed to integrate it into JDK-8 -so anyone can review and make sense
of it-.

Not deleting modified files in Step 0 (files that exist before and after
TLS 1.3) would create a collection of changes in the repository that is
hard to understand because it includes a number of patches collapsed
into one. The reason to collapse these changes (do file-replacing) is
justified by time and resources constraints. Even though a
patch-by-patch backport would be ideal -in my opinion-, I'm okay with
going through this path as long as we take reasonable assurances to
minimize risks.

In regards to the repository history, we are introducing a 'limitation'
with the approach taken when tracking changes back in two ways:

1) tracking changes to the legacy TLS engine won't be possible because
history has a hard stop with a whole new file committed; and,

2) tracking TLS engine changes between 8196584 (which is the TLS 1.3
implementation) and 11.0.7 with granularity won't be possible.

Given that the TLS engine was completely rewritten, I'm inclined to
think that there won't be a need to establish a relationship to the
previous TLS engine code. In fact, 8196584 is not a collection of small
incremental patches that allow you to follow the history; but a large
changeset and a new start.

Changes after 8196584 are mostly stabilization and security fixes, which
could have been in 8196584. In the worst case, a lookup in the 11u
repository would be needed to analyze individual changesets. I don't
expect this to happen frequently because even if there were a bug, it
will probably be analyzed/fixed in 11u first -given that the TLS engine
will be the same-: I don't expect many bugs in code within
sun.security.ssl to be 8u specific. The fact that we replaced all the
files -including superfluous changes such as code formatting- increases
the chances that a JDK-11 fix applies cleanly to JDK-8.

I went through the list of files deleted by 8245467 (Step 0)/webrev.v0
AND through the list of files deleted or modified by 8196584.

All the deleted files deleted by 8245467 (Step 0) are within the
sun/security/ssl directory, as expected.

There are a few files deleted by 8196584 which were not deleted by
8245467 (Step 0) because they have never been introduced to JDK-8. I'll
list them below, grouped by bug ID:

 * 8046321: JEP 249: OCSP Stapling for TLS

  * classes/sun/security/ssl/CertStatusReqExtension.java
  * classes/sun/security/ssl/CertStatusReqItemV2.java
  * classes/sun/security/ssl/CertStatusReqListV2Extension.java
  * classes/sun/security/ssl/OCSPStatusRequest.java
  * classes/sun/security/ssl/StatusRequest.java
  * classes/sun/security/ssl/StatusRequestType.java
  * classes/sun/security/ssl/StatusResponseManager.java
  * classes/sun/security/ssl/UnknownStatusRequest.java

 * 8043758: JEP 219: Datagram Transport Layer Security (DTLS)

  * classes/sun/security/ssl/Ciphertext.java
  * classes/sun/security/ssl/DTLSInputRecord.java
  * classes/sun/security/ssl/DTLSOutputRecord.java
  * classes/sun/security/ssl/DTLSRecord.java
  * classes/sun/security/ssl/HelloCookieManager.java
  * classes/sun/security/ssl/MaxFragmentLengthExtension.java
  * classes/sun/security/ssl/Plaintext.java
  * classes/sun/security/ssl/SSLEngineInputRecord.java
  * classes/sun/security/ssl/SSLEngineOutputRecord.java
  * classes/sun/security/ssl/SSLRecord.java
  * classes/sun/security/ssl/SSLSocketInputRecord.java
  * classes/sun/security/ssl/SSLSocketOutputRecord.java

 * 8038089: TLS optional support for Kerberos cipher suites needs to be
re-examined

  * classes/sun/security/ssl/ClientKeyExchange.java
  * classes/sun/security/ssl/ClientKeyExchangeService.java
  * classes/sun/security/krb5/internal/ssl/KerberosPreMasterSecret.java
  * classes/sun/security/krb5/internal/ssl/Krb5KeyExchangeService.java

 * 8140436: Negotiated Finite Field Diffie-Hellman Ephemeral Parameters
for TLS

  * classes/sun/security/ssl/NamedGroup.java
  * classes/sun/security/ssl/NamedGroupType.java
  * classes/sun/security/ssl/PredefinedDHParameterSpecs.java
  * classes/sun/security/ssl/SupportedGroupsExtension.java

There are a few files deleted by 8245467 (Step 0) that were not deleted
by 8196584 because by the time 8196584 was introduced, previous
changesets already deleted them. I'll list them below, grouped by bug ID:

 * 8043758: JEP 219: Datagram Transport Layer Security (DTLS)

  * classes/sun/security/ssl/EngineArgs.java
  * classes/sun/security/ssl/EngineInputRecord.java
  * classes/sun/security/ssl/EngineOutputRecord.java
  * classes/sun/security/ssl/EngineWriter.java

 * 8140436: Negotiated Finite Field Diffie-Hellman Ephemeral Parameters
for TLS

  * classes/sun/security/ssl/EllipticCurvesExtension.java

Some files are out of sun/security/ssl, we they are not deleted by Step 0:

 * conf/security/java.security
 * classes/com/sun/jndi/ldap/ext/StartTlsResponseImpl.java
 * classes/sun/security/jgss/GSSCaller.java
 * classes/sun/security/jgss/LoginConfigImpl.java
 * classes/sun/security/krb5/EncryptedData.java
 * classes/module-info.java

Some files are within sun/security/ssl but are not deleted by Step 0 and
are not deleted or modified by 8196584 either:

 * classes/sun/security/ssl/KerberosClientKeyExchange.java
 * classes/sun/security/ssl/Krb5Helper.java
 * classes/sun/security/ssl/Krb5Proxy.java

This is an exception because the Kerberos feature -removed in JDK-11-
will stay in JDK-8 for compatibility reasons. We decided to preserve the
repository history here, as the 'new file' hard-stop would add no value.

The classes/sun/security/ssl/SunJSSE.java file was not deleted -even
though it has been modified by 8196584- and is an exception too. The
reason is that this file includes changes from 7092821, which escape
sun/security/ssl and would be better handled separately. We could have
done file-replacement here and created a new Step to remove them, but it
is probably not worth the effort. Note that 7092821 is required for
parity with Oracle's JDK and will be introduced later. At some point in
the future, there should be almost no differences with JDK 11.0.7's
SunJSSE.java file.

With all that said, 8245467 (Step 0) webrev.v0 looks okay to me.

Note: we had a previous iteration of Step 0 here [1], before splitting
the discussion into multiple threads.

If any other reviewer has comments related to Step 0 -or to the approach
we have taken-, the time is *now* because changes in Step 0 may cause
following Step patches to be re-generated in cascade.

@Alexey: please hold the push until we review all the steps and a JDK-8
maintainer approves the series of patches.

Thanks,
Martin.-

--
[1] - https://mail.openjdk.java.net/pipermail/jdk8u-dev/2020-May/011716.html

More information about the jdk8u-dev mailing list