On Mon, 13 May 2024 03:46:50 GMT, Kevin Driver <kdri...@openjdk.org> wrote:
>> Introduce an API for Key Derivation Functions (KDFs), which are >> cryptographic algorithms for deriving additional keys from a secret key and >> other data. See [JEP 478](https://openjdk.org/jeps/478). > > Kevin Driver has updated the pull request incrementally with one additional > commit since the last revision: > > re-enable preview annotations src/java.base/share/classes/javax/crypto/KDF.java line 46: > 44: > 45: /** > 46: * This class provides the functionality of a key derivation algorithm > for JCE. I think it's important to spell out the KDF acronym in the first sentence, and then you can use KDF subsequently instead of "Key derivation". Suggest: "This class provides the functionality of a Key Derivation Function (KDF). {@code KDF} objects are instantiated with the {@code getInstance} family of methods. KDF algorithm names follow a naming convention of <I>Algorithm</I>with<I>PRF</I>. ..." src/java.base/share/classes/javax/crypto/KDF.java line 51: > 49: * of methods. Key derivation algorithm names follow a naming convention > of > 50: * <I>Algorithm</I>with<I>PRF</I>. The algorithm field is the KDF > algorithm > 51: * (e.g. HKDF, etc.), while the PRF specifier identifies the underlying s/specifier/field/ src/java.base/share/classes/javax/crypto/KDF.java line 53: > 51: * (e.g. HKDF, etc.), while the PRF specifier identifies the underlying > 52: * pseudorandom function (e.g. HmacSHA256). For instance, a KDF > implementation > 53: * of HKDF using HMAC-SHA256 will have an algorithm string of s/will have/has/ s/string/name/ src/java.base/share/classes/javax/crypto/KDF.java line 60: > 58: * {@snippet lang = java: > 59: * KDF kdfHkdf = KDF.getInstance("HKDFWithHmacSHA256", > 60: * (AlgorithmParameterSpec) null); I think it would be clearer to call the one-arg `getInstance` w/o null params here. src/java.base/share/classes/javax/crypto/KDF.java line 81: > 79: "Provider"); > 80: private static final boolean skipDebug = Debug.isOn("engine=") > 81: && !Debug.isOn("kdf"); None of these debug variables are used AFAICT. src/java.base/share/classes/javax/crypto/KDF.java line 89: > 87: private KDFSpi spi; > 88: > 89: // The name of the MAC algorithm. s/MAC/KDF/ src/java.base/share/classes/javax/crypto/KDF.java line 157: > 155: > 156: /** > 157: * Returns a {@code KDF} object that implements the specified > algorithm.. Extra period at end of sentence. src/java.base/share/classes/javax/crypto/KDF.java line 209: > 207: } catch (InvalidAlgorithmParameterException e) { > 208: throw new NoSuchAlgorithmException( > 209: "Received an InvalidParameterSpecException. Does this " s/InvalidParameterSpecException/InvalidAlgorithmParameterException/ Also, preserve the cause when throwing `NoSuchAlgorithmException` (pass `e` as the 2nd param). Same comment applies to other getInstance methods. src/java.base/share/classes/javax/crypto/KDF.java line 314: > 312: Objects.requireNonNull(algorithm, "null algorithm name"); > 313: try { > 314: Instance instance = GetInstance.getInstance("KDF", > KDFSpi.class, I think you can call `JceSecurity.getInstance` here, and then you don't need lines 318-322. Same comment applies to other `getInstance` methods. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/18924#discussion_r1598479103 PR Review Comment: https://git.openjdk.org/jdk/pull/18924#discussion_r1598484799 PR Review Comment: https://git.openjdk.org/jdk/pull/18924#discussion_r1598482080 PR Review Comment: https://git.openjdk.org/jdk/pull/18924#discussion_r1598489265 PR Review Comment: https://git.openjdk.org/jdk/pull/18924#discussion_r1598494982 PR Review Comment: https://git.openjdk.org/jdk/pull/18924#discussion_r1598496701 PR Review Comment: https://git.openjdk.org/jdk/pull/18924#discussion_r1598499646 PR Review Comment: https://git.openjdk.org/jdk/pull/18924#discussion_r1598509971 PR Review Comment: https://git.openjdk.org/jdk/pull/18924#discussion_r1598523001
