Merge pull request #645 from dan0dbfe/fix/issue-509

Correctly return NOERROR even if host resolver returned empty list

Author: djs55

src/hostnet/host.ml

@@ -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"
 
@@ -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
@@ -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

src/hostnet/hostnet_dns.ml

@@ -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"))

src/hostnet/hostnet_http.ml

@@ -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

src/hostnet/sig.ml

@@ -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
 

src/hostnet_test/test_dns.ml

@@ -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 ",
@@ -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 *)