8297878: KEM: Implementation #13256

wangweij · 2023-03-31T02:25:04Z

The KEM API and DHKEM impl. Note that this PR uses new methods in #13250.

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Change requires CSR request JDK-8305384 to be approved
Commit message must refer to an issue

Issues

JDK-8297878: KEM: Implementation
JDK-8305384: KEM: Implementation (CSR)

Reviewers

Anthony Scarpino (@ascarpino - Reviewer) ⚠️ Review applies to 7cace182
Sean Mullan (@seanjmullan - Reviewer) ⚠️ Review applies to b3982bdb

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 13256

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

Using diff file

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

Webrev

Link to Webrev Comment

bridgekeeper · 2023-03-31T02:26:36Z

👋 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-03-31T02:28: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 · 2023-03-31T02:32:18Z

Webrevs

src/java.base/share/classes/javax/crypto/DecapsulateException.java

src/java.base/share/classes/javax/crypto/KEMSpi.java

src/java.base/share/classes/javax/crypto/KEM.java

seanjmullan · 2023-04-13T18:21:51Z

src/java.base/share/classes/javax/crypto/KEM.java

+     *
+     * @param pk the receiver's public key, must not be {@code null}
+     * @param secureRandom the source of randomness for encapsulation,
+     *                     If {@code null}, the implementation should provide


s/should/must/

This spec is not written for the implementor. How about "If {@code} null, a default one from the implementation will be used.".

src/java.base/share/classes/javax/crypto/KEM.java

mlbridge · 2023-04-13T21:41:15Z

Mailing list message from Stephen Farrell on security-dev:

Hi,

Apologies for the interruption from the sidelines but I
have a query if that's ok.

Is there any relationship between this work and RFC1980
which defines HPKE, being a way of encrypting to a public
value using a KEM?

Reason to ask is HPKE is a mechanism that'll be needed for
TLS Encrypted Client Hello and the MLS protocol, so it'd
be a fine thing if these additions were suitable for that
too.

Cheers,
S.

PS: I implemented HPKE for OpenSSL so if there's interest
in supporting that here too, I'd be happy to help a bit.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_0xE4D8E9F997A833DD.asc
Type: application/pgp-keys
Size: 1197 bytes
Desc: OpenPGP public key
URL: <https://mail.openjdk.org/pipermail/security-dev/attachments/20230413/a1b0f044/OpenPGP_0xE4D8E9F997A833DD-0001.asc>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 236 bytes
Desc: OpenPGP digital signature
URL: <https://mail.openjdk.org/pipermail/security-dev/attachments/20230413/a1b0f044/OpenPGP_signature-0001.sig>

src/java.base/share/classes/javax/crypto/KEMSpi.java

src/java.base/share/classes/javax/crypto/KEM.java

wangweij · 2023-04-13T22:32:17Z

Mailing list message from Stephen Farrell on security-dev:

Hi,

Apologies for the interruption from the sidelines but I have a query if that's ok.

Is there any relationship between this work and RFC1980 which defines HPKE, being a way of encrypting to a public value using a KEM?

We know about HPKE, and it can makes use of the DHKEM implementation here (if the AuthEncap/AuthDecap functions are not used). However, we (Oracle's Java SE Security Team) don't have a plan to include HPKE inside OpenJDK yet.

This PR is mainly about adding the KEM SPI so 3rd security providers can implement other KEM algorithms. DHKEM is included mainly to prove that the API is usable.

mlbridge · 2023-04-14T08:18:16Z

Mailing list message from Stephen Farrell on security-dev:

Hiya,

On 13/04/2023 23:35, Weijun Wang wrote:

Apologies for the interruption from the sidelines but I have a
query if that's ok.

Is there any relationship between this work and RFC1980 which
defines HPKE, being a way of encrypting to a public value using a
KEM?

We know about HPKE,

Of course:-)

and it can makes use of the DHKEM implementation
here (if the AuthEncap/AuthDecap functions are not used).

FWIW, I'm not aware of any protocol yet attempting to make
use of the authenticated HPKE modes, so that seems very
reasonable. (OTOH, it's not that hard for a library to
support all modes, so it may be worth some consideration.)

However, we
(Oracle's Java SE Security Team) don't have a plan to include HPKE
inside OpenJDK yet.

Entirely fair. If doing so is of interest (to you or others),
I'd be happy to try help. (Ping me on/off-list if that is of
interest.)

This PR is mainly about adding the KEM SPI so 3rd security providers
can implement other KEM algorithms. DHKEM is included mainly to prove
that the API is usable.

Grand. I'll get out of the way of this thread so:-) But
again, if interested, do reach out, as I'm keen to see ECH
support ending up widespread and HPKE is a fine precursor
for that.

Cheers,
S.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_0xE4D8E9F997A833DD.asc
Type: application/pgp-keys
Size: 1197 bytes
Desc: OpenPGP public key
URL: <https://mail.openjdk.org/pipermail/security-dev/attachments/20230414/418fe369/OpenPGP_0xE4D8E9F997A833DD-0001.asc>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 236 bytes
Desc: OpenPGP digital signature
URL: <https://mail.openjdk.org/pipermail/security-dev/attachments/20230414/418fe369/OpenPGP_signature-0001.sig>

src/java.base/share/classes/javax/crypto/KEM.java

wangweij · 2023-04-14T14:13:17Z

Grand. I'll get out of the way of this thread so:-) But
again, if interested, do reach out, as I'm keen to see ECH
support ending up widespread and HPKE is a fine precursor
for that.

Thanks a lot. I'll remember that.

src/java.base/share/classes/javax/crypto/KEM.java

src/java.base/share/classes/javax/crypto/KEMSpi.java

mlbridge · 2023-04-15T03:59:17Z

Mailing list message from Kevin Driver on security-dev:

Some interesting side discussion here. I wanted to chime in to point out that HPKE is built upon the ?primitives? of KEM and HKDF. As mentioned on the list, KEM is underway. I am also spearheading our effort reviving the HKDF JEP<https://bugs.openjdk.org/browse/JDK-8189808> which has gone a bit stale.

HPKE is certainly something we?re looking into as well. Once the building blocks of KEM and HKDF are in place, HPKE will ramp up next.

Kevin Driver
Mobile: +1.512.431.5690
Java Security Libraries

Subject: Re: RFR: 8297878: KEM: Implementation
Date: Thu, 13 Apr 2023 21:31:43 +0100
From: Stephen Farrell <stephen.farrell at cs.tcd.ie>
To: Xue-Lei Andrew Fan <xuelei at openjdk.org>, security-dev at openjdk.org

Hi,

Apologies for the interruption from the sidelines but I
have a query if that's ok.

Is there any relationship between this work and RFC1980
which defines HPKE, being a way of encrypting to a public
value using a KEM?

Reason to ask is HPKE is a mechanism that'll be needed for
TLS Encrypted Client Hello and the MLS protocol, so it'd
be a fine thing if these additions were suitable for that
too.

Cheers,
S.

PS: I implemented HPKE for OpenSSL so if there's interest
in supporting that here too, I'd be happy to help a bit.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/security-dev/attachments/20230414/0efa22f3/attachment-0001.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_0xE4D8E9F997A833DD.asc
Type: application/pgp-keys
Size: 1221 bytes
Desc: OpenPGP_0xE4D8E9F997A833DD.asc
URL: <https://mail.openjdk.org/pipermail/security-dev/attachments/20230414/0efa22f3/OpenPGP_0xE4D8E9F997A833DD-0001.asc>

wangweij · 2023-04-18T16:17:33Z

New commit pushed. The "the caller has validated" sentences are removed since that will be an implementation detail. Now each provider needs to validate the arguments themselves.

Also, when a null key is provided to newEncapsulator and newDecapsulator, they throw an InvalidKeyException instead of a NullPointerException now.

ascarpino · 2023-04-28T17:19:15Z

My feeling is that modern APIs use from and to instead of from and length. For example, Arrays.copyOfRange, Arrays.equals and Arrays.compare vs new String(daata, from, length).

I don't see this as a modern vs old API style, it's consistency. Arrays.copyOfRange() is from 1.6. I think it's being consistent with other JCA classes, like Cipher, Signature, MessageDigest, and Mac. I'm not aware of any JCA that uses (from, to).

Also Arrays methods use length of any size. KEM does not. In fact it is very obvious what the length is as it must be the key size when looking at DHKEM.engineDecapsulate(...):

Objects.checkFromToIndex(from, to, params.Nsecret);

One could argue that to isn't necessary given the length is fixed by the params.

wangweij · 2023-04-28T21:50:57Z

Maybe 1.6 is already modern enough. ;-)

to is useful if secretSize is 48 but you want an AES-256 key, then you would call encapsulate(0, 32, "AES") or encapsulate(16, 48, "AES"), depending on how your protocol demands.

wangweij · 2023-05-02T17:53:07Z

New commit pushed. s/provider/providerName/. Now we can claim the Encapsulator and Decapsulator classes are immutable.

test/jdk/com/sun/crypto/provider/DHKEM/Compliance.java

src/java.base/share/classes/javax/crypto/KEM.java

ascarpino

I'm fine with the changes

openjdk · 2023-05-23T01:56:01Z

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

8297878: KEM: Implementation

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

3eced01: 8304074: [JMX] Add an approximation of total bytes allocated on the Java heap by the JVM
15e0285: 8309110: Build failure after JDK-8307795 due to warnings in micro-benchmark StoreMaskTrueCount.java
4526282: 8308977: gtest:codestrings fails on riscv
f600d03: 8307795: AArch64: Optimize VectorMask.truecount() on Neon
07f2070: 8309095: Remove UTF-8 character from TaskbarPositionTest.java
2b186e2: 8309042: MemorySegment::reinterpret cleanup action is not called for all overloads
78aac24: 8308881: Strong CLD oop handle roots are demoted to non-roots concurrently
1f1f604: 8302670: use-after-free related to PhaseIterGVN interaction with Unique_Node_List and Node_Stack
d35a550: 8309077: Problemlist compiler/jvmci/TestUncaughtErrorInCompileMethod.java
457e1cb: 8308817: RISC-V: Support VectorTest node for Vector API
... and 151 more: https://git.openjdk.org/jdk/compare/bb24c36759f19448c8539f6b11017753f304bb56...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.

test/jdk/com/sun/crypto/provider/DHKEM/Compliance.java

test/jdk/javax/crypto/KEM/RSA_KEM.java

wangweij · 2023-05-25T20:46:12Z

New commit. Resolve Sean's comments. Make SecureRandom settable.

wangweij · 2023-05-30T16:28:16Z

/integrate

openjdk · 2023-05-30T16:29:31Z

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

21af8ba: 8290499: new File(parent, "/") breaks normalization – creates File with slash at the end
804f198: 8308992: New test TestHFA fails with zero
fb0b1f0: 8051725: Improve expansion of Conv2B nodes in the middle-end
3eced01: 8304074: [JMX] Add an approximation of total bytes allocated on the Java heap by the JVM
15e0285: 8309110: Build failure after JDK-8307795 due to warnings in micro-benchmark StoreMaskTrueCount.java
4526282: 8308977: gtest:codestrings fails on riscv
f600d03: 8307795: AArch64: Optimize VectorMask.truecount() on Neon
07f2070: 8309095: Remove UTF-8 character from TaskbarPositionTest.java
2b186e2: 8309042: MemorySegment::reinterpret cleanup action is not called for all overloads
78aac24: 8308881: Strong CLD oop handle roots are demoted to non-roots concurrently
... and 154 more: https://git.openjdk.org/jdk/compare/bb24c36759f19448c8539f6b11017753f304bb56...master

Your commit was automatically rebased without conflicts.

openjdk · 2023-05-30T16:29:42Z

@wangweij Pushed as commit 6b90b05.

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

the fix

e953d9e

openjdk bot added the rfr Pull request is ready for review label Mar 31, 2023

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

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

wangweij changed the title ~~8297878: Implementing Key Encapsulation Mechanism API~~ 8297878: KEM: Implementation Apr 11, 2023

seanjmullan reviewed Apr 12, 2023

View reviewed changes

src/java.base/share/classes/javax/crypto/DecapsulateException.java Outdated Show resolved Hide resolved

XueleiFan reviewed Apr 12, 2023

View reviewed changes

src/java.base/share/classes/javax/crypto/KEMSpi.java Show resolved Hide resolved

seanjmullan reviewed Apr 12, 2023

View reviewed changes

seanjmullan reviewed Apr 13, 2023

View reviewed changes

src/java.base/share/classes/javax/crypto/KEM.java Outdated Show resolved Hide resolved

src/java.base/share/classes/javax/crypto/KEM.java Outdated Show resolved Hide resolved

src/java.base/share/classes/javax/crypto/KEM.java Outdated Show resolved Hide resolved

seanjmullan reviewed Apr 13, 2023

View reviewed changes

src/java.base/share/classes/javax/crypto/KEM.java Show resolved Hide resolved

seanjmullan reviewed Apr 13, 2023

View reviewed changes

XueleiFan suggested changes Apr 13, 2023

View reviewed changes

src/java.base/share/classes/javax/crypto/KEMSpi.java Outdated Show resolved Hide resolved

src/java.base/share/classes/javax/crypto/KEMSpi.java Outdated Show resolved Hide resolved

src/java.base/share/classes/javax/crypto/KEMSpi.java Outdated Show resolved Hide resolved

seanjmullan reviewed Apr 13, 2023

View reviewed changes

spec change, getAlgorithm

8a227da

seanjmullan reviewed Apr 14, 2023

View reviewed changes

spec changes

b398b03

seanjmullan reviewed Apr 14, 2023

View reviewed changes

let implementor validate arguments, null key throws IKE

6455a52

fine tune spec

39c3a20

providerName

c2daf4e

sisahoo reviewed May 3, 2023

View reviewed changes

test/jdk/com/sun/crypto/provider/DHKEM/Compliance.java Outdated Show resolved Hide resolved

test/jdk/com/sun/crypto/provider/DHKEM/Compliance.java Outdated Show resolved Hide resolved

wangweij added 2 commits May 3, 2023 08:44

Resolve Siba's comment

c9f1d8f

deterministic randomness

128cefe

ascarpino reviewed May 15, 2023

View reviewed changes

src/java.base/share/classes/javax/crypto/KEM.java Show resolved Hide resolved

to and length and comments

ea8f796

ferakocz reviewed May 17, 2023

View reviewed changes

src/java.base/share/classes/javax/crypto/KEM.java Show resolved Hide resolved

src/java.base/share/classes/javax/crypto/KEM.java Show resolved Hide resolved

wangweij added 2 commits May 17, 2023 14:23

Merge branch 'master' into 8297878

655d252

Merge branch 'master' into 8297878

7cace18

ascarpino approved these changes May 22, 2023

View reviewed changes

openjdk bot added ready Pull request is ready to be integrated and removed csr Pull request needs approved CSR before integration labels May 23, 2023

seanjmullan reviewed May 25, 2023

View reviewed changes

test/jdk/com/sun/crypto/provider/DHKEM/Compliance.java Outdated Show resolved Hide resolved

test/jdk/javax/crypto/KEM/RSA_KEM.java Outdated Show resolved Hide resolved

test update

b3982bd

seanjmullan approved these changes May 25, 2023

View reviewed changes

wangweij added 2 commits May 25, 2023 19:15

comment for RSA-KEM

2c7ef3d

New and old SunJCE entries in class comment

bdd0c63

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

openjdk bot closed this May 30, 2023

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

wangweij deleted the 8297878 branch May 30, 2023 16:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8297878: KEM: Implementation #13256

8297878: KEM: Implementation #13256

wangweij commented Mar 31, 2023 •

edited by openjdk bot

Loading

bridgekeeper bot commented Mar 31, 2023

openjdk bot commented Mar 31, 2023

mlbridge bot commented Mar 31, 2023 •

edited

Loading

seanjmullan Apr 13, 2023

wangweij Apr 13, 2023

mlbridge bot commented Apr 13, 2023

wangweij commented Apr 13, 2023

mlbridge bot commented Apr 14, 2023

wangweij commented Apr 14, 2023

mlbridge bot commented Apr 15, 2023

wangweij commented Apr 18, 2023

ascarpino commented Apr 28, 2023 •

edited

Loading

wangweij commented Apr 28, 2023

wangweij commented May 2, 2023 •

edited

Loading

ascarpino left a comment

openjdk bot commented May 23, 2023 •

edited

Loading

wangweij commented May 25, 2023

wangweij commented May 30, 2023

openjdk bot commented May 30, 2023

openjdk bot commented May 30, 2023

8297878: KEM: Implementation #13256

8297878: KEM: Implementation #13256

Conversation

wangweij commented Mar 31, 2023 • edited by openjdk bot Loading

Progress

Issues

Reviewers

Reviewing

Webrev

bridgekeeper bot commented Mar 31, 2023

openjdk bot commented Mar 31, 2023

mlbridge bot commented Mar 31, 2023 • edited Loading

Webrevs

seanjmullan Apr 13, 2023

Choose a reason for hiding this comment

wangweij Apr 13, 2023

Choose a reason for hiding this comment

mlbridge bot commented Apr 13, 2023

wangweij commented Apr 13, 2023

mlbridge bot commented Apr 14, 2023

wangweij commented Apr 14, 2023

mlbridge bot commented Apr 15, 2023

wangweij commented Apr 18, 2023

ascarpino commented Apr 28, 2023 • edited Loading

wangweij commented Apr 28, 2023

wangweij commented May 2, 2023 • edited Loading

ascarpino left a comment

Choose a reason for hiding this comment

openjdk bot commented May 23, 2023 • edited Loading

wangweij commented May 25, 2023

wangweij commented May 30, 2023

openjdk bot commented May 30, 2023

openjdk bot commented May 30, 2023

wangweij commented Mar 31, 2023 •

edited by openjdk bot

Loading

mlbridge bot commented Mar 31, 2023 •

edited

Loading

ascarpino commented Apr 28, 2023 •

edited

Loading

wangweij commented May 2, 2023 •

edited

Loading

openjdk bot commented May 23, 2023 •

edited

Loading