-
Notifications
You must be signed in to change notification settings - Fork 5.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
8337664: Distrust TLS server certificates issued after Oct 2024 and anchored by Entrust Root CAs #20731
Conversation
👋 Welcome back mpowers! A progress list of the required criteria for merging this PR into |
@mcpowers This change now passes all automated pre-integration checks. ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details. After integration, the commit message for the final commit will be:
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 322 new commits pushed to the
As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details. ➡️ To integrate this PR with the above commit message to the |
/csr |
@mcpowers has indicated that a compatibility and specification (CSR) request is needed for this pull request. @mcpowers this pull request must refer to an issue in JBS to be able to link it to a CSR request. To refer this pull request to an issue in JBS, please update the title of this pull request to just the issue ID. |
Webrevs
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Please also have Rajan review.
@@ -0,0 +1,136 @@ | |||
/* | |||
* Copyright (c) 2018, 2024, Oracle and/or its affiliates. All rights reserved. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copyright only needs 2024.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test/jdk/sun/security/ssl/X509TrustManagerImpl/Entrust/Distrust.java has the same problem.
Both have been fixed.
@rhalade You'll need to review this. |
X509TrustManager xtm = (X509TrustManager)tm; | ||
return xtm; | ||
} | ||
throw new Exception("No TrustManager for " + type); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please update this and other Exception
thrown in loadCertificateChain
and testTM
function to RuntimeException
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will address with separate bug.
} catch (CertificateException ce) { | ||
// expired TLS certificates should not be treated as failure | ||
if (expired(ce)) { | ||
System.err.println("Test is N/A, chain is expired"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be updated to throw SkippedException so we know that certificates are expired?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment indicates this is normal and not deserving of an exception to grab someone's attention. Sean might think otherwise.
/integrate |
Going to push as commit bbb5161.
Your commit was automatically rebased without conflicts. |
Please review this change to distrust TLS server certificates issued after October 31, 2024 and anchored by Entrust Root CAs. This change is in line with similar plans recently announced by Google and Mozilla. TLS server certificates issued before this date will continue to be valid until they expire. This restriction should have minimal compatibility impact since Entrust has announced they will be using a partner (SSL.com) for all TLS server certificates issued after Oct 31, 2024.
See the CSR for more details: https://bugs.openjdk.org/browse/JDK-8339194
Progress
Issues
Reviewers
Reviewing
Using
git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/20731/head:pull/20731
$ git checkout pull/20731
Update a local copy of the PR:
$ git checkout pull/20731
$ git pull https://git.openjdk.org/jdk.git pull/20731/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 20731
View PR using the GUI difftool:
$ git pr show -t 20731
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/20731.diff
Webrev
Link to Webrev Comment