[8u] RFR 8202343: Disable TLS 1.0 and 1.1

[Severin Gehwolf]

Hi Martin,

On Fri, 2021-01-22 at 12:40 -0300, Martin Balao wrote:
> Hi Severin,
> 
> Thanks for having a look at this.
> 
> On 1/21/21 3:31 PM, Severin Gehwolf wrote:
> > >  *
> > > test/javax/net/ssl/sanity/ciphersuites/SystemPropCipherSuitesOrder.ja
> > > va
> > >   * 8u does not have JDK-8234728 which introduces this test file. This
> > > case is similar to the JdkProcClient.java one, so I propose the same
> > > action now.
> > 
> > I tend to disagree. JDK-8234728 seems like a good patch to have for
> > OpenJDK 8u. It verifies proper ciphers are available for certain
> > algorithms. With the TLS 1.3 backport in 8u, it would be good to have
> > associated tests too. Oracle backported it to 8 as well. I'd suggest to
> > first backport JDK-8234728 and rebase this one on top.
> 
> Even when it would be desirable to have JDK-8234728 in 8u, I see it
> analogous to JDK-8243029 and tend to disagree on the
> dependencies-enforcement criteria. It's not a fundamental dependency:
> they conflict accidentally because 8202343 is touching many test files
> with the only goal of re-enabling TLS 1.0/1.1 for them to keep passing.
> It's perhaps more clear if we think that 8202343 would have been
> developed before 8234728 without any issues; and the fact that 8234728
> was before is just a coincidence.
> 
> Even when JDK-8234728 applies cleanly, it fails to pass and it's far
> from a trivial backport. In fact, my current investigation is suggesting
> that JDK-8234728 is failing to properly execute
> HandshakeHashCloneExhaustion with TLS 1.3; and is making it to
> trivially-pass. TLS 1.3 uses the HKDF class to derive keys, which gets a
> MessageDigest instance. However, because of JDK-8157579, there is a
> fallback scheme for non-clonable MessageDigest implementations. As a
> result, the MessageDigest instance used is not DigestBase$<SMTH> (from
> the test BaseDigest.java file) but one from the SUN provider. Looks to
> me that this is different than in TLS 1.2 or below, where the
> TlsPrfGenerator class is used to derive keys and we have no
> fallback-scheme for non-clonable MessageDigests. I've still not verified
> this last statement, though. Verifying this last statement would confirm
> HandshakeHashCloneExhaustion author's intention of using
> DigestBase$<SMTH> instead of any other MessageDigest implementation.
> Note: 8u does not have the JDK-8157579 fallback-scheme and is exposing
> the bug.
> 
> My point is that we now need to shift focus to investigate 8234728 in
> jdk/jdk, fix it (eventually), create backports for it, backport 8157579
> perhaps, and then continue with the backport of 8234728 -which has a
> followup for another test, and I'm not sure it does not cause conflicts
> or leads to a whole new chain of dependencies-.
> 
> I agree in that would be ideal to have all these tests and fixes, or
> even to backport dependencies when they are non-fundamental but trivial.
> But my concern is that a low-priority test may now be blocking an
> important backport.

The basis of my suggestion to pull in that dep first was that the
backport of JDK-8234728 would be reasonable straight-forward. It sounds
like it isn't. Given the aforementioned, that it's a hunk to test-only
files missing and on the grounds that this patch only adds a hunk to
re-enable old TLS like this:

+        // Re-enable protocol if disabled.
+        if (protocol.equals("TLSv1") || protocol.equals("TLSv1.1")) {
+            SecurityUtils.removeFromDisabledTlsAlgs(protocol);
+        }

it should be reasonable to omit those and get 8202343 in. Please add a
comment on JDK-8234728 to that effect. This would ensure that those
hunks will get added should JDK-8234728 get backported later.

To me this isn't the same ballpark as JDK-8243029 since A) for JDK-
8243029, JDK 11's version of the framework can be used to test JDK 8
compatibility this isn't the case for JDK-8234728 B) JDK-8234728 checks
correctness of some TLS 1.3 aspects not otherwise covered. YMMV.

Either way, please proceed with the backport of this with hunks to
test/javax/net/ssl/sanity/ciphersuites/SystemPropCipherSuitesOrder.java
test/javax/net/ssl/sanity/ciphersuites/TLSCipherSuitesOrder.java
omitted and adding a comment on JDK-8234728.

Thanks,
Severin