On Tue, 27 Apr 2021 02:41:12 GMT, Valerie Peng <valer...@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
