Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

openssl deterministic test fails #293

Merged
merged 9 commits into from
Mar 29, 2024
Merged

openssl deterministic test fails #293

merged 9 commits into from
Mar 29, 2024

Conversation

LCBH
Copy link
Contributor

@LCBH LCBH commented Dec 14, 2023

  • Implementing PartialEq and Debug for TraceContext so that we can compare the two TraceContext obtained at different times from executing the same trace.
  • Using that to test PUT's determinism after executing client_attacker_full.
  • Currently fails for OpenSSL 111j. See the Issue OpenSSL is not made fully derministic #294 for this new failing uni test openssl::deterministic::tests::test_openssl_no_randomness_full.

 - distinguish re-set RAND from reseed
 - reset RAND once for all for all factories with `determinism_set_reseed_all_factories`
 - reseed all factories before executing a trace with `determinism_reseed_all_factories`
 --> New integration test for determinsim. Passes for OpenSSL!
@LCBH
Copy link
Contributor Author

LCBH commented Dec 20, 2023

Commit #aa340 fixed the issue:

New determinism architecture:

  • distinguish re-set RAND from reseed
  • reset RAND once for all for all factories with determinism_set_reseed_all_factories
  • reseed all factories before executing a trace with determinism_reseed_all_factories
    --> New integration test for determinism. Passes for OpenSSL!

The test test_attacker_full_det_recreate now passes for openssl111j.

@LCBH
Copy link
Contributor Author

LCBH commented Dec 20, 2023

The test is actually not really reproducible. b1cdbbb is doing the same on multiple traces and it fails at some point :(
Sometimes at step 0, sometimes at step 20, 15, etc...
Why is so?

@LCBH
Copy link
Contributor Author

LCBH commented Feb 2, 2024

Conclusion: this test fails because of session tickets sent after the final Server FInishes. Those contain time-related information, which thus differ across executions.

In BoringSSL, the flag BORINGSSL_UNSAFE_DETERMINISTIC_MODE reseed the PRNG but also make BoringSSL use a hard-coded time instead of the actual time. We have not found the equivalent for OpenSSL.
A solution would be to use libfaketime but this will also impact tlspuffin's timestamps.
One could also hijackossl_time_now but that's not really robust in the long term.

@LCBH
Copy link
Contributor Author

LCBH commented Feb 2, 2024

Will close without merging, the useful more complete deterministic uni test is also in #290 that will me merged.

@LCBH LCBH added the wontfix This will not be worked on label Feb 20, 2024
@LCBH LCBH added the ci:full Run full CI checks on labeled PR label Mar 13, 2024
@LCBH LCBH merged commit d8f1e5a into main Mar 29, 2024
@LCBH LCBH deleted the openssl-det branch March 29, 2024 14:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci:full Run full CI checks on labeled PR wontfix This will not be worked on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants