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

resolver: do dnssec validation as an option #325

Merged
merged 2 commits into from
Nov 29, 2022

Conversation

hannesm
Copy link
Member

@hannesm hannesm commented Oct 31, 2022

in #290 / 4276306 we added DNSSec support to the resolver. With this PR, the DNSSec validation is made optional (Dns_resolver.create takes a labelled dnssec:bool argument to switch it on or off for the lifetime of the resolver).

Copy link
Member

@reynir reynir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From a first impression it looks good to me. I'll look over it again Monday (maybe Sunday).

resolver/dns_resolver.ml Show resolved Hide resolved
resolver/dns_resolver.ml Outdated Show resolved Hide resolved
@@ -152,7 +152,7 @@ let resolve t ~rng ip_proto ts name typ =
*)
let rec go t types name =
Logs.debug (fun m -> m "go %a" Domain_name.pp name) ;
match find_nearest_ns rng ip_proto ts t (Domain_name.raw name) with
match find_nearest_ns rng ip_proto dnssec ts t (Domain_name.raw name) with
| `NeedAddress ns -> go t addresses ns
| `NeedDnskey (zone, ip) -> zone, zone, [`K (Rr_map.K Dnskey)], ip, t
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

`NeedDnskey should never be returned if dnssec is false. I don't think it's worth it to try encode it in the types, though.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indeed.

@reynir reynir merged commit b1ab0f5 into mirage:main Nov 29, 2022
@reynir
Copy link
Member

reynir commented Nov 29, 2022

Thanks!

@hannesm hannesm deleted the resolver-dnssec-optional branch November 29, 2022 13:25
hannesm added a commit to hannesm/opam-repository that referenced this pull request Dec 2, 2022
…er, dns-mirage, dns-client, dns-cli and dns-certify (6.4.1)

CHANGES:

* dns-client: adapt to happy eyeballs 0.4.0 (mirage/ocaml-dns#329 @reynir @hannesm)
* dns-resolver: dnssec validation is optional via a labeled parameter ~dnssec
  passed to Dns_resolver.create (mirage/ocaml-dns#325 @hannesm)
* upgrade to dune 2 (mirage/ocaml-dns#327 @reynir)
hannesm added a commit to hannesm/opam-repository that referenced this pull request Dec 2, 2022
…er, dns-mirage, dns-client, dns-cli and dns-certify (6.4.1)

CHANGES:

* dns-client: adapt to happy eyeballs 0.4.0 (mirage/ocaml-dns#329 @reynir @hannesm)
* dns-resolver: dnssec validation is optional via a labeled parameter ~dnssec
  passed to Dns_resolver.create (mirage/ocaml-dns#325 @hannesm)
* upgrade to dune 2 (mirage/ocaml-dns#327 @reynir)
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.

2 participants