KDF API review, round 2

[Adam Petcher]

On 11/20/2017 5:12 AM, Jamil Nimeh wrote:

>
> On 11/19/2017 12:45 PM, Michael StJohns wrote:
>> On 11/17/2017 1:07 PM, Adam Petcher wrote:
>>>
>>> I agree that this is challenging because there is so much variety in 
>>> KDFs. But I don't think that SP 800-108 is a good example of 
>>> something that should be exposed as an algorithm in JCA, because it 
>>> is too broad. SP 800-108 is more of a toolbox that can be used to 
>>> construct KDFs. Particular specializations of SP 800-108 are widely 
>>> used, and they will get names that can be used in getInstance. For 
>>> example, HKDF-Expand is a particular specialization of SP 800-108.
>>> So I think the existing pattern of using algorithm names to specify 
>>> concrete algorithms should work just as well in this API as it does 
>>> in the rest of JCA. Of course, more flexibility in the API is a nice 
>>> feature, but supporting this level of generality may be out of scope 
>>> for this effort.
>>
>> The more I think about it the more I think you're mostly right. But 
>> let's split this slightly as almost every KDF allows for the 
>> specification of the PRF.  So
>>
>> <kdfname>/<prf>    as the standard naming convention.
>>
>> Or TLS13/HMAC-SHA256 and HKDF/HMAC-SHA256 (which are different 
>> because of the mandatory inclusion of "L" in the derivation 
>> parameters and each component object for TLS13)

This approach seems fine to me. We would probably want to allow any 
algorithm name after the / (rather than limiting it to PRFs), because 
JCA doesn't have a notion of PRF, and because some KDFs take other kinds 
of functions (e.g. PBKDF1 uses a bare hash function).

>>
>> Still - let's include the .setParameters() call as a failsafe as 
>> looking forward I can see the need for flexibility rearing its ugly 
>> head (e.g. adding PSS parameters to RSA signatures way late in the 
>> game.....) and it does match the pattern for Signature so its not a 
>> new concept. A given provider need not support the call, but its 
>> there if needed.
> Signature appears to have setParameter because the initSign and 
> initVerify didn't have APS parameters in their method signatures. 
> Since we're talking about providing APS objects through both 
> getInstance() for those locked to the algorithm and init() for things 
> like salts, info, etc. that can be changed on successive inits it 
> seems like we're covered without the need for a setParameter method.

My argument is that providing APS in getInstance doesn't appear to be 
necessary. Of course, if you want to tackle this, that's fine with me.  
But I think it complicates the API and I expect it will lead to other 
API/design problems that will need to be sorted out.

I agree that setParameter() in Signature appears to be there to solve a 
different problem. This API doesn't have that problem because the init 
method takes an APS.

>
> One additional topic for discussion: Late in the week we talked about 
> the current state of the API internally and one item to revisit is 
> where the DerivationParameterSpec objects are passed. It was brought 
> up by a couple people that it would be better to provide the DPS 
> objects pertaining to keys at the time they are called for through 
> deriveKey() and deriveKeys() (and possibly deriveData).
>
> Originally we had them all grouped in a List in the init method. One 
> reason for needing it up there was to know the total length of 
> material to generate.  If we can provide the total length through the 
> AlgorithmParameterSpec passed in via init() then things like:
>
> Key deriveKey(DerivationParameterSpec param);
> List<Key> deriveKeys(List<DerivationParameterSpec> params);
>
> become possible.  To my eyes at least it does make it more clear what 
> DPS you're processing since they're provided at derive time, rather 
> than the caller having to keep track in their heads where in the DPS 
> list they might be with each successive deriveKey or deriveKeys 
> calls.  And I think we could do away with deriveKeys(int), too.

I like this change. It simplifies the API, and forcing the JCA client to 
be explicit and supply the output length in the APS is a good thing.