Skip to content

Commit

Permalink
Correctly return NOERROR even if host resolver returned empty list
Browse files Browse the repository at this point in the history
  • Loading branch information
dan0dbfe committed Nov 19, 2024
1 parent 604ab7f commit 14556e2
Show file tree
Hide file tree
Showing 5 changed files with 73 additions and 27 deletions.
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

0 comments on commit 14556e2

Please sign in to comment.