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

Rationale for ecdh_skip_hash_extract_x_and_y #1

Closed
mikekelly opened this issue Jun 30, 2022 · 3 comments
Closed

Rationale for ecdh_skip_hash_extract_x_and_y #1

mikekelly opened this issue Jun 30, 2022 · 3 comments

Comments

@mikekelly
Copy link

This libarary is great, I really like the minimalist API but I'm having interoperability issues using it for ECDH.

I believe it is due to PrivateKey.sharedSecret using ecdh_skip_hash_extract_x_and_y

ecdh_skip_hash_extract_x_and_y, // hashfp

The result is Data that when put through SHA256.hash produces a inconsistent result from libraries like this:

https://github.com/bitcoin-core/secp256k1/blob/185a6af22792531a629959834fff9257e396abb5/src/modules/ecdh/main_impl.h#L13-L24

I was wondering if this was an intentional design decision and what the rationale was?

@Sajjon
Copy link
Owner

Sajjon commented Jun 30, 2022

👋 thanks a lot! Thanks for this, will investigate next week! Can't remember now why I did it like that. Can add parameter to swift function maybe? So the hash function can be chosen.

@mikekelly
Copy link
Author

mikekelly commented Jun 30, 2022

yeah, combined with an enum of the available functions (defaulting to .ecdh_skip_hash_extract_x_and_y, so the API doesn't break) would be a nice API :)

@Sajjon
Copy link
Owner

Sajjon commented Mar 16, 2023

@mikekelly First of all, so so sorry for slowest response every! I was busy with other work. In #7 I've solved this issue, I now vendor three different ECDH variants, one of which is CryptoKit compatible, because it returns CryptoKit's SharedSecret struct which is really nice, since offers KDFs!

One which follows standards libsecp256k1 and lastly the old one (custom, no hash), please see the PR #7 for details.

I will make a new release 0.0.9 with these new APIs

@Sajjon Sajjon closed this as completed Mar 16, 2023
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

No branches or pull requests

2 participants