-
Notifications
You must be signed in to change notification settings - Fork 51
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Endless Loop on signature size #39
Comments
I changed line 1814 back to *signature_length = 64; and added increment line to: if (rv == CKR_BUFFER_TOO_SMALL) { A bit cleaner I suppose. |
I'm not sure if it is quite correct. According to PKCS#11 standard , when buffer for signature is not large enough, PKCS#11 library implementation MUST return So I think you have some buggy implementation of PKCS#11 which return We can make a workaround to make it work, but simply incrementing what was returned from PKCS#11 implementation is in my opinion not right solution. On the other hand signature is done using RSA key and for ALL currently used sizes of RSA keys 64 bytes is much less than required, minimum key sizes now are 2048 bits, which results in 256 bytes of signature and call to C_Sign() will be done at least twice (after first call for most of PKCS#11 implementations will update required length), but for @matlabs solution and RSA 2048-bit key will be called four times. Usually, when we need to minimize memory usage, we call But for the case for So summarizing changes, it could look like (my proposed changes marked as *signature = NULL;
*signature_length = 1024; /* MS */
while (*signature == NULL) {
CK_ULONG current_signature_length = *signature_length; /* MS */
*signature = malloc(*signature_length);
if (*signature == NULL) {
set_error("not enough free memory available");
return -1;
}
rv = h->fl->C_Sign(h->session, hash + h_offset, sizeof(hash) - h_offset, *signature, signature_length);
if (rv == CKR_BUFFER_TOO_SMALL) {
/* increase signature length as long as it it to short */
free(*signature);
*signature = NULL;
DBG1("increased signature buffer-length to %ld", *signature_length);
if (current_signature_length >= *signature_length) { /* MS */
/* workaround for buggy PKCS#11 implementation: it didn't change
or even lowered buffer size - forcing using larger (double size) buffer */
*signature_length = current_signature_length * 2;
}
} else if (rv != CKR_OK) {
free(*signature);
*signature = NULL;
set_error("C_Sign() failed: 0x%08lX", rv);
return -1;
}
} |
Created PR #40. |
I made your requested modifications and everything works great. I have tested this change on: Thank you for your quick response. |
I have this same issue. |
Yes. Say, in a week or two. Is that ok for you? |
I cannot see any references to fix this bug in |
@mskalski is right. It is not yet merged. However, the patch works for me on Ubuntu 20.04 and the Yubikey 5 with |
Hi! I have some questions on #40 . |
#40 had been merged about a year ago. Shouldn't this issue be closed now? |
I am entering an endless loop because my signature length size is greater that 64. I modified the code in src/commom/pkcs11_lib.c line 1814 *signature_length = 64; to *signature_length = 1024; and everything appears to be fine for me. The code in question is as follows:
I don't see where the string is getting incremented or increased.
I really don't want my own version of pam_pkcs11 and was wondering if you could take a look at the problem and advise.
Thanks...
The text was updated successfully, but these errors were encountered: