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

[Mark Powers]

On Tue, 30 Sep 2025 19:29:07 GMT, Sean Mullan <mullan at openjdk.org> wrote:

>> Mark Powers has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   another day another iteration
>
> src/java.base/share/classes/com/sun/crypto/provider/PBMAC1Parameters.java line 84:
> 
>> 82:     private int keyLength = -1;
>> 83: 
>> 84:     protected void Init(AlgorithmParameterSpec paramSpec)
> 
> I don't think you need a separate method. Put this code in the constructor. Same for the other `Init` method. Then you can also make the fields final.

fixed

> src/java.base/share/classes/sun/security/pkcs12/MacData.java line 174:
> 
>> 172:     }
>> 173: 
>> 174:     void processMacData(AlgorithmParameterSpec params,
> 
> Can be static?

No - perhaps in an earlier iteration of the code, but not in the latest.

> src/java.base/share/classes/sun/security/pkcs12/MacData.java line 186:
> 
>> 184:             kdfHmac = macAlgorithm;
>> 185:             Hmac = macAlgorithm;
>> 186:         }
> 
> Did you consider adding another kdfHmac parameter to the method and passing in the correct values?

This comment appears to apply to an earlier iteration. It is not easy to keep track of all these iterations. I believe the latest iteration fixes this.

> src/java.base/share/classes/sun/security/util/PBKDF2Parameters.java line 71:
> 
>> 69:  * </pre>
>> 70:  */
>> 71: final public class PBKDF2Parameters {
> 
> `public` should be before `final`. For reference, see the last sentence of https://docs.oracle.com/javase/specs/jls/se25/html/jls-8.html#jls-8.1.1

fixed

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

PR Review Comment: https://git.openjdk.org/jdk/pull/24429#discussion_r2404173249
PR Review Comment: https://git.openjdk.org/jdk/pull/24429#discussion_r2404173197
PR Review Comment: https://git.openjdk.org/jdk/pull/24429#discussion_r2404173157
PR Review Comment: https://git.openjdk.org/jdk/pull/24429#discussion_r2404173428