8278851: Correct signer logic for jars signed with multiple digest algorithms

[jerboaa]

Please review this backport which fixes a regression new in 8u362 due to JDK-8269039. For some jars which are signed with an obsolete digest algorithm, after JDK-8269039, they're treated as unsigned even though one of the signatories used a valid (non-obsolete) digest algo.

The backport is not clean, mainly because of JDK-8275887 (which in 11u wasn't present when that bug was backported), copyright headers and different test infra in 8u. So the test looks a bit different so that it works on 8u. All of the changes are fairly trivial to resolve. Changes in SecurityUtils.java aren't needed as 8u-dev already has them.

Testing:

• jdk/test/sun/security/tools/jarsigner tests pass.
• new regression test. Fails prior product fix, passes after.

---

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

Issue

• JDK-8278851: Correct signer logic for jars signed with multiple digest algorithms

Reviewers

• Martin Balao (@martinuy - Reviewer)
• Andrew John Hughes (@gnu-andrew - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 282

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

Using diff file

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

Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back sgehwolf! 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 (7ecdf217)

[jerboaa]

GHA failure on Linux x86 (32 bit) of gc/logging/TestGCId.java seems unrelated.

[jerboaa]

@martinuy Hi! Could you please help review this? Thanks.

[martinuy]

Hi @jerboaa ,

Thanks for contributing this backport. Looks good to me. I've run the JarWithOneNonDisabledDigestAlg test locally and passes.

Minor note —which is not worth any change to the current proposal, in my view—: the list that List.of returns is unmodifiable, which is not the case for Arrays.asList.

Just out of curiosity, was there a reason for placing JarWithOneNonDisabledDigestAlg into a new disdigest directory? I see that in 11u and above is in the jarsigner directory.

Thanks,
Martin.-

[openjdk]

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

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

8278851: Correct signer logic for jars signed with multiple digest algorithms

Reviewed-by: mbalao, 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 12 new commits pushed to the master branch:

• 1ee22a7: 8263059: security/infra/java/security/cert/CertPathValidator/certification/ComodoCA.java fails due to revoked cert
• d41618f: 8271199: Mutual TLS handshake fails signing client certificate with custom sensitive PKCS11 key
• cd40350: Merge
• 89aeae1: 8304053: Revert os specific stubs for SystemMetrics
• ebbd3a8: 8303828: [Solaris] Broken jdk8u build after JDK-8266391
• 5dc33f2: 8295530: Update Zlib Data Compression Library to Version 1.2.13
• ca1c3b1: 8302791: Add specific ClassLoader object to Proxy IllegalArgumentException message
• de0e5a2: 8152432: Implement setting jtreg @requires properties vm.flavor, vm.bits, vm.compMode
• ae6405f: 8289301: P11Cipher should not throw out of bounds exception during padding
• ad41d90: 8293232: Fix race condition in pkcs11 SessionManager
• ... and 2 more: https://git.openjdk.org/jdk8u-dev/compare/7c5d77cbd2cf993643d48ded46342e01772a96cc...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.

[jerboaa]

Thanks for the review!

Minor note —which is not worth any change to the current proposal, in my view—: the list that List.of returns is unmodifiable, which is not the case for Arrays.asList.

OK. I could wrap it in Collections.unmodifiableList(...), but it's probably not worth it in this case.

Just out of curiosity, was there a reason for placing JarWithOneNonDisabledDigestAlg into a new disdigest directory? I see that in 11u and above is in the jarsigner directory.

Yes, the test failed when adding to the jarsigner directory directly. At least when the full suite of tests was run in the jarsigner directory. If I remember correctly, this caused some weird CNFE.

[gnu-andrew]

8u backport looks correct. I had the same comment about List.of vs asList as Martin. It's not worth changing here, but, given how frequently this comes up, we should probably look at having a utility method to handle the conversion.

I see the lack of JDK-8056174 results in having to rewrite the signJarFile method completely to call the external program, as there is no programmatic access to jar signing. Too risky to backport this just for a nicer test.

Approved for 8u

[jerboaa]

Thanks!

/integrate

[openjdk]

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

• 1ee22a7: 8263059: security/infra/java/security/cert/CertPathValidator/certification/ComodoCA.java fails due to revoked cert
• d41618f: 8271199: Mutual TLS handshake fails signing client certificate with custom sensitive PKCS11 key
• cd40350: Merge
• 89aeae1: 8304053: Revert os specific stubs for SystemMetrics
• ebbd3a8: 8303828: [Solaris] Broken jdk8u build after JDK-8266391
• 5dc33f2: 8295530: Update Zlib Data Compression Library to Version 1.2.13
• ca1c3b1: 8302791: Add specific ClassLoader object to Proxy IllegalArgumentException message
• de0e5a2: 8152432: Implement setting jtreg @requires properties vm.flavor, vm.bits, vm.compMode
• ae6405f: 8289301: P11Cipher should not throw out of bounds exception during padding
• ad41d90: 8293232: Fix race condition in pkcs11 SessionManager
• ... and 2 more: https://git.openjdk.org/jdk8u-dev/compare/7c5d77cbd2cf993643d48ded46342e01772a96cc...master

Your commit was automatically rebased without conflicts.

[openjdk]

@jerboaa Pushed as commit f1c1f96.

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