RFR: 8343232: PKCS#12 KeyStore support for RFC 9879: Use of Password-Based Message Authentication Code 1 (PBMAC1) [v8]

[Mark Powers]

On Wed, 24 Sep 2025 19:28:12 GMT, Sean Mullan <mullan at openjdk.org> wrote:

>> Mark Powers has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   fix behavior with keytool
>
> src/java.base/share/classes/sun/security/pkcs12/MacData.java line 94:
> 
>> 92:                 pbeSpec =
>> 93:                         this.digestAlgorithmParams.getParameterSpec(
>> 94:                         PBEParameterSpec.class);
> 
> I think you may already be working on this, so mainly registering this as a comment. This code should be replaced with an internal method that calls `AlgorithmId.getEncodedParams()` and decodes the parameters, reusing much of the code you have already written in `PBMAC1Parameters.engineInit()`. This will avoid having to create an `AlgorithmParameters` implementation as part of this feature, which isn't strictly needed. We can consider adding that on a follow-on Enhancement.

Done.

> src/java.base/share/classes/sun/security/pkcs12/PKCS12KeyStore.java line 1953:
> 
>> 1951:     private void processMacData(AlgorithmParameterSpec params,
>> 1952:             MacData macData, char[] password, byte[] data, String macAlgorithm)
>> 1953:             throws  Exception {
> 
> Try just throwing the exceptions that can be thrown by code in this method, rather than `Exception` for everything. I know there is a "try/catch (Exception)" block in `engineLoad` when calling this method, but I think it is cleaner to only declare the exceptions that can be thrown here.

Agreed. I'll fix this.

> src/java.base/share/classes/sun/security/pkcs12/PKCS12KeyStore.java line 2214:
> 
>> 2212:                                 new PBEParameterSpec(salt, ic);
>> 2213:                         processMacData(params, macData, password, authSafeData,
>> 2214:                                 macAlgorithm);
> 
> These 4 lines can be moved below after the if/else block since they are the same for both conditions.

Done.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/24429#discussion_r2388311568
PR Review Comment: https://git.openjdk.org/jdk/pull/24429#discussion_r2388312083
PR Review Comment: https://git.openjdk.org/jdk/pull/24429#discussion_r2388312524