8269039: Disable SHA-1 Signed JARs

[GoeLin]

src/java.base/share/conf/security/java.security
Does not resolve because 11 mentions "include jdk.disabled.namedCurves"

src/jdk.jartool/share/classes/sun/security/tools/jarsigner/Main.java
Some hunks did not apply because DISABLED_CHECK was renamed
to JAR_DISABLED_CHECK in 17.
Other hunks patch methods not in 11: checkWeakKey(), checkWeakAlg()
as well as the calls to these methods.

test/jdk/java/security/Security/signedfirst/Dyn.sh
test/jdk/java/security/Security/signedfirst/Static.sh
Deleting did not apply.

test/jdk/java/util/jar/JarInputStream/signed.jar
Patching this binary file failed. I just copied
the file from 17.

test/jdk/sun/security/tools/jarsigner/CheckSignerCertChain.java
Patch skipped, test not in 11.

test/jdk/sun/security/tools/jarsigner/TimestampCheck.java
Resolved. Checked output differed.

test/lib/jdk/test/lib/security/SecurityUtils.java
The change to this file was already backported.

In addition, I adapted
sun/security/tools/jarsigner/DefaultOptions.java
sun/security/tools/jarsigner/NameClash.java
sun/security/tools/jarsigner/EC.java
according to
"8172404: Tools should warn if weak algorithms are used before restricting them"
which makes the tests pass.

---

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 a CSR request to be approved

Issues

• JDK-8269039: Disable SHA-1 Signed JARs
• JDK-8264362: Disable SHA-1 Signed JARs (CSR)

Reviewers

• Martin Balao (@martinuy - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk11u-dev pull/1244/head:pull/1244
$ git checkout pull/1244

Update a local copy of the PR:
$ git checkout pull/1244
$ git pull https://git.openjdk.org/jdk11u-dev pull/1244/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 1244

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk11u-dev/pull/1244.diff

[bridgekeeper]

👋 Welcome back goetz! 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]

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

[mlbridge]

Webrevs

• 00: Full (942e3c29)

[GoeLin]

@mbalao @martinuy
Hi, could you have a look at this change? I would appreciate a review by someone with experience with this code. Thanks!

[martinuy]

Hi Goetz,

Thanks for proposing this backport.

Comments:

• src/jdk.jartool/share/classes/sun/security/tools/jarsigner/Main.java

  • Looks to me that the parameter 'CertPathConstraintsParameters cpcp' is added to the method printCert but not used. In 17u, this parameter is passed to checkWeakKey (introduced in 8259401), which has not been backported to 11u. I'd consider either removing it or backporting 8259401 too, which does not seem (at first glance) too complex. Give that this is a minor comment and that we are targeting 11.0.17, this suggestion won't be a blocker.

• test/jdk/java/security/Security/signedfirst/exp.jar

  • I was comparing against 17u-dev and found that this file was not deleted there. The correct action is to delete it, as proposed in the 11u backport and in the original patch. When the test was a script, this file was used. Now that the test is in Java, it uses the test libs to create the JAR in run time. No harm but, technically, we should delete this file from 17u.
    • Note: this comment also applies to the file test/jdk/java/security/Security/signedfirst/keystore.jks.

In the test/jdk/sun/security/tools/jarsigner directory, I've not found references to "SHA1" that are not in 17u. The changes for tests removed in later releases look good to me.

I've also checked that there are no regressions in the following test categories:

• test/jdk/java/security/Security/signedfirst
• test/jdk/sun/security/tools/jarsigner

In summary, the 11u backport looks good to me. Good job.

I've seen that the 17u backport links the original CSR. In the old days we used to have a CSR specific for the backport. Please check that with the 11u release maintainers.

Thanks,
Martin.-

--
[1] - 942e3c2#diff-7083af3b8473a092987afa0bbb4d1694664649534bac716f6b9cd3c3b9833219R1449

[openjdk]

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

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

8269039: Disable SHA-1 Signed JARs

Reviewed-by: mbalao

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 37 new commits pushed to the master branch:

• 3c33047: 8268185: Update GitHub Actions for jtreg 6
• 00d9483: 8266254: Update to use jtreg 6
• c61acb5: 8258946: Fix optimization-unstable code involving signed integer overflow
• 5d20cb6: 8210960: Allow --with-boot-jdk-jvmargs to work during configure
• f00776b: 8285728: Alpine Linux build fails with busybox tar
• a625058: 8239379: ProblemList serviceability/sa/sadebugd/DebugdConnectTest.java on OSX
• 9de7c4c: 8282860: Write a regression test for JDK-4164779
• 0174134: 8284367: JQuery UI upgrade from 1.12.1 to 1.13.1
• 37ca256: 8285097: Duplicate XML keys in XPATHErrorResources.java and XSLTErrorResources.java
• a1a0459: 8290886: [11u]: Backport of JDK-8266250 introduced test failures
• ... and 27 more: https://git.openjdk.org/jdk11u-dev/compare/1f4028960a3934853104efd1d95991b137b5f520...master

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 master branch, type /integrate in a new comment.

[GoeLin]

Hi Martin,
thanks for having a look that fast.

• Yes, I added the wrong CSR. Fixed for 17.
• I will remove the file in 17.
• I'll have a look at 8259401 and backport if feasible.

[GoeLin]

Hi Martin,
I filed 8291595: [17u] Delete files missed in backport of 8269039
Also, I had a look at 8259401. It can be backported clean and passes our testing. But it comes with a release note about the warning, and there is no such note for 11. So I think we should rather not backport this change. So should I remove the argument?
On the other side, eventual follow up backports apply better if I keep it as-is, and this should not harm performance as it is only called for printing.

[GoeLin]

Having double-checked with my colleagues I would like to push it as-is.

[GoeLin]

/integrate

[openjdk]

Going to push as commit 5a0824b.
Since your change was applied there have been 47 commits pushed to the master branch:

• 5f79e3b: 8290334: Update FreeType to 2.12.1
• 60a6890: 8290246: test fails "assert(init != __null) failed: initialization not found"
• 8a88a69: 8286314: Trampoline not created for far runtime targets outside small CodeCache
• 81ffdc1: 8283015: Create a test for JDK-4715496
• fc4868e: 8283493: Create an automated regression test for RFE 4231298
• 672e3c2: 8282789: Create a regression test for the JTree usecase of JDK-4618767
• a92d71d: 8280913: Create a regression test for JRootPane.setDefaultButton() method
• 4b85189: 8275689: [TESTBUG] Use color tolerance only for XRender in BlitRotateClippedArea test
• d928ab7: 8270312: Error: Not a test or directory containing tests: java/awt/print/PrinterJob/XparColor.java
• 3957731: 8264792: The NumberFormat for locale sq_XK formats price incorrectly.
• ... and 37 more: https://git.openjdk.org/jdk11u-dev/compare/1f4028960a3934853104efd1d95991b137b5f520...master

Your commit was automatically rebased without conflicts.

[openjdk]

@GoeLin Pushed as commit 5a0824b.

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