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 NIST P curve support to ECDH #351

Merged
merged 3 commits into from
May 31, 2024
Merged

Add NIST P curve support to ECDH #351

merged 3 commits into from
May 31, 2024

Conversation

hko-s
Copy link
Contributor

@hko-s hko-s commented May 11, 2024

No description provided.

src/crypto/ecdh.rs Outdated Show resolved Hide resolved
src/crypto/ecdh.rs Outdated Show resolved Hide resolved
src/crypto/ecdh.rs Outdated Show resolved Hide resolved
src/crypto/ecdh.rs Outdated Show resolved Hide resolved
@dignifiedquire
Copy link
Member

would be nice to add benchmarks and tests for these new formats, following the existing ones

@hko-s hko-s force-pushed the ecdh branch 2 times, most recently from 043b57e to 7374480 Compare May 16, 2024 10:55
- SecretKey is now an enum
- add curve parameter to KeyType::ECDH and ecdh::generate_key
- generalize ecdh shared_secret handling to flexible length
@hko-s
Copy link
Contributor Author

hko-s commented May 16, 2024

would be nice to add benchmarks and tests for these new formats, following the existing ones

I extended the benchmarks, which seemed straightforward.

Regarding tests i am unsure what to extend. I did already add nist curve tests to

rpgp/src/crypto/ecdh.rs

Lines 543 to 548 in 3b91018

for curve in [
ECCCurve::Curve25519,
ECCCurve::P256,
ECCCurve::P384,
ECCCurve::P521,
] {

It looks like the corpus in tests doesn't have any nist artifacts.
Did you have specific ideas what kinds of tests should be added?

@dignifiedquire
Copy link
Member

overall looking great, will take some time in the next day to go over the actual crypto algorithms once more

@dignifiedquire
Copy link
Member

@hko-s want to move it out of draft, or is there sth else you want to do?

@hko-s hko-s marked this pull request as ready for review May 29, 2024 20:47
@hko-s
Copy link
Contributor Author

hko-s commented May 29, 2024

@hko-s want to move it out of draft, or is there sth else you want to do?

i thought it'd be nice to shape it into two commits:

one that just transforms the previous code, and one that implements nistp.
i just pushed this changed git shape (with unchanged code).

@dignifiedquire dignifiedquire merged commit b20d841 into rpgp:master May 31, 2024
18 checks passed
@dignifiedquire
Copy link
Member

thanks :)

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.

None yet

2 participants