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/P11Util.java line 34: > 32: import java.nio.charset.Charset; > 33: import java.security.*; > 34: import java.util.Arrays; Not used? src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11Util.java line 36: > 34: import java.util.Arrays; > 35: > 36: import jdk.internal.access.SharedSecrets; Not used? src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11Util.java line 93: > 91: while (passwordBytes.hasRemaining()) { > 92: encPassword[i] = (char) (passwordBytes.get() & 0xFF); > 93: passwordBytes.put(i++, (byte) 0); nit: add a comment // for erasing bytes in 'passwordBytes' src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/wrapper/CK_MECHANISM.java line 213: > 211: sb.append(Constants.INDENT); > 212: sb.append("pParameter:"); > 213: sb.append(Constants.NEWLINE); Is this intended? It seems NEWLINE is not added beforehand for other fields. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/12396#discussion_r1205970312 PR Review Comment: https://git.openjdk.org/jdk/pull/12396#discussion_r1205969903 PR Review Comment: https://git.openjdk.org/jdk/pull/12396#discussion_r1205969334 PR Review Comment: https://git.openjdk.org/jdk/pull/12396#discussion_r1205972334
