On Fri, 22 Aug 2025 19:33:15 GMT, Weijun Wang <wei...@openjdk.org> wrote:
>> Implement HPKE as defined in https://datatracker.ietf.org/doc/rfc9180/. >> >> <img >> src="https://github.com/user-attachments/assets/45625334-903b-4a3d-a987-7fddeab9a604"; >> /> > > Weijun Wang has updated the pull request incrementally with one additional > commit since the last revision: > > 8325448: Hybrid Public Key Encryption src/java.base/share/classes/javax/crypto/spec/HPKEParameterSpec.java line 40: > 38: * This immutable class specifies the set of parameters used with a > {@code Cipher} for the > 39: * <a href="https://www.rfc-editor.org/info/rfc9180";>Hybrid Public Key > Encryption</a> > 40: * (HPKE) algorithm. The <a href= I think would be useful to add one or two (but no more) sentences describing HPKE and what it should be used for. src/java.base/share/classes/javax/crypto/spec/HPKEParameterSpec.java line 52: > 50: * {@linkplain Cipher#init(int, Key, AlgorithmParameterSpec) cipher > initialization}. > 51: * <p> > 52: * {@link #of(int, int, int)} creates an {@code HPKEParameterSpec} > instance with Suggest "The {@link #of(int, int, int)} static method ..." to avoid starting a sentence with a lower-case letter. src/java.base/share/classes/javax/crypto/spec/HPKEParameterSpec.java line 53: > 51: * <p> > 52: * {@link #of(int, int, int)} creates an {@code HPKEParameterSpec} > instance with > 53: * specified KEM, KDF, and AEAD algorithm identifiers. s/specified/the specified/ src/java.base/share/classes/javax/crypto/spec/HPKEParameterSpec.java line 69: > 67: * {@link #info(byte[])} method by both sides. > 68: * <li> > 69: * To support authentication using a pre-shared key ({@code mode_psk}), > the "To support" sounds like something more needs to be done to implement it. What about just saying "To authenticate using ..." Same comment on lines below. src/java.base/share/classes/javax/crypto/spec/HPKEParameterSpec.java line 73: > 71: * {@link #psk(SecretKey, byte[])} method by both sides. > 72: * <li> > 73: * To support authentication using an asymmetric Key ({@code mode_auth}), s/Key/key/ src/java.base/share/classes/javax/crypto/spec/HPKEParameterSpec.java line 98: > 96: * <p> > 97: * If an HPKE cipher is initialized without parameters, an > 98: * {@code InvalidKeyException} will be thrown. Suggest clarifying which method: "... thrown by the init method." src/java.base/share/classes/javax/crypto/spec/HPKEParameterSpec.java line 101: > 99: * <p> > 100: * At HPKE cipher initialization, if no HPKE implementation supports the > provided > 101: * key type, an {@code InvalidKeyException} should be thrown. If the > provided Suggest saying "is thrown" instead of "should be" which leaves some question as to whether it is acceptable for some other behavior. src/java.base/share/classes/javax/crypto/spec/HPKEParameterSpec.java line 102: > 100: * At HPKE cipher initialization, if no HPKE implementation supports the > provided > 101: * key type, an {@code InvalidKeyException} should be thrown. If the > provided > 102: * {@code HPKEParameterSpec} is not supported by any HPKE implementation, "is not supported" is typically used when an algorithm is not supported. I think you need different wording like "invalid parameters". src/java.base/share/classes/javax/crypto/spec/HPKEParameterSpec.java line 103: > 101: * key type, an {@code InvalidKeyException} should be thrown. If the > provided > 102: * {@code HPKEParameterSpec} is not supported by any HPKE implementation, > 103: * an {@code InvalidAlgorithmParameterException} will be thrown. For > example: s/will be/is/ src/java.base/share/classes/javax/crypto/spec/HPKEParameterSpec.java line 103: > 101: * key type, an {@code InvalidKeyException} should be thrown. If the > provided > 102: * {@code HPKEParameterSpec} is not supported by any HPKE implementation, > 103: * an {@code InvalidAlgorithmParameterException} will be thrown. For > example: Suggest being more specific than "For example:". i.e. - "The following are cases of invalid parameters:" src/java.base/share/classes/javax/crypto/spec/HPKEParameterSpec.java line 109: > 107: * <li> An attempt to use {@code authKey(key)} is made with an > incompatible key. > 108: * <li> An attempt to use {@code authKey(key)} is made but {@code > mode_auth} > 109: * or {@code mode_auth_psk}) is not supported by the KEM used. no matching left parenthesis src/java.base/share/classes/javax/crypto/spec/HPKEParameterSpec.java line 119: > 117: * of RFC 9180. > 118: * <p> > 119: * As with any AEAD cipher, each {@code doFinal} call on the server side > must Hmm, I see a bit of a disconnect here since it starts talking about AEAD cipher, but this is an HPKE cipher. Suggest you need a lead-in sentence with what part/step of the HPKE you are referring to. Also, instead of "server" probably better to say "receiver". src/java.base/share/classes/javax/crypto/spec/HPKEParameterSpec.java line 121: > 119: * As with any AEAD cipher, each {@code doFinal} call on the server side > must > 120: * correspond to exactly one complete ciphertext, and the number and > order of > 121: * calls must match on both sides. Unlike raw AEAD usage, however, an > HPKE I think you need to define "raw". src/java.base/share/classes/javax/crypto/spec/HPKEParameterSpec.java line 131: > 129: * {@snippet lang=java class="PackageSnippets" > region="hpke-spec-example"} > 130: * > 131: * @implNote This class has defined constants for some of the standard > algorithm s/has defined/defines/ src/java.base/share/classes/javax/crypto/spec/HPKEParameterSpec.java line 132: > 130: * > 131: * @implNote This class has defined constants for some of the standard > algorithm > 132: * identifiers. For example, {@link #KEM_DHKEM_P_256_HKDF_SHA256}, Maybe combine the first two sentences, and say "... identifiers such as KEM_DHKEM_P_256_HKDF_SHA256, ..." src/java.base/share/classes/javax/crypto/spec/HPKEParameterSpec.java line 133: > 131: * @implNote This class has defined constants for some of the standard > algorithm > 132: * identifiers. For example, {@link #KEM_DHKEM_P_256_HKDF_SHA256}, > 133: * {@link #KDF_HKDF_SHA256}, and {@link #AEAD_AES_128_GCM}. An > implementation Suggest combining 2 sentences and starting with "An HPKE Cipher implementation may support ..." src/java.base/share/classes/javax/crypto/spec/HPKEParameterSpec.java line 135: > 133: * {@link #KDF_HKDF_SHA256}, and {@link #AEAD_AES_128_GCM}. An > implementation > 134: * may support all, some, or none of the algorithm identifiers defined > here. > 135: * It may also support additional identifiers not listed here, including > private Suggest starting with "An implementation may also support ..." (just to be clear what "it" is referring to.") src/java.base/share/classes/javax/crypto/spec/HPKEParameterSpec.java line 142: > 140: * @spec security/standard-names.html > 141: * Java Security Standard Algorithm Names > 142: * @since 25 26? ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/18411#discussion_r2298892745 PR Review Comment: https://git.openjdk.org/jdk/pull/18411#discussion_r2298835940 PR Review Comment: https://git.openjdk.org/jdk/pull/18411#discussion_r2298836953 PR Review Comment: https://git.openjdk.org/jdk/pull/18411#discussion_r2298865555 PR Review Comment: https://git.openjdk.org/jdk/pull/18411#discussion_r2298862053 PR Review Comment: https://git.openjdk.org/jdk/pull/18411#discussion_r2298873486 PR Review Comment: https://git.openjdk.org/jdk/pull/18411#discussion_r2298877104 PR Review Comment: https://git.openjdk.org/jdk/pull/18411#discussion_r2298884971 PR Review Comment: https://git.openjdk.org/jdk/pull/18411#discussion_r2298877580 PR Review Comment: https://git.openjdk.org/jdk/pull/18411#discussion_r2298883374 PR Review Comment: https://git.openjdk.org/jdk/pull/18411#discussion_r2298888564 PR Review Comment: https://git.openjdk.org/jdk/pull/18411#discussion_r2298911678 PR Review Comment: https://git.openjdk.org/jdk/pull/18411#discussion_r2298961040 PR Review Comment: https://git.openjdk.org/jdk/pull/18411#discussion_r2298962347 PR Review Comment: https://git.openjdk.org/jdk/pull/18411#discussion_r2298964123 PR Review Comment: https://git.openjdk.org/jdk/pull/18411#discussion_r2298967871 PR Review Comment: https://git.openjdk.org/jdk/pull/18411#discussion_r2298969699 PR Review Comment: https://git.openjdk.org/jdk/pull/18411#discussion_r2298970025
