8308592: Framework for CA interoperability testing #390
Conversation
|
👋 Welcome back andrew! A progress list of the required criteria for merging this PR into |
openjdk
bot
changed the title
|
This backport pull request has now been updated with issue from the original commit. |
|
/issue add 8272708,8288132,8268678,8270280 |
|
@gnu-andrew Adding additional issue to issue list: Adding additional issue to issue list: Adding additional issue to issue list: |
|
Webrevs
|
There was a problem hiding this comment.
This looks good. The reason why certignarootca test fails is the different defaults for jdk.tls.client.enableStatusRequestExtension between JDK 11 and JDK 8 in SSLContextImpl.java. The former has it set to true the latter to false as per the TLS 1.3 backport to 8. Without it the ClientHello won't have the status_request extension, which is required for the test to pass.
I suggest to set this to true in the affected test only (or set it to true globally in CAInterop.java). Either way I'd include this in this backport.
openjdk-notifier
bot
force-pushed
the
pr/389
branch
from
6929927
to
5d885d5
Compare
|
@gnu-andrew this pull request can not be integrated into git checkout JDK-8308592
git fetch https://git.openjdk.org/jdk8u-dev.git pr/389
git merge FETCH_HEAD
# resolve conflicts and follow the instructions given by git merge
git commit -m "Merge pr/389"
git push |
openjdk
bot
added
the
merge-conflict
|
The parent pull request that this pull request depends on has now been integrated and the target branch of this pull request has been updated. This means that changes from the dependent pull request can start to show up as belonging to this pull request, which may be confusing for reviewers. To remedy this situation, simply merge the latest changes from the new target branch into this pull request by running commands similar to these in the local repository for your personal fork: git checkout JDK-8308592
git fetch https://git.openjdk.org/jdk8u-dev.git master
git merge FETCH_HEAD
# if there are conflicts, follow the instructions given by git merge
git commit -m "Merge master"
git push |
gnu-andrew
force-pushed
the
JDK-8308592
branch
from
0dc8bad
to
04ae02c
Compare
|
@gnu-andrew Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. See OpenJDK Developers’ Guide for more information. |
openjdk
bot
removed
the
merge-conflict
…ckport of TLS 1.3
Good catch. Looks like all tests pass with this enabled in |
Yeah, I'm not going to merge over half a dozen commits manually, bot, when Git is smart enough to match them against the ones you pushed to 8u-dev and just drop them. |
|
|
|
/approval request This change reworks the certificate tests to use test portals rather than having to maintain copies of the certificates in the OpenJDK repository and so should reduce the maintenance cost of these tests going forward. The fix has already been backported to the other LTS JDKs (11u, 17u, 21u). The backport required manually removing some tests which were not in their latest versions, a couple of dependent backports to provide us with SkippedException and a passing Actalis test, and 8u-specific modifications (change of cacerts location and type, enable jdk.tls.client.enableStatusRequestExtension). All tests passed successfully. |
|
@gnu-andrew |
|
/approve yes |
|
@jerboaa |
|
@gnu-andrew This change now passes all automated pre-integration checks. 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 no new commits pushed to the ➡️ To integrate this PR with the above commit message to the |
openjdk
bot
added
ready
|
@gnu-andrew Could you please integrate this? Thanks! |
|
Thought I had, sorry. |
|
Going to push as commit b610be3.
Your commit was automatically rebased without conflicts. |
openjdk
bot
removed
ready
|
@gnu-andrew Pushed as commit b610be3. 💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored. |
Hi all,
This pull request contains a backport of commit da57d2a1 from the openjdk/jdk repository.
The commit being backported was authored by Rajan Halade on 19 Sep 2023 and was reviewed by Sean Mullan. It was backported to 17u on 2023-09-28 and 11u on 2023-10-11
It reworks the certificate tests to use test portals rather than having to maintain copies of the certificates in the OpenJDK repository and so should reduce the maintenance cost of these tests going forward. The fix has already been backported to 21u and Oracle-17u.
There were conflicts in the removals of
ActalisCA.java,BuypassCA.java,LetsEncryptCA.javaandQuoVadisCA.java, due to changes not in 8u. For the first of these, we backported JDK-8297955 (see #388) as it also contains a code change. As the other three are only test changes, we assumed these bugs to be resolved by the replacement of the individual tests withCAInterop.java.The
ValidatePathWithURL.javaclass had to be modified slightly to work with the 8ucacerts. These changes were based on the existingValidatePathWithParams.java.All tests pass with the exception of the OCSP case of
certignarootca. This fails withCaused by: java.security.cert.CertPathValidatorException: Certificate does not specify OCSP responder. While not ideal, I don't think this is an issue with the testing backport so I'd suggest we look into this and fix it later, while getting this in so that pending certificate updates can go in. It seems to be 8u specific (the case passed on 11u). The CRL case does pass as doesCertignaCA.java(not sure why there are two tests for this one).Progress
Issues
Reviewers
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk8u-dev.git pull/390/head:pull/390$ git checkout pull/390Update a local copy of the PR:
$ git checkout pull/390$ git pull https://git.openjdk.org/jdk8u-dev.git pull/390/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 390View PR using the GUI difftool:
$ git pr show -t 390Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk8u-dev/pull/390.diff
Webrev
Link to Webrev Comment