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

Use the standard way to hash data to a curve point/scalar #35

Closed
fjarri opened this issue Mar 3, 2021 · 3 comments · Fixed by #92
Closed

Use the standard way to hash data to a curve point/scalar #35

fjarri opened this issue Mar 3, 2021 · 3 comments · Fixed by #92
Labels
ABI Changes the format of serialized objects blocked Blocked by a third-party library cryptography Needs attention of someone who knows what they're doing enhancement New feature or request
Milestone

Comments

@fjarri
Copy link
Contributor

fjarri commented Mar 3, 2021

At the moment unsafe_hash_to_point() uses a simple ad-hoc algorithm. There is a (draft) standard for such hashing: https://www.ietf.org/archive/id/draft-irtf-cfrg-hash-to-curve-10.html

We also need to use the standard hash_to_field() to hash to Scalar (see ScalarDigest), since there may be problems when SHA256 is used for that without the expansions (as is the case in RustCrypto libraries now).

Seems like it would be a good practice to use the standard, but the required implementation is quite bulky and needs access to some internals not currently exposed by the RustCrypto ECC stack we use. It would be much better if it was implemented there. Tracking issue: RustCrypto/traits#481

As an alternative, we could use the reference implementation and convert its result to a RustCrypto object. Is it worth all the extra dependencies? (The dependencies might only be used at build time if #3 is fixed by implementing a build-stage macro).

Reference implementation:
https://github.com/armfazh/redox-ecc
https://github.com/armfazh/h2c-rust-ref

@fjarri fjarri added enhancement New feature or request cryptography Needs attention of someone who knows what they're doing blocked Blocked by a third-party library labels Mar 3, 2021
@fjarri fjarri changed the title Use a standard way to hash data to a curve point Use the standard way to hash data to a curve point/scalar Mar 6, 2021
@fjarri fjarri added this to the v1.0.0 milestone Jun 13, 2021
@fjarri fjarri added the ABI Changes the format of serialized objects label Jun 21, 2021
@cygnusv
Copy link
Member

cygnusv commented Aug 4, 2021

Given that unsafe_hash_to_point() is only used to compute a parameter in a nothing-up-my-sleeve way (and not because there's actual security requirement for the Umbral scheme), I don't think we should spend more effort here, specially after the new developments with nube.

@fjarri
Copy link
Contributor Author

fjarri commented Aug 19, 2021

Status update: I suppose unsafe_hash_to_point() can be stabilized as it is, and doesn't need to wait for a reference hash-to-curve implementation. On the other hand, hashing to scalars can still be changed to conform to standard (possibly in RustCrypto stack). See discussion in #64 for more details.

To recap, we need to stabilize two things:

  1. Hashing to non-zero scalars. While an astronomically small possibility of getting a zero is probably not worth worrying about, a rigorous implementation (taking a modulo order - 1) will change the results and break compatibility.
  2. Using a big enough hash size to modulo reduce. Currently RustCrypto uses 32 bytes, while the standard requires 64 bytes.

@fjarri
Copy link
Contributor Author

fjarri commented Dec 30, 2021

Update, after a discussion with @cygnusv:

Hashing to non-zero scalars. While an astronomically small possibility of getting a zero is probably not worth worrying about, a rigorous implementation (taking a modulo order - 1) will change the results and break compatibility.

k256 now has this functionality available, and we will use it, which is standard enough for our purposes.

Using a big enough hash size to modulo reduce. Currently RustCrypto uses 32 bytes, while the standard requires 64 bytes.

We want to preserve the compatibility with Ethereum contracts, and only 32-byte hashes are available there. We could concatenate two 32-byte hashes, like hash(x || 0) || hash(x || 1), but it is not necessary - we will reduce from a 32-byte output.

So all in all, the remainders of this issue should be fixed in #87.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ABI Changes the format of serialized objects blocked Blocked by a third-party library cryptography Needs attention of someone who knows what they're doing enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants