From 5e3258825c339622185eefa161f60125aefbca33 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?=
 <marmarek@invisiblethingslab.com>
Date: Fri, 1 Nov 2024 05:20:36 +0100
Subject: [PATCH] network: do not restart tinyproxy if DNS hasn't changed

NetworkManager will call the hook when anything about the connection
changes. In some cases (like multiple connections) this can be quite
often and restarting tinyproxy each time will eventually trigger restart
rate limit and keep the service down. But even if not hitting this
limit, restart interrupts any download in progress.

Add a check if DNS addresses actually changed since the las hook call,
and restart the service only when necessary.
As a side effect, this will also avoid restarting the service on DHCP
lease renew.

Fixes QubesOS/qubes-issues#9110

(cherry picked from commit 01aa298f3f1b444d1c497dbdc8bfbe22f92740ef)
---
 network/qubes-nmhook           | 14 +++++++++-----
 network/qubes-setup-dnat-to-ns | 29 ++++++++++++++++++++++++-----
 network/setup-ip               |  4 +++-
 3 files changed, 36 insertions(+), 11 deletions(-)

diff --git a/network/qubes-nmhook b/network/qubes-nmhook
index a2065110..322b8fde 100755
--- a/network/qubes-nmhook
+++ b/network/qubes-nmhook
@@ -7,10 +7,14 @@
 if [ "$2" = "up" ] || [ "$2" = "vpn-up" ] || [ "$2" = "vpn-down" ] || [ "$2" = "dhcp4-change" ]; then
     /usr/lib/qubes/qubes-setup-dnat-to-ns
 
-    # FIXME: Tinyproxy does not reload DNS servers.
-    if under_systemd ; then
-        systemctl --no-block try-restart qubes-updates-proxy.service
-    else
-        service qubes-updates-proxy try-restart
+    # Do not restart updates proxy if DNS hasn't changed
+    # (qubes-setup-dnat-to-ns exits with 100 in such a case)
+    if [ $? -ne 100 ]; then
+        # FIXME: Tinyproxy does not reload DNS servers.
+        if under_systemd ; then
+            systemctl --no-block try-restart qubes-updates-proxy.service
+        else
+            service qubes-updates-proxy try-restart
+        fi
     fi
 fi
diff --git a/network/qubes-setup-dnat-to-ns b/network/qubes-setup-dnat-to-ns
index 70d2926e..3dcbe904 100755
--- a/network/qubes-setup-dnat-to-ns
+++ b/network/qubes-setup-dnat-to-ns
@@ -20,6 +20,10 @@
 #
 
 from __future__ import annotations
+
+import subprocess
+import sys
+
 import dbus
 import qubesdb
 from typing import List
@@ -85,7 +89,7 @@ def install_firewall_rules(dns):
             qubesdb_dns.append(IPv4Address(ns_maybe.decode("ascii", "strict")))
         except (UnicodeDecodeError, ValueError):
             pass
-    res = [
+    preamble = [
         'add table ip qubes',
         # Add the chain so that the subsequent delete will work. If the chain already
         # exists this is a harmless no-op.
@@ -95,6 +99,8 @@ def install_firewall_rules(dns):
         # atomic operation, so there is no period where neither chain is present or
         # where both are present.
         'delete chain ip qubes dnat-dns',
+    ]
+    rules = [
         'table ip qubes {',
         'chain dnat-dns {',
         'type nat hook prerouting priority dstnat; policy accept;',
@@ -105,7 +111,7 @@ def install_firewall_rules(dns):
         # Or maybe user wants to enforce DNS-Over-HTTPS.
         # Drop IPv4 DNS requests to qubesdb_dns addresses.
         for vm_nameserver in qubesdb_dns:
-            res += [
+            rules += [
                 f"ip daddr {vm_nameserver} udp dport 53 drop",
                 f"ip daddr {vm_nameserver} tcp dport 53 drop",
             ]
@@ -115,12 +121,25 @@ def install_firewall_rules(dns):
             dns_resolved = dns_resolved + dns_resolved
         for vm_nameserver, dest in zip(qubesdb_dns, dns_resolved):
             dns_ = str(dest)
-            res += [
+            rules += [
                 f"ip daddr {vm_nameserver} udp dport 53 dnat to {dns_}",
                 f"ip daddr {vm_nameserver} tcp dport 53 dnat to {dns_}",
             ]
-    res += ["}\n}\n"]
-    os.execvp("nft", ("nft", "--", "\n".join(res)))
+    rules += ["}", "}"]
+
+    # check if new rules are the same as the old ones - if so, don't reload
+    # and return that info via exit code
+    try:
+        old_rules = subprocess.check_output(
+            ["nft", "list", "chain", "ip", "qubes", "dnat-dns"]).decode().splitlines()
+    except subprocess.CalledProcessError:
+        old_rules = []
+    old_rules = [line.strip() for line in old_rules]
+
+    if old_rules == rules:
+        sys.exit(100)
+
+    os.execvp("nft", ("nft", "--", "\n".join(preamble + rules)))
 
 if __name__ == '__main__':
     install_firewall_rules(get_dns_resolved())
diff --git a/network/setup-ip b/network/setup-ip
index 11144a50..0c3aff35 100755
--- a/network/setup-ip
+++ b/network/setup-ip
@@ -185,7 +185,9 @@ configure_qubes_ns() {
     secondary_dns=$(qubesdb-read /qubes-netvm-secondary-dns)
     echo "NS1=$primary_dns" > /var/run/qubes/qubes-ns
     echo "NS2=$secondary_dns" >> /var/run/qubes/qubes-ns
-    /usr/lib/qubes/qubes-setup-dnat-to-ns
+    ret=0
+    /usr/lib/qubes/qubes-setup-dnat-to-ns || ret=$?
+    [ "$ret" -eq 0 ] || [ "$ret" -eq 100 ] || exit "$ret"
 }
 
 qubes_ip_change_hook() {