feat: add BLAKE3 support - #172
thevilledev wants to merge 2 commits into
Conversation
Make BLAKE3 available through Digestif's existing backend selection so Mirage and TLS users do not need a separate hashing package. Support keyed hashing, key derivation, and extensible output in both backends. Keep the imported C code auditable with pinned upstream provenance and a clear separation from the Digestif-specific integration. Signed-off-by: Ville Vesilehto <ville@vesilehto.fi>
795bf2a to
f716840
Compare
|
The PR seems ok from a quick view, I need time to look into the OCaml implementation but as far as I can tell, I can give a green flag. |
| __define_hash (blake2s, BLAKE2S) | ||
| __define_hash (rmd160, RMD160) | ||
|
|
||
| CAMLprim value |
There was a problem hiding this comment.
Do you have a reason of not using __define_hash? blake2{b,s} is particular because it has multiple parameters when we would like to initialize it but I don't see (quickly) a particular case regarding blake3.
There was a problem hiding this comment.
Thanks, definitely a better approach. I initially kept it separate because of minor differences in the upstream API (like different context type). I can look into this later today!
dinosaure
left a comment
dinosaure
left a comment
There was a problem hiding this comment.
I can see a certain merge path between BLAKE2 and BLAKE3, I need more time to figure out how to organize all of that but I think we can save some lines via functors.
Route BLAKE3's standard operations through the shared hash definition. A small adapter preserves the upstream API differences while keeping keyed and extensible-output operations explicit. Keep the fuzzing change focused on registering BLAKE3 by retaining Crowbar's existing failure-formatting helper. Signed-off-by: Ville Vesilehto <ville@vesilehto.fi>
ca954d0 to
c332ab4
Compare
| return Val_int (upper ## _CTX_SIZE); \ | ||
| } | ||
|
|
||
| struct blake3_ctx { |
There was a problem hiding this comment.
I don't understand such indirection, why we need to wrap the hasher into a struct. Can we just use the digetif_blake3_hasher directly?
There was a problem hiding this comment.
To your previous comment #172 (comment): if we want to use __define__hash and not wrap digestif_blake3_hasher we would have to add an explicit third parameter to __define_hash. To demonstrate:
-#define __define_hash(name, upper)
+#define __define_hash(name, upper, ctx_type)And then something like this:
__define_hash (md5, MD5, struct md5_ctx)
__define_hash (sha1, SHA1, struct sha1_ctx)
__define_hash (sha224, SHA224, struct sha224_ctx)
__define_hash (sha256, SHA256, struct sha256_ctx)
__define_hash (sha384, SHA384, struct sha384_ctx)
__define_hash (sha512, SHA512, struct sha512_ctx)
__define_hash (whirlpool, WHIRLPOOL, struct whirlpool_ctx)
__define_hash (blake2b, BLAKE2B, struct blake2b_ctx)
__define_hash (blake2s, BLAKE2S, struct blake2s_ctx)
__define_hash (blake3, BLAKE3, digestif_blake3_hasher)
__define_hash (rmd160, RMD160, struct rmd160_ctx)The blake3_ctx is there simply to be aligned with the macro & not refactor at this point. Which way do we go?
There was a problem hiding this comment.
#174 should help, I'm waiting the CI and see if everything is correct but your PR should just define a typedef then to rename blake3_hasher to blake3_ctx.
|
Replaced by #180 where the diff is much more smaller. |
2 participants
Summary
Add
Digestif.BLAKE3to the C and pure OCaml backends.Fixes #93.
Changes
src-c/native, with marked upstream/Digestif sections.Tests
Tested locally on macOS.