On Tue, 27 Apr 2021 04:27:10 GMT, Greg Rubin
<github.com+829871+salusasecon...@openjdk.org> wrote:
>> Anyone can help review this somewhat trivial fix? The main change is inside
>> src/jdk.crypto.cryptoki/share/native/libj2pkcs11/p11_objmgmt.c. This is to
>> help better troubleshooting by reporting the type of unavailable attributes
>> in PKCS11 exception message when C_GetAttributeValue(...) call failed. The
>> java file changes are just cleanup for consolidating the CKR_* constants
>> definition into PKCS11Exception class.
>>
>> Thanks,
>> Valerie
>
> src/jdk.crypto.cryptoki/share/native/libj2pkcs11/p11_objmgmt.c line 253:
>
>> 251: if (rv != CKR_OK) {
>> 252: if (rv == CKR_ATTRIBUTE_SENSITIVE || rv ==
>> CKR_ATTRIBUTE_TYPE_INVALID) {
>> 253: msg = malloc(80); // should be more than sufficient
>
> I'm worried about asserting that 80 is sufficient especially as extremely
> large numbers of attributes could be retrieved and the limit check later on
> seems a bit lax.
80 should be sufficient, as we only include attribute type of those UNAVAILABLE
ones. For example, there may be 6 attributes specified, but realistically only
one or two being unavailable. Are you aware of any case which a large number of
attributes are unavailable? I added the limit check to ensure no out-of-bound
writes. In the worst case, the reported types may be incomplete, i.e. up to 80
bytes. Otherwise, we may need two passes, one to find out the number of
unavailable attributes, and one to allocate and set the content. Or, do you
have other suggestion?
> src/jdk.crypto.cryptoki/share/native/libj2pkcs11/p11_util.c line 189:
>
>> 187: * returnValue is CKR_OK. Otherwise, it returns the returnValue as a
>> jLong.
>> 188: *
>> 189: * @param env - used to call JNI funktions and to get the Exception
>> class
>
> NitPick: here and above some German seems to have slipped in. I think we want
> "functions"
I didn't notice the German, just copy-n-paste. Will fix the typos.
-------------
PR: https://git.openjdk.java.net/jdk/pull/3709
