CSR Review Request: JDK-8220531, SecretKeyFactory.getInstance( algo_, provider_ ) ignores the provider argument.

[Adam Petcher]

Looks good. I added myself as a reviewer.

On the subject of PBKDF2With<PRF-We-Don't-Have>: your response makes 
sense. I was concerned about a situation in which we parse the algorithm 
name (similar to Cipher transforms), but it looks like that doesn't 
happen here.

On 3/13/2019 11:48 AM, Jamil Nimeh wrote:
> Hi Adam, thanks for the feedback.  I have some comments below:
>
> On 3/13/2019 6:44 AM, Adam Petcher wrote:
>> On 3/12/2019 2:33 PM, Jamil Nimeh wrote:
>>
>>> Hello all,
>>>
>>> Please review the CSR for the behavioral change to SunJCE's PBKDF2 
>>> implementaion.  This change will make the underlying Mac also come 
>>> from SunJCE.  This change only affects the SunJCE implementation of 
>>> PBKDF2, not any other implementation from any different provider.
>>>
>>> https://bugs.openjdk.java.net/browse/JDK-8220531
>>
>> Looks pretty straightforward. I just have a couple of questions 
>> related to compatibility:
>>
>> 1) Is it possible that the requested Mac would not be available in 
>> SunJCE, but it would be available in some other provider? If so, then 
>> PBKDF2 would fail after this change. Should we fall back to the 
>> current behavior if we get a NoSuchAlgorithmException from SunJCE?
> JN: It seems unlikely that we would make a SunJCE implementation of a 
> given PBKDF2With<prf> if we also don't support the PRF portion.  At 
> least up to now we have always supported the PRFs (Hmacs mainly) on 
> SunJCE as well.  It would concern me if we were trying to make a 
> SecretKeyFactory PBKDF2With<PRF-We-Don't-Have> because, short of a 3rd 
> party provider that has it, it would never work.
>>
>> 2) Do you (or anyone else on the mailing list) have any reason to be 
>> concerned that the Mac in SunJCE won't work as well in some cases 
>> where it could also come from another (higher-priority) provider? If 
>> so, then we should think about adding a system property or other 
>> toggle for this behavior. This is a question---not a suggestion. I 
>> don't think we should include this toggle unless we have some 
>> motivation to do so.
> JN: I'd really prefer to not add yet another property to control 
> something like this if we don't have to.  If you're selecting SunJCE 
> to do PBKDF2 it seems (to me at least) that you're already willing to 
> accept that the crypto operations happen in SunJCE software and the 
> PRF is really the core of that operation.  Given that a higher 
> priority 3rd party provider can hamstring (in admittedly rare cases) 
> SunJCE's implementation, I'd much rather see SunJCE always work 
> regardless of 3rd party provider configuration than see the underlying 
> PRF move to another provider.
>>
>> Also, if there is no change to any spec, then I think that means the 
>> scope is "Implementation" rather than "SE".
> JN: I'll make that change.
>>
>>