Skip to content

windows/dkml: opam install with deps-only and with-test only packages to test, deps-only for only build - #198

Merged
hannesm merged 6 commits into
mirage:mainfrom
hannesm:win
Feb 24, 2024
Merged

windows/dkml: opam install with deps-only and with-test only packages to test, deps-only for only build#198
hannesm merged 6 commits into
mirage:mainfrom
hannesm:win

Conversation

@hannesm

@hannesm hannesm commented Feb 22, 2024

Copy link
Copy Markdown
Member

@hannesm

hannesm commented Feb 22, 2024

Copy link
Copy Markdown
Member Author

the std_cflags are properly picked up (in contrast to #137 (comment)), also this fixes #137 (comment)

remaining is the following issue, on windows 64 bit systems, the fiat 32 bit versions are used (reason behind this is that 128bit unsigned integers are not available) -- now with our pre-computed tables, this results in an error. I guess on a windows 64 bit system, using the 32 bit tables is fine. any other opinion @Firobe @jonahbeckford ? I added that in 975a881 (but unfortunately our mirage-crypto-ec tests aren't running on dkml/windows since mirage-crypto-pk and asn1-combinators don't work there (gmp/zarith).

@hannesm
hannesm force-pushed the win branch 2 times, most recently from 5589386 to 65e0b63 Compare February 23, 2024 12:39
@hannesm

hannesm commented Feb 23, 2024

Copy link
Copy Markdown
Member Author

Thanks to #200, we can now run mirage-crypto-ec tests. Unfortunately, they fail:

File "tests/dune", line 60, characters 7-25:
60 |  (name test_ec_wycheproof)
            ^^^^^^^^^^^^^^^^^^
(cd _build/default/tests && ./test_ec_wycheproof.exe)
Fatal error: exception File "tests/test_ec_wycheproof.ml", line 418, characters 4-10: Assertion failed

It is not entirely clear to me, why, though. The assetion in question:

let to_ed25519_keys (key : eddsa_key) =
  let priv_cs = Cstruct.of_string key.sk
  and pub_cs = Cstruct.of_string key.pk
  in
  match Ed25519.priv_of_cstruct priv_cs, Ed25519.pub_of_cstruct pub_cs with
  | Ok priv, Ok pub ->
    assert (Cstruct.equal Ed25519.(pub_to_cstruct (pub_of_priv priv)) pub_cs); (* that's the failing one *)
    priv, pub
  | _ -> assert false

This means that the (pub_to_cstruct (pub_of_priv _)) isn't equal to the public key. Looks like something is going wrong down the line, but what?

@hannesm

hannesm commented Feb 23, 2024

Copy link
Copy Markdown
Member Author

to be clear, I don't have any further energy and time to look deeper into "CL.EXE" and what goes wrong. If someone can pick this up, best with an actual setup to have quick compile-test-edit cycles, that'd be amazing.

I propose to do some printf debugging (looking where pub_cs is different from pub_to_cstruct (pub_to_priv priv). It may have to do that OCaml assumes 64bit, and the C code 32bit. There are also a huge amount of warnings from the C compiler, which may be worth to look into.

If there's no interest/time by others, I plan to revert the DKML changes in the main branch to cut a mirage-crypto release.

@jonahbeckford

Copy link
Copy Markdown
Contributor

I have no time to do this at the moment. While I think it would be better just to mark the ec opam release as unavailable on windows, do what you think is best.

@hannesm

hannesm commented Feb 24, 2024

Copy link
Copy Markdown
Member Author

ok, I removed mirage-crypto-ec from the windows-dkml-ci script. plan to merge this when CI is green.

I have no idea @jonahbeckford about your plans to use mirage-crypto on dkml/windows -- but I'm sure your suggestion "remove mirage-crypto-ec from the build" fits your needs. I'll open an issue about the mirage-crypto-ec and dkml.

@hannesm
hannesm merged commit ccbf964 into mirage:main Feb 24, 2024
@hannesm
hannesm deleted the win branch February 24, 2024 14:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants