8347596: Update HSS/LMS public key encoding #23083

wangweij · 2025-01-13T17:42:12Z

Update the encoding of HSS/LMS public key to be consistent with https://www.rfc-editor.org/rfc/rfc9708.html#name-changes-since-rfc-8708 and https://datatracker.ietf.org/doc/html/draft-ietf-lamps-x509-shbs-13#name-hss-public-keys.

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-8347596: Update HSS/LMS public key encoding (Bug - P3)

Reviewers

Sean Mullan (@seanjmullan - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 23083

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

Using diff file

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

Using Webrev

Link to Webrev Comment

bridgekeeper · 2025-01-13T17:43:18Z

👋 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-13T17:43:26Z

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

8347596: Update HSS/LMS public key encoding

Reviewed-by: mullan

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

9b1bed0: 8290043: serviceability/attach/ConcAttachTest.java failed "guarantee(!CheckJNICalls) failed: Attached JNI thread exited without being detached"
2de71d0: 8347129: cpuset cgroups controller is required for no good reason
4c30933: 8346971: [ubsan] psCardTable.cpp:131:24: runtime error: large index is out of bounds
06ff4c1: 8347146: Convert IncludeLocalesPluginTest to use JUnit
db76f47: 8347720: [BACKOUT] Portable implementation of FORBID_C_FUNCTION and ALLOW_C_FUNCTION
e6902cf: 8323740: java.lang.ExceptionInInitializerError when trying to load XML classes in wrong order
a01e92c: 8347724: Replace SIZE_FORMAT in jfr directory
d002933: 8347286: (fs) Remove some extensions from java/nio/file/Files/probeContentType/Basic.java
d532019: 8347143: [aix] Fix strdup use in os::dll_load
dfd215b: 8347376: tools/jlink/runtimeImage/JavaSEReproducibleTest.java and PackagedModulesVsRuntimeImageLinkTest.java failed after JDK-8321413
... and 28 more: https://git.openjdk.org/jdk/compare/fa5ff82eb3f0f2df74acd117509bac6e3c634a3f...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-13T17:44:00Z

@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-13T17:47:16Z

Webrevs

mcpowers

Does this interoperate with BC?

mcpowers · 2025-01-13T22:34:44Z

src/java.base/share/classes/sun/security/provider/HSS.java

@@ -783,11 +779,18 @@ public String toString() {
        @Override
        protected void parseKeyBits() throws InvalidKeyException {
            byte[] keyArray = getKey().toByteArray();
-            if ((keyArray[0] != DerValue.tag_OctetString) || (keyArray[1] != keyArray.length -2)) {
-                throw new InvalidKeyException("Bad X509Key");
+            // Check less than minimum length to make sure this method works as expected


s/Check less/Check if less/

or just delete the comment since it adds no useful information

OK. My intent was to say this check is not final (because minimal length of an HSS/LMS public key is more than 12) but it is still necessary to prevent OOIBE in this method. I can see it is not very useful.

mcpowers · 2025-01-13T22:45:41Z

src/java.base/share/classes/sun/security/provider/HSS.java

+            }
+            if (keyArray[0] == DerValue.tag_OctetString
+                    && keyArray[1] == keyArray.length - 2) {
+                // pre-8347596 format that has an inner OCTET STRING.


I think I would delete the "pre-8347596 format" part of the comment.

Why? Curious people can further check out what happened from here.

Maybe it's a Solaris thing where bug IDs in the code was frowned upon.

wangweij · 2025-01-13T23:46:16Z

Does this interoperate with BC?

It will, once BC 1.80 is out.

wangweij · 2025-01-14T23:06:23Z

/integrate

openjdk · 2025-01-14T23:06:46Z

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

ec2aaaa: 8326236: assert(ce != nullptr) failed in Continuation::continuation_bottom_sender
02d2493: 8347613: Remove leftover doPrivileged call in Currency test: CheckDataVersion.java
10d08db: 8346142: [perf] scalability issue for the specjvm2008::xml.validation workload
9b1bed0: 8290043: serviceability/attach/ConcAttachTest.java failed "guarantee(!CheckJNICalls) failed: Attached JNI thread exited without being detached"
2de71d0: 8347129: cpuset cgroups controller is required for no good reason
4c30933: 8346971: [ubsan] psCardTable.cpp:131:24: runtime error: large index is out of bounds
06ff4c1: 8347146: Convert IncludeLocalesPluginTest to use JUnit
db76f47: 8347720: [BACKOUT] Portable implementation of FORBID_C_FUNCTION and ALLOW_C_FUNCTION
e6902cf: 8323740: java.lang.ExceptionInInitializerError when trying to load XML classes in wrong order
a01e92c: 8347724: Replace SIZE_FORMAT in jfr directory
... and 31 more: https://git.openjdk.org/jdk/compare/fa5ff82eb3f0f2df74acd117509bac6e3c634a3f...master

Your commit was automatically rebased without conflicts.

openjdk · 2025-01-14T23:06:55Z

@wangweij Pushed as commit 0ee6ba9.

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

wangweij · 2025-01-14T23:08:43Z

/backport :jdk24

openjdk · 2025-01-14T23:10:15Z

@wangweij the backport was successfully created on the branch backport-wangweij-0ee6ba9c-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 0ee6ba9c from the openjdk/jdk repository.

The commit being backported was authored by Weijun Wang on 14 Jan 2025 and was reviewed by Sean Mullan.

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-0ee6ba9c-jdk24:backport-wangweij-0ee6ba9c-jdk24
$ git checkout backport-wangweij-0ee6ba9c-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-0ee6ba9c-jdk24

the fix

cd93417

openjdk bot added the rfr Pull request is ready for review label Jan 13, 2025

openjdk bot added the security security-dev@openjdk.org label Jan 13, 2025

remove the wrapping in a helper method

5d23683

mcpowers reviewed Jan 13, 2025

View reviewed changes

simpler comment

c8c1ec3

seanjmullan approved these changes Jan 14, 2025

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Jan 14, 2025

openjdk bot added the integrated Pull request has been integrated label Jan 14, 2025

openjdk bot closed this Jan 14, 2025

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Jan 14, 2025

wangweij deleted the 8347596 branch January 15, 2025 01:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8347596: Update HSS/LMS public key encoding #23083

8347596: Update HSS/LMS public key encoding #23083

wangweij commented Jan 13, 2025 •

edited by openjdk bot

Loading

bridgekeeper bot commented Jan 13, 2025

openjdk bot commented Jan 13, 2025 •

edited

Loading

openjdk bot commented Jan 13, 2025

mlbridge bot commented Jan 13, 2025 •

edited

Loading

mcpowers left a comment

mcpowers Jan 13, 2025

wangweij Jan 13, 2025

mcpowers Jan 13, 2025

wangweij Jan 13, 2025

mcpowers Jan 13, 2025

wangweij commented Jan 13, 2025

wangweij commented Jan 14, 2025

openjdk bot commented Jan 14, 2025

openjdk bot commented Jan 14, 2025

wangweij commented Jan 14, 2025

openjdk bot commented Jan 14, 2025

8347596: Update HSS/LMS public key encoding #23083

8347596: Update HSS/LMS public key encoding #23083

Conversation

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

Progress

Issue

Reviewers

Reviewing

bridgekeeper bot commented Jan 13, 2025

openjdk bot commented Jan 13, 2025 • edited Loading

openjdk bot commented Jan 13, 2025

mlbridge bot commented Jan 13, 2025 • edited Loading

Webrevs

mcpowers left a comment

Choose a reason for hiding this comment

mcpowers Jan 13, 2025

Choose a reason for hiding this comment

wangweij Jan 13, 2025

Choose a reason for hiding this comment

mcpowers Jan 13, 2025

Choose a reason for hiding this comment

wangweij Jan 13, 2025

Choose a reason for hiding this comment

mcpowers Jan 13, 2025

Choose a reason for hiding this comment

wangweij commented Jan 13, 2025

wangweij commented Jan 14, 2025

openjdk bot commented Jan 14, 2025

openjdk bot commented Jan 14, 2025

wangweij commented Jan 14, 2025

openjdk bot commented Jan 14, 2025

wangweij commented Jan 13, 2025 •

edited by openjdk bot

Loading

openjdk bot commented Jan 13, 2025 •

edited

Loading

mlbridge bot commented Jan 13, 2025 •

edited

Loading