On Tue, 23 May 2023 19:29:47 GMT, Martin Balao <mba...@openjdk.org> wrote:

>> We would like to propose an implementation for the [JDK-8301553: Support 
>> Password-Based Cryptography in 
>> SunPKCS11](https://bugs.openjdk.org/browse/JDK-8301553) enhancement 
>> requirement.
>> 
>> In addition to pursuing the requirement goals and guidelines of 
>> [JDK-8301553](https://bugs.openjdk.org/browse/JDK-8301553), we want to share 
>> the following implementation notes (grouped per altered file):
>> 
>>   * 
>> ```src/java.base/share/classes/com/sun/crypto/provider/HmacPKCS12PBECore.java```
>>  (modified)
>>     * This file contains the ```SunJCE``` implementation for the [PKCS #12 
>> General Method for Password 
>> Integrity](https://datatracker.ietf.org/doc/html/rfc7292#appendix-B) 
>> algorithms. It has been modified with the intent of consolidating all 
>> parameter checks in a common file 
>> (```src/java.base/share/classes/sun/security/util/PBEUtil.java```), that can 
>> be used both by ```SunJCE``` and ```SunPKCS11```. This change does not only 
>> serve the purpose of avoiding duplicated code but also ensuring alignment 
>> and compatibility between different implementations of the same algorithms. 
>> No changes have been made to parameter checks themselves.
>>     * The new ```PBEUtil::getPBAKeySpec``` method introduced for parameters 
>> checking takes both a ```Key``` and a ```AlgorithmParameterSpec``` instance 
>> (same as the ```HmacPKCS12PBECore::engineInit``` method), and returns a 
>> ```PBEKeySpec``` instance which consolidates all the data later required to 
>> proceed with the computation (password, salt and iteration count).
>> 
>>   * ```src/java.base/share/classes/com/sun/crypto/provider/PBES2Core.java``` 
>> (modified)
>>     * This file contains the ```SunJCE``` implementation for the [PKCS #5 
>> Password-Based Encryption 
>> Scheme](https://datatracker.ietf.org/doc/html/rfc8018#section-6.2) 
>> algorithms, which use PBKD2 algorithms underneath for key derivation. In the 
>> same spirit than for the ```HmacPKCS12PBECore``` case, we decided to 
>> consolidate common code for parameters validation and default values in a 
>> single file 
>> (```src/java.base/share/classes/sun/security/util/PBEUtil.java```), that can 
>> serve both ```SunJCE``` and ```SunPKCS11``` and ensure compatibility. 
>> However, instead of a single static method at the implementation level (see 
>> ```PBEUtil::getPBAKeySpec```), we create an instance of an auxiliary class 
>> and invoke an instance method (```PBEUtil.PBES2Params::getPBEKeySpec```). 
>> The reason is to persist parameters data that has to be consistent between 
>> calls to ```PBES2Core::engineInit``` (in its multiple ...
>
> Martin Balao has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   8301553: Support Password-Based Cryptography in SunPKCS11 (iteration #3)
>   
>   Co-authored-by: Francisco Ferrari <fferr...@redhat.com>
>   Co-authored-by: Martin Balao <mba...@redhat.com>

src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11PBECipher.java 
line 214:

> 212:     protected int engineGetKeySize(Key key)
> 213:             throws InvalidKeyException {
> 214:         return cipher.engineGetKeySize(key);

I am not too sure that this would work for all types of key... Based on the 
current impl, cipher is an AES cipher, but the specified key here can be P11Key 
or a PBEKey object. The key algorithms for both would be PBE-related, but then 
you are using a AES cipher to retrieve the key size?

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

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

Reply via email to