-
Notifications
You must be signed in to change notification settings - Fork 31
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
secp256k1 DHKEM #59
secp256k1 DHKEM #59
Conversation
Thank you! I'm sorry I'm so busy with school rn I will get around to this in 2-3 weeks. Two small things until then:
Thanks again, this looks great |
I don't appear to be able to create or target new branches on this repository. If you were to open an unstable-k256 branch from main then I could target that. May you please share why you'd like a new branch as well? I think I could learn something from your development flow in understanding why.
The code for generating k256-ecdh test vectors can be found in this repository. Is this sufficient, or do you mean that you would like them to be committed into this repository? best of luck with school |
Ok np I can do the branch stuff. The reason is just bc I don't want to put non-specc'd things in the main branch. Unless I misunderstood and K256 is an accepted IETF extension? |
I suppose it is still a draft https://datatracker.ietf.org/doc/draft-wahby-cfrg-hpke-kem-secp256k1/01/ but perhaps it could be gated behind an |
KEM name listed at iana.org see: https://www.iana.org/assignments/hpke/hpke.xhtml
Any movement on this? I'm working on a project that needs to use hpke with secp256k1. It would be fine, IMO, to see this gated behind a |
@erskingardner, Yes @nyonson and I are working on a version that uses the You can find that branch Here: https://github.com/DanGould/rust-hpke/tree/secp Since chacha20poly1305 is also a bitcoin-native dependency now, there's also an effort to get those primitives into I'm curious what rozbb thinks of using that dependency for secp. |
Hi all. Apologies for the stall. I’m happy to merge now into a separate branch. I don’t see a strong reason not to put this in main beyond what I said above. I’ll discuss on the RustCrypto Zulip shortly and make a decision. In the meantime, merging. Thank you!! |
Re using libsecpk256, I’m much more inclined to keep it as-is, ie with |
I plan to use the C-based version because that's already used across the rust-bitcoin and nostr ecosystems, which is where my use case (and presumably @erskingardner's) for HPKE lies. Some targets (like Cash App) would likely refuse to ship both the C-based libsecp256k1 and rust-k256 in one binary. |
Thanks for the quick turnaround here. @DanGould My situation is a bit different. The OpenMLS library that I'm working in uses the RustCrypto k256 library which in turn uses this library. However, OpenMLS has built the library to allow for library users to select which crypto library they want to use so I'm looking at whether it makes sense to try and add a simple wrapper there to use the rust-secp256k1 library instead of using the RustCrypto based version. From everyone I've talked to, the rust-secp256k1 library is the one that most in the bitcoin/nostr space are using so it'd be best to align there eventually. Admittedly, I'm still learning my way around Rust and the Rust ecosystem of crates though so you guys shouldn't base any decisions on what I'm doing at this point. 😅 JS and Ruby to Rust has been a steep learning curve. One other note. I emailed the guy who proposed adding secp256k1 to the HKPE RFC. I'm pasting his response below but tl;dr, sounds like IANA has approved and given a number to secp256k1 DHKEM in the 9180 RFC, it's just that the updates don't propagate back to the actual RFC doc. 🤷♂️ In any case, this change is actually official so we probably shouldn't refer to it as unstable or off-spec.
|
Ok understood. There are some ambiguities in the RFC though. |
JFYI - I've created a PR on another HPKE crate |
@erskingardner While I would like to ultimately land this feature into this crate so it can be used easily with |
Thanks for the heads up @DanGould. I guess we'll have multiple libraries doing hpke with secp256k1, no harm there since different downstream libraries are likely to use different hpke implementations. I'm having an issue currently with the test_vectors from the HPKE RFC, doesn't look like your tests try and test against those? |
I see your tests here and since the rust-hpke crate here wasn't doing using those test vectors, I wasn't either, but perhaps I should. Seems critical to ensure compatibility across multiple implementations. |
Yeah, I agree. We need to have some test vector coverage. The newer test vectors from the secp256k1 addition have a few issues. The The |
Close #50
Supersede #52 by filling in the test cases.
I found no k256 test vectors including a
K256_DH_RES_XCOORD
and so calculated one myself using a separate production ecdh library, libsecp256k1.Test vector keys were taken from the draft's ChaCha20-Poly1305 first base test case. The test vector ecdh x coordinate was calculated using libsecp256k's rust bindings with this code since the "secp256k1-based DHKEM for HPKE" doc only displays the "shared_secret" which has been extracted and expanded, not a DH result x coordinate.