robur-coop / udns

[deprecated, developmeht moved to https://github.com/mirage/ocaml-dns] µDNS - an opinionated Domain Name System (DNS) library
BSD 2-Clause "Simplified" License
55 stars 5 forks source link

adapt to domain-name phantom type API #31

Open hannesm opened 5 years ago

hannesm commented 5 years ago

esp last commit is of interest, will clean up other commits. I'm sure there's more [ `host ] Domain_name.t to be introduced (i.e. all zone boundaries should be on hostnames, ...) -- but it's also a bit unclear where to require hostnames, and where domain names are sufficient (i.e. zone parser -- should origin be a hostname? can we verify that each zone we add to dns_trie is a hostname?).

in respect to regression2/3 -- I'm not sure whether SOA nameserver (/MX) should really be hostname -- from what our server should accept, this is very reasonable, for what odig should accept & print, I think this is a bit too limited.

hannesm commented 5 years ago

What does the ?mac argument in the tsig_verify code represent / do? It feels a bit scary reading this with so many optional arguments.

well, the request does not contain a mac, but the reply the mac of the request, as defined in https://tools.ietf.org/html/rfc2845

Why are we waiting 5 seconds here before closing?

that's my home-grown reconnection logic ;) wait 5 seconds till next attempt ;p (yes, the connection logic in mirage requires some work)

I assume int64 is timestamp, int is retry count, maybe document what the Cstruct.option is?

yes, good idea. its the mac of the request (in this case a notify)

How about Randomconv.int16 server.rng, or does this need to be 0 .. 65534 specifically?

I guess #9 should include this..

I opened #32 which superseeds this (but does not address your points, will attempt to work on a separate PR with your concerns above) and #29