NIST elliptic curves via mirage-crypto-ec #101

hannesm · 2021-02-03T12:05:24Z

This PR integrated elliptic curves into this repository. It is based on fiat, and due to some dependencies for ECDSA (compute k in a deterministic way, hash algorithms) it has been imported into this repository.

The API (Mirage_crypto_ec) is not yet set into stone (multiple NIST curves should be reflected, documentation is lacking). It is work in progress, with the main goal here to test with the various CI systems in place.

The extraction process (from fiat-crypto) is done via GNUmakefile in ec/native. Note that it has been updated, and the fiat-crypto provided inversion is used now (previously, there was a fe_inv). The test cases pass, though ECDSA may need some Cstruct.rev adaptions, and more tests (from wycheproof) are needed for ECDSA.

Hope that @NathanReb @emillon @pascutto are fine with the code import here.

Sponsored by: Nitrokey GmbH

…E.md)

This is an import of https://github.com/mirage/fiat from commit 341318ad08b9224c87ed4bd26b29cebb46e10812 (with a rename of fiat_p256 to mirage_crypto_ec). The motivation is to extend "fiat-p256" in multiple dimensions: more curves (NIST, ..), and also ECDSA - which requires hash functions, and computation of a deterministic k (RFC 6979) which is already part of this repository. Co-Authored-By: Nathan Rebours <[email protected]> Co-Authored-By: Etienne Millon <[email protected]> Co-Authored-By: Clément Pascutto <[email protected]>

JasonGross · 2021-02-04T17:07:03Z

The extraction process (from fiat-crypto) is done via GNUmakefile in ec/native.

If you're already using dune and OCaml, perhaps you want to depend on our opam package which installs the relevant binaries to %{bin}%? I'm happy to release a non-dev version of fiat-crypto if you'd prefer that, or update the opam package if it doesn't currently meet your needs.

hannesm · 2021-02-04T21:06:50Z

@JasonGross thanks for your pointer to the opam package. This sounds like a nice way forward on documenting (and running) how to re-do the extraction. For the common OCaml user, I don't think it is reasonable to generate the C code themselves (but instead we'll ship pre-generated C code).

JasonGross · 2021-02-04T21:19:18Z

For the common OCaml user, I don't think it is reasonable to generate the C code themselves (but instead we'll ship pre-generated C code

Indeed, I was just suggesting that perhaps your makefile setup, presumably there for documentation / regeneration purposes, can be configured for the opam-based setup rather than assuming fiat-crypto in a parent directory.

mirage-crypto-ec.opam

avsm · 2021-02-17T15:34:29Z

.cirrus.yml

+  dependencies_script: eval `opam env` && opam pin add -y mirage-crypto.dev . && opam pin add -y mirage-crypto-rng.dev . && opam pin add -y mirage-crypto-pk.dev . && opam pin add -y mirage-crypto-ec.dev . && opam install -y -t --deps-only mirage-crypto mirage-crypto-rng mirage-crypto-pk mirage-crypto-ec
+  build_script: eval `opam env` && opam exec -- dune build -p mirage-crypto,mirage-crypto-rng,mirage-crypto-pk,mirage-crypto-ec
+  test_script: eval `opam env` && opam exec -- dune runtest -p mirage-crypto,mirage-crypto-rng,mirage-crypto-pk,mirage-crypto-ec


minor: the eval is not necessary here as the various opam commands will add the right env variables

hannesm · 2021-02-22T17:22:50Z

current status:

P-224 (ECDH & ECDSA & test cases from RFC 6979 and wycheproof, 32 and 64 bit)
P-256 (ECDH & ECDSA & test cases from RFC 6979 and wycheproof, 32 and 64 bit)
P-384 (ECDH & ECDSA & test cases from RFC 6979 and wycheproof, 32 and 64 bit)
P-521 (ECDH & ECDSA & test cases from wycheproof, 32 and 64 bit, RFC 6979 test cases fail since k is computed differently (bit_length % 8 <> 0)

Left items:

validate API (by using it in X509 & TLS & ...)
write documentation for public API

Delayed until later:

SECP256K1 (needs more thorough investigation)
RFC6979 test cases for P521 (will be delayed until someone has a good idea avoiding bit-fiddling)
workflow to extract code via an opam switch / CI / ...

Reviews / comments / feedback appreciated :) [a different step / PR is to integrate 25519 (ECDH & EdDSA) and 448 (ECDH & EdDSA)]

hannesm · 2021-02-24T17:46:20Z

I added priv/pub_to/of_cstruct, and with hannesm/ocaml-x509@c063ff7 I can successfully decode & encode PKCS8 (private key) data.

avsm · 2021-02-24T20:00:35Z

ec/native/p224_stubs.c

+CAMLprim value mc_p224_sub(value out, value a, value b)
+{
+	CAMLparam3(out, a, b);
+	fiat_p224_sub(Caml_ba_data_val(out), Caml_ba_data_val(a), Caml_ba_data_val(b));
+	CAMLreturn(Val_unit);


Most of the C calls of this class could be marked noalloc since they never raise or allocate values in the C binding. That would save some overhead in calling them.

However, if the fiat functions are very CPU intensive, it may be better to leave them as alloc functions and release the global runtime lock instead during the actual C crypto.

Uhm, this is done at the OCaml declaration-side (e.g. for P-224 at https://github.com/mirage/mirage-crypto/pull/101/files#diff-b65c54aec12f050b8540b66b5b49e4fc2289e3137ec35bd9a7ef23f9dec38ec8R611-R638). Anything else that is needed there, or any occassion I forgot to mark [@@noalloc]?

Essentially the question above: if these are computationally intensive and could take some time, then we should drop the runtime lock and not make them noalloc.

Essentially the question above: if these are computationally intensive and could take some time, then we should drop the runtime lock and not make them noalloc.

It should be easy due to the use of non-relocatable bigarrays.

CPU-intensive is a comparative term. Certainly these functions are the CPU-hotspots in performing curve operations, but the time spent in any single invocation is quite small. The are some numbers in Andres Erbsen, Jade Philipoom, Jason Gross, Robert Sloan, Adam Chlipala. Simple High-Level Code For Cryptographic Arithmetic -- With Proofs, Without Compromises. Proceedings of the IEEE Symposium on Security & Privacy 2019 (S&P'19). May 2019. The table on page 10 shows that running the entire scalar multiplication (which will call these functions a handful of times) takes about 1k--3k CPU cycles. The plot in in Fig. 10 corresponds to 1000 sequential
computations of a 256-bit Montgomery ladder, and even the slowest (~500 bit primes on 32-bit machines) amount to less than 0.001--0.01 seconds per call.

Additionally, these primitives are low-level, intended to be used to implement higher-level curve operations. I expect that the boundary of these functions is not the place to be releasing runtime locks, and that should instead happen at the boundary of the curve-level operations (which, I believe, should also not be allocating anything on the heap). Another point in this direction is that ideally we'd like to generate the curve-level operations in C as well and expose that API, rather than this lower-level one.

hannesm · 2021-03-05T22:17:12Z

@EduardoRFS any idea what the root cause of the cross compile failures is? are they temporary?

This includes: - ECDSA implementation with blinding - tests from RFC 6979 (deterministic k) - wycheproof tests for ECDH and ECDSA (commit 2196000605e45d91097147c9c71f26b72af58003) - ec/native contains a Makefile which runs the extraction (using fiat-crypto's word-by-word-montgomery)

hannesm · 2021-03-05T22:24:46Z

this is ready to be merged: some initial documentation is included in this PR, also the P521 32bit code has been generated and added. I squashed the development commits together, and am fine with the remaning ones.

from the comment above, future PRs may:

add more curves (SECP_K1, require a different implementation of scalar_mult)
reduce allocations (or in general, improve performance)
adapt 'deterministic k' to use the bit size of the curve (and have p521 rfc6979 tests not fail)
add edwards curves (25519, 448) with dh and eddsa

EduardoRFS · 2021-03-05T22:39:19Z

@hannesm yeah they're, github actions connections seems to be failing, will be addressing it tomorrow, feel free to ignore it

@pascutto

…ge-crypto-rng, mirage-crypto-rng-mirage and mirage-crypto-rng-async (0.9.0) CHANGES: - Elliptic curve support in the new package mirage-crypto-ec The arithmetic code is generated by [fiat-crypto](https://github.com/mit-plv/fiat-crypto), a development in Coq which includes proofs of constant time behaviour. The generation can be reproduced (see ec/native/GNUmakefile). The group operation implementations are taken from BoringSSL. The high-level mechanisms (signature DSA and key exchange ECDH) are implemented in OCaml. The ECDSA implementation (as our DSA one) uses a deterministic k (RFC 6979). The NIST curves P224 (SECP224R1), P256 (SECP256R1), P384 (SECP384R1), and P521 (SECP521R1) are supported (ECDH and ECDSA), in addition to Curve25519 (X25519 and Ed25519). Performance of X25519 has been measured and is roughly the same as the hacl_x25519 and also the hacl opam package (see mirage/mirage-crypto#107 for numbers). Tests vectors are from RFCs and wycheproof. Import mirage/fiat repository (@pascutto @emillon @NathanReb @hannesm mirage/mirage-crypto#101) Check bounds of message (reported by @greg42, fixed by @hannesm mirage/mirage-crypto#108) Remove blinding, since constant time arithmetics is used (@hannesm mirage/mirage-crypto#106) Curve 25519 (X25519 & Ed25519) support (@hannesm mirage/mirage-crypto#107 imported from BoringSSL) Partially reviewed by @JasonGross @avsm @dinosaure Partially sponsored by Nitrokey GmbH

hannesm added 2 commits February 1, 2021 19:25

move freestanding to src/freestanding

5a13873

mirage-crypto-rng-mirage.opam: Adjust license (from rng/mirage/LICENS…

5326083

…E.md)

hannesm force-pushed the ec branch 4 times, most recently from 3174b26 to c00c7ac Compare February 3, 2021 14:02

hannesm force-pushed the ec branch 4 times, most recently from ced07ac to d18d2d8 Compare February 3, 2021 22:25

hannesm mentioned this pull request Feb 4, 2021

ECDSA sample code mit-plv/fiat-crypto#885

Closed

hannesm force-pushed the ec branch 4 times, most recently from 94467d8 to ecb3e3d Compare February 11, 2021 16:24

dinosaure reviewed Feb 17, 2021

View reviewed changes

mirage-crypto-ec.opam Outdated Show resolved Hide resolved

avsm reviewed Feb 17, 2021

View reviewed changes

hannesm force-pushed the ec branch from 6a37f53 to 06a1324 Compare February 18, 2021 19:25

hannesm force-pushed the ec branch from b1b249a to 8cc1c07 Compare February 23, 2021 11:16

avsm reviewed Feb 24, 2021

View reviewed changes

hannesm force-pushed the ec branch 4 times, most recently from 34c6359 to 53c6363 Compare March 5, 2021 11:05

hannesm force-pushed the ec branch from 53c6363 to 0a6a430 Compare March 5, 2021 11:46

GitHub actions: add 4.12, update to 4.11.2, 4.10.2, 4.09.1

a85f4fc

hannesm force-pushed the ec branch 2 times, most recently from 724b2a6 to 1243e85 Compare March 5, 2021 11:50

hannesm added 2 commits March 5, 2021 23:19

CI: remove formatting

523f2fd

hannesm force-pushed the ec branch from 9d752fa to 5e042bb Compare March 5, 2021 22:20

hannesm merged commit 14a306e into mirage:main Mar 6, 2021

hannesm deleted the ec branch March 6, 2021 16:20

hannesm mentioned this pull request Mar 6, 2021

workflow for extraction from fiat #104

Open

hannesm mentioned this pull request Mar 31, 2021

[new release] mirage-crypto-pk, mirage-crypto-ec, mirage-crypto, mirage-crypto-rng, mirage-crypto-rng-mirage and mirage-crypto-rng-async (0.9.0) ocaml/opam-repository#18424

Merged

smondet mentioned this pull request Nov 30, 2023

Support for secp256k1 #187

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

NIST elliptic curves via mirage-crypto-ec #101

NIST elliptic curves via mirage-crypto-ec #101

hannesm commented Feb 3, 2021

JasonGross commented Feb 4, 2021

hannesm commented Feb 4, 2021

JasonGross commented Feb 4, 2021 •

edited

Loading

avsm Feb 17, 2021

hannesm commented Feb 22, 2021 •

edited

Loading

hannesm commented Feb 24, 2021 •

edited

Loading

avsm Feb 24, 2021

hannesm Feb 27, 2021

avsm Mar 3, 2021

dinosaure Mar 3, 2021

JasonGross Mar 3, 2021

hannesm commented Mar 5, 2021

hannesm commented Mar 5, 2021

EduardoRFS commented Mar 5, 2021

NIST elliptic curves via mirage-crypto-ec #101

NIST elliptic curves via mirage-crypto-ec #101

Conversation

hannesm commented Feb 3, 2021

JasonGross commented Feb 4, 2021

hannesm commented Feb 4, 2021

JasonGross commented Feb 4, 2021 • edited Loading

avsm Feb 17, 2021

Choose a reason for hiding this comment

hannesm commented Feb 22, 2021 • edited Loading

hannesm commented Feb 24, 2021 • edited Loading

avsm Feb 24, 2021

Choose a reason for hiding this comment

hannesm Feb 27, 2021

Choose a reason for hiding this comment

avsm Mar 3, 2021

Choose a reason for hiding this comment

dinosaure Mar 3, 2021

Choose a reason for hiding this comment

JasonGross Mar 3, 2021

Choose a reason for hiding this comment

hannesm commented Mar 5, 2021

hannesm commented Mar 5, 2021

EduardoRFS commented Mar 5, 2021

JasonGross commented Feb 4, 2021 •

edited

Loading

hannesm commented Feb 22, 2021 •

edited

Loading

hannesm commented Feb 24, 2021 •

edited

Loading