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

Unable to Load Certain EC Keys in Botan 3.6+ #4539

Closed
benj-zen opened this issue Jan 10, 2025 · 5 comments
Closed

Unable to Load Certain EC Keys in Botan 3.6+ #4539

benj-zen opened this issue Jan 10, 2025 · 5 comments
Assignees
Labels

Comments

@benj-zen
Copy link

When generating EC keys prior to version 3.6, the private key byte size is not stretched to match the byte size of the curve order. In Botan 3.6+, EC keys will not be loaded if these sizes do not match:

std::unique_ptr<EC_Scalar_Data> EC_Group_Data::scalar_deserialize(std::span<const uint8_t> bytes) const {
if(bytes.size() != m_order_bytes) {
return nullptr;
}

This issue occurs 50% of the time when generating keys for secp521r1, as 521 mod 8 = 1. When generating an example key with Botan 3.5 or below, which has a byte size of 65:

$ botan keygen --algo=ECDH --params=secp521r1
-----BEGIN PRIVATE KEY-----
MIHrAgEAMA4GBSuBBAEMBgUrgQQAIwSB1TCB0gIBAQRBrUnLx/qoPTdu7xSEZNnO
QHODpOgVgI60AOQjfK7oj45U9J1gG7G+k8hJcmVis2Dys64Ggqw2hhYgNYuDsBDw
EX2hgYkDgYYABAF/MiwGdehgdlNzFWRQywQtcUy4QHXBlREF1q7/ecclXv1xzFAW
ZvgUKGe9r1ox7piGSP7gteS4PJ4RE17lqNmgIAACl9dvTRMGxMbBgfWXLctYtAhv
xS0wery1wQvskt9+FjwPiHSXi4rmjLDM6vcyUliITumR4ArULaowUm5OGvD/Lw==
-----END PRIVATE KEY-----

Attempting to load the key in Botan 3.6 results in an error:

$ botan pkcs8 test.pem
Error: EC_Scalar::from_bytes is not a valid scalar value
@randombit randombit added the bug label Jan 10, 2025
@randombit randombit self-assigned this Jan 10, 2025
@randombit
Copy link
Owner

Interestingly this encoding bug was fixed in #4110 but what I failed to realize at the time is, thinking that the bug had been introduced in #4056 and had never been in a release. But in fact we were always encoding incorrectly prior to #4056 as well

.encode(BigInt::encode_1363(m_private_key, m_private_key.bytes()), ASN1_Type::OctetString)

Using the size of the key vs using the size of the group order as it should have been. :/

@randombit
Copy link
Owner

Are you sure about the version timeline? As best I can tell the encoding was fixed in 3.5.0.

@benj-zen
Copy link
Author

Oh, I see. Would still be great if there would be an API option to somehow import the old keys in 3.6+.
I'm pretty sure about the versions. I compiled both versions before submitting the issue, generated the key in 3.5.0 and tried to load in 3.6.0.

randombit added a commit that referenced this issue Jan 10, 2025
Starting in 3.6.0 a logic change started rejecting EC keys where the
byte encoding is shorter than required.

Most implementations including OpenSSL accept such keys.

GH #4539
@randombit
Copy link
Owner

Would still be great if there would be an API option to somehow import the old keys in 3.6+.

We just have to support this in the default codepath. It might be a different story had this been done correctly the whole time, but it wasn't. Can you try the patch in #4541

randombit added a commit that referenced this issue Jan 10, 2025
Starting in 3.6.0 a logic change started rejecting EC keys where the
byte encoding is shorter than required.

Most implementations including OpenSSL accept such keys.

GH #4539
randombit added a commit that referenced this issue Jan 10, 2025
Starting in 3.6.0 a logic change started rejecting EC keys where the
byte encoding is shorter than required.

Most implementations including OpenSSL accept such keys.

GH #4539
@benj-zen
Copy link
Author

I thought that throwing an error when loading a short private key via the default function may be desirable, hence my comment about the API option. However, #4541 works perfectly. Thanks!

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

2 participants