mirleft / ocaml-x509

X509 (RFC5280) handling in OCaml
BSD 2-Clause "Simplified" License
52 stars 33 forks source link

mirage-crypto 0.8.9 breaks regression test #143

Closed sternenseemann closed 3 years ago

sternenseemann commented 3 years ago

Excerpt from the tests:

> [FAIL]        Regression                                         11   sloppy private key parsing.

…

┌───────────────────────────────────────────────────────────────────────────────────────────────────┐
│ [FAIL]        Regression                                         11   sloppy private key parsing. │
└───────────────────────────────────────────────────────────────────────────────────────────────────┘
ASSERT private key gcloud, expected decoding error
FAIL private key gcloud, expected decoding error
Raised at file "src/alcotest-engine/test.ml", line 143, characters 20-48
Called from file "tests/regression.ml", line 132, characters 13-74
Called from file "src/alcotest-engine/core.ml", line 269, characters 17-23
Called from file "src/alcotest-engine/monad.ml", line 34, characters 31-35

Logs saved to `~/x509-v0.11.2/_build/default/tests/_build/_tests/X509 tests/Regression.011.output'.
 ──────────────────────────────────────────────────────────────────────────────

Full test results in `~/x509-v0.11.2/_build/default/tests/_build/_tests/X509 tests'.
1 failure! in 0.000s. 2195 tests run.

As far as I understanded 0.8.9 fixed not being able to handle gcloud keys properly, so it seems to me that x509 now holds wrong assumptions about mirage-crypto‽

Is it safe to ignore this test failure?

hannesm commented 3 years ago

yes, see #142 for the corresponding fix for x509 -- in opam-repository, there's a with-test & < "0.8.9" dependency for the latest released x509.

psafont commented 3 years ago

I don't think this is an issue with this library, mirage-crypto* 0.8.9 also broke other packages using mirage-crypto. 0.8.10 is being released in opam and 0.8.9 is being disabled: https://github.com/ocaml/opam-repository/pull/18010

sternenseemann commented 3 years ago

Still broken with mirage-crypto 0.8.10.

psafont commented 3 years ago

It's expecting the gcloud key to be invalid unless sloppy is used. The changes in mirage-crypto make the library rightfully decode the key. #142 is needed to fix the assumption.

hannesm commented 3 years ago

@sternenseemann yes, this is expected (to be still broken with mirage-crypto 0.8.10). for your packaging demands, you could either include the patch of #142, disable the x509 tests, or wait a few days until we have a new x509 release in place.

sternenseemann commented 3 years ago

Yeah, I'm gonna wait for the PR to be merged and then patch x509 until the next release :)

hannesm commented 3 years ago

it took me some more time, but finally 0.12.0 is out (https://github.com/ocaml/opam-repository/pull/18444). Closing this issue.