8337664: Distrust TLS server certificates issued after Oct 2024 and anchored by Entrust Root CAs #61

franferrax · 2024-09-10T20:55:35Z

Hi, here is a JDK-8337664: Distrust TLS server certificates issued after Oct 2024 and anchored by Entrust Root CAs backport, based on openjdk/jdk11u#95.

After adjusting the file paths from 11u to 8u, the backport isn't clean, but conflicts are minimal. These include a copyright line and minor java.security-<platform> context mismatches. You can verify this comparing 00beb50 against openjdk/jdk11u@90ad5b1.

On top of that, the code still needed adjustments for the 8u codebase, which were addressed in a separate commit, 53e8134. I made these adjustments in line with 68e393c, the 8u backport of JDK-8207258: Distrust TLS server certificates anchored by Symantec Root CAs.

Testing

I run jdk/tier1 and all the tests under jdk/test/sun/security/ssl, using 64-bit slowdebug and release images, locally built in Fedora Linux 40. Please note that this includes the new X509TrustManagerImpl/Entrust/Distrust.java, which I've also made fail by temporarily undoing the java.security-linux changes. I haven't found any regression against master (currently e32d62e).

Regarding the failures in GitHub Actions, we can see that this also occurred in recent jdk8u-dev pull requests. For example:

security/infra/java/security/cert/CertPathValidator/certification/CAInterop.java#sslrooteccca
security/infra/java/security/cert/CertPathValidator/certification/CAInterop.java#sslrootevrsaca
- Failed in this PR, in Linux x64 (jdk/tier1), Linux x86 (jdk/tier1), Windows x64 (jdk/tier1) and Windows x86 (jdk/tier1)
- Same as 8066708: JMXStartStopTest fails to connect to port 38112 jdk8u-dev#430, in Linux x64 (jdk/tier1), Linux x86 (jdk/tier1), Windows x64 (jdk/tier1) and Windows x86 (jdk/tier1)
com/sun/jdi/PrivateTransportTest.sh
security/infra/java/security/cert/CertPathValidator/certification/CAInterop.java#sslrooteccca
security/infra/java/security/cert/CertPathValidator/certification/CAInterop.java#sslrootevrsaca
sun/misc/CopyMemory.java
- Failed in this PR, in macOS x64 (jdk/tier1)
- Same as 8066708: JMXStartStopTest fails to connect to port 38112 jdk8u-dev#430, in macOS x64 (jdk/tier1)
gc/concurrentMarkSweep/CheckAllocateAndSystemGC.java
- Failed in this PR, in Linux x86 (hotspot/tier1)
- Same as 8066708: JMXStartStopTest fails to connect to port 38112 jdk8u-dev#430, in Linux x86 (hotspot/tier1)
compiler/unsafe/OpaqueAccesses.java
- Failed in this PR, in macOS x64 (hotspot/tier1)
- Same as 8066708: JMXStartStopTest fails to connect to port 38112 jdk8u-dev#430, in macOS x64 (hotspot/tier1)

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Commit message must refer to an issue
Change requires CSR request JDK-8339194 to be approved
JDK-8337664 needs maintainer approval

Issues

JDK-8337664: Distrust TLS server certificates issued after Oct 2024 and anchored by Entrust Root CAs (Enhancement - P3 - Approved)
JDK-8339194: Distrust TLS server certificates issued after Oct 2024 and anchored by Entrust Root CAs (CSR)

Reviewers

Severin Gehwolf (@jerboaa - Reviewer)
Andrew John Hughes (@gnu-andrew - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk8u.git pull/61/head:pull/61
$ git checkout pull/61

Update a local copy of the PR:
$ git checkout pull/61
$ git pull https://git.openjdk.org/jdk8u.git pull/61/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 61

View PR using the GUI difftool:
$ git pr show -t 61

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk8u/pull/61.diff

Webrev

Link to Webrev Comment

bridgekeeper · 2024-09-10T20:56:37Z

👋 Welcome back fferrari! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2024-09-10T20:57:59Z

@franferrax This change now passes all automated pre-integration checks.

After integration, the commit message for the final commit will be:

8337664: Distrust TLS server certificates issued after Oct 2024 and anchored by Entrust Root CAs

Reviewed-by: sgehwolf, andrew

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been no new commits pushed to the master branch. If another commit should be pushed before you perform the /integrate command, your PR will be automatically rebased. If you prefer to avoid any potential automatic rebasing, please check the documentation for the /integrate command for further details.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@jerboaa, @gnu-andrew) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

openjdk · 2024-09-10T20:58:18Z

This backport pull request has now been updated with issue from the original commit.

mlbridge · 2024-09-12T12:13:00Z

Webrevs

01: Full - Incremental (e96486dc)
00: Full (53e81347)

jerboaa

Looks mostly fine.

jerboaa · 2024-09-12T15:58:42Z

jdk/src/share/classes/sun/security/validator/EntrustTLSPolicy.java

+    private static final Debug debug = Debug.getInstance("certpath");
+
+    // SHA-256 certificate fingerprints of distrusted roots
+    private static final Set<String> FINGERPRINTS = new HashSet<>(Arrays.asList(


Suggested change

private static final Set<String> FINGERPRINTS = new HashSet<>(Arrays.asList(

private static final Set<String> FINGERPRINTS = Collections.unmodifiableSet(new HashSet<>(Arrays.asList(...)));

Updated in e96486d, please note that the SYMANTEC_TLS distrust policy (which I checked for this backport) does not make the FINGERPRINTS set immutable either:

jdk8u/jdk/src/share/classes/sun/security/validator/SymantecTLSPolicy.java

Lines 66 to 70 in e32d62e

// SHA-256 certificate fingerprints of distrusted roots

private static final Set<String> FINGERPRINTS = new HashSet<>(Arrays.asList(

// cacerts alias: geotrustglobalca

// DN: CN=GeoTrust Global CA, O=GeoTrust Inc., C=US

"FF856A2D251DCD88D36656F450126798CFABAADE40799C722DE4D2B5DB36A73A",

Should we also update SymantecTLSPolicy.java?

Thanks!

Should we also update SymantecTLSPolicy.java?

No, not in this bug. Feel free to do this as an 8u-only fix via jdk8u-dev pr. The original backport to 8 of https://bugs.openjdk.org/browse/JDK-8207258 didn't do this. That's one of the gotchas of Set.of() backports. It's not terribly important.

This is a common case, as Severin implies. For the test library, we added internal versions of the listOf and setOf methods to better handle this. It has the advantage that the same error checking (null values, duplicates) is present as on the methods in later JDKs.

I can look at moving that to an internal JDK class instead to cover library cases, and fix the Symantec case then. No reason to delay this critical fix for that.

Opened https://bugs.openjdk.org/browse/JDK-8340414

Opened https://bugs.openjdk.org/browse/JDK-8340414

Great!

jerboaa

Looks good.

openjdk · 2024-09-12T19:45:18Z

⚠️ @franferrax This change is now ready for you to apply for maintainer approval. This can be done directly in each associated issue or by using the /approval command.

franferrax · 2024-09-18T16:43:57Z

Adding @gnu-andrew for awareness. According to the Crypto Roadmap, this change is targeting 8u, and planned for the October CPU. 23u, 21u, 17u and 11u backports are already integrated.

gnu-andrew

Backport looks good to me. Missing collection and time methods are ported to suitable 8u equivalents. java.security changes are duplicated to the set of files in 8u.

I see differences with entrustrootcag4-chain.pem but this seems to be because 11u's version has CRLF line endings for some reason. The 8u one is actually correct in using the usual line endings.

gnu-andrew · 2024-09-18T17:29:56Z

Adding @gnu-andrew for awareness. According to the Crypto Roadmap, this change is targeting 8u, and planned for the October CPU. 23u, 21u, 17u and 11u backports are already integrated.

I'm aware of the fix from 21u. Backport looks good so feel free to apply for approval so we can get this in the October release.

gnu-andrew · 2024-09-18T17:30:49Z

I'll also take a look at the PR you reference for the tests and see if we can get that fixed in -dev. It won't change these results though.

franferrax · 2024-09-19T11:04:38Z

I see differences with entrustrootcag4-chain.pem but this seems to be because 11u's version has CRLF line endings for some reason. The 8u one is actually correct in using the usual line endings.

I hadn't noticed this, the original version has mixed CR and CRLF, and it got fixed when adjusting and applying the patch. If you prefer it to be a verbatim copy I can easily modify it to match the original.

I'm aware of the fix from 21u. Backport looks good so feel free to apply for approval so we can get this in the October release.

Thanks, here is the approval request.

gnu-andrew · 2024-09-19T21:50:08Z

I see differences with entrustrootcag4-chain.pem but this seems to be because 11u's version has CRLF line endings for some reason. The 8u one is actually correct in using the usual line endings.

I hadn't noticed this, the original version has mixed CR and CRLF, and it got fixed when adjusting and applying the patch. If you prefer it to be a verbatim copy I can easily modify it to match the original.

No need to change it. If anything, 11u should be fixed.

I'm aware of the fix from 21u. Backport looks good so feel free to apply for approval so we can get this in the October release.

Thanks, here is the approval request.

Thanks, I didn't see the /approval command for it on this PR.

/approve yes

openjdk · 2024-09-19T21:51:07Z

@gnu-andrew
8337664: The approval request has been approved.

franferrax · 2024-09-20T10:41:03Z

No need to change it. If anything, 11u should be fixed.

Ok, just for the record, 17u, 21u, 23u and mainline also have the mixed line endings in entrustrootcag4-chain.pem.

Thanks, I didn't see the /approval command for it on this PR.

Yes, I manually filled the request in the JBS because other backports requests were done in that way, and I was wondering which label the /approval command would add (jdk8u-fix-request vs jdk8u-critical-request).

franferrax · 2024-09-20T10:41:21Z

/integrate

openjdk · 2024-09-20T10:42:42Z

@franferrax
Your change (at version e96486d) is now ready to be sponsored by a Committer.

gnu-andrew · 2024-09-20T10:44:09Z

/sponsor

openjdk · 2024-09-20T10:44:41Z

Going to push as commit 39221f8.

openjdk · 2024-09-20T10:44:49Z

@gnu-andrew @franferrax Pushed as commit 39221f8.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

franferrax added 2 commits September 10, 2024 21:52

Backport 7d49c52272b54070a13b02708dd7ce5f8e375a06

00beb50

Adjust code for OpenJDK 8

53e8134

franferrax changed the title ~~Backport 7d49c52272b54070a13b02708dd7ce5f8e375a06 @franferrax~~ Backport 7d49c52272b54070a13b02708dd7ce5f8e375a06 Sep 10, 2024

openjdk bot changed the title ~~Backport 7d49c52272b54070a13b02708dd7ce5f8e375a06~~ 8337664: Distrust TLS server certificates issued after Oct 2024 and anchored by Entrust Root CAs Sep 10, 2024

openjdk bot added the backport label Sep 10, 2024

franferrax marked this pull request as ready for review September 12, 2024 12:06

openjdk bot added the rfr Pull request is ready for review label Sep 12, 2024

jerboaa reviewed Sep 12, 2024

View reviewed changes

Wrap the FINGERPRINTS set as immutable

e96486d

jerboaa approved these changes Sep 12, 2024

View reviewed changes

openjdk bot added the approval label Sep 13, 2024

gnu-andrew approved these changes Sep 18, 2024

View reviewed changes

openjdk bot added ready Pull request is ready to be integrated and removed approval labels Sep 19, 2024

openjdk bot added the sponsor Pull request is ready to be sponsored label Sep 20, 2024

openjdk bot added the integrated Pull request has been integrated label Sep 20, 2024

openjdk bot closed this Sep 20, 2024

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review sponsor Pull request is ready to be sponsored labels Sep 20, 2024

franferrax deleted the backport_8337664 branch September 26, 2024 12:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8337664: Distrust TLS server certificates issued after Oct 2024 and anchored by Entrust Root CAs #61

8337664: Distrust TLS server certificates issued after Oct 2024 and anchored by Entrust Root CAs #61

franferrax commented Sep 10, 2024 •

edited by openjdk bot

Loading

bridgekeeper bot commented Sep 10, 2024

openjdk bot commented Sep 10, 2024 •

edited

Loading

openjdk bot commented Sep 10, 2024

mlbridge bot commented Sep 12, 2024 •

edited

Loading

jerboaa left a comment

jerboaa Sep 12, 2024

franferrax Sep 12, 2024

jerboaa Sep 12, 2024

gnu-andrew Sep 18, 2024

gnu-andrew Sep 18, 2024

franferrax Sep 19, 2024

jerboaa left a comment

openjdk bot commented Sep 12, 2024

franferrax commented Sep 18, 2024

gnu-andrew left a comment

gnu-andrew commented Sep 18, 2024

gnu-andrew commented Sep 18, 2024

franferrax commented Sep 19, 2024

gnu-andrew commented Sep 19, 2024

openjdk bot commented Sep 19, 2024

franferrax commented Sep 20, 2024

franferrax commented Sep 20, 2024

openjdk bot commented Sep 20, 2024

gnu-andrew commented Sep 20, 2024

openjdk bot commented Sep 20, 2024

openjdk bot commented Sep 20, 2024

	private static final Set<String> FINGERPRINTS = new HashSet<>(Arrays.asList(
	private static final Set<String> FINGERPRINTS = Collections.unmodifiableSet(new HashSet<>(Arrays.asList(...)));

	// SHA-256 certificate fingerprints of distrusted roots
	private static final Set<String> FINGERPRINTS = new HashSet<>(Arrays.asList(
	// cacerts alias: geotrustglobalca
	// DN: CN=GeoTrust Global CA, O=GeoTrust Inc., C=US
	"FF856A2D251DCD88D36656F450126798CFABAADE40799C722DE4D2B5DB36A73A",

8337664: Distrust TLS server certificates issued after Oct 2024 and anchored by Entrust Root CAs #61

8337664: Distrust TLS server certificates issued after Oct 2024 and anchored by Entrust Root CAs #61

Conversation

franferrax commented Sep 10, 2024 • edited by openjdk bot Loading

Testing

Progress

Issues

Reviewers

Reviewing

Webrev

bridgekeeper bot commented Sep 10, 2024

openjdk bot commented Sep 10, 2024 • edited Loading

openjdk bot commented Sep 10, 2024

mlbridge bot commented Sep 12, 2024 • edited Loading

Webrevs

jerboaa left a comment

Choose a reason for hiding this comment

jerboaa Sep 12, 2024

Choose a reason for hiding this comment

franferrax Sep 12, 2024

Choose a reason for hiding this comment

jerboaa Sep 12, 2024

Choose a reason for hiding this comment

gnu-andrew Sep 18, 2024

Choose a reason for hiding this comment

gnu-andrew Sep 18, 2024

Choose a reason for hiding this comment

franferrax Sep 19, 2024

Choose a reason for hiding this comment

jerboaa left a comment

Choose a reason for hiding this comment

openjdk bot commented Sep 12, 2024

franferrax commented Sep 18, 2024

gnu-andrew left a comment

Choose a reason for hiding this comment

gnu-andrew commented Sep 18, 2024

gnu-andrew commented Sep 18, 2024

franferrax commented Sep 19, 2024

gnu-andrew commented Sep 19, 2024

openjdk bot commented Sep 19, 2024

franferrax commented Sep 20, 2024

franferrax commented Sep 20, 2024

openjdk bot commented Sep 20, 2024

gnu-andrew commented Sep 20, 2024

openjdk bot commented Sep 20, 2024

openjdk bot commented Sep 20, 2024

franferrax commented Sep 10, 2024 •

edited by openjdk bot

Loading

openjdk bot commented Sep 10, 2024 •

edited

Loading

mlbridge bot commented Sep 12, 2024 •

edited

Loading