Merge remote-tracking branch 'origin/pr/232'

* origin/pr/232:
  Use netvm_gw_ip instead of netvm_ip
  Remove commented-out code
  Add NetVM-facing neighbor entry in NAT namespace
  Optimization: use `ip -n` over `ip netns exec`
  NAT network namespaces need neighbor entries
  vif-route-qubes: better input validation
  Don’t use onlink flag for nexthop
  Fix running under -euo pipefail
  Don’t hardcode MAC addresses
  Add gateway IP+MAC, not VM’s own
  Add permanent neighbor entries
  network: prevent IP spoofing on upstream (eth0) interface
  network: setup anti-spoofing firewall rules before enabling the interface

network/setup-ip

@@ -3,6 +3,15 @@
 # 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
+}
+
+add_default_route () {
+    /sbin/ip -- route replace to unicast default via "$1" dev "$2" onlink
+}
 
 configure_network() {
     local MAC="$1"
@@ -15,22 +24,27 @@ 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 "$gateway" dev "$INTERFACE" \
+        lladdr "$netvm_mac" nud permanent
     if [ -n "$ip6" ]; then
         /sbin/ifconfig "$INTERFACE" add "$ip6/$netmask6"
+        /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
-            /sbin/route -6 add "$gateway6/$netmask6" dev "$INTERFACE"
+            add_host_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
@@ -163,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)"
+            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="$(/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="$(/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="$(/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="$(/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="$(/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
 
@@ -194,8 +208,8 @@ if [ -n "$INTERFACE" ]; then
                 netmask6="128"
             fi
 
-            primary_dns=$(/usr/bin/qubesdb-read /qubes-primary-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=$(/usr/bin/qubesdb-read /qubes-secondary-dns 2>/dev/null) || secondary_dns=
 
             if [ -n "$ip" ]; then
                 /sbin/ethtool -K "$INTERFACE" sg off
@@ -208,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

network/vif-qubes-nat.sh

@@ -26,6 +26,7 @@ netns_appvm_if="${vif}"
 #               '----------------------------------'
 #
 
+readonly netvm_mac=fe:ff:ff:ff:ff:ff
 
 function run
 {
@@ -35,7 +36,12 @@ function run
 
 function 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"
@@ -46,9 +52,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
+    # ignore it.
-    run ip link add "$netns_netvm_if" type veth peer name "$netvm_if" address fe:ff:ff:ff:ff:ff
+    # for the peer interface, make sure that it has the same MAC address
-    run ip link set "$netns_netvm_if" netns "$netns"
+    # 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 "$netvm_mac"
+    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 +94,8 @@ 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 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"
 
@@ -94,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
-

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
@@ -77,30 +78,34 @@ 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
+readonly max_domid=32752
-# See vif-qubes-nat.sh for details
+
-# XXX: supported only for the first IPv4 address, IPv6 is dropped if this
+# if domid is 0 something is seriously wrong, so don’t check for that case
-# feature is enabled
+if ! [[ $vif =~ ^vif([1-9][0-9]{,4})\.(0|[1-9][0-9]*)$ ]]; then
-if [ -n "$appvm_ip" ] && [ -n "$appvm_gw_ip" ] && [ "$appvm_ip" != "$netvm_ip" ]; then
+    printf 'Bad interface name %q\n' "$vif">&2
-    # shellcheck disable=SC2154
+    exit 1
-    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
+domid=${BASH_REMATCH[1]} sub=${BASH_REMATCH[2]}
-    . "$dir/vif-qubes-nat.sh"
+
+# metric must be positive, but prefer later interface
+#  32752 is max XID aka domid
+if (( domid > max_domid )); then
+    printf %s\\n "domid $domid too large"
+    exit 1
 fi
+metric=$(( max_domid - domid ))
 
 # 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'
         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}"
@@ -112,12 +117,46 @@ case "$command" in
         ;;
 esac
 
-domid=${vif/vif/}
+# Apply NAT if IP visible from the VM is different than the "real" one
-domid=${domid/.*/}
+# See vif-qubes-nat.sh for details
-# metric must be possitive, but prefer later interface
+# XXX: supported only for the first IPv4 address, IPv6 is dropped if this
-#  32752 is max XID aka domid
+# feature is enabled
-metric=$(( 32752 - domid ))
+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
+    # the VMs we serve using those addresses.
+    for addr in ${ip};
+    do
+        if [[ "$addr" = *:* ]]; then
+            ipt=ip6tables-restore
+        else
+            ipt=iptables-restore
+        fi
+        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
+        if [[ "$command" = 'online' ]]; then
+            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
+    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 +168,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}"