mirage / ocaml-dns

OCaml implementation of the DNS protocol
BSD 2-Clause "Simplified" License
105 stars 43 forks source link

Add some utilities to be able to provide a proper DNS device for MirageOS #297

Closed dinosaure closed 2 years ago

dinosaure commented 2 years ago

This PR wants to provide a connect_device which can be used by a MirageOS device and expects raw arguments (due to the fact that it's not possible to extend converters in Mirage) and try to parse them. If these arguments are wrong, we just fails. The semantic of arguments is not yet clear but it's a draft. The idea is to have 3 arguments:

With all of these arguments, the Dns_client_mirage.S, the new connect_device function and mirage/mirage#1292, we are able to provide a simple DNS device which can be re-used then by Happy_eyeballs. Related to mirage/mirage#1291.

dinosaure commented 2 years ago

I said on others PRs:

NOTE: by design, connect_device should not appear into the signature but only into the functor. I put it but if you are more confident to put it outside (as any devices in mirage), I can do it.

dinosaure commented 2 years ago

I updated the PR with what we did on ocaml-x509 (it deleted the direct dependency to hex and astring). Then, I decided to split authenticator argument by a comma such as: --dns-authenticator <ipaddr>,<authenticator>.

dinosaure commented 2 years ago

I force-pushed a new version according the new version of x509.0.16.0 and the new release of ipaddr.5.3.0 (which is not yet released but it will be soon). I integrated your comment @hannesm and provide a better way to unserialize nameservers given by the mirage tool. Tell me what you think about that.

hannesm commented 2 years ago

Tanks again for your work, I reviewed it and pushed two commits. The behavioural change is that udp is no more supported by nameserver_of_string -- since "let create" errors on `Udp, I don't think it is very useful to support udp in the nameserver_of_string. Also, "connect_device" has been renamed to "connect". The default authenticator is now Ca_certs_nss.authenticator.

WDYT?

dinosaure commented 2 years ago

I don't have strong opinion with what you did, it's perfect for me to, then, be able to propose a new device on the mirage side 👍 .

hannesm commented 2 years ago

(force-pushed after merge of #300 to allow CI to succeed)