8325254: CKA_TOKEN private and secret keys are not necessarily sensitive

[franferrax]

Hi, this is a second take of #2223, the backport of JDK-8325254: CKA_TOKEN private and secret keys are not necessarily sensitive, backed out by #2249.

Even though the original patch applies cleanly to 17u, it introduces a dependency on P11Util::isNSS() from JDK-8301553: Support Password-Based Cryptography in SunPKCS11.

I only picked the P11Util::isNSS() changes since the whole PBE patch would require a deeper assessment and testing.

Testing

• Build linux-x86_64-server-release and linux-x86_64-server-slowdebug
• Locally execute SunPKCS11 tests (test/jdk/sun/security/pkcs11) in both builds
  • Ensure all the patched lines have coverage by attaching a debugger during the execution
• Locally execute jdk:tier1 in both builds
• Review GitHub Actions from run 8285374490 on backport-8325254@franferrax/jdk17u-dev
  • The only failure is due to JDK-8326960: GHA: RISC-V linux-cross-compile is failing

Regards,
Francisco

---

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
• JDK-8325254 needs maintainer approval

Issue

• JDK-8325254: CKA_TOKEN private and secret keys are not necessarily sensitive (Bug - P4 - Approved)

Reviewers

• Martin Balao (@martinuy - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 2299

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

Using diff file

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

Webrev

Link to Webrev Comment

[bridgekeeper]

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

@franferrax 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:

8325254: CKA_TOKEN private and secret keys are not necessarily sensitive

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

• 7ced722: 8326529: JFR: Test for CompilerCompile events fails due to time out
• 08debe8: 8306040: HttpResponseInputStream.available() returns 1 on empty stream
• 78f482d: 8318727: Enable parallelism in vmTestbase/vm/gc/concurrent tests
• 5ecac7a: 8280377: MethodHandleProxies does not correctly invoke default methods with varags
• 4ececad: 8306714: Open source few Swing event and AbstractAction tests
• 151091c: 8305942: Open source several AWT Focus related tests
• 996dfb0: 8305943: Open source few AWT Focus related tests
• 615c01b: 8297292: java/nio/channels/FileChannel/FileExtensionAndMap.java is too slow
• 8e132c4: 8163229: several regression tests have a main method that is never executed
• 7680369: 8296610: java/net/HttpURLConnection/SetAuthenticator/HTTPSetAuthenticatorTest.java failed with "BindException: Address already in use: connect"
• ... and 46 more: https://git.openjdk.org/jdk17u-dev/compare/4b9ab0dd9eea74d1d381db3c4f31b69609f752ad...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.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@martinuy) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

[openjdk]

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

[mlbridge]

Webrevs

• 00: Full (66cb4995)

[martinuy]

Looks good to me.

[franferrax]

/approval request 17u is affected by this bug because it has JDK-8271566 that introduced it. This is a second take of the previous (backed out) backport which addresses the missing P11Util::isNSS() issue. Better testing has been done to ensure the change does not introduce any regression, please see the "Testing" section in the pull request for further details.

[openjdk]

@franferrax
8325254: Approval has already been requested and approved.

[franferrax]

/integrate

[openjdk]

@franferrax
Your change (at version 66cb499) is now ready to be sponsored by a Committer.

[martinuy]

/sponsor

[openjdk]

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

• 7ced722: 8326529: JFR: Test for CompilerCompile events fails due to time out
• 08debe8: 8306040: HttpResponseInputStream.available() returns 1 on empty stream
• 78f482d: 8318727: Enable parallelism in vmTestbase/vm/gc/concurrent tests
• 5ecac7a: 8280377: MethodHandleProxies does not correctly invoke default methods with varags
• 4ececad: 8306714: Open source few Swing event and AbstractAction tests
• 151091c: 8305942: Open source several AWT Focus related tests
• 996dfb0: 8305943: Open source few AWT Focus related tests
• 615c01b: 8297292: java/nio/channels/FileChannel/FileExtensionAndMap.java is too slow
• 8e132c4: 8163229: several regression tests have a main method that is never executed
• 7680369: 8296610: java/net/HttpURLConnection/SetAuthenticator/HTTPSetAuthenticatorTest.java failed with "BindException: Address already in use: connect"
• ... and 46 more: https://git.openjdk.org/jdk17u-dev/compare/4b9ab0dd9eea74d1d381db3c4f31b69609f752ad...master

Your commit was automatically rebased without conflicts.

[openjdk]

@martinuy @franferrax Pushed as commit d29fe03.

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