On Thu, 16 Feb 2023 04:52:57 GMT, Martin Balao <mba...@openjdk.org> wrote:

>> src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/wrapper/PKCS11.java
>>  line 165:
>> 
>>> 163:                 getInfo();
>>> 164:                 this.version.major = pInfo.cryptokiVersion.major;
>>> 165:                 this.version.minor = pInfo.cryptokiVersion.minor;
>> 
>> Now that you added getInfo(), then the C_GetInfo() could be made private, so 
>> that all callers can just leverage getInfo(), right?
>
> Right! We will make C_GetInfo private and delete the overload in 
> SynchronizedPKCS11 for the next iteration.

We have analyzed this issue further and think that we need one more iteration. 
It's possible that the constructor of PKCS11 finishes with PKCS11::pInfo in 
null. In fact, it's very likely to happen and PKCS11::getInfo is not 
multi-thread safe. The second problem is that it's possible that a 
SyncrhonizedPKCS11 instance ends up calling a non-synchronized version of 
PKCS11::C_GetInfo, if we remove the override. The reason why we added 
PKCS11::getInfo in the first place was because PKCS11::getVersion is not 
currently available in 17u —where this feature was originally developed— and 
there were 2 calls to PKCS11::C_GetInfo. We can safely undo the PKCS11::getInfo 
change and call PKCS11::getVersion in P11SecretKeyFactory::derivePBEKey.

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

PR: https://git.openjdk.org/jdk/pull/12396

Reply via email to