mirage / digestif Goto Github PK

It could be useful to be able to feed a hmac in a similar way to how a digest is fed. This allows computing a hmac over data that can't fit in memory at once. I can probably suggest a change.

I forked the Blake2 OCaml implementation to add the personalisation, with some tests from Zcash, see https://gitlab.com/dannywillems/ocaml-ec/-/merge_requests/18, directory src/digestif.
I am happy to make an MR, but a discussion for the interface is necessary. The fork in ocaml-ec changes it slightly and only uses blake2.

Now that BLAKE3 is released, would be nice to add support for it.

My dune project uses digestif and just adds digestif.cto the list of libraries:
(libraries digestif.c ....)

- and it works. However with version 0.6.1 inline tests no longer link, resulting in the error:

Error: Files /home/afu/.opam/4.06.1/lib/digestif/c/digestif_c.cmxa
       and /home/afu/.opam/4.06.1/lib/digestif/rakia/rakia.cmxa
       both define a module named Rakia

building regular binaries still works with 0.6.1
Switching to digestif.ocaml` does solve the linking problem.

Could it be that version 0.6.1 requires different linking? I see some mention in the release notes for version 0.6.1.

See @cfcs comment on #70

This leads to undefined behavior if off or len are out of bounds.
I think that's fine for the unsafe_feed_bigstring interface, but I am a bit skeptical about the lack of length validation in feed_bigstring.

BLAKE2b (or just BLAKE2) is optimized for 64-bit platforms—including NEON-enabled ARMs—and produces digests of any size between 1 and 64 bytes
BLAKE2s is optimized for 8- to 32-bit platforms and produces digests of any size between 1 and 32 bytes

I need a possbility to set the digest size :)

Currently, because digestif uses dune's variants, we are not able to trick on the META file. By this way, the last release of digestif (0.8.0) is not compatible with MirageOS. Due to the uncertain way of the integration of C stubs with MirageOS, I decided to keep the compatibility in an other (non usual) way - eg. ocaml/opam-repository#15768.

However, this solution is not great and we should integrate this patch into the distribution (instead to have a free file into the OPAM repository) - and update the digestif's OPAM file.

I couldn't find a link to the HTML doc here, but there's doc in the ocaml hub here: https://docs.ocaml.pro/docs/OPAM.digestif.0.9.0/index.html
unfortunately the link to Digestif is broken

On another note, the code doc of feed_bytes refers to feed, which is not there https://github.com/mirage/digestif/blob/master/src/digestif.mli#L36

I just cloned the mirageos website repo and run
make switch (Which worked fine)
and then
make deps
Output:

#=== ERROR while compiling digestif.1.1.1 =====================================#
# context     2.1.0 | linux/x86_64 | ocaml.4.14.0 | git+https://github.com/mirage/mirage-dev.git
# path        ~/gits/mirage-www/_opam/.opam-switch/build/digestif.1.1.1
# command     ~/.opam/opam-init/hooks/sandbox.sh build ./install/install.ml
# exit-code   2
# env-file    ~/.opam/log/digestif-238769-bd9bc9.env
# output-file ~/.opam/log/digestif-238769-bd9bc9.out
### output ###
# ./install/install.ml: 3: let: not found
# ./install/install.ml: 4: freestanding_linkopts = "-l:libdigestif_freestanding_stubs.a": not found
# ./install/install.ml: 6: let: not found
# ./install/install.ml: 7: match: not found
# ./install/install.ml: 8: Syntax error: "|" unexpected

OS: Linux Mint 20.02 Uma
OCaml: 4.14
Opam: 2.1.0
Dune: 3.0.3

Even if it's interesting to use GADT here, at some point, it can be painful to use them along a big project. So we probably should also add a polymorphic variant.

In some context, it can be useful to get the hash as a bytes instead of a string. It's an outcome of digestif, so the user is responsible about this bytes. It permits to not use Bytes.unsafe_to_string in this context.

Hey,

I'm not entirely sure I understand the API design. We have two types, ctx and t -- but what is the fundamental difference?

A ctx we can create by either empty or init -- hold on, should the ctx returned by empty be immutable or better not being used?

A t we receive from get, but also from the functions that take care of computing the hash of a string / string list /...

Now, the nicely introduced get_into_bytes is defined on ctx, so if I computed the hash with digest_string, I can't extract it into an existing byte vector.

Would it be possible to simplify the API and only provide a single type t? Since it is private anyways, I'd expect no issues... What do you think?

I need it for Bitcoin.

First brought up here: #30 (comment)

see also my comment below: #30 (comment)

I'd be happy to adapt the code example I posted into a PR if you would like?
ping @dinosaure

Hi,

compare relies on caml_hash, but this function has collisions that are easy to find (in particular because of its small codomain).

This behaviour is demonstrated by the following program:

let s1 = "O-)"

let s2 = "$@$"

let () =
  let open Digestif.SHA256 in
  let d1 = digest_string s1 in
  let d2 = digest_string s2 in
  Printf.printf
    "compare:        %d\nunsafe_compare: %d\neq:             %B\n"
    (compare d1 d2)
    (unsafe_compare d1 d2)
    (eq d1 d2)

Output:

compare:        0
unsafe_compare: -1
eq:             false

Thanks!

When compiling with digestif.ocaml, the SHA256 hash result obtained via feeding and getting does not match the Nocrypto's one.

But when compiled with digestif.c, the hash matches.

When compiled with digestif.ocaml, it seems that the hashes mismatch as well if feeding size is different for each round.

I have not tried other hash algorithms.

Please let me know if you want me to provide full code to demonstrate the issue.

I'm not sure that neq is needed here, wouldn't it be easier to just wrap eq in not ( .. ) ?

Speaking of interface, it would be useful if eq was constant-time so it could be used in security-sensitive applications (see https://codahale.com/a-lesson-in-timing-attacks/ for an example of a bug caused by non-constant comparison).

Currently, digestif provides:

val hmac_string : key:string -> ?off:int -> ?len:int -> string -> t
val hmac_bytes : key:bytes -> ?off:int -> ?len:int -> bytes -> t
val hmac_bigstring : key:bigstring -> ?off:int -> ?len:int -> bigstring -> t

A look into GitHub repositories which use digestif (which is not relevant) shows me that nobody uses hmac_{bytes,bigstring}. The purpose of the key is to be hidden and constant. So we probably should enforce the fact that the key must be a string for all of these functions.

This is a proposal of @hannesm and I agree with that. If someone has some regards about this breaking-changes, this issue exists for that 👍. I will let this issue open for 1 week, then, I will propose a pull-request according this issue.

See https://github.com/mirage/digestif/blob/master/src-c/digestif.ml#L133 and https://github.com/mirage/digestif/blob/master/src-ocaml/digestif.ml#L119.

Then, Digestif_eq.eq is invert result.

By the trick to link with the C stub or the pure OCaml implementation, ocamlbuild can not make a documentation.

Running the test-suite of digestif-0.8.0 with alcotest-1.0.1 yields the following error:

running tests
   test_conv alias test/conv/runtest (exit 2)
(cd _build/default/test/conv && ./test_conv.exe --color=always)
seed: [|118; 4; 12; 129; 19; 6; 209; 128; 205; 167; 23; 143|].
test_conv.exe: internal error, uncaught exception:
               Alcotest__Core.Registration_error("Duplicate test name: of_hex")

Fatal error: exception Alcotest__Core.Make(M).Test_error
        test alias test/c/runtest

Below is a possible (trivial) fix:

--- a/test/conv/test_conv.ml	2020-04-14 07:05:23.347470133 +0200
+++ b/test/conv/test_conv.ml	2020-04-14 07:06:15.580175419 +0200
@@ -98,12 +98,12 @@
 
 let tests () =
   Alcotest.run "digestif"
-    [ "of_hex", List.mapi test_hex_success consistent_hex
-    ; "consistent_of_hex", List.mapi test_consistent_hex_success consistent_hex
-    ; "of_hex", List.mapi test_hex_success spaces_hex
-    ; "consistent_of_hex", List.mapi test_consistent_hex_success spaces_hex
-    ; "of_hex", List.mapi test_hex_success inconsistent_hex
-    ; "consistent_of_hex", List.mapi test_consistent_hex_fail inconsistent_hex
+    [ "of_hex 1", List.mapi test_hex_success consistent_hex
+    ; "consistent_of_hex 1", List.mapi test_consistent_hex_success consistent_hex
+    ; "of_hex 2", List.mapi test_hex_success spaces_hex
+    ; "consistent_of_hex 2", List.mapi test_consistent_hex_success spaces_hex
+    ; "of_hex 3", List.mapi test_hex_success inconsistent_hex
+    ; "consistent_of_hex 3", List.mapi test_consistent_hex_fail inconsistent_hex
     ; ( "iso of_hex"
       , List.mapi test_hex_iso
           (list_init (random_string Digestif.SHA1.digest_size) 64) )

See https://dune.readthedocs.io/en/stable/variants.html#default-implementation

This will fix loading in the toplevel and simplify the usage of the library quite a lot.

maybe related to #92 and to ocaml/opam-repository#15880

 $ opam install digestif
The following actions will be performed:
  - install digestif 0.8.0-1

<><> Gathering sources ><><><><><><><><><><><><><><><><><><><><><><><><><><><><>
[digestif.0.8.0-1] found in cache

<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><>
[ERROR] The compilation of digestif failed at
        "/home/smondet/.opam/opam-init/hooks/sandbox.sh build patch -p1 -i
        mirage.patch".

#=== ERROR while compiling digestif.0.8.0-1 ===================================#
# context              2.0.5 | linux/x86_64 | ocaml-base-compiler.4.09.1 | git+https://gitlab.com/nomadic-labs/opam-repository.git#4c53a2b62737b54f9a9ef6e0b777ad3561ab2407
# path                 .../_opam/.opam-switch/build/digestif.0.8.0-1
# command              ~/.opam/opam-init/hooks/sandbox.sh build patch -p1 -i mirage.patch
# exit-code            1
# env-file             ~/.opam/log/digestif-7159-6d07ae.env
# output-file          ~/.opam/log/digestif-7159-6d07ae.out
### output ###
# patching file _build/default/META.digestif
# Hunk #1 FAILED at 55.
# 1 out of 1 hunk FAILED -- saving rejects to file _build/default/META.digestif.rej

 $ cat .../_opam/.opam-switch/build/digestif.0.8.0-1/_build/default/META.digestif.rej
--- _build/default/META.digestif
+++ _build/default/META.digestif
@@ -55,3 +55,6 @@
     plugin(native) = "rakia_xen.cmxs"
   )
 )
+
+xen_linkopts = "-l:rakia/xen/librakia_xen_stubs.a"
+freestanding_linkopts = "-l:rakia/freestanding/librakia_freestanding_stubs.a"

it's still GNU patch (not freeBSD):

 $ patch --version
GNU patch 2.7.6
Copyright (C) 2003, 2009-2012 Free Software Foundation, Inc.
Copyright (C) 1988 Larry Wall

License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>.
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.

Written by Larry Wall and Paul Eggert

Currently, the GADT defined by digestif is not quite so good when it's not possible to write something like:

val decode : 'hash Digestif.t -> ... -> 'hash t

The definition of the GADT should be:

type _ hash =
  | SHA1 : SHA1.t hash
  | BLAKE2B : BLAKE2B.t hash
  | ...

instead to rely on a polymorphic variant (eg. [ SHA1 | BLAKE2B | ... ]). The old design comes from the special case about BLAKE2{B,S} where the length of the hash can be a parameter. So a BLAKE2B 64 should be different to a BLAKE2B 128 for example. I think it's better to reduce (and be less expressive) possibilities about BLAKE2{B,S} (and stay on default values) over our GADT - then, if someone wants to configure length of the hash, he can but outside the use of the GADT.

This issue should remove this old code.

looking into the API, I was wondering whether you've any plans for adapting mirleft/ocaml-nocrypto@a9f94f8 -- imho this improves the usage in certain scenarios such as streaming (and having feed returning a ctx, rather than mutating, is a good idea imho)!?

Hi,

For BLAKE2, the hmac_* functions do not actually implement HMAC.

More precisely, HMAC is a way to turn any hash function into a MAC. But some hash functions also have a keyed variant which is another MAC function.

Here, Digestif.BLAKE2S.hmac_string implements keyed BLAKE2s, not HMAC-BLAKE2s as expected.

I'm happy to provide a patch, but this will break compatibility obviously.

Let me know what you think.

Hi,
I am porting a personal project (it will be open source soon) from C++ to OCaml.
I think that Whirlpool is a good hash function and I would like to use it in OCaml if possible.

What do you think of a Whirlpool implementation ?

Thanks

Ethereum uses Keccak, i.e. SHA3 with different padding constants (pre-standard version).
Could you please add support?

[1] https://ethereum.stackexchange.com/questions/550/which-cryptographic-hash-function-does-ethereum-use

Hello, I'm working on an implementation of the WireGuard VPN protocol for MirageOS, and I need BLAKE2s for that.

I don't know if this would be the right place to request it / add it, but if it is trivial for you to add it, that would make everything easier for me! <3

Would be nice to have a feed_string function in addition to feed_bytes so we don't have to allocate a Bytes.t to hash a string.

Hi!

I'm computing a hash and I'm trying to access the resulting digest as bytes (not hex-encoded). Is there a way to access the underlying string? At the moment I'm using to_hex and decoding it back, but this seems wasteful.

Thanks!

Please 🙏

I get the following error building mirage/ocaml-git/master with mirage/digestif/master installed:

#=== ERROR while compiling git-unix.dev =======================================#
# context     2.0.0 | linux/x86_64 | ocaml-variants.4.07.1+fp+flambda | pinned(file:///home/g2p/src/storage-unikernel/vendor/ocaml-git)
# path        ~/.opam/4.07.1+fp+flambda/.opam-switch/build/git-unix.dev
# command     ~/.opam/opam-init/hooks/sandbox.sh build dune build -p git-unix -j 3
# exit-code   1
# env-file    ~/.opam/log/git-unix-19321-d0f245.env
# output-file ~/.opam/log/git-unix-19321-d0f245.out
### output ###
# [...]
# /home/g2p/.opam/4.07.1+fp+flambda/lib/digestif/c/digestif_c.a(digestif_conv.o): In function `camlDigestif_conv__code_end':
# :(.text+0xb6d): multiple definition of `camlDigestif_conv__code_end'
# /home/g2p/.opam/4.07.1+fp+flambda/lib/digestif/digestif.a(digestif_conv.o)::(.text+0xb6d): first defined here
# /home/g2p/.opam/4.07.1+fp+flambda/lib/digestif/c/digestif_c.a(digestif_conv.o): In function `camlDigestif_conv__data_end':
# :(.data+0x478): multiple definition of `camlDigestif_conv__data_end'
# /home/g2p/.opam/4.07.1+fp+flambda/lib/digestif/digestif.a(digestif_conv.o)::(.data+0x478): first defined here
# /home/g2p/.opam/4.07.1+fp+flambda/lib/digestif/c/digestif_c.a(digestif_conv.o): In function `camlDigestif_conv__frametable':
# :(.data+0x480): multiple definition of `camlDigestif_conv__frametable'
# /home/g2p/.opam/4.07.1+fp+flambda/lib/digestif/digestif.a(digestif_conv.o)::(.data+0x480): first defined here
# collect2: error: ld returned 1 exit status
# File "caml_startup", line 1:
# Error: Error during linking

This line might be the culprit:

src-c/dune:(rule (copy# %{lib:digestif:digestif_conv.ml} digestif_conv.ml))

Here are the rest of the logs:
git-unix-19321-d0f245.out.txt

in ocaml/opam-repository#20029 we found out that for example Digestif.SHA256 is only available after 0.7 or 0.7.2 (see lower bound). The docs could mention that.

It has been a while… Thanks for RMD160 btw.

Would it be possible to change the library to release the runtime lock for update operations in the native version of the library, to allow the user to offload update operations to other cpu's?

I can make a PR to call caml_release_runtime_system() and caml_acquire_runtime_system() from the update functions operating on bigarrays.
However this would require that the memory location of ctx is not moved by the ocaml gc while update function is running. One easy solution for this would be to change ctx from type By.t to Bi.t (bigarray). Alternatively create a custom type that allocates the internal structure outside the ocaml heap.

I believe the existing ocaml implementations are correct in src/digestif_conv.ml are correct, but they allocate buffers of unitialized (aka "old freed data") and overwrite them later, which gives me a nervous feeling.

I provided some example alternative implementations here:
https://github.com/mirage/digestif/pull/31/files/ac17c284be51994697de2e2fd09a4ca99edbb6a0#diff-0e4a0d0e4fd4a1679a3417f2c48841c2

• My comment about to_hex avoiding branches doesn't make sense since the current implementation does not branch either.

• My of_hex example is a bit complicated, but it was the best that I could come up with that matches the existing behavior. Not sure if it should be replaced by a cleaner version?

ping @dinosaure

Might be useful to add some instruction on how to install if this lib is on opam :o

let to_hex hash =
  let chr x = match x with
      | 0 | 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 -> Char.chr (0x30 + x)
      | _ -> Char.chr (87 + x) in
  String.init (D.digest_size * 2)
  (fun i ->
    let ch = Char.code hash.[i / 2] in
    let shift = ((i land 1) lxor 1) lsl 2 in (* even: 0; odd: 4*)
    chr ((ch lsr shift) land 0x0f)
  )

and

let of_hex hex =
  let code x = match x with
      | '0' .. '9' -> Char.code x - 48
      | 'A' .. 'F' -> Char.code x - 55
      | 'a' .. 'z' -> Char.code x - 87
      | _ -> invalid_arg "of_hex" in
  let decode_char ch1 ch2 = Char.chr ((code ch1 lsl 4) lor (code ch2)) in
  let offset = ref 0 in
  let rec next have_first idx =
    if !offset + idx >= String.length v then
      '\x00' (* right-pad with 0x00 *)
    else match hex.[!offset + idx] with
    | ' ' | '\t' | '\r' | '\n' ->
      incr offset; next has_first idx
    | ch2 when has_first -> ch2
    | ch1 ->
      incr offset ;
      let ch2 = next true idx in
      if ch2 <> '\x00' then
        decode_char ch1 ch2
      else
        invalid_arg "of_hex: odd number of hex characters"
  in
  String.init D.digest_size (next false)

I don't need it right now, but I guess would be nice to have.

$ utop -require digestif,digestif.c
# open Digestif;;
# SHA.digest_string "a";;
Error: Reference to undefined global `Digestif'

Close to #44

just a general comment, but I'm trying to understand some code making use of this library and there's code like

  let h = Digestif.blake2s 32 in

and there doesn't seem to be any official documentation anywhere I can find on what 32 is here.

(The doc is here: https://ocaml.org/p/digestif/0.9.0/doc/Digestif/index.html#val-blake2s but there is no actual documentation)