From 2ec6f856e3ff0228c8061df0c1b5dc26a5062585 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Fri, 20 Mar 2026 23:33:04 -0700 Subject: [PATCH 1/3] prevent named crash on rndc modzone for a zone in named.conf If named is built without LMDB and has a zone in named.conf, then rndc modzone for that zone triggers an assertion failure unless there's already an NZF file. This is because load_nzf doesn't create 'nzf_config' when NZF is missing, while a valid nzf_config is assumed in do_modzone when it tries to add the modified zone config to add_parser. The crash is fixed by skipping the call to cfg_parser_mapadd when nzf_config is NULL. Skipping it should be okay since the config stored in add_parser would be needed only for subsequently deleting a zone by rndc delzone when the zone was originally added by rndc addzone, but in this case the zone was not 'added'. Checking if nzf_config is NULL before using it also seems to be consistent with other parts of the implementation. --- bin/named/server.c | 7 +++- .../addzone/tests_rndc_modzone_without_add.py | 37 +++++++++++++++++++ 2 files changed, 42 insertions(+), 2 deletions(-) create mode 100644 bin/tests/system/addzone/tests_rndc_modzone_without_add.py diff --git a/bin/named/server.c b/bin/named/server.c index 2cb6ba991b..28ca7dc922 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -14277,8 +14277,11 @@ do_modzone(named_server_t *server, ns_cfgctx_t *cfg, dns_view_t *view, #ifndef HAVE_LMDB /* Store the new zone configuration; also in NZF if applicable */ - z = UNCONST(zoneobj); - CHECK(cfg_parser_mapadd(cfg->add_parser, cfg->nzf_config, z, "zone")); + if (cfg->nzf_config != NULL) { + z = UNCONST(zoneobj); + CHECK(cfg_parser_mapadd(cfg->add_parser, cfg->nzf_config, z, + "zone")); + } #endif /* HAVE_LMDB */ if (added) { diff --git a/bin/tests/system/addzone/tests_rndc_modzone_without_add.py b/bin/tests/system/addzone/tests_rndc_modzone_without_add.py new file mode 100644 index 0000000000..3658f5f518 --- /dev/null +++ b/bin/tests/system/addzone/tests_rndc_modzone_without_add.py @@ -0,0 +1,37 @@ +# 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. + +import pytest + +pytestmark = pytest.mark.extra_artifacts( + [ + "ns*/*.nzf*", + "ns*/*.nzd*", + "ns1/redirect.db", + "ns2/new-zones", + "ns2/redirect.db", + "ns3/redirect.db", + ] +) + + +def test_rndc_modzone_without_add(ns3): + """ + Confirm "rndc modzone" works for a zone that was not added by "addzone". + """ + # We begin with a zone that has a normal configuration, and then modify it + # by rndc modzone. This should succeed and shouldn't cause any disruption. + # Previously, it triggered an assertion failure unless LMDB was enabled. + cmd = ns3.rndc( + 'modzone . {type primary; file "redirect.db"; allow-query {none;};};', + raise_on_exception=False, + ) + assert cmd.rc == 0 From 17416af24878d305000ca9035b48a7d837973662 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Mon, 23 Mar 2026 11:29:28 -0700 Subject: [PATCH 2/3] Revert "Store zone config also on modzone" This reverts commit 85453d393d1b591ae1108603308c79dd63acca85. This commit doesn't seem to be a complete solution of what it appears to fix: showzone succeeds and shows the modified config after first modzone, but subsequent attempts of modzone fail (though not because of the commit being reverted), let alone showing the correct new config. Revering the change for now, and will provide a more comprehensive fix in the next commit. --- bin/named/server.c | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/bin/named/server.c b/bin/named/server.c index 28ca7dc922..95b387e342 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -14301,18 +14301,6 @@ do_modzone(named_server_t *server, ns_cfgctx_t *cfg, dns_view_t *view, TCHECK(putstr(text, zname)); TCHECK(putstr(text, "' reconfigured.")); } else { -#ifdef HAVE_LMDB - CHECK(nzd_open(view, 0, &txn, &dbi)); - CHECK(nzd_save(&txn, dbi, zone, zoneobj)); -#else /* ifdef HAVE_LMDB */ - result = nzf_append(view, zoneobj); - if (result != ISC_R_SUCCESS) { - TCHECK(putstr(text, "\nNew zone config not saved: ")); - TCHECK(putstr(text, isc_result_totext(result))); - goto cleanup; - } -#endif /* HAVE_LMDB */ - TCHECK(putstr(text, "zone '")); TCHECK(putstr(text, zname)); TCHECK(putstr(text, "' must also be reconfigured in\n")); From f2115e9d58ee6584a091d697499875ff6a78a243 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Mon, 23 Mar 2026 09:58:39 -0700 Subject: [PATCH 3/3] ensure rndc modzone succeeds twice for a zone in named.conf If a zone is in named.conf, not originally added by rndc addzone, rndc modzone for that zone succeeds once, but subsequent modzone attempts fail. This is because do_modzone removes the zone config from global or view options, but it would fail due to 'not found' once the config is removed. The fix is to ensure re-adding the updated zone config to the global or view options. This also works as a more complete fix for the issue 85453d3 atempted to solve, ensuring rndc showzone shows the latest config: it now works for multple attemps of modzone, and with named that is not built with LMDB. The change in this commit relies on UNCONST in a few places. That's not clean, but 'add/mod/delzone' generally seems to need it (for example, delete_zoneconf uses it to modify the list of zones). In that sense, this change follows the convention (for a longer term, there may have to be a better API so that we can modify config obtions that were once parsed). --- bin/named/server.c | 18 +++++++++--------- .../addzone/tests_rndc_modzone_without_add.py | 19 +++++++++++++++++++ 2 files changed, 28 insertions(+), 9 deletions(-) diff --git a/bin/named/server.c b/bin/named/server.c index 95b387e342..ad472112e2 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -14113,6 +14113,7 @@ do_modzone(named_server_t *server, ns_cfgctx_t *cfg, dns_view_t *view, dns_zone_t *zone = NULL; bool added; bool locked = false; + const cfg_obj_t *options = NULL; #ifndef HAVE_LMDB FILE *fp = NULL; cfg_obj_t *z; @@ -14220,17 +14221,13 @@ do_modzone(named_server_t *server, ns_cfgctx_t *cfg, dns_view_t *view, if (!added) { if (cfg->vconfig == NULL) { - result = delete_zoneconf( - view, cfg->conf_parser, cfg->config, - dns_zone_getorigin(zone), NULL, locked); + options = cfg->config; } else { - const cfg_obj_t *voptions = cfg_tuple_get(cfg->vconfig, - "options"); - result = delete_zoneconf( - view, cfg->conf_parser, voptions, - dns_zone_getorigin(zone), NULL, locked); + options = cfg_tuple_get(cfg->vconfig, "options"); } - + result = delete_zoneconf(view, cfg->conf_parser, options, + dns_zone_getorigin(zone), NULL, + locked); if (result != ISC_R_SUCCESS) { TCHECK(putstr(text, "former zone configuration " "not deleted: ")); @@ -14301,6 +14298,9 @@ do_modzone(named_server_t *server, ns_cfgctx_t *cfg, dns_view_t *view, TCHECK(putstr(text, zname)); TCHECK(putstr(text, "' reconfigured.")); } else { + CHECK(cfg_parser_mapadd(cfg->conf_parser, UNCONST(options), + UNCONST(zoneobj), "zone")); + TCHECK(putstr(text, "zone '")); TCHECK(putstr(text, zname)); TCHECK(putstr(text, "' must also be reconfigured in\n")); diff --git a/bin/tests/system/addzone/tests_rndc_modzone_without_add.py b/bin/tests/system/addzone/tests_rndc_modzone_without_add.py index 3658f5f518..b2a7335625 100644 --- a/bin/tests/system/addzone/tests_rndc_modzone_without_add.py +++ b/bin/tests/system/addzone/tests_rndc_modzone_without_add.py @@ -35,3 +35,22 @@ def test_rndc_modzone_without_add(ns3): raise_on_exception=False, ) assert cmd.rc == 0 + + # Confirm that the modzone took effect in 'rndc showzone'. + cmd = ns3.rndc("showzone .", raise_on_exception=False) + assert cmd.rc == 0 + assert 'allow-query { "none"; }' in cmd.out + + # Confirm that 'rndc modzone' still works after the first modzone. + # This was not the case before as the zone config was incorrectly + # removed in-memory after the first modzone. + cmd = ns3.rndc( + 'modzone . {type primary; file "redirect.db"; allow-query {any;};};', + raise_on_exception=False, + ) + assert cmd.rc == 0 + + # Confirm that the second modzone took effect in 'rndc showzone'. + cmd = ns3.rndc("showzone .", raise_on_exception=False) + assert cmd.rc == 0 + assert 'allow-query { "any"; }' in cmd.out