On Fri, 18 Apr 2025 21:04:51 GMT, Valerie Peng <valer...@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:
> 
>   address review comments from Mark

A tiny comment. Otherwise, LGTM.

test/jdk/sun/security/pkcs11/SecretKeyFactory/TestPBKD.java line 46:

> 44:  * @summary test key derivation on a SunPKCS11 SecretKeyFactory service
> 45:  * @library /test/lib ..
> 46:  * @modules java.base/com.sun.crypto.provider:open

This `:open` line is useless now. No more reflection code.

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

PR Review: https://git.openjdk.org/jdk/pull/24068#pullrequestreview-2809825973
PR Review Comment: https://git.openjdk.org/jdk/pull/24068#discussion_r2070293015

Reply via email to