On Thu, 1 Jun 2023 22:07:07 GMT, Francisco Ferrari Bihurriet <d...@openjdk.org> wrote:
>> src/java.base/share/classes/sun/security/util/PBEUtil.java line 182: >> >>> 180: // salt should be non-null per PBEParameterSpec >>> 181: iCountInit = check(pbeParams.getIterationCount()); >>> 182: saltInit = check(pbeParams.getSalt()); >> >> Why not override params with the IvParameterSpec inside the PBEParameterSpec >> here? Then the PBES2Params.initialize(...) method does not need to handle >> PBEParameterSpec type? Seems more consistent this way. > > @valeriepeng: I agree, the only downside I see is that we'll need to do the > same `pbeParams.getParameterSpec()` extraction before [this > `PBES2Params.initialize()` > call](https://github.com/openjdk/jdk/blob/1c596e147dac5102b31a946c905843b9b19b428e/src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11PBECipher.java#L144-L146). > But for this case, it's clearly more consistent to have the extraction here. > We'll address this in the next iteration. I'm okay with this change. While I don't like extracting in two places (on the caller side) as @franferrax said, perhaps it makes easier to understand why the iteration count and salt are not obtained from params in PBES2Params::initialize. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/12396#discussion_r1218808892
