8347289: HKDF delayed provider selection failed with non-extractable PRK #22976

wangweij · 2025-01-08T18:43:30Z

A non-extractable PRK in HKDF Expand-Only is an invalid input, not an internal error.

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-8347289: HKDF delayed provider selection failed with non-extractable PRK (Bug - P3)

Reviewers

Valerie Peng (@valeriepeng - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/22976/head:pull/22976
$ git checkout pull/22976

Update a local copy of the PR:
$ git checkout pull/22976
$ git pull https://git.openjdk.org/jdk.git pull/22976/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 22976

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/22976.diff

Using Webrev

Link to Webrev Comment

bridgekeeper · 2025-01-08T18:44:14Z

👋 Welcome back weijun! 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 · 2025-01-08T18:44:49Z

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

8347289: HKDF delayed provider selection failed with non-extractable PRK

Reviewed-by: valeriep

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

ea49537: 8310340: assert(_thread->is_interp_only_mode() || stub_caller) failed: expected a stub-caller
2e00816: 8346671: java/nio/file/Files/probeContentType/Basic.java fails on Windows 2025
88fa3b2: 8346998: Test nsk/jvmti/ResourceExhausted/resexhausted003 fails with java.lang.OutOfMemoryError when CDS is off
6ee2bd2: 8347147: [REDO] AccessFlags can be u2 in metadata
a641932: 8346310: Duplicate !HAS_PENDING_EXCEPTION check in DynamicArchive::dump_at_exit
49ee4df: 8166983: Remove old/legacy unused tzdata files
3fe0818: 8346099: JFR: Query for 'jfr view' can't handle wildcard with multiple event types
55bcf4c: 8346047: JFR: Incorrect percentile value in 'jfr view'
ae3fc46: 8345580: Remove const from Node::_idx which is modified
4d18e5a: 8346872: tools/jpackage/windows/WinLongPathTest.java fails
... and 45 more: https://git.openjdk.org/jdk/compare/07c9f7138affdf0d42ecdc30adcb854515569985...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.

openjdk · 2025-01-08T18:45:36Z

@wangweij The following label will be automatically applied to this pull request:

security

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

mlbridge · 2025-01-08T18:49:22Z

Webrevs

00: Full (a5af08a7)

driverkt · 2025-01-08T19:21:29Z

src/java.base/share/classes/com/sun/crypto/provider/HKDFKeyDerivation.java

@@ -180,8 +180,8 @@ protected byte[] engineDeriveData(AlgorithmParameterSpec derivationSpec)
        } else if (derivationSpec instanceof HKDFParameterSpec.Expand anExpand) {
            // set this value in the "if"
            if ((pseudoRandomKey = anExpand.prk().getEncoded()) == null) {
-                throw new AssertionError(
-                        "PRK is required for HKDFParameterSpec.Expand");
+                throw new InvalidAlgorithmParameterException(


My only question here is whether the Expand could be created without a PRK for any other reason besides it being non-extractable. If we think so (even if it's just user-error), then perhaps the wording of the message for the IAPE should be revised from the currently proposed text.

In the creation of the Expand object, we've already guaranteed that PRK the object must be non null. The only problem here is its encoding.

I don't want to explicitly mention the null encoding so the wording is a little vague. What do you suggest?

Ah, you're right. I wondered if it was just the encoding that was null in this case, rather than the object itself. I didn't double-check but just read through the delta.

I think your wording should suffice, upon further reflection. It would be overly verbose to say something like "Cannot retrieve PRK bytes ..." or even "Cannot retrieve encoded PRK bytes ...".

This is probably fine to leave as-is.

valeriepeng

Looks good to me.

wangweij · 2025-01-09T01:20:15Z

/integrate

openjdk · 2025-01-09T01:21:03Z

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

0a5ef82: 8347162: Update problemlist CR for vmTestbase/nsk/jdi/VMOutOfMemoryException
ea49537: 8310340: assert(_thread->is_interp_only_mode() || stub_caller) failed: expected a stub-caller
2e00816: 8346671: java/nio/file/Files/probeContentType/Basic.java fails on Windows 2025
88fa3b2: 8346998: Test nsk/jvmti/ResourceExhausted/resexhausted003 fails with java.lang.OutOfMemoryError when CDS is off
6ee2bd2: 8347147: [REDO] AccessFlags can be u2 in metadata
a641932: 8346310: Duplicate !HAS_PENDING_EXCEPTION check in DynamicArchive::dump_at_exit
49ee4df: 8166983: Remove old/legacy unused tzdata files
3fe0818: 8346099: JFR: Query for 'jfr view' can't handle wildcard with multiple event types
55bcf4c: 8346047: JFR: Incorrect percentile value in 'jfr view'
ae3fc46: 8345580: Remove const from Node::_idx which is modified
... and 46 more: https://git.openjdk.org/jdk/compare/07c9f7138affdf0d42ecdc30adcb854515569985...master

Your commit was automatically rebased without conflicts.

openjdk · 2025-01-09T01:21:14Z

@wangweij Pushed as commit db7fa6a.

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

wangweij · 2025-01-09T03:08:43Z

/backport :jdk24

openjdk · 2025-01-09T03:09:38Z

@wangweij the backport was successfully created on the branch backport-wangweij-db7fa6a2-jdk24 in my personal fork of openjdk/jdk. To create a pull request with this backport targeting openjdk/jdk:jdk24, just click the following link:

➡️ Create pull request

The title of the pull request is automatically filled in correctly and below you find a suggestion for the pull request body:

Hi all,

This pull request contains a backport of commit db7fa6a2 from the openjdk/jdk repository.

The commit being backported was authored by Weijun Wang on 9 Jan 2025 and was reviewed by Valerie Peng.

Thanks!

If you need to update the source branch of the pull then run the following commands in a local clone of your personal fork of openjdk/jdk:

$ git fetch https://github.com/openjdk-bots/jdk.git backport-wangweij-db7fa6a2-jdk24:backport-wangweij-db7fa6a2-jdk24
$ git checkout backport-wangweij-db7fa6a2-jdk24
# make changes
$ git add paths/to/changed/files
$ git commit --message 'Describe additional changes made'
$ git push https://github.com/openjdk-bots/jdk.git backport-wangweij-db7fa6a2-jdk24

⚠️ @wangweij You are not yet a collaborator in my fork openjdk-bots/jdk. An invite will be sent out and you need to accept it before you can proceed.

the fix

a5af08a

openjdk bot added the rfr label Jan 8, 2025

openjdk bot added the security label Jan 8, 2025

driverkt reviewed Jan 8, 2025

View reviewed changes

valeriepeng approved these changes Jan 8, 2025

View reviewed changes

openjdk bot added the ready label Jan 8, 2025

openjdk bot added the integrated label Jan 9, 2025

openjdk bot closed this Jan 9, 2025

openjdk bot removed ready rfr labels Jan 9, 2025

wangweij deleted the 8347289 branch January 10, 2025 03:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8347289: HKDF delayed provider selection failed with non-extractable PRK #22976

8347289: HKDF delayed provider selection failed with non-extractable PRK #22976

wangweij commented Jan 8, 2025 •

edited by openjdk bot

Loading

bridgekeeper bot commented Jan 8, 2025

openjdk bot commented Jan 8, 2025 •

edited

Loading

openjdk bot commented Jan 8, 2025

mlbridge bot commented Jan 8, 2025

driverkt Jan 8, 2025

wangweij Jan 8, 2025

wangweij Jan 8, 2025

driverkt Jan 8, 2025

valeriepeng left a comment

wangweij commented Jan 9, 2025

openjdk bot commented Jan 9, 2025

openjdk bot commented Jan 9, 2025

wangweij commented Jan 9, 2025

openjdk bot commented Jan 9, 2025

8347289: HKDF delayed provider selection failed with non-extractable PRK #22976

8347289: HKDF delayed provider selection failed with non-extractable PRK #22976

Conversation

wangweij commented Jan 8, 2025 • edited by openjdk bot Loading

Progress

Issue

Reviewers

Reviewing

bridgekeeper bot commented Jan 8, 2025

openjdk bot commented Jan 8, 2025 • edited Loading

openjdk bot commented Jan 8, 2025

mlbridge bot commented Jan 8, 2025

Webrevs

driverkt Jan 8, 2025

Choose a reason for hiding this comment

wangweij Jan 8, 2025

Choose a reason for hiding this comment

wangweij Jan 8, 2025

Choose a reason for hiding this comment

driverkt Jan 8, 2025

Choose a reason for hiding this comment

valeriepeng left a comment

Choose a reason for hiding this comment

wangweij commented Jan 9, 2025

openjdk bot commented Jan 9, 2025

openjdk bot commented Jan 9, 2025

wangweij commented Jan 9, 2025

openjdk bot commented Jan 9, 2025

wangweij commented Jan 8, 2025 •

edited by openjdk bot

Loading

openjdk bot commented Jan 8, 2025 •

edited

Loading