Skip to content

Fix x25519 and ed25519 if Cstruct with off <> 0 is given as input #118

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

Merged
merged 2 commits into from
Apr 8, 2021

Conversation

hannesm
Copy link
Member

@hannesm hannesm commented Apr 8, 2021

Looks like I'm always making the same mistakes (and should really find (or develop) tooling which is more strict when passing pointers from OCaml to C): when passing a Cstruct.t (well, the underlying buffer), take care to add the "off" to the pointer. I reviewed the Mirage_crypto_ec module (with the assumption that Cstruct.create will always create something with off = 0) and couldn't find more cases where the off needs to be added (the tradeoff is clearly "number of arguments passed from OCaml to C").

Anyways, this PR contains some fixes, and when I reviewed the NIST curves I spotted some easy small pieces where to avoid allocations.

hannesm added 2 commits April 8, 2021 18:46
…ffset <> 0

when passing a Cstruct.buffer to C, we need to ensure that either offset = 0
or take that offset into account (adding that to the pointer). Enhance tests
of X25519, Ed25519, ECDSA, and ECDH with such a case (to avoid problems in the
future).
@hannesm hannesm merged commit b0030d3 into mirage:main Apr 8, 2021
@hannesm hannesm deleted the fix-x25519 branch April 8, 2021 17:36
hannesm added a commit to hannesm/opam-repository that referenced this pull request Apr 8, 2021
…ge-crypto-rng, mirage-crypto-rng-mirage and mirage-crypto-rng-async (0.9.2)

CHANGES:

- mirage-crypto-ec: fix X25519 and Ed25519 if called with Cstruct.t whose
  offset is not equal to 0. Add unit tests to avoid the same issue in the
  future (mirage/mirage-crypto#118 by @hannesm)
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.

1 participant