-
Notifications
You must be signed in to change notification settings - Fork 6.2k
8369282: Distrust TLS server certificates anchored by Chunghwa ePKI Root CA #28930
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
base: master
Are you sure you want to change the base?
Conversation
|
👋 Welcome back mpowers! A progress list of the required criteria for merging this PR into |
|
❗ This change is not yet ready to be integrated. |
| * is at index 0, the trust anchor at index n-1. | ||
| * @throws ValidatorException if the certificate is distrusted | ||
| */ | ||
| static void checkDistrust(X509Certificate[] chain) |
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.
nit: checkDistrust is an odd name for a method throwing an error and not returning anything in my opinion. But since it's used in other files and behaves the same I think it's fine.
| return X509CertImpl.getFingerprint("SHA-256", cert, debug); | ||
| } | ||
|
|
||
| private static void checkNotBefore(LocalDate notBeforeDate, |
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.
I might be wrong, but wouldn't 'Not Before' mean that it would also include the date ('Equals or After'). I think renaming it to checkIsAfter would be better, what do you think?
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.
I agree. However, notBefore was probably chosen because it is also a field in the X509Certificate. The name appears in many places. What about adding a comment:
Check whether the certificate's notBeforeDate is after the distrust date for the anchor (root CA).
Throw ValidatorException if it is after the distrust date.
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.
That would be fine with me and I think would be helpful. Thank you!
JDK-8369282
Progress
Issues
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/28930/head:pull/28930$ git checkout pull/28930Update a local copy of the PR:
$ git checkout pull/28930$ git pull https://git.openjdk.org/jdk.git pull/28930/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 28930View PR using the GUI difftool:
$ git pr show -t 28930Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/28930.diff
Using Webrev
Link to Webrev Comment