Skip to content
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

PKCS11 pkcs11_signature_sign does not check ulDataLen==32 for ECDSA #387

Closed
jacobschloss opened this issue Jul 28, 2024 · 1 comment
Closed
Labels

Comments

@jacobschloss
Copy link

jacobschloss commented Jul 28, 2024

Describe the bug
When signing (ECDSA) a too-long buffer via the PKCS11 interface, eg with Botan PKCS11 interface here signing a 33 byte buffer

Botan::PK_Signer signer(atecc_privkey_handle, m_rng, "Raw",  Botan::Signature_Format::Standard, "pkcs11");

std::array<uint8_t, 33> incorrect_long_msg;
incorrect_long_msg.fill(0);
incorrect_long_msg[32] = 1;
pkcs11sig = signer.sign_message(incorrect_long_msg, m_rng);

"C_Sign" / pkcs11_signature_sign is passed in the length of the buffer from the calling 3rd party library, checks if ulDataLen is 0, but not if ulDataLen is 32 which atcab_sign_ext assumes.

Later we can match the signature by only passing in the first 32 bytes

Botan::PK_Verifier sig_verifier(*tmpkey, "Raw(SHA-256)", Botan::Signature_Format::Standard);

std::array<uint8_t, 32> msg;
msg.fill(0);
sig_verifier.verify_message(msg, pkcs11sig)) < -- will return true, since the last byte in the above signing operation was silently ignored.

Expected behavior
Input message buffer length to be validated and an error code to be returned (maybe CKR_DATA_LEN_RANGE or CKR_ARGUMENTS_BAD?) since pkcs11_signature_sign is told the buffer length and knows it is an incorrect size for the requested signing operation.

Additional context
If a buffer is passed in that is less than 32 bytes long and near a section boundary, presumably the program could crash as atcab_sign_ext will try to read 32 bytes.

The function pkcs11_signature_check_params checks the length of the signature output buffer, but not the length of the input message buffer. Adding the precondition check to this function could also make sense.

@jacobschloss jacobschloss changed the title PKCS11 pkcs11_signature_sign silently ignores too long pData PKCS11 pkcs11_signature_sign does not check ulDataLen==32 for ECDSA Jul 28, 2024
Copy link

This issue has been marked as stale - please confirm the issue still exists with the latest version of the library and update the issue if it remains

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant