8302233: HSS/LMS: keytool and jarsigner changes #14254

wangweij · 2023-05-31T22:38:20Z

Code changes for HSS/LMS that's related to keytool and jarsigner:

No need to add -sigalg for both tools when HSS/LMS key is involved, it can only be HSS/LMS.
The digestAlgorithm field in a PKCS7 SignerInfo. It must be the same as the hash algorithm used by the HSS/LMS key. This needs to be enforced both at signing and verification.
HSS/LMS reuses .DSA as the signature block file extension inside a signed JAR file

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

Issues

JDK-8302233: HSS/LMS: keytool and jarsigner changes (Enhancement - P3)
JDK-8308730: HSS/LMS: keytool and jarsigner changes (CSR)

Reviewers

Sean Mullan (@seanjmullan - Reviewer) ⚠️ Review applies to fae4757a

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 14254

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

Using diff file

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

Webrev

Link to Webrev Comment

bridgekeeper · 2023-05-31T22:39:07Z

👋 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 · 2023-05-31T22:40:47Z

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

bridgekeeper · 2023-07-27T01:12:53Z

@wangweij This pull request has been inactive for more than 8 weeks and will be automatically closed if another 8 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

wangweij · 2023-09-07T16:53:09Z

Still alive.

bridgekeeper · 2023-11-02T21:26:07Z

@wangweij This pull request has been inactive for more than 8 weeks and will be automatically closed if another 8 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

wangweij · 2023-11-02T21:28:02Z

please

mlbridge · 2023-11-16T13:47:55Z

Webrevs

test/lib/jdk/test/lib/security/timestamp/TsaSigner.java

src/java.base/share/classes/sun/security/util/KeyUtil.java

src/java.base/share/classes/sun/security/pkcs/PKCS7.java

src/java.base/share/classes/sun/security/pkcs/SignerInfo.java

seanjmullan

Looks good - one minor comment. However, I still would like @ferakocz to review the KeyUtil.hashAlgFromHSS method.

src/java.base/share/classes/sun/security/util/SignatureFileVerifier.java

openjdk · 2023-11-27T18:50:44Z

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

8302233: HSS/LMS: keytool and jarsigner changes

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

82967f4: 8310159: Bulk copy with Unsafe::arrayCopy is slower compared to memcpy
f0a12c5: 8320763: Fix spacing arround assignment in spec.gmk.in
12e983a: 8194743: Compiler implementation for Statements before super()
5e24aaf: 8320001: javac crashes while adding type annotations to the return type of a constructor
f9e9131: 8319703: Serial: Remove generationSpec
a006d7e: 8294549: configure script should detect unsupported path
4977922: 8320330: Improve implementation of RShift Value
a40d8d9: 8314220: Configurable InlineCacheBuffer size
1272368: 8318113: CSS.BackgroundImage doesn't implement equals
28d3762: 8320618: NPE: Cannot invoke "java.lang.constant.ClassDesc.isArray()" because "this.sym" is null
... and 107 more: https://git.openjdk.org/jdk/compare/b05e69f789fa8c9a5320be5a841317abd3b3a235...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.

ferakocz · 2023-11-30T16:13:09Z

src/java.base/share/classes/sun/security/pkcs/SignerInfo.java

                }
+                break;
+            case "HSS/LMS":
+                // RFC 8708 requires the same hash algorithm used as in the HSS/LMS algorithm


Actually it is RFC 8554 that requires it, RFC 8708 just references RFC 8554

What I meant here is that, RFC 8708 requires that the hash algorithm used by the HSS/LMS signature (which should be a single one used in every corner of HSS/LMS, as required by RFC 8554) should be the same as the digestAlgorithm in the CMS SignerInfo object. See RFC 8708 Section 5:

digestAlgorithm MUST contain the one-way hash function used in the HSS/LMS tree.

https://www.rfc-editor.org/rfc/rfc8708.html#name-signed-data-conventions

wangweij · 2023-11-30T19:13:15Z

/integrate

openjdk · 2023-11-30T19:14:31Z

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

7ad7005: 8315034: File.mkdirs() occasionally fails to create folders on Windows shared folder
41daa3b: 8320239: add dynamic switch for JvmtiVTMSTransitionDisabler sync protocol
7c135c3: 8321066: Multiple JFR tests have started failing
8bedb28: 8321119: Disable java/foreign/TestHandshake.java on Zero VMs
b1cbf55: 8321018: Parallel: Make some methods in ParCompactionManager private
c6a8278: 8321127: ProblemList java/util/stream/GatherersTest.java
a3eb664: 8315701: [macos] Regression: KeyEvent has different keycode on different keyboard layouts
6aba6aa: 8320347: Emulate vblendvp[sd] on ECore
6938474: 8320916: jdk/jfr/event/gc/stacktrace/TestParallelMarkSweepAllocationPendingStackTrace.java failed with "OutOfMemoryError: GC overhead limit exceeded"
da09eab: 8319980: [JVMCI] libgraal should reuse Thread instances as C2 does
... and 186 more: https://git.openjdk.org/jdk/compare/b05e69f789fa8c9a5320be5a841317abd3b3a235...master

Your commit was automatically rebased without conflicts.

openjdk · 2023-11-30T19:14:37Z

@wangweij Pushed as commit 0a60b0f.

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

the fix

438f254

openjdk bot added the csr Pull request needs approved CSR before integration label May 31, 2023

openjdk bot added the security security-dev@openjdk.org label May 31, 2023

wangweij added 2 commits June 1, 2023 18:16

always DSA for new algorithms

7ad4e9f

test change

2864f5f

openjdk bot removed the csr Pull request needs approved CSR before integration label Nov 15, 2023

wangweij marked this pull request as ready for review November 16, 2023 13:41

openjdk bot added the rfr Pull request is ready for review label Nov 16, 2023

seanjmullan reviewed Nov 16, 2023

View reviewed changes

test/lib/jdk/test/lib/security/timestamp/TsaSigner.java Show resolved Hide resolved

src/java.base/share/classes/sun/security/util/KeyUtil.java Show resolved Hide resolved

seanjmullan reviewed Nov 16, 2023

View reviewed changes

src/java.base/share/classes/sun/security/pkcs/PKCS7.java Show resolved Hide resolved

src/java.base/share/classes/sun/security/pkcs/SignerInfo.java Outdated Show resolved Hide resolved

wangweij added 5 commits November 16, 2023 11:39

Merge branch 'master' into 8302233

c667ecd

small change on copyright year and method modifier

e3b1a7c

no need to say new

ff15014

explain hashAlgFromHSS

04a7a0b

no need to talk about the limitation here

fae4757

seanjmullan approved these changes Nov 27, 2023

View reviewed changes

src/java.base/share/classes/sun/security/util/SignatureFileVerifier.java Outdated Show resolved Hide resolved

openjdk bot added the ready Pull request is ready to be integrated label Nov 27, 2023

reword comment

fd5473b

ferakocz reviewed Nov 30, 2023

View reviewed changes

openjdk bot added the integrated Pull request has been integrated label Nov 30, 2023

openjdk bot closed this Nov 30, 2023

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Nov 30, 2023

wangweij deleted the 8302233 branch February 1, 2024 22:41

8302233: HSS/LMS: keytool and jarsigner changes #14254

8302233: HSS/LMS: keytool and jarsigner changes #14254

Uh oh!

Conversation

wangweij commented May 31, 2023 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issues

Reviewers

Reviewing

Webrev

Uh oh!

bridgekeeper bot commented May 31, 2023

Uh oh!

openjdk bot commented May 31, 2023

Uh oh!

bridgekeeper bot commented Jul 27, 2023

Uh oh!

wangweij commented Sep 7, 2023

Uh oh!

bridgekeeper bot commented Nov 2, 2023

Uh oh!

wangweij commented Nov 2, 2023

Uh oh!

mlbridge bot commented Nov 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

seanjmullan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

openjdk bot commented Nov 27, 2023

Uh oh!

ferakocz Nov 30, 2023

Choose a reason for hiding this comment

Uh oh!

wangweij Nov 30, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wangweij commented Nov 30, 2023

Uh oh!

openjdk bot commented Nov 30, 2023

Uh oh!

openjdk bot commented Nov 30, 2023

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

wangweij commented May 31, 2023 •

edited by openjdk bot

Loading

mlbridge bot commented Nov 16, 2023 •

edited

Loading

wangweij Nov 30, 2023 •

edited

Loading