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

Correctly return NOERROR even if host resolver returned empty list #645

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 18 additions & 13 deletions src/hostnet/host.ml
Original file line number Diff line number Diff line change
Expand Up @@ -1294,10 +1294,11 @@ module Dns = struct
in
return (Ok ips)))
>>= function
| Error (`Msg _) ->
(* FIXME: error handling completely missing *)
Lwt.return []
| Ok ips -> Lwt.return ips
| Error e ->
(* FIXME: error handling completely missing *)
Lwt.return (Error e)
| Ok ips ->
Lwt.return (Ok ips)

let localhost_local = Dns.Name.of_string "localhost.local"

Expand All @@ -1306,17 +1307,19 @@ module Dns = struct
(match question with
| { q_class = Q_IN; q_name; _ } when q_name = localhost_local ->
Log.debug (fun f -> f "DNS lookup of localhost.local: return NXDomain");
Lwt.return (q_name, [])
Lwt.return (Error q_name)
| { q_class = Q_IN; q_type = Q_A; q_name; _ } ->
getaddrinfo (Dns.Name.to_string q_name) `INET >>= fun ips ->
Lwt.return (q_name, ips)
(getaddrinfo (Dns.Name.to_string q_name) `INET >>= function
| Error _ -> Lwt.return (Error q_name)
| Ok ips -> Lwt.return (Ok (q_name, ips)))
| { q_class = Q_IN; q_type = Q_AAAA; q_name; _ } ->
getaddrinfo (Dns.Name.to_string q_name) `INET6 >>= fun ips ->
Lwt.return (q_name, ips)
| _ -> Lwt.return (Dns.Name.of_string "", []))
(getaddrinfo (Dns.Name.to_string q_name) `INET6 >>= function
| Error _ -> Lwt.return (Error q_name)
| Ok ips -> Lwt.return (Ok (q_name, ips)))
| _ ->
Lwt.return (Error (Dns.Name.of_string "")))
>>= function
| _, [] -> Lwt.return []
| q_name, ips ->
| Ok (q_name, ips) ->
let answers =
List.map
(function
Expand All @@ -1338,7 +1341,9 @@ module Dns = struct
})
ips
in
Lwt.return answers
Lwt.return (Ok answers)
| Error _ ->
Lwt.return (Error ())

let resolve = resolve_getaddrinfo
end
Expand Down
7 changes: 4 additions & 3 deletions src/hostnet/hostnet_dns.ml
Original file line number Diff line number Diff line change
Expand Up @@ -446,10 +446,11 @@ struct
| _, Host ->
D.resolve question
>>= function
| [] ->
Lwt.return (Ok (Some (marshal nxdomain)))
| answers ->
| Ok answers ->
Lwt.return (Ok (marshal_reply answers))
(* Currently return all errors as NXDOMAIN *)
| Error _ ->
Lwt.return (Ok (Some (marshal nxdomain)))
end
| _ ->
Lwt.return (Error (`Msg "DNS packet had multiple questions"))
Expand Down
23 changes: 13 additions & 10 deletions src/hostnet/hostnet_http.ml
Original file line number Diff line number Diff line change
Expand Up @@ -151,16 +151,19 @@ module Make
let question =
make_question ~q_class:Q_IN Q_A (Dns.Name.of_string name_or_ip)
in
Dns_resolver.resolve question
>>= fun rrs ->
(* Any IN record will do (NB it might be a CNAME) *)
let rec find_ip = function
| { cls = RR_IN; rdata = A ipv4; _ } :: _ ->
Lwt.return (Ok (Ipaddr.V4 ipv4))
| _ :: rest -> find_ip rest
| [] -> errorf "Failed to lookup host: %s" name_or_ip in
find_ip rrs
| Ok x -> Lwt.return (Ok x)
(Dns_resolver.resolve question >>= function
| Ok rrs ->
(* Any IN record will do (NB it might be a CNAME) *)
let rec find_ip = function
| { cls = RR_IN; rdata = A ipv4; _ } :: _ ->
Lwt.return (Ok (Ipaddr.V4 ipv4))
| _ :: rest -> find_ip rest
| [] -> errorf "Failed to lookup host: %s" name_or_ip
in
find_ip rrs
| Error _ ->
Lwt.return (Error (`Msg (Printf.sprintf "Failed to lookup host: %s" name_or_ip))))
| Ok ip -> Lwt.return (Ok ip)

let to_json t =
let open Ezjsonm in
Expand Down
2 changes: 1 addition & 1 deletion src/hostnet/sig.ml
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ module type FILES = sig
end

module type DNS = sig
val resolve: Dns.Packet.question -> Dns.Packet.rr list Lwt.t
val resolve: Dns.Packet.question -> (Dns.Packet.rr list, unit) result Lwt.t
(** Given a question, find associated resource records *)
end

Expand Down
37 changes: 37 additions & 0 deletions src/hostnet_test/test_dns.ml
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,37 @@ let test_etc_hosts_priority server config () =
in
run ~pcap:"test_etc_hosts_priority.pcap" t

let test_noerror_empty_response server config () =
let name = "ipv4.tlund.se" in (* FIXME: Use a better address? *)
let t _ stack =
set_dns_policy config;
let resolver = DNS.create stack.Client.t in
(* Query for an AAAA record on a domain that might only have A records *)
DNS.gethostbyname ~server ~q_type:Dns.Packet.Q_AAAA resolver name >>= function
| [] -> (* Expected: NOERROR with an empty answer list *)
Log.info (fun f -> f "%s returned NOERROR with no AAAA records" name);
Lwt.return ()
| _ ->
Log.err (fun f -> f "Expected no AAAA records for %s, but got some" name);
failwith ("Expected NOERROR with no AAAA records for " ^ name)
in
run ~pcap:"test_noerror_empty_response.pcap" t

let test_nxdomain_response server config () =
let name = "nonexistent.example.com" in (* Use a domain that doesn’t exist *)
let t _ stack =
set_dns_policy config;
let resolver = DNS.create stack.Client.t in
DNS.gethostbyname ~server resolver name >>= function
| [] -> (* Expected: NXDOMAIN (empty answer list) *)
Log.info (fun f -> f "%s returned NXDOMAIN as expected" name);
Lwt.return ()
| _ ->
Log.err (fun f -> f "Expected NXDOMAIN for %s, but got answers" name);
failwith ("Expected NXDOMAIN for " ^ name)
in
run ~pcap:"test_nxdomain_response.pcap" t

let test_dns config =
let prefix = Dns_policy.(Config.to_string @@ config ()) in [
prefix ^ ": lookup ",
Expand All @@ -123,6 +154,12 @@ let test_dns config =

prefix ^ ": _etc_hosts_priority",
[ "", `Quick, test_etc_hosts_priority primary_dns_ip config ];

prefix ^ ": noerror_empty_response",
[ "", `Quick, test_noerror_empty_response primary_dns_ip config ];

prefix ^ ": nxdomain_response",
[ "", `Quick, test_nxdomain_response primary_dns_ip config ];
]

(* A real UDP server listening on a physical port *)
Expand Down