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

dream.middleware and dream.cipher is MirageOS compatible #115

Merged
merged 7 commits into from
Jul 7, 2021

Conversation

dinosaure
Copy link
Contributor

This patch is small but a bit complex. mirage-crypto-rng.lwt is not compatible with MirageOS because it plays with Lwt_engine to add a hook to continuously feed the random number generator - where MirageOS has its own scheduler. So we need to delete it from dream.cipher to be MirageOS compatible.

However, at this step, we have multiple choices:

I chosen the second solution but from the abstraction perspective, we have a strong link between dream and mirage-crypto-rng (even for MirageOS). It implies that the end-user must trust on mirage-crypto-rng and he is not able to chose something else then - even for MirageOS.

This is an hard issue and well-know in MirageOS, so I just would like to have the opinion of @hannesm on that and, of course, your opinion @aantron.

The second commit is an implication of the first one. Due to the deletion of mirage-crypto-rng.lwt from dream.cipher and with my others patches (see #100, #102, #107, #108 and #114), Lwt_engine/Lwt_unix/Lwt_io are not available anymore via subsequent dependencies. So, with the first patch, static.ml does not compile (which requires Lwt_io). So I decided to, internally, make a sub-package dream.middleware.static (which brings lwt.unix) and includes it into the main dream.ml module.

Such design gives me the opportunity to not use static middleware into my MirageOS (which does not have a file-system). Of course, at the end, nothing change on the Dream interface.

…- and move the mirage-crypto-rng.lwt dependency to the dream package
@hannesm
Copy link
Contributor

hannesm commented Jul 7, 2021

My point of view: Indeed for cryptographic keys you need a strong (cryptographically secure) random number generator. This also needs to be initialized (and best periodically fed entropy into). Using Mirage_crypto_rng is a good choice. The generate function allows an optional custom generator g being passed -- in the case you want something different from the standard Fortuna one (this g could be exposed if there are users who wish to use their own).

Initialization, as done here, is fine. The Mirage_crypto_rng_lwt.initialize adds a periodic task (Lwt.async) and adds a callback to Lwt_main.Enter_iter_hook -- both are good to be done at the top level (they won't actually do something until Lwt_main.run is executed).

src/dream.ml Outdated Show resolved Hide resolved
src/middleware/dune Outdated Show resolved Hide resolved
src/cipher/dune Outdated Show resolved Hide resolved
src/cipher/random.ml Outdated Show resolved Hide resolved
src/cipher/random.ml Outdated Show resolved Hide resolved
src/cipher/random.ml Outdated Show resolved Hide resolved
src/cipher/random.ml Outdated Show resolved Hide resolved
src/cipher/random.ml Outdated Show resolved Hide resolved
@aantron aantron merged commit 5d5c682 into aantron:master Jul 7, 2021
@aantron
Copy link
Owner

aantron commented Jul 7, 2021

Thanks @dinosaure for PR and @hannesm for review!

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.

None yet

3 participants