On Thu, 1 Aug 2024 20:47:21 GMT, Weijun Wang <wei...@openjdk.org> wrote:
>> The existing sentence seems to cover this scenario. Let me know if you >> disagree. >> >> `In some cases the WithPRF portion of the algorithm field may be omitted if >> the KDF algorithm has a fixed or default PRF.` >> >> However, if Argon2 (or another relevant example) were to require a PRF >> specifier at some point in the future, we would still want it to follow that >> form, right? `<em>Algorithm</em>With<em>PRF</em>` > > The PRF or any other config can be put in a `KDFParameters`. The algorithm > name does not necessarily takes this form. I've made an update for this one in [69e89fd](https://github.com/openjdk/jdk/pull/20301/commits/69e89fd61555e60c9bcff1febb8062e65c6550b1). >> 1) An example of a KDF implementation within the class header seems a bit >> excessive to me. >> >> 2) I felt that changes to the wording describing `KDFParameters` elsewhere >> accomplished this goal. > > I don't have strong opinions now. 2) is fine now. You can still consider 1) > if you can write a short one. Not all details need to be implemented. Just > the structure. I have made a note to re-visit a minimal KDF implementation for the class header javadoc. >> Can you add some more detail here? > > Something like: If the result key is extractable, then its getEncoded value > should have the same content as the result of deriveData. I was trying to think through this as well. I can think of cases where there may not be a standard DER/ASN.1 encoding for the `SecretKey` object. In that case, do we really want to stipulate this? >> I looked at `Cipher` and `Mac`. They do not specify in `doFinal` whether or >> not the result may be `null`. >> >> I think this specification is sufficient for implementations (more precise >> than the above mentioned classes) and allows the implementors of `KDFSpi` to >> return `null` in cases which we may not yet have anticipated. > > In the era of `Cipher` and `Mac`, we don't treat nullability as a big issue > and does not always specify it. Now it's called Java's Billion Dollar Mistake. > > In fact, I don't think `doFinal` has ever intended to allow returning null. Commit https://github.com/openjdk/jdk/pull/20301/commits/783e8b4da10c2a76fc8d46f3907551d646ee2ca1 has disallowed `null`, according to the javadoc/spec. >> I am open to changing this wording, but here is my logic on the above: >> >> `or if {@code alg} is invalid or unsupported by the KDF implementation` >> >> Since the kdfParameterSpec (the `AlgorithmParameterSpec`) determines the KDF >> implementation, my feeling is that **if** the implementation cannot return a >> key of the desired algorithm, this could be because it is: `unsupported by >> the KDF implementation`. To me, this covers the combination case. > > One example is asking for a 42-byte AES key. We cannot say "AES" is > unsupported, we also cannot say length 42 is not a valid params set itself, > but the combination is invalid. As indicated above, I can rephrase. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/20301#discussion_r1700908623 PR Review Comment: https://git.openjdk.org/jdk/pull/20301#discussion_r1702084130 PR Review Comment: https://git.openjdk.org/jdk/pull/20301#discussion_r1700899690 PR Review Comment: https://git.openjdk.org/jdk/pull/20301#discussion_r1702080952 PR Review Comment: https://git.openjdk.org/jdk/pull/20301#discussion_r1700976859
