From 68b61c2c6d1b18dff4db1c06f6c82521202b7a2c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?=
 <marmarek@invisiblethingslab.com>
Date: Thu, 19 Dec 2019 01:14:39 +0100
Subject: [PATCH 01/13] network: setup anti-spoofing firewall rules before
 enabling the interface

Previously enabling the interface was the first action in the setup
steps. Linux theoretically do not forward the traffic until proper
IP address and route is added to the interface (depending on rp_filter
setting). But instead of relying on this opaque behavior better setup
anti-spoofing rules earlier. Also, add 'set -o pipefail' for more
reliable error handling.
Note the rules for actual VM traffic (qvm-firewall) are properly
enforced - until those rules are loaded, traffic from appropriate vif
interface is blocked. But this relies on proper source IP address,
anti-spoofing rules need to be setup race-free.

Reported-by: Demi M. Obenour <demiobenour@gmail.com>
---
 network/vif-route-qubes | 38 +++++++++++++++++++++++++-------------
 1 file changed, 25 insertions(+), 13 deletions(-)

diff --git a/network/vif-route-qubes b/network/vif-route-qubes
index 724d22a..8c1426f 100755
--- a/network/vif-route-qubes
+++ b/network/vif-route-qubes
@@ -24,6 +24,7 @@ dir=$(dirname "$0")
 # shellcheck disable=SC1091,SC1090
 . "$dir/vif-common.sh"
 
+set -o pipefail
 #main_ip=$(dom0_ip)
 
 # Network Hooks for triggering supplementary actions on AppVM connect
@@ -95,7 +96,6 @@ fi
 # shellcheck disable=SC2154
 case "$command" in
     online)
-        ifconfig "${vif}" up
         echo 1 >"/proc/sys/net/ipv4/conf/${vif}/proxy_arp"
         ipcmd='add'
         iptables_cmd='-I PREROUTING 1'
@@ -118,6 +118,30 @@ domid=${domid/.*/}
 #  32752 is max XID aka domid
 metric=$(( 32752 - domid ))
 
+# add anti-spoofing rules before enabling the interface
+if [ "${ip}" ]; then
+    # If we've been given a list of IP addresses, then add routes from dom0 to
+    # the guest using those addresses.
+    for addr in ${ip};
+    do
+        if [[ "$addr" = *:* ]]; then
+            ipt=ip6tables-restore
+        else
+            ipt=iptables-restore
+        fi
+        echo -e "*raw\\n$iptables_cmd -i ${vif} ! -s ${addr} -j DROP\\nCOMMIT" | \
+            ${cmdprefix} $ipt --noflush $ipt_arg
+    done
+    # if no IPv6 is assigned, block all IPv6 traffic on that interface
+    if ! [[ "$ip" = *:* ]]; then
+        echo -e "*raw\\n$iptables_cmd -i ${vif} -j DROP\\nCOMMIT" | \
+            ${cmdprefix} ip6tables-restore --noflush $ipt_arg
+    fi
+fi
+
+if [ "$command" = "online" ]; then
+    ifconfig "${vif}" up
+fi
 
 if [ "${ip}" ]; then
     # If we've been given a list of IP addresses, then add routes from dom0 to
@@ -129,21 +153,9 @@ if [ "${ip}" ]; then
             continue
         fi
         ${cmdprefix} ip route "${ipcmd}" "${addr}" dev "${vif}" metric "$metric"
-        if [[ "$addr" = *:* ]]; then
-            ipt=ip6tables-restore
-        else
-            ipt=iptables-restore
-        fi
-        echo -e "*raw\\n$iptables_cmd -i ${vif} ! -s ${addr} -j DROP\\nCOMMIT" | \
-            ${cmdprefix} $ipt --noflush $ipt_arg
 
         network_hooks "${command}" "${vif}" "${addr}"
     done
-    # if no IPv6 is assigned, block all IPv6 traffic on that interface
-    if ! [[ "$ip" = *:* ]]; then
-        echo -e "*raw\\n$iptables_cmd -i ${vif} -j DROP\\nCOMMIT" | \
-            ${cmdprefix} ip6tables-restore --noflush $ipt_arg
-    fi
     ${cmdprefix} ip addr "${ipcmd}" "${back_ip}/32" dev "${vif}"
     if [ "${back_ip6}" ] && [[ "${back_ip6}" != "fe80:"* ]] && [[ "$ipv6_disabled" != 1 ]]; then
         ${cmdprefix} ip addr "${ipcmd}" "${back_ip6}/128" dev "${vif}"

From 74f5fb5ac7160f145e8383d39d7c41c240f1f0ce Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?=
 <marmarek@invisiblethingslab.com>
Date: Wed, 18 Dec 2019 05:57:45 +0100
Subject: [PATCH 02/13] network: prevent IP spoofing on upstream (eth0)
 interface

Currently there is just one anti-spoofing firewall rule ensuring packets
coming through vif+ interfaces have the right source address. Add
another rule ensuring that addresses that belongs to VMs behind those
vif+ interface do not appear on other interfaces (specifically eth0, but
also physical ones).

Normally it wouldn't be an issue because of rp_filter (doing the same
based on route table), default DROP in FORWARD chain and also conntrack
(the need to guess exact port numbers and sequence numbers). But it
appears all three mechanisms are ineffective in some cases:
 - rp_filter in many distributions (including Fedora and Debian) was
 switched to Loose Mode, which doesn't verify exact interface
 - there is a rule in FORWARD table allowing established connections and
 conntrack does not keep track of input/output interfaces
 - CVE-2019-14899 allows to guess all the data needed to inject packets

Reported-by: Demi M. Obenour <demiobenour@gmail.com>
---
 network/vif-route-qubes | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/network/vif-route-qubes b/network/vif-route-qubes
index 8c1426f..345b120 100755
--- a/network/vif-route-qubes
+++ b/network/vif-route-qubes
@@ -129,7 +129,10 @@ if [ "${ip}" ]; then
         else
             ipt=iptables-restore
         fi
-        echo -e "*raw\\n$iptables_cmd -i ${vif} ! -s ${addr} -j DROP\\nCOMMIT" | \
+        printf '%s\n' "*raw" \
+            "$iptables_cmd -i ${vif} ! -s ${addr} -j DROP" \
+            "$iptables_cmd ! -i vif+ -s ${addr} -j DROP" \
+            "COMMIT" | \
             ${cmdprefix} $ipt --noflush $ipt_arg
     done
     # if no IPv6 is assigned, block all IPv6 traffic on that interface

From aa71677cbd881516b8e61f5e7a8c3a80a0dbbe50 Mon Sep 17 00:00:00 2001
From: Demi Marie Obenour <athena@invisiblethingslab.com>
Date: Tue, 10 Nov 2020 16:28:53 -0500
Subject: [PATCH 03/13] Add permanent neighbor entries

This allows network traffic to flow even if ARP and NDP do not work or
ave explicitly been disabled.
---
 network/setup-ip        | 18 +++++++++++++++---
 network/vif-route-qubes |  6 +++++-
 2 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/network/setup-ip b/network/setup-ip
index 917b028..b8900d0 100755
--- a/network/setup-ip
+++ b/network/setup-ip
@@ -4,6 +4,14 @@
 # shellcheck disable=SC1091
 . /usr/lib/qubes/init/functions
 
+add_host_route () {
+    /sbin/ip -- route replace to unicast "$1" dev "$2" onlink scope host
+}
+
+add_default_route () {
+    /sbin/ip -- route replace to unicast default via "$1" dev "$2" onlink
+}
+
 configure_network() {
     local MAC="$1"
     local INTERFACE="$2"
@@ -17,20 +25,24 @@ configure_network() {
     local secondary_dns="${10}"
 
     /sbin/ifconfig "$INTERFACE" "$ip" netmask "$netmask"
+    /sbin/ip -- neighbour replace to "$ip" dev "$INTERFACE" \
+        lladdr "$MAC" nud permanent
     if [ -n "$ip6" ]; then
         /sbin/ifconfig "$INTERFACE" add "$ip6/$netmask6"
+        /sbin/ip -- neighbour replace to "$ip6" dev "$INTERFACE" \
+            lladdr "$MAC" nud permanent
     fi
     /sbin/ifconfig "$INTERFACE" up
 
     if [ -n "$gateway" ]; then
         /sbin/route add -host "$gateway" dev "$INTERFACE"
         if [ -n "$gateway6" ] && ! echo "$gateway6" | grep -q "^fe80:"; then
-            /sbin/route -6 add "$gateway6/$netmask6" dev "$INTERFACE"
+            add_route "$gateway6/$netmask6" "$INTERFACE"
         fi
         if ! qsvc disable-default-route ; then
-            /sbin/route add default gw "$gateway"
+            add_default_route "$gateway" "$INTERFACE"
             if [ -n "$gateway6" ]; then
-                /sbin/route -6 add default gw "$gateway6" dev "$INTERFACE"
+                add_default_route "$gateway6" "$INTERFACE"
             fi
         fi
     fi
diff --git a/network/vif-route-qubes b/network/vif-route-qubes
index 345b120..82c0549 100755
--- a/network/vif-route-qubes
+++ b/network/vif-route-qubes
@@ -134,6 +134,10 @@ if [ "${ip}" ]; then
             "$iptables_cmd ! -i vif+ -s ${addr} -j DROP" \
             "COMMIT" | \
             ${cmdprefix} $ipt --noflush $ipt_arg
+        if [[ "$command" = 'online' ]]; then
+            ip neighbour "${ipcmd}" to "${addr}" \
+                dev "${vif}" lladdr 00:16:3e:5e:6c:6c:00 nud permanent
+        fi
     done
     # if no IPv6 is assigned, block all IPv6 traffic on that interface
     if ! [[ "$ip" = *:* ]]; then
@@ -155,7 +159,7 @@ if [ "${ip}" ]; then
             log error "Cannot set IPv6 route to ${addr}, IPv6 disabled in the kernel"
             continue
         fi
-        ${cmdprefix} ip route "${ipcmd}" "${addr}" dev "${vif}" metric "$metric"
+        ${cmdprefix} ip route "${ipcmd}" "${addr}" dev "${vif}" metric "$metric" onlink
 
         network_hooks "${command}" "${vif}" "${addr}"
     done

From 0a322958e4cc399cfa15382aa6aa5b713dbb3cb9 Mon Sep 17 00:00:00 2001
From: Demi Marie Obenour <athena@invisiblethingslab.com>
Date: Tue, 10 Nov 2020 22:09:54 -0500
Subject: [PATCH 04/13] =?UTF-8?q?Add=20gateway=20IP+MAC,=20not=20VM?=
 =?UTF-8?q?=E2=80=99s=20own?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 network/setup-ip | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/network/setup-ip b/network/setup-ip
index b8900d0..b050d63 100755
--- a/network/setup-ip
+++ b/network/setup-ip
@@ -23,21 +23,22 @@ configure_network() {
     local gateway6="$8"
     local primary_dns="$9"
     local secondary_dns="${10}"
+    local netvm_mac=fe:ff:ff:ff:ff:ff
 
     /sbin/ifconfig "$INTERFACE" "$ip" netmask "$netmask"
-    /sbin/ip -- neighbour replace to "$ip" dev "$INTERFACE" \
-        lladdr "$MAC" nud permanent
+    /sbin/ip -- neighbour replace to "$gateway" dev "$INTERFACE" \
+        lladdr "$netvm_mac" nud permanent
     if [ -n "$ip6" ]; then
         /sbin/ifconfig "$INTERFACE" add "$ip6/$netmask6"
-        /sbin/ip -- neighbour replace to "$ip6" dev "$INTERFACE" \
-            lladdr "$MAC" nud permanent
+        /sbin/ip -- neighbour replace to "$gateway6" dev "$INTERFACE" \
+            lladdr "$netvm_mac" nud permanent
     fi
     /sbin/ifconfig "$INTERFACE" up
 
     if [ -n "$gateway" ]; then
-        /sbin/route add -host "$gateway" dev "$INTERFACE"
+        add_host_route "$gateway" "$INTERFACE"
         if [ -n "$gateway6" ] && ! echo "$gateway6" | grep -q "^fe80:"; then
-            add_route "$gateway6/$netmask6" "$INTERFACE"
+            add_host_route "$gateway6/$netmask6" "$INTERFACE"
         fi
         if ! qsvc disable-default-route ; then
             add_default_route "$gateway" "$INTERFACE"

From 377add43d19c0ac93d9e060675f6de4ce1eb42c5 Mon Sep 17 00:00:00 2001
From: Demi Marie Obenour <athena@invisiblethingslab.com>
Date: Tue, 10 Nov 2020 22:31:18 -0500
Subject: [PATCH 05/13] =?UTF-8?q?Don=E2=80=99t=20hardcode=20MAC=20addresse?=
 =?UTF-8?q?s?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 network/vif-route-qubes | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/network/vif-route-qubes b/network/vif-route-qubes
index 82c0549..ec33462 100755
--- a/network/vif-route-qubes
+++ b/network/vif-route-qubes
@@ -112,8 +112,8 @@ case "$command" in
         ;;
 esac
 
-domid=${vif/vif/}
-domid=${domid/.*/}
+[[ $vif =~ ^vif([0-9]+)\.([0-9]+)$ ]] || exit 1
+domid=${BASH_REMATCH[1]} sub=${BASH_REMATCH[2]}
 # metric must be possitive, but prefer later interface
 #  32752 is max XID aka domid
 metric=$(( 32752 - domid ))
@@ -135,8 +135,9 @@ if [ "${ip}" ]; then
             "COMMIT" | \
             ${cmdprefix} $ipt --noflush $ipt_arg
         if [[ "$command" = 'online' ]]; then
-            ip neighbour "${ipcmd}" to "${addr}" \
-                dev "${vif}" lladdr 00:16:3e:5e:6c:6c:00 nud permanent
+            mac=$(xenstore-read "backend/vif/$domid/$sub/mac") &&
+            ip -- neighbour "${ipcmd}" to "${addr}" \
+                dev "${vif}" lladdr "$mac" nud permanent
         fi
     done
     # if no IPv6 is assigned, block all IPv6 traffic on that interface

From 3e7552856fe78bae1070ab3f35ee82514c0d2359 Mon Sep 17 00:00:00 2001
From: Demi Marie Obenour <athena@invisiblethingslab.com>
Date: Wed, 11 Nov 2020 14:00:24 -0500
Subject: [PATCH 06/13] Fix running under -euo pipefail

Some qubesdb-read commands are expected to fail.  I ultimately did not
wind up including -e, but this version should be ready for it.
---
 network/setup-ip | 29 +++++++++++++++--------------
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/network/setup-ip b/network/setup-ip
index b050d63..35e9fba 100755
--- a/network/setup-ip
+++ b/network/setup-ip
@@ -3,6 +3,7 @@
 # Source Qubes library.
 # shellcheck disable=SC1091
 . /usr/lib/qubes/init/functions
+set -uo pipefail
 
 add_host_route () {
     /sbin/ip -- route replace to unicast "$1" dev "$2" onlink scope host
@@ -176,27 +177,27 @@ if [ -n "$INTERFACE" ]; then
     if [ "$ACTION" == "add" ]; then
         MAC="$(get_mac_from_iface "$INTERFACE")"
         if [ -n "$MAC" ]; then
-            ip="$(/usr/bin/qubesdb-read "/net-config/$MAC/ip" 2> /dev/null)"
-            ip6="$(/usr/bin/qubesdb-read "/net-config/$MAC/ip6" 2> /dev/null)"
-            netmask="$(/usr/bin/qubesdb-read "/net-config/$MAC/netmask" 2> /dev/null)"
-            netmask6="$(/usr/bin/qubesdb-read "/net-config/$MAC/netmask6" 2> /dev/null)"
-            gateway="$(/usr/bin/qubesdb-read "/net-config/$MAC/gateway" 2> /dev/null)"
-            gateway6="$(/usr/bin/qubesdb-read "/net-config/$MAC/gateway6" 2> /dev/null)"
+            ip="$(/usr/bin/qubesdb-read "/net-config/$MAC/ip" 2> /dev/null)" || ip=
+            ip6="$(/usr/bin/qubesdb-read "/net-config/$MAC/ip6" 2> /dev/null)" || ip6=
+            netmask="$(/usr/bin/qubesdb-read "/net-config/$MAC/netmask" 2> /dev/null)" || netmask=
+            netmask6="$(/usr/bin/qubesdb-read "/net-config/$MAC/netmask6" 2> /dev/null)" || netmask6=
+            gateway="$(/usr/bin/qubesdb-read "/net-config/$MAC/gateway" 2> /dev/null)" || gateway=
+            gateway6="$(/usr/bin/qubesdb-read "/net-config/$MAC/gateway6" 2> /dev/null)" || gateway6=
 
             # Handle legacy values
-            LEGACY_MAC="$(/usr/bin/qubesdb-read /qubes-mac 2> /dev/null)"
+            LEGACY_MAC="$(/usr/bin/qubesdb-read /qubes-mac 2> /dev/null)" || LEGACY_MAC=
             if [ "$MAC" == "$LEGACY_MAC" ] || [ -z "$LEGACY_MAC" ]; then
                 if [ -z "$ip" ]; then
-                    ip="$(/usr/bin/qubesdb-read /qubes-ip 2> /dev/null)"
+                    ip="$(/usr/bin/qubesdb-read /qubes-ip 2> /dev/null)" || ip=
                 fi
                 if [ -z "$ip6" ]; then
-                    ip6="$(/usr/bin/qubesdb-read /qubes-ip6 2> /dev/null)"
+                    ip6="$(/usr/bin/qubesdb-read /qubes-ip6 2> /dev/null)" || ip6=
                 fi
                 if [ -z "$gateway" ]; then
-                    gateway="$(/usr/bin/qubesdb-read /qubes-gateway 2> /dev/null)"
+                    gateway="$(/usr/bin/qubesdb-read /qubes-gateway 2> /dev/null)" || gateway=
                 fi
                 if [ -z "$gateway6" ]; then
-                    gateway6="$(/usr/bin/qubesdb-read /qubes-gateway6 2> /dev/null)"
+                    gateway6="$(/usr/bin/qubesdb-read /qubes-gateway6 2> /dev/null)" || gateway6=
                 fi
             fi
 
@@ -207,8 +208,8 @@ if [ -n "$INTERFACE" ]; then
                 netmask6="128"
             fi
 
-            primary_dns=$(/usr/bin/qubesdb-read /qubes-primary-dns 2>/dev/null)
-            secondary_dns=$(/usr/bin/qubesdb-read /qubes-secondary-dns 2>/dev/null)
+            primary_dns=$(/usr/bin/qubesdb-read /qubes-primary-dns 2>/dev/null) || primary_dns=
+            secondary_dns=$(/usr/bin/qubesdb-read /qubes-secondary-dns 2>/dev/null) || secondary_dns=
 
             if [ -n "$ip" ]; then
                 /sbin/ethtool -K "$INTERFACE" sg off
@@ -221,7 +222,7 @@ if [ -n "$INTERFACE" ]; then
                     configure_network "$MAC" "$INTERFACE" "$ip" "$ip6" "$netmask" "$netmask6" "$gateway" "$gateway6" "$primary_dns" "$secondary_dns"
                 fi
 
-                network=$(qubesdb-read /qubes-netvm-network 2>/dev/null)
+                network=$(qubesdb-read /qubes-netvm-network 2>/dev/null) || network=
                 if [ -n "$network" ]; then
                     if ! qsvc disable-dns-server; then
                         configure_qubes_ns

From 9646acb18e6eaa2e29e8398833c058d79c7d1167 Mon Sep 17 00:00:00 2001
From: Demi Marie Obenour <athena@invisiblethingslab.com>
Date: Fri, 13 Nov 2020 12:51:15 -0500
Subject: [PATCH 07/13] =?UTF-8?q?Don=E2=80=99t=20use=20onlink=20flag=20for?=
 =?UTF-8?q?=20nexthop?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This is rejected by the kernel.
---
 network/vif-route-qubes | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/network/vif-route-qubes b/network/vif-route-qubes
index ec33462..e87806c 100755
--- a/network/vif-route-qubes
+++ b/network/vif-route-qubes
@@ -160,7 +160,7 @@ if [ "${ip}" ]; then
             log error "Cannot set IPv6 route to ${addr}, IPv6 disabled in the kernel"
             continue
         fi
-        ${cmdprefix} ip route "${ipcmd}" "${addr}" dev "${vif}" metric "$metric" onlink
+        ${cmdprefix} ip route "${ipcmd}" "${addr}" dev "${vif}" metric "$metric"
 
         network_hooks "${command}" "${vif}" "${addr}"
     done

From 791b08c2ecf6021946531c38fa228f0926c86ac4 Mon Sep 17 00:00:00 2001
From: Demi Marie Obenour <athena@invisiblethingslab.com>
Date: Fri, 13 Nov 2020 13:15:24 -0500
Subject: [PATCH 08/13] vif-route-qubes: better input validation

The input is trusted, but this will help debugging if something goes
wrong.
---
 network/vif-route-qubes | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/network/vif-route-qubes b/network/vif-route-qubes
index e87806c..85881b7 100755
--- a/network/vif-route-qubes
+++ b/network/vif-route-qubes
@@ -112,16 +112,28 @@ case "$command" in
         ;;
 esac
 
-[[ $vif =~ ^vif([0-9]+)\.([0-9]+)$ ]] || exit 1
+readonly max_domid=32752
+
+# if domid is 0 something is seriously wrong, so don’t check for that case
+if ! [[ $vif =~ ^vif([1-9][0-9]{,4})\.(0|[1-9][0-9]*)$ ]]; then
+    printf 'Bad interface name %q\n' "$vif">&2
+    exit 1
+fi
+
 domid=${BASH_REMATCH[1]} sub=${BASH_REMATCH[2]}
-# metric must be possitive, but prefer later interface
+
+# metric must be positive, but prefer later interface
 #  32752 is max XID aka domid
-metric=$(( 32752 - domid ))
+if (( domid > max_domid )); then
+    printf %s\\n "domid $domid too large"
+    exit 1
+fi
+metric=$(( max_domid - domid ))
 
 # add anti-spoofing rules before enabling the interface
 if [ "${ip}" ]; then
-    # If we've been given a list of IP addresses, then add routes from dom0 to
-    # the guest using those addresses.
+    # If we’ve been given a list of IP addresses, then add routes from us to
+    # the VMs we serve using those addresses.
     for addr in ${ip};
     do
         if [[ "$addr" = *:* ]]; then

From 6517cca2a4f56568812f53b9a93784b16d894012 Mon Sep 17 00:00:00 2001
From: Demi Marie Obenour <athena@invisiblethingslab.com>
Date: Thu, 19 Nov 2020 12:04:02 -0500
Subject: [PATCH 09/13] NAT network namespaces need neighbor entries

If we are using a NAT network namespace, it needs its own neighbor
entries.  For consistency, give it the same MAC address as the VM it
connects to.
---
 network/vif-qubes-nat.sh | 10 +++++--
 network/vif-route-qubes  | 65 +++++++++++++++++++---------------------
 2 files changed, 37 insertions(+), 38 deletions(-)

diff --git a/network/vif-qubes-nat.sh b/network/vif-qubes-nat.sh
index 9833e45..2789d34 100755
--- a/network/vif-qubes-nat.sh
+++ b/network/vif-qubes-nat.sh
@@ -46,9 +46,12 @@ if test "$command" == online; then
     run ip link set "$netns_appvm_if" netns "$netns"
 
     # keep the same MAC as the real vif interface, so NetworkManager will still
-    # ignore it
-    run ip link add "$netns_netvm_if" type veth peer name "$netvm_if" address fe:ff:ff:ff:ff:ff
-    run ip link set "$netns_netvm_if" netns "$netns"
+    # ignore it.
+    # for the peer interface, make sure that it has the same MAC address
+    # as the actual VM, so that our neighbor entry works.
+    run ip link add name "$netns_netvm_if" address "$mac" type veth \
+        peer name "$netvm_if" address fe:ff:ff:ff:ff:ff
+    run ip link set dev "$netns_netvm_if" netns "$netns"
 
     netns ip6tables -t raw -I PREROUTING -j DROP
     netns ip6tables -P INPUT DROP
@@ -85,6 +88,7 @@ if test "$command" == online; then
         netns iptables -t nat -I POSTROUTING -o "$netns_appvm_if" -s "$netvm_dns2_ip" -j SNAT --to-source "$appvm_dns2_ip"
     fi
 
+    netns ip neighbour add to "$appvm_ip" dev "$netns_appvm_if" lladdr "$mac" nud permanent
     netns ip addr add "$netvm_ip" dev "$netns_netvm_if"
     netns ip addr add "$appvm_gw_ip" dev "$netns_appvm_if"
 
diff --git a/network/vif-route-qubes b/network/vif-route-qubes
index 85881b7..1cf43d2 100755
--- a/network/vif-route-qubes
+++ b/network/vif-route-qubes
@@ -78,40 +78,6 @@ if [ "${ip}" ]; then
     appvm_ip="$(qubesdb-read "/mapped-ip/$ip4/visible-ip" 2>/dev/null || :)"
 fi
 
-# Apply NAT if IP visible from the VM is different than the "real" one
-# See vif-qubes-nat.sh for details
-# XXX: supported only for the first IPv4 address, IPv6 is dropped if this
-# feature is enabled
-if [ -n "$appvm_ip" ] && [ -n "$appvm_gw_ip" ] && [ "$appvm_ip" != "$netvm_ip" ]; then
-    # shellcheck disable=SC2154
-    if test "$command" == online; then
-        # shellcheck disable=SC2154
-        echo 1 >"/proc/sys/net/ipv4/conf/${vif}/proxy_arp"
-    fi
-
-    # shellcheck source=network/vif-qubes-nat.sh
-    . "$dir/vif-qubes-nat.sh"
-fi
-
-# shellcheck disable=SC2154
-case "$command" in
-    online)
-        echo 1 >"/proc/sys/net/ipv4/conf/${vif}/proxy_arp"
-        ipcmd='add'
-        iptables_cmd='-I PREROUTING 1'
-        cmdprefix=''
-        ipv6_disabled=$(cat /proc/sys/net/ipv6/conf/"${vif}"/disable_ipv6 || echo 1)
-        ;;
-    offline)
-        do_without_error ifdown "${vif}"
-        ipcmd='del'
-        iptables_cmd='-D PREROUTING'
-        cmdprefix='do_without_error'
-        # cleanup IPv6 config even if _now_ it is disabled
-        ipv6_disabled=0
-        ;;
-esac
-
 readonly max_domid=32752
 
 # if domid is 0 something is seriously wrong, so don’t check for that case
@@ -130,6 +96,36 @@ if (( domid > max_domid )); then
 fi
 metric=$(( max_domid - domid ))
 
+# shellcheck disable=SC2154
+case "$command" in
+    online)
+        echo 1 >"/proc/sys/net/ipv4/conf/${vif}/proxy_arp"
+        ipcmd='add'
+        iptables_cmd='-I PREROUTING 1'
+        cmdprefix=''
+        ipv6_disabled=$(cat /proc/sys/net/ipv6/conf/"${vif}"/disable_ipv6 || echo 1)
+        # without a MAC address we will fail later with a confusing error
+        mac=$(xenstore-read "backend/vif/$domid/$sub/mac") || exit 1
+        ;;
+    offline)
+        do_without_error ifdown "${vif}"
+        ipcmd='del'
+        iptables_cmd='-D PREROUTING'
+        cmdprefix='do_without_error'
+        # cleanup IPv6 config even if _now_ it is disabled
+        ipv6_disabled=0
+        ;;
+esac
+
+# Apply NAT if IP visible from the VM is different than the "real" one
+# See vif-qubes-nat.sh for details
+# XXX: supported only for the first IPv4 address, IPv6 is dropped if this
+# feature is enabled
+if [ -n "$appvm_ip" ] && [ -n "$appvm_gw_ip" ] && [ "$appvm_ip" != "$netvm_ip" ]; then
+    # shellcheck source=network/vif-qubes-nat.sh
+    . "$dir/vif-qubes-nat.sh"
+fi
+
 # add anti-spoofing rules before enabling the interface
 if [ "${ip}" ]; then
     # If we’ve been given a list of IP addresses, then add routes from us to
@@ -147,7 +143,6 @@ if [ "${ip}" ]; then
             "COMMIT" | \
             ${cmdprefix} $ipt --noflush $ipt_arg
         if [[ "$command" = 'online' ]]; then
-            mac=$(xenstore-read "backend/vif/$domid/$sub/mac") &&
             ip -- neighbour "${ipcmd}" to "${addr}" \
                 dev "${vif}" lladdr "$mac" nud permanent
         fi

From 097342bd08a3fd2004038436d4f07e68c0af223d Mon Sep 17 00:00:00 2001
From: Demi Marie Obenour <athena@invisiblethingslab.com>
Date: Thu, 19 Nov 2020 12:10:26 -0500
Subject: [PATCH 10/13] Optimization: use `ip -n` over `ip netns exec`

This saves an exec call.
---
 network/vif-qubes-nat.sh | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/network/vif-qubes-nat.sh b/network/vif-qubes-nat.sh
index 2789d34..672f407 100755
--- a/network/vif-qubes-nat.sh
+++ b/network/vif-qubes-nat.sh
@@ -35,7 +35,12 @@ function run
 
 function netns
 {
-    run ip netns exec "$netns" "$@"
+    if [[ "$1" = 'ip' ]]; then
+        shift
+        run ip -n "$netns" "$@"
+    else
+        run ip netns exec "$netns" "$@"
+    fi
 }
 
 run ip addr flush dev "$netns_appvm_if"

From e4eeb2ee1bc8e69e25f7333961ff4da64b511f2b Mon Sep 17 00:00:00 2001
From: Demi Marie Obenour <athena@invisiblethingslab.com>
Date: Thu, 19 Nov 2020 12:16:15 -0500
Subject: [PATCH 11/13] Add NetVM-facing neighbor entry in NAT namespace

Since AppVMs will have their own NetVM-facing neighbor entries, a user
might (correctly) conclude that NetVMs do not need ARP or NDP enabled.
For this to work with NAT namespaces, they need their own neighbor
entries.
---
 network/vif-qubes-nat.sh | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/network/vif-qubes-nat.sh b/network/vif-qubes-nat.sh
index 672f407..058d8a3 100755
--- a/network/vif-qubes-nat.sh
+++ b/network/vif-qubes-nat.sh
@@ -26,6 +26,7 @@ netns_appvm_if="${vif}"
 #               '----------------------------------'
 #
 
+readonly netvm_mac=fe:ff:ff:ff:ff:ff
 
 function run
 {
@@ -55,7 +56,7 @@ if test "$command" == online; then
     # for the peer interface, make sure that it has the same MAC address
     # as the actual VM, so that our neighbor entry works.
     run ip link add name "$netns_netvm_if" address "$mac" type veth \
-        peer name "$netvm_if" address fe:ff:ff:ff:ff:ff
+        peer name "$netvm_if" address "$netvm_mac"
     run ip link set dev "$netns_netvm_if" netns "$netns"
 
     netns ip6tables -t raw -I PREROUTING -j DROP
@@ -94,6 +95,7 @@ if test "$command" == online; then
     fi
 
     netns ip neighbour add to "$appvm_ip" dev "$netns_appvm_if" lladdr "$mac" nud permanent
+    netns ip neighbour add to "$netvm_ip" dev "$netns_netvm_if" lladdr "$netvm_mac" nud permanent
     netns ip addr add "$netvm_ip" dev "$netns_netvm_if"
     netns ip addr add "$appvm_gw_ip" dev "$netns_appvm_if"
 

From 9d10ecc08f8903aa94b76b644038f155a9453c7f Mon Sep 17 00:00:00 2001
From: Demi Marie Obenour <athena@invisiblethingslab.com>
Date: Thu, 19 Nov 2020 15:19:40 -0500
Subject: [PATCH 12/13] Remove commented-out code

---
 network/vif-qubes-nat.sh | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/network/vif-qubes-nat.sh b/network/vif-qubes-nat.sh
index 058d8a3..21b3bb1 100755
--- a/network/vif-qubes-nat.sh
+++ b/network/vif-qubes-nat.sh
@@ -105,10 +105,4 @@ if test "$command" == online; then
     netns ip route add "$appvm_ip" dev "$netns_appvm_if" src "$appvm_gw_ip"
     netns ip route add "$netvm_gw_ip" dev "$netns_netvm_if" src "$netvm_ip"
     netns ip route add default via "$netvm_gw_ip" dev "$netns_netvm_if" src "$netvm_ip"
-
-
-    #run ip addr add "$netvm_gw_ip" dev "$netvm_if"
-    #run ip link set "$netvm_if" up
-    #run ip route add "$netvm_ip" dev "$netvm_if" src "$netvm_gw_ip"
 fi
-

From 0580fe545b7b4616f963694e1eddcc9788b53f38 Mon Sep 17 00:00:00 2001
From: Demi Marie Obenour <athena@invisiblethingslab.com>
Date: Sun, 22 Nov 2020 17:52:54 -0500
Subject: [PATCH 13/13] Use netvm_gw_ip instead of netvm_ip

They are usually identical, but this is not guaranteed.
---
 network/vif-qubes-nat.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/network/vif-qubes-nat.sh b/network/vif-qubes-nat.sh
index 21b3bb1..0c38731 100755
--- a/network/vif-qubes-nat.sh
+++ b/network/vif-qubes-nat.sh
@@ -95,7 +95,7 @@ if test "$command" == online; then
     fi
 
     netns ip neighbour add to "$appvm_ip" dev "$netns_appvm_if" lladdr "$mac" nud permanent
-    netns ip neighbour add to "$netvm_ip" dev "$netns_netvm_if" lladdr "$netvm_mac" nud permanent
+    netns ip neighbour add to "$netvm_gw_ip" dev "$netns_netvm_if" lladdr "$netvm_mac" nud permanent
     netns ip addr add "$netvm_ip" dev "$netns_netvm_if"
     netns ip addr add "$appvm_gw_ip" dev "$netns_appvm_if"