On Wed, 17 May 2023 03:11:54 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 with a new target base due to a > merge or a rebase. The pull request now contains three commits: > > - Rebase fix after JDK-8306033. Replace called functions with their new > names. > - 8301553: Support Password-Based Cryptography in SunPKCS11 (iteration #1) > > Co-authored-by: Francisco Ferrari <fferr...@redhat.com> > Co-authored-by: Martin Balao <mba...@redhat.com> > - 8301553: Support Password-Based Cryptography in SunPKCS11 > > Co-authored-by: Francisco Ferrari <fferr...@redhat.com> > Co-authored-by: Martin Balao <mba...@redhat.com> We found several more cases of passwords and encoded keys not cleared that were addressed in out Iteration # 2 commit. These cases were both in Java and native code. We still have doubts about the effectiveness and need for these actions, but prefer to have the discussion on a different channel. We also found that invalid keys (not starting with the name "PBE") passed to PBES2Core::engineInit or P11PBECipher::engineInit were being cleared and this could be unexpected to the caller. This is related to my comment [here](https://github.com/openjdk/jdk/pull/12396#discussion_r1199513839). For these cases, we decided not to invoke ::getEncoded and not to clean. As said, we have concerns about these undocumented mutations to objects passed by argument. No test regressions observed in jdk/com/sun/crypto/provider or jdk/sun/security/pkcs11. ------------- PR Comment: https://git.openjdk.org/jdk/pull/12396#issuecomment-1555410153
