RFR: 8301553: Support Password-Based Cryptography in SunPKCS11

Martin Balao mbalao at openjdk.org
Mon Mar 20 18:14:38 UTC 2023


On Wed, 15 Mar 2023 18:02:58 GMT, Valerie Peng <valeriep at openjdk.org> wrote:

>> src/java.base/share/classes/sun/security/util/PBEUtil.java line 47:
>> 
>>> 45: 
>>> 46:     // Used by SunJCE and SunPKCS11
>>> 47:     public final static class PBES2Params {
>> 
>> This class confuses me. An instance is constructed without any parameter. The method `getPBEKeySpec` is able to mutate it and return something, the method `getAlgorithmParameters` is able to read the content and possibly mutate again and return something, and finally another method `getIvSpec` is able to return the content.
>> 
>> Please either precisely document how these methods will be called and enforce this rule with internal checks, or refactor it to something immutable.
>
> +1.

This class mutates because we have to keep state that may change an undefined number of times. This is not new, in the previous code state was kept as well. The only difference is that instead of keeping the state spread across different fields —which would now require mirroring fields between SunJCE and SunPKCS11 sides—, we have it aggregated in a single class. We are also proposing that the methods that change this state —which are used equally from SunJCE and SunPKCS11— are also part of the same class. The intention is to help with consistency and code reuse.

I'll give a brief reminder of how state changes work, even before the PBE enhancement. When engineInit is called, parameter values (passed, defaults or randomly generated) will change the state. All following calls to engineGetParameters must return the same state. When engineInit is called again —with the same CipherSpi instance—, state is re-initialized and the previous sequence repeats. The only special case is when engineGetParameters is called before the first call to engineInit. In such case, engineGetParameters generates the initial state and this state will be overwritten by the first call to engineInit later.

I'll add comments to the class to help explaining this logic.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/12396#discussion_r1142516414



More information about the security-dev mailing list