On Tue, 6 Aug 2024 22:25:30 GMT, Martin Balao <[email protected]> wrote:
>> Can someone help review this fix? Changed the required-mechanism check by
>> checking if the particular mechanism is inside the list of enabled supported
>> mechanisms. This should be more reliable than calling C_GetMechanismInfo(..)
>> on the required mechanism given vendors may return various sorts of error
>> codes.
>>
>> Thanks,
>> Valerie
>
> src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/SunPKCS11.java line
> 1265:
>
>> 1263: (Collectors.toCollection(HashSet::new));
>> 1264: if (config.getDisabledMechanisms() != null) {
>> 1265: enabledMechSet.removeAll(config.getDisabledMechanisms());
>
> Should `enabledMechSet` be further restricted to `Config::enabledMechanisms`
> (if set)? `Config::disabledMechanisms` looks like a fallback for when
> `Config::enabledMechanisms` is not set, according to `Config::isEnabled`. I'd
> keep the logic that makes `Config::enabledMechanisms` work in pair with
> `Config::disabledMechanisms` in a single place, as duplicating it into two
> different places may lead to misalignment.
Yes, you have a good point. Let me think about it and see how to better handle
this.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/20207#discussion_r1722376089
