RFR: 8348732: SunJCE and SunPKCS11 have different PBE key encodings [v2]

Thu Mar 27 00:35:13 UTC 2025

On Tue, 25 Mar 2025 00:22:23 GMT, Valerie Peng <valeriep at openjdk.org> wrote:

>> As part of [https://bugs.openjdk.org/browse/JDK-8301553](JDK-8301553), SunPKCS11 provider added support for PBE SecretKeyFactories for `HmacPBESHAxxx` and `PBEWithHmacSHAxxxAndAES_yyy`. These impls produce keys whose encoding contains the PBKDF2 derived bytes. Given that SunJCE provider have supported `PBEWithHmacSHAxxxAndAES_yyy` SecretKeyFactories whose key encoding is the password bytes for long time. Such difference may be very confusing, e.g. using the same KeySpec and same-name SecretKeyFactory (from different providers), the resulting keys have same algorithm and format but different encodings.
>> 
>> Given that the `P11Mac` and `P11PBECipher` classes already do key derivation internally, these PKCS11 SecretKeyFactories aren't a must-have and are proposed to be removed. I've also aligned the com.sun.crypto.provider.PBEKey class with com.sun.crypto.provider.PPBKDF2KeyImpl class to switch to "UTF-8" when converting the char[] to byte[]. This is to accomodate unicode passwords and given that "UTF-8" encoding is same for ASCII characters, this change should not affect backward compatibility.
>
> Valerie Peng has updated the pull request incrementally with one additional commit since the last revision:
> 
>   apply the suggested changes and minor code refactoring.

Hi @valeriepeng,

I have left a couple more comments, and taken advantage to do a more complete review.

Please note that I haven't finished reviewing `TestPBKD.java` (for example, we can still do some of the deleted checks). However I wanted to left a partial review in-advance, as I will be on PTO until next Tuesday.

src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11Mac.java line 203:

> 201:                 // because this is a PBE Mac service. In addition to checking
> 202:                 // the key, check that params (if passed) are consistent.
> 203:                 PBEUtil.checkKeyAndParams(key, params, algorithm);

Both usages of `PBEUtil.checkKeyAndParams` have been removed (this is the first one), so we should now remove it from `src/java.base/share/classes/sun/security/util/PBEUtil.java`.

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

> 133:             // because this is a PBE Cipher service. In addition to checking the
> 134:             // key, check that params (if passed) are consistent.
> 135:             PBEUtil.checkKeyAndParams(key, params, pbeAlg);

Both usages of `PBEUtil.checkKeyAndParams` have been removed (this is the second one), so we should now remove it from `src/java.base/share/classes/sun/security/util/PBEUtil.java`.

test/jdk/sun/security/pkcs11/Mac/PBAMac.java line 114:

> 112:                     "ae6b69cf9edfd9cd8c3b51cdf2b0243502f35a3e6007f33b1ab73568" +
> 113:                     "2ea81ea562f4383bb9512ff70752367b7259b16f"),
> 114:              macAssertionData("HmacPBESHA512", "HmacSHA512",

nit: all the `macAssertionData` calls are now indented with an extra space (13 leading spaces in total).

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

PR Review: https://git.openjdk.org/jdk/pull/24068#pullrequestreview-2714825938
PR Review Comment: https://git.openjdk.org/jdk/pull/24068#discussion_r2012727892
PR Review Comment: https://git.openjdk.org/jdk/pull/24068#discussion_r2012749286
PR Review Comment: https://git.openjdk.org/jdk/pull/24068#discussion_r2012738803