Add regression test for TOCTOU race in DNS UPDATE SSU handling

Race rndc reconfig (toggling between allow-update and update-policy)
against a stream of DNS UPDATEs for 5 seconds and verify that named
does not crash.

Before the fix, the race between send_update() and update_action()
reading the SSU table independently could trigger an assertion
failure (INSIST) when the zone's update policy changed between the
two reads.

diff --git a/REUSE.toml b/REUSE.toml
index e87ec915fd8437cbed4f7b9fb7c8595c293ac88b..70604ed568e0d7318cedf8c457fc746df842ade2 100644 (file)
--- a/REUSE.toml
+++ b/REUSE.toml
@@ -10,6 +10,7 @@ path = [
        "**/**.batch",
        "**/**.before**",
        "**/**.ccache",
+       "**/**.db**",
        "**/**.good",
        "**/**.key",
        "**/**.key.in",

diff --git a/bin/tests/system/ssutoctou/ns1/example.db.in b/bin/tests/system/ssutoctou/ns1/example.db.in
new file mode 100644 (file)
index 0000000..9b0498c
--- /dev/null
+++ b/bin/tests/system/ssutoctou/ns1/example.db.in
@@ -0,0 +1,10 @@
+$TTL 300
+@      IN SOA  ns.example. admin.example. (
+                       1       ; serial
+                       3600    ; refresh
+                       900     ; retry
+                       604800  ; expire
+                       300     ; minimum
+               )
+@      IN NS   ns.example.
+ns     IN A    10.53.0.1

diff --git a/bin/tests/system/ssutoctou/ns1/named.conf.j2 b/bin/tests/system/ssutoctou/ns1/named.conf.j2
new file mode 100644 (file)
index 0000000..87b867a
--- /dev/null
+++ b/bin/tests/system/ssutoctou/ns1/named.conf.j2
@@ -0,0 +1,45 @@
+/*
+ * Copyright (C) Internet Systems Consortium, Inc. ("ISC")
+ *
+ * SPDX-License-Identifier: MPL-2.0
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0.  If a copy of the MPL was not distributed with this
+ * file, you can obtain one at https://mozilla.org/MPL/2.0/.
+ *
+ * See the COPYRIGHT file distributed with this work for additional
+ * information regarding copyright ownership.
+ */
+
+{% set use_ssu = use_ssu | default(False) %}
+
+options {
+       query-source address 10.53.0.1;
+       notify-source 10.53.0.1;
+       transfer-source 10.53.0.1;
+       port @PORT@;
+       pid-file "named.pid";
+       listen-on { 10.53.0.1; };
+       listen-on-v6 { none; };
+       recursion no;
+       dnssec-validation no;
+};
+
+key rndc_key {
+       secret "1234abcd8765";
+       algorithm @DEFAULT_HMAC@;
+};
+
+controls {
+       inet 10.53.0.1 port @CONTROLPORT@ allow { any; } keys { rndc_key; };
+};
+
+zone "example" {
+       type primary;
+       file "example.db";
+{% if use_ssu %}
+       update-policy { grant * self * A; };
+{% else %}
+       allow-update { any; };
+{% endif %}
+};

diff --git a/bin/tests/system/ssutoctou/setup.sh b/bin/tests/system/ssutoctou/setup.sh
new file mode 100755 (executable)
index 0000000..24a0026
--- /dev/null
+++ b/bin/tests/system/ssutoctou/setup.sh
@@ -0,0 +1,17 @@
+#!/bin/sh -e
+
+# Copyright (C) Internet Systems Consortium, Inc. ("ISC")
+#
+# SPDX-License-Identifier: MPL-2.0
+#
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0.  If a copy of the MPL was not distributed with this
+# file, you can obtain one at https://mozilla.org/MPL/2.0/.
+#
+# See the COPYRIGHT file distributed with this work for additional
+# information regarding copyright ownership.
+
+# shellcheck source=conf.sh
+. ../conf.sh
+
+cp ns1/example.db.in ns1/example.db

diff --git a/bin/tests/system/ssutoctou/tests_ssutoctou.py b/bin/tests/system/ssutoctou/tests_ssutoctou.py
new file mode 100644 (file)
index 0000000..9e7b322
--- /dev/null
+++ b/bin/tests/system/ssutoctou/tests_ssutoctou.py
@@ -0,0 +1,104 @@
+# Copyright (C) Internet Systems Consortium, Inc. ("ISC")
+#
+# SPDX-License-Identifier: MPL-2.0
+#
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0.  If a copy of the MPL was not distributed with this
+# file, you can obtain one at https://mozilla.org/MPL/2.0/.
+#
+# See the COPYRIGHT file distributed with this work for additional
+# information regarding copyright ownership.
+
+"""
+Regression test for GL#5006: TOCTOU race in DNS UPDATE SSU table handling.
+
+send_update() and update_action() used to independently read the zone's
+SSU table.  If rndc reconfig changed the zone's update policy between
+these two reads, the values could diverge, causing an assertion failure.
+
+This test races rndc reconfig (toggling between allow-update and
+update-policy) against a stream of DNS UPDATEs to verify that named
+survives without crashing.
+"""
+
+import threading
+import time
+
+import dns.query
+import dns.rdatatype
+import dns.update
+import pytest
+
+import isctest
+
+pytestmark = pytest.mark.extra_artifacts(
+    [
+        "*/*.db",
+        "*/*.jnl",
+    ]
+)
+
+
+def send_updates(ip, port, stop_event):
+    """Send DNS UPDATEs in a tight loop until stopped."""
+    n = 0
+    while not stop_event.is_set():
+        n += 1
+        try:
+            up = dns.update.UpdateMessage("example.")
+            up.add(
+                f"test{n}.example.",
+                300,
+                dns.rdatatype.A,
+                f"10.0.0.{n % 256}",
+            )
+            dns.query.tcp(up, ip, port=port, timeout=2)
+        except Exception:  # pylint: disable=broad-exception-caught
+            pass
+
+
+def toggle_config(ns1, templates, stop_event):
+    """Toggle zone config between allow-update and update-policy."""
+    use_ssu = False
+    while not stop_event.is_set():
+        use_ssu = not use_ssu
+        try:
+            templates.render("ns1/named.conf", {"use_ssu": use_ssu})
+            ns1.rndc("reconfig")
+        except Exception:  # pylint: disable=broad-exception-caught
+            pass
+        time.sleep(0.01)
+
+
+def test_ssu_toctou_race(ns1, templates):
+    """Race rndc reconfig against DNS UPDATEs -- named must not crash."""
+    port = int(isctest.vars.ALL["PORT"])
+    stop = threading.Event()
+
+    update_thread = threading.Thread(
+        target=send_updates,
+        args=("10.53.0.1", port, stop),
+    )
+    reconfig_thread = threading.Thread(
+        target=toggle_config,
+        args=(ns1, templates, stop),
+    )
+
+    update_thread.start()
+    reconfig_thread.start()
+
+    # Let them race for a few seconds
+    time.sleep(5)
+
+    stop.set()
+    update_thread.join(timeout=10)
+    reconfig_thread.join(timeout=10)
+
+    # Restore original config
+    templates.render("ns1/named.conf", {"use_ssu": False})
+    ns1.rndc("reconfig")
+
+    # Verify named is still alive
+    msg = isctest.query.create("ns.example.", "A")
+    res = isctest.query.udp(msg, "10.53.0.1")
+    isctest.check.noerror(res)

diff --git a/lib/ns/update.c b/lib/ns/update.c
index aba0b80456831548bd5efe5ddd6dcaf38ab3b3a7..8dd26a8ad6f1cffae44f2a4643dd344626ab8c26 100644 (file)
--- a/lib/ns/update.c
+++ b/lib/ns/update.c
@@ -1803,8 +1803,8 @@ send_update(ns_client_t *client, dns_zone_t *zone) {
        *uev = (update_t){
                .zone = zone,
                .client = client,
-               .ssutable = TAKE_OWNERSHIP(ssutable),
-               .maxbytype = TAKE_OWNERSHIP(maxbytype),
+               .ssutable = MOVE_OWNERSHIP(ssutable),
+               .maxbytype = MOVE_OWNERSHIP(maxbytype),
                .maxbytypelen = maxbytypelen,
                .result = ISC_R_SUCCESS,
        };