mirage / hacl

Archived. Curve25519 support has been integrated into mirage-crypto-ec (via fiat-crypto). Hacl bindings are available from the hacl-star opam package. OCaml bindings for HACL* elliptic curves
https://github.com/mirage/mirage-crypto
Other
20 stars 5 forks source link

This conflicts with tezos bindings (gitlab.com/nomadic-labs/ocaml-hacl) #32

Open pirbo opened 4 years ago

pirbo commented 4 years ago

Currently, if you try to opam install tezos-node, you end up with

/usr/bin/ld: /home/pirbo/.opam/4.09.1+flambda/lib/hacl/libhacl_stubs.a(hacl_stubs.o): in function `ml_Hacl_Curve25519_crypto_scalarmult':
/home/pirbo/.opam/4.09.1+flambda/.opam-switch/build/hacl.0.3/_build/default/src/hacl_stubs.c:94: multiple definition of `ml_Hacl_Curve25519_crypto_scalarmult'; /home/pirbo/.opam/4.09.1+flambda/lib/hacl_x25519/libhacl_x25519_stubs.a(hacl_x25519_stubs.o):/home/pirbo/.opam/4.09.1+flambda/.opam-switch/build/hacl_x25519.0.1.1/_build/default/src/hacl_x25519_stubs.c:6: first defined here
collect2: error: ld returned 1 exit status
File "caml_startup", line 1:
Error: Error during linking

I'm opening this issue to track the short and mid terms solutions to that:

Short term: I'll declare that hacl (tezos) is in conflict with hacl_x25519 (this) in ocaml/opam-repository. This way, when you install tezos-node, you'll install tls.0.11.1 that does not depend upon this and we should be fine. (ocaml/opam-repository#16435)

Mid term: the project everest will release itself bindings for hacl in opam: ocaml/opam-repository#16243. It looks to me like nice clean work. Especially, it is spitted in 2 separate libraries: hacl-star-raw which is supposed to be strictly "HaCL C functions exposed in OCaml" and hacl-star which tries to offer a more OCamly API for HaCL capabilities. Tezos is the sponsor of hacl-star and we'll update to use it asap. I understand that mirage has special needs and does its own overlay but could we imagine to share hacl-star-raw?

msprotz commented 4 years ago

Curious to hear about Mirage's special needs. We're hoping that the OCaml bindings we're submitting are the one package to rule them all ™️.

If you need to cherry-pick a single algorithm, we've also streamlined that use-case now, meaning that you should be able to cherry-pick just the files you need from our self-contained snapshot, without having to assemble kremlib and stuff yourself, it's all under version control now.

Let us know, happy to help.

avsm commented 4 years ago

We do in the current release need special runes to build C bindings with our various unikernels backends. But luckily, as soon as https://github.com/mirage/mirage/pull/1153 is merged, this will happen automatically as we will build them in dedicated dune workspaces with the right CFLAGS. So I'm hoping this is a short term thing.

@pirbo does Tezos need TLS 1.3? We could look into renaming C stubs in our package until we can match with upstream.

dinosaure commented 4 years ago

As @avsm said, we currently need a cross compilation of C files provided by hacl to be able to compile tls (1.3 supports) in any targets (eg. Solo5 and Xen). Such constraint, as @avsm said should be fixed by the dunification of MirageOS. However, for a court term, we must respect expected layout needed by the current mirage compilation.

I look into your PR for a long time and it seems very interesting for us! Again, the simpler patch to avoid any conflicts should be to rename C functions. However this package needs to exists as long as mirage/mirage#153 is not merged.

A long term solution should be to replace this package by your package finally to avoid duplicate of code. (/cc @hannesm @samoht)

avsm commented 4 years ago

@msprotz just one thing is missing from your generated bindings: dune files. If you have those, then dune takes care of passing all the various cross compilation runes we need. Would it be possible to add those to hacl-raw, you think?

pirbo commented 4 years ago

I would say that tezos can live with a tls that is not able to speak TLS 1.3 for now. tls is used for the http based rpc server of tezos-node which must be restricted to listen on a controled network space! So we can live with only TLS 1.2 on the local link/VPNs :-)

msprotz commented 4 years ago

the hacl-star package uses dune, but the hacl-star-raw package does not... it uses a Makefile... the reason is as follows:

We briefly contemplated building only the OCaml part with dune, but I'm no Dune expert and it seemed dubious to have Dune rely on an object (libevercrypt.{a,so}) that was produced by a previous build system. (Also, ctypes bindings are hard to build, and Dune didn't have built-in support back then.)

What do you advise @avsm? Have a second build system that relies on Dune for everything? Switch to a hybrid build system where the native C parts are built with a Makefile and the OCaml part is built with Dune?

avsm commented 4 years ago

Thanks for the details. I think a hybrid build system as you describe makes most sense.

The OCaml dune file is fairly straightforward; you just specify the names of the C files in the dune (library) stanza, and it takes care of building the .so and the META files and the ocamlmklib invocations required. The big advantage from an OCaml downstream perspective is that the build then composes with other dune projects -- for example, Mirage uses a dune-workspace file to cross compile all C bindings using special kernel CFLAGS, and that will just happen automatically if hacl is bundled with a dune file.

That final makefile you link to looks pretty clean. I think it's just a matter of replacing ctype.depend and the bottom half of the Makefile with a generated dune file instead, and then the usual dune build runes will work. The C rules of course stay untouched.

@dinosaure @samoht or I can advise on this if you're stuck at any point.

dinosaure commented 4 years ago

Mirage uses a dune-workspace file to cross compile all C bindings using special kernel CFLAGS, and that will just happen automatically if hacl is bundled with a dune file.

If the Makefile of hacl-star-raw give to us the ability to tweak CFLAGS, dune is able to call it as a foreign archive. A possible solution is to provide a dune file which call the Makefile with CFLAGS given by the dune-workspace.

An example of such case is available here. Considering hacl-star-raw as package, it will depends on dune and ctypes finally.

msprotz commented 4 years ago

The Makefile should honor CC and CFLAGS, yes. If you're able to give it a go and submit a PR for the hacl-star repository, that would be super helpful -- I'm afraid my dune skills are too limited to try it myself. There's also a configure script to run.