Replace ct_find_uint8 by Eqaf_cstruct.find_uint8

[dinosaure]

A fix of #23

[hannesm]

this is fine, though:

• CI fails (would you mind to look into this and fix it?)
• can we remove the ct_find funcion from Uncommon, and have it in pk/common.ml or pk/rsa.ml (the only use-side)? <- I'm not keen to introduce dependencies in the main library

[hannesm]

ah, the test failure (from https://pipelines.actions.githubusercontent.com/qoofUrGaI3vjbXV0l1nsaHtqOWUOy1R9qZ8Weal5PXKQiJlyok/_apis/pipelines/1/runs/28/signedlogcontent/11?urlExpires=2020-04-17T11%3A04%3A42.5829588Z&urlSigningMethod=HMACV1&urlSignature=yynvhHhYHHE%2Bp0YvoUktZOIeTSsKr2EZL95Ynsr2r%2B8%3D):

2020-04-17T10:40:09.1333537Z The following dependencies couldn't be met:
2020-04-17T10:40:09.1334251Z   - mirage-crypto -> eqaf >= 0.7
2020-04-17T10:40:09.1334503Z       no matching version

so, this will eventually solve itself (once the windows runner has a fresher opam repository!?)

[dinosaure]

can we remove the ct_find funcion from Uncommon, and have it in pk/common.ml or pk/rsa.ml (the only use-side)? <- I'm not keen to introduce dependencies in the main library

It's done 👍.

[hannesm]

thanks again, sorry for being picky, but I'd appreciate a function definition (a la let ct_find_uint8 ..) and avoid repeating the two lines multiple times :)

[hannesm]

hmm, or not - I'm not sure anymore. why should select_int be used instead of Option.get?

[dinosaure]

hmm, or not - I'm not sure anymore. why should select_int be used instead of Option.get?

select_int is checked as a constant-time way to simulate a branch (like if a then b else c). I use it to replace Option.get which needs an allocation (and may be delay the computation). Finally, we an assert that:

let res = Eqaf.find_uint8 ... in
let v = Eqaf.select_int (res - 1) default res in

Is constant-time where:

let res = Eqaf.find_uint8 ... in
let res = if res = (-1) then None else Some res in
let v = Option.get ~def res in

is not. Instead, we can implement a ct_find_uint8 : default:int -> ?off:int -> f:(int -> bool) -> Cstruct.t -> int which uses select_int - then, it will be a one-liner where find_uint8 is used.

[hannesm]

@dinosaure thanks for your answer. yes, I'd appreciate a find_uint8 that takes a default: argument instead of returning -1 (which is slightly surprising in a typed OCaml world). maybe worth to do and cut a 0.7.1 or 0.8.0 release of eqaf?

[hannesm]

oh, another way to move here is: develop such a find_uint8 ~default: locally in rsa and use this at the three locations. :)

[dinosaure]

returning -1 (which is slightly surprising in a typed OCaml world)

Yes, it's mostly due to the limitation to avoid any call to the GC in your eqaf's codebase.

maybe worth to do and cut a 0.7.1 or 0.8.0 release of eqaf?

Sure, may be today, or this week 👍.

[dinosaure]

oh, another way to move here is: develop such a find_uint8 ~default: locally in rsa and use this at the three locations. :)

Done in this way 👍 into my last commit. Should I rebase?

[hannesm]

looks fine to me, can be squashed and merged into a single commit when CI passes

[hannesm]

thanks, merged. the CI failures (on windows) are lacking the eqaf 0.7 release in opam-repository-mingw (that will eventually appear at some point) ;the OCaml-CI has an update cycle of "around a week" (there's discussion on ocurrent/ocaml-ci#26 about ways out -- no, embedding submodules into this repository won't happen, sorry).

since we use a variety of CI systems, we still have some that test this PR successfully :)