RFR: JDK-6676643: Improve current C_GetAttributeValue native implementation [v2]

Xue-Lei Andrew Fan xuelei at openjdk.java.net
Thu May 13 21:20:12 UTC 2021


On Wed, 12 May 2021 21:58:13 GMT, Valerie Peng <valeriep at 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
>
> Valerie Peng has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Fix typos, e.g. funktion, lsit, functin.

Looks good to me except a few trivial comments.

src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/wrapper/PKCS11Exception.java line 193:

> 191:         register("CKR_TOKEN_RESOURCE_EXCEEDED", 0x00000201L);
> 192:         register("CKR_OPERATION_CANCEL_FAILED", 0x00000202L);
> 193:         register("CKR_VENDOR_DEFINED", 0x80000000L);

See below comment, I may have the hash map unmodifiable after the registration.

src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/wrapper/PKCS11Exception.java line 197:

> 195: 
> 196:     private static void register(String name, long errorCode) {
> 197:         errorMap.put(errorCode, name);

I may check the put() return value and throw error for duplicated items, just in case duplicated copy-and-past register.

src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/wrapper/PKCS11Exception.java line 205:

> 203:         if (res == null) {
> 204:             res = "0x" + Functions.toFullHexString((int)errorCode);
> 205:             errorMap.put(errorCode, res);

It would be nice if the errorMap is immutable once the known error codes get registered.

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

Marked as reviewed by xuelei (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/3709



More information about the security-dev mailing list