RFR: JDK-6676643: Improve current C_GetAttributeValue native implementation

[Greg Rubin]

On Tue, 27 Apr 2021 02:41:12 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

src/jdk.crypto.cryptoki/share/native/libj2pkcs11/p11_objmgmt.c line 252:

> 250: 
> 251:     if (rv != CKR_OK) {
> 252:         if (rv == CKR_ATTRIBUTE_SENSITIVE || rv == CKR_ATTRIBUTE_TYPE_INVALID) {

According to the PKCS#11v2.40 spec, `CKR_BUFFER_TOO_SMALL` should be handled in the same special ways as these too (in that it isn't a "true error").

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.

src/jdk.crypto.cryptoki/share/native/libj2pkcs11/p11_objmgmt.c line 262:

> 260:             temp1 = msg;
> 261:             temp2 = msg + 80;
> 262:             for (i = 0; i < ckAttributesLength && temp1 < temp2; i++) {

I think that this loop will append at most 11 bytes to the string each time (is this correct?), if so, we can make the check `temp1 < temp2 - 12` to count the final null and avoid running off the end with a buffer overflow.

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"

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

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