RFR: 8301553: Support Password-Based Cryptography in SunPKCS11 [v6]

[Francisco Ferrari Bihurriet]

On Tue, 30 May 2023 22:03:44 GMT, Valerie Peng <valeriep at openjdk.org> wrote:

>> Martin Balao has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   8301553: Support Password-Based Cryptography in SunPKCS11 (iteration #4)
>>   
>>   Co-authored-by: Francisco Ferrari <fferrari at redhat.com>
>>   Co-authored-by: Martin Balao <mbalao at redhat.com>
>
> src/java.base/share/classes/sun/security/util/PBEUtil.java line 105:
> 
>> 103:                                 "needed for decryption");
>> 104:                     }
>> 105:                 }
> 
> Isn't there also default value for iteration count? 'params' can be PBEParameterSpec (line 82) but its salt and iteration count values aren't used comparing to the IvParameterSpec inside. Seems a bit inconsistent?

@valeriepeng: I agree, `DEFAULT_ITERATIONS` should be used here and only here, so we consistently initialize any defaults in a single place. We'll update that.

> src/java.base/share/classes/sun/security/util/PBEUtil.java line 182:
> 
>> 180:                     // salt should be non-null per PBEParameterSpec
>> 181:                     iCountInit = check(pbeParams.getIterationCount());
>> 182:                     saltInit = check(pbeParams.getSalt());
> 
> Why not override params with the IvParameterSpec inside the PBEParameterSpec here? Then the PBES2Params.initialize(...) method does not need to handle PBEParameterSpec type? Seems more consistent this way.

@valeriepeng: I agree, the only downside I see is that we'll need to do the same `pbeParams.getParameterSpec()` extraction before [this `PBES2Params.initialize()` call](https://github.com/openjdk/jdk/blob/1c596e147dac5102b31a946c905843b9b19b428e/src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11PBECipher.java#L144-L146). But for this case, it's clearly more consistent to have the extraction here. We'll address this in the next iteration.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/12396#discussion_r1213728201
PR Review Comment: https://git.openjdk.org/jdk/pull/12396#discussion_r1213728793