Merge pull request #61 from talex5/fix-mac

Force backend MAC to fe:ff:ff:ff:ff:ff to fix HVM clients

Dockerfile

@@ -7,7 +7,7 @@ FROM ocaml/opam2@sha256:f7125924dd6632099ff98b2505536fe5f5c36bf0beb24779431bb62b
 # Pin last known-good version for reproducible builds.
 # Remove this line (and the base image pin above) if you want to test with the
 # latest versions.
-RUN git fetch origin && git reset --hard e77756e92274790668ed1f6f998d66fa2e744fb6 && opam update
+RUN git fetch origin && git reset --hard d1b2a1cbc28d43926b37e61f46fc403b48ab9c23 && opam update
 
 RUN sudo apt-get install -y m4 libxen-dev pkg-config
 RUN opam pin add -yn cmdliner 'https://github.com/talex5/cmdliner.git#repro-builds'

build-with-docker.sh

@@ -5,5 +5,5 @@ docker build -t qubes-mirage-firewall .
 echo Building Firewall...
 docker run --rm -i -v `pwd`:/home/opam/qubes-mirage-firewall qubes-mirage-firewall
 echo "SHA2 of build:   $(sha256sum qubes_firewall.xen)"
-echo "SHA2 last known: 765cf16c2e85feb7e5dfd3e409a3013c91c2b07f5680ed9f4e487e27213f1355"
+echo "SHA2 last known: dbf7460fa628bea5d132a96fe7ba2cd832e3d9da7005ae74f6a124957f4848ea"
 echo "(hashes should match for released versions)"

client_eth.ml

@@ -27,16 +27,16 @@ let client_gw t = t.client_gw
 let add_client t iface =
   let ip = iface#other_ip in
   let rec aux () =
-    if IpMap.mem ip t.iface_of_ip then (
+    match IpMap.find ip t.iface_of_ip with
+    | Some old ->
       (* Wait for old client to disappear before adding one with the same IP address.
          Otherwise, its [remove_client] call will remove the new client instead. *)
-      Log.info (fun f -> f "Waiting for old client %a to go away before accepting new one" Ipaddr.V4.pp ip);
+      Log.info (fun f -> f ~header:iface#log_header "Waiting for old client %s to go away before accepting new one" old#log_header);
       Lwt_condition.wait t.changed >>= aux
-    ) else (
+    | None ->
       t.iface_of_ip <- t.iface_of_ip |> IpMap.add ip iface;
       Lwt_condition.broadcast t.changed ();
       Lwt.return_unit
-    )
   in
   aux ()
 
@@ -70,7 +70,11 @@ module ARP = struct
 
   let lookup t ip =
     if ip = t.net.client_gw then Some t.client_link#my_mac
-    else None
+    else if (Ipaddr.V4.to_bytes ip).[3] = '\x01' then (
+      Log.info (fun f -> f ~header:t.client_link#log_header
+                   "Request for %a is invalid, but pretending it's me (see Qubes issue #5022)" Ipaddr.V4.pp ip);
+      Some t.client_link#my_mac
+    ) else None
   (* We're now treating client networks as point-to-point links,
      so we no longer respond on behalf of other clients. *)
     (*
@@ -83,16 +87,18 @@ module ARP = struct
 
   let input_query t arp =
     let req_ipv4 = arp.Arp_packet.target_ip in
-    Log.info (fun f -> f "who-has %s?" (Ipaddr.V4.to_string req_ipv4));
+    let pf (f : ?header:string -> ?tags:_ -> _) fmt =
+      f ~header:t.client_link#log_header ("who-has %a? " ^^ fmt) Ipaddr.V4.pp req_ipv4
+    in
     if req_ipv4 = t.client_link#other_ip then (
-      Log.info (fun f -> f "ignoring request for client's own IP");
+      Log.info (fun f -> pf f "ignoring request for client's own IP");
       None
     ) else match lookup t req_ipv4 with
       | None ->
-        Log.info (fun f -> f "unknown address; not responding");
+        Log.info (fun f -> pf f "unknown address; not responding");
         None
       | Some req_mac ->
-        Log.info (fun f -> f "responding to: who-has %s?" (Ipaddr.V4.to_string req_ipv4));
+        Log.info (fun f -> pf f "responding with %a" Macaddr.pp req_mac);
         Some { Arp_packet.
                operation = Arp_packet.Reply;
                (* The Target Hardware Address and IP are copied from the request *)
@@ -105,15 +111,16 @@ module ARP = struct
   let input_gratuitous t arp =
     let source_ip = arp.Arp_packet.source_ip in
     let source_mac = arp.Arp_packet.source_mac in
+    let header = t.client_link#log_header in
     match lookup t source_ip with
     | Some real_mac when Macaddr.compare source_mac real_mac = 0 ->
-      Log.info (fun f -> f "client suggests updating %s -> %s (as expected)"
+      Log.info (fun f -> f ~header "client suggests updating %s -> %s (as expected)"
                    (Ipaddr.V4.to_string source_ip) (Macaddr.to_string source_mac));
     | Some other_mac ->
-      Log.warn (fun f -> f "client suggests incorrect update %s -> %s (should be %s)"
+      Log.warn (fun f -> f ~header "client suggests incorrect update %s -> %s (should be %s)"
                    (Ipaddr.V4.to_string source_ip) (Macaddr.to_string source_mac) (Macaddr.to_string other_mac));
     | None ->
-      Log.warn (fun f -> f "client suggests incorrect update %s -> %s (unexpected IP)"
+      Log.warn (fun f -> f ~header "client suggests incorrect update %s -> %s (unexpected IP)"
                    (Ipaddr.V4.to_string source_ip) (Macaddr.to_string source_mac))
 
   let input t arp =

client_net.ml

@@ -26,17 +26,20 @@ let writev eth dst proto fillfn =
        Lwt.return ()
     )
 
-class client_iface eth ~gateway_ip ~client_ip client_mac : client_link = object
-  val queue = FrameQ.create (Ipaddr.V4.to_string client_ip)
-  method my_mac = ClientEth.mac eth
-  method other_mac = client_mac
-  method my_ip = gateway_ip
-  method other_ip = client_ip
-  method writev proto fillfn =
-    FrameQ.send queue (fun () ->
-        writev eth client_mac proto fillfn
-    )
-end
+class client_iface eth ~domid ~gateway_ip ~client_ip client_mac : client_link =
+  let log_header = Fmt.strf "dom%d:%a" domid Ipaddr.V4.pp client_ip in
+  object
+    val queue = FrameQ.create (Ipaddr.V4.to_string client_ip)
+    method my_mac = ClientEth.mac eth
+    method other_mac = client_mac
+    method my_ip = gateway_ip
+    method other_ip = client_ip
+    method writev proto fillfn =
+      FrameQ.send queue (fun () ->
+          writev eth client_mac proto fillfn
+        )
+    method log_header = log_header
+  end
 
 let clients : Cleanup.t Dao.VifMap.t ref = ref Dao.VifMap.empty
 
@@ -73,10 +76,10 @@ let add_vif { Dao.ClientVif.domid; device_id } ~client_ip ~router ~cleanup_tasks
   Netback.make ~domid ~device_id >>= fun backend ->
   Log.info (fun f -> f "Client %d (IP: %s) ready" domid (Ipaddr.V4.to_string client_ip));
   ClientEth.connect backend >>= fun eth ->
-  let client_mac = Netback.mac backend in
+  let client_mac = Netback.frontend_mac backend in
   let client_eth = router.Router.client_eth in
   let gateway_ip = Client_eth.client_gw client_eth in
-  let iface = new client_iface eth ~gateway_ip ~client_ip client_mac in
+  let iface = new client_iface eth ~domid ~gateway_ip ~client_ip client_mac in
   Router.add_client router iface >>= fun () ->
   Cleanup.on_cleanup cleanup_tasks (fun () -> Router.remove_client router iface);
   let fixed_arp = Client_eth.ARP.create ~net:client_eth iface in
@@ -99,7 +102,7 @@ let add_vif { Dao.ClientVif.domid; device_id } ~client_ip ~router ~cleanup_tasks
 (** A new client VM has been found in XenStore. Find its interface and connect to it. *)
 let add_client ~router vif client_ip =
   let cleanup_tasks = Cleanup.create () in
-  Log.info (fun f -> f "add client vif %a" Dao.ClientVif.pp vif);
+  Log.info (fun f -> f "add client vif %a with IP %a" Dao.ClientVif.pp vif Ipaddr.V4.pp client_ip);
   Lwt.async (fun () ->
       Lwt.catch (fun () ->
           add_vif vif ~client_ip ~router ~cleanup_tasks

config.ml

@@ -27,7 +27,7 @@ let main =
       package "ethernet";
       package "mirage-protocols";
       package "shared-memory-ring" ~min:"3.0.0";
-      package "netchannel" ~min:"1.10.2";
+      package "netchannel" ~min:"1.11.0" ~pin:"git+https://github.com/mirage/mirage-net-xen.git";
       package "mirage-net-xen";
       package "ipaddr" ~min:"3.0.0";
       package "mirage-qubes";

fw_utils.ml

@@ -30,6 +30,7 @@ end
 class type client_link = object
   inherit interface
   method other_mac : Macaddr.t
+  method log_header : string  (* For log messages *)
 end
 
 (** An Ethernet header from [src]'s MAC address to [dst]'s with an IPv4 payload. *)