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

Add helper for making EC key from components #358

Merged
merged 1 commit into from
Aug 9, 2024

Conversation

tomhughes
Copy link
Contributor

This parallels the work done in #307 for EC keys by providing a helper to construct them from their components which is useful when the provider does not publish an x5c version of the key.

@prince-chrismc
Copy link
Collaborator

prince-chrismc commented Jul 30, 2024

First pass on mobile this looks absolutely stellar 👌 🙌

@ItsAMeMarcel
Copy link

ItsAMeMarcel commented Aug 5, 2024

I would really appreciate to see this functionality in the main branch 😍

Copy link
Owner

@Thalhammer Thalhammer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

Copy link
Collaborator

@prince-chrismc prince-chrismc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even the docs look good 👏

image

If I have to be picky, an example and the openssl command to generate the test certificate would take this above 100%

I believe the correct one for this curve is

➜  /tmp openssl ecparam -genkey -name secp384r1 -noout -out private-key.pem       
➜  /tmp openssl ec -in private-key.pem -pubout -out public-key.pem         
read EC key
writing EC key
➜  /tmp cat public-key.pem 
-----BEGIN PUBLIC KEY-----
MHYwEAYHKoZIzj0CAQYFK4EEACIDYgAEmMgWcIvCrgWF9zvYy5y5fO8EQutEYzJl
K6DF8S7DN/RUsorSJdBMvJ8l66Crlq1mSWWFztseK6URXfUJSK1vj2dE5QoUkaxh
S6WqXuZ2jT0dHjJxDAZc7M2e+EImdDU5
-----END PUBLIC KEY-----

the curve names are a nightmare 🙊

std::string group = helper::curve2group(curve, ec);
if (ec) return {};

// https://github.com/openssl/openssl/issues/16270#issuecomment-895734092
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤢

@prince-chrismc
Copy link
Collaborator

@tomhughes This is one of the best contributions a maintainer can hope for so it's super appreciated!

@prince-chrismc prince-chrismc merged commit c870903 into Thalhammer:master Aug 9, 2024
59 of 60 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants