On Wed, 15 May 2024 16:58:05 GMT, Kevin Driver 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 two additional
> commits since the last revision:
>
> - remove unused declared exception in impls
> - throw a ProviderException instead of "eating" an NSAE for Mac
> The changes in the PR have been updated 64 times so far, hard to keep up :-)
> Just a few comments on the current API, revision
> [4bb0d78](https://github.com/openjdk/jdk/commit/4bb0d78b7c70076a2702578299bb07a5e49c798e)
>
> * The KDF.deriveXXX methods mention "Delayed provider selection". Is this
> idempotent? If I create a KDF and several threads race to derive keys or
> data, is it guaranteed that the same provider will be selection for any
> ordering of these threads? What does KDF::getProviderName if no provider has
> been selected?
I believe it would *not* be guaranteed that the same provider would be selected
for any ordering of the threads (depending on their possibly unique
KDFParameterSpec values). Is this a documentation call-out? Or did you have a
concern about this?
For the second part of your question, we have updated Javadoc to describe that
`getProviderName` situation:
> However, if
> * {@code getProviderName} is called before calling the {@code deriveKey} or
> * {@code deriveData} methods, the first provider supporting the KDF algorithm
> * is chosen which may not be the desired one; therefore it is recommended not
> * to call {@code getProviderName} until after a key derivation operation.
---
> * KDFSpi. Can "cryptographic service provider" link to anything? I mentioned
> this in a previous comment but there is nothing to show that this provider
> interface fits in. It's not a factory for a KDF so you can't just implement
> it and plop an implementation on your class path. What does
> KDFSpi::engineDeriveKey throw if the value of "alg" is not a recognised
> algorithm name?
I'll look to add a link to a document for "cryptographic service provider".
There will not be stringent checks on `alg` to ensure validity. Only a `null`
check. There is not really a way to enumerate all the possibilities from all
providers.
---
> * HKDFParameterSpec.Builder.extractOnly. Is it an error to call the build
> methods (currently named extractOnly and thenExpand) before adding key
> material? Asking if these methods need to throw IllegalStateException if they
> don't yet have the key material.
> * HKDFParameterSpec.Extract ikms and salts methods, are you planning to
> document the ordering of the elements?
An empty ikm and/or salt list is a supported scenario, so this should be fine.
I have added the documentation comment about the order of elements.
---
> * HKDFParameterSpec.Extract.info uses the phrase "or null if not specified".
> An ExtractThenExpand object can be created with optional context/info, it
> looks like an Extract can't be created with context/info. Just trying to see
> if it is possible to get a non-null context/info here.
I'm a bit confused by this question. HKDFParameterSpec.Extract does not contain
an `info`. Maybe I am missing something.
---
> * HKDFParameterSpec.buildExtract. The naming is a bit unusual here. Look at
> Thread.ofPlatform and Thread.ofVirtual for ideas, it might be that this
> method should be OfExtract.
I've made the suggested change.
-
PR Comment: https://git.openjdk.org/jdk/pull/18924#issuecomment-2113060882
