Fix a bug with template filename reuse

When a zone filename is defined in named.conf which will be
written to by the server - i.e., secondary or dynamically updated
zones - there is a test at configuration time to ensure that the
filename is non-unique.

This test is run before the zone is actually created, so a zone
configured using a template may not have had its filename expanded
yet.  This can cause a configuration to fail because, for example,
multiple zones appear to using the filename "$name.db".

This has been fixed by calling dns_zone_expandzonefile() from
isccfg_check_zoneconf(), to expand the names when checking for
uniqueness.

diff --git a/bin/tests/system/checkconf/bad-template-file-reuse.conf b/bin/tests/system/checkconf/bad-template-file-reuse.conf
new file mode 100644 (file)
index 0000000..2887071
--- /dev/null
+++ b/bin/tests/system/checkconf/bad-template-file-reuse.conf
@@ -0,0 +1,15 @@
+template a {
+       type secondary;
+        primaries { 192.0.0.2; };
+       file "$name.db";
+};
+
+zone example {
+       template a;
+};
+
+zone other {
+       type secondary;
+        primaries { 192.0.0.2; };
+        file "example.db";
+};

diff --git a/bin/tests/system/checkconf/good-template-file-reuse.conf b/bin/tests/system/checkconf/good-template-file-reuse.conf
new file mode 100644 (file)
index 0000000..8788655
--- /dev/null
+++ b/bin/tests/system/checkconf/good-template-file-reuse.conf
@@ -0,0 +1,17 @@
+template a {
+       type secondary;
+        primaries { 192.0.0.2; };
+       file "$name.db";
+};
+
+zone one {
+       template a;
+};
+
+zone two {
+       template a;
+};
+
+zone three {
+       template a;
+};

diff --git a/bin/tests/system/masterfile/ns2/named.conf.j2 b/bin/tests/system/masterfile/ns2/named.conf.j2
index cdebfae3689bdb816ce84a35984e28e75e107c65..72fa31d161b23740a68feb03ebcf17cdbab29f43 100644 (file)
--- a/bin/tests/system/masterfile/ns2/named.conf.j2
+++ b/bin/tests/system/masterfile/ns2/named.conf.j2
@@ -50,3 +50,17 @@ zone "present" {
        file "present.db";
        initial-file "example.db";
 };
+
+template secondary {
+       type secondary;
+       primaries { 10.53.0.1; };
+       file "$name.db";
+};
+
+zone "extra1" {
+       template secondary;
+};
+
+zone "extra2" {
+       template secondary;
+};

diff --git a/bin/tests/system/masterfile/tests_masterfile.py b/bin/tests/system/masterfile/tests_masterfile.py
index 598445555db2f258cbd2284af3c28f7219cb42bb..7109407a8f9951a31bca69e096c86dfd5abce9ef 100644 (file)
--- a/bin/tests/system/masterfile/tests_masterfile.py
+++ b/bin/tests/system/masterfile/tests_masterfile.py
@@ -18,7 +18,13 @@ import pytest
 import isctest
 
 pytestmark = pytest.mark.extra_artifacts(
-    ["ns2/copied.db", "ns2/present.db", "ns2/alternate.db"]
+    [
+        "ns2/copied.db",
+        "ns2/present.db",
+        "ns2/alternate.db",
+        "ns2/extra1.db",
+        "ns2/extra2.db",
+    ]
 )
 
 

diff --git a/lib/isccfg/check.c b/lib/isccfg/check.c
index 6596b50637142e5c48610b499e534553a0ee38bc..232d4d1890aed16a800dc7412894df576fa6e784 100644 (file)
--- a/lib/isccfg/check.c
+++ b/lib/isccfg/check.c
@@ -57,6 +57,7 @@
 #include <dns/rrl.h>
 #include <dns/secalg.h>
 #include <dns/ssu.h>
+#include <dns/zoneproperties.h>
 
 #include <dst/dst.h>
 
@@ -74,7 +75,9 @@
 static in_port_t dnsport = 53;
 
 static isc_result_t
-fileexist(const cfg_obj_t *obj, isc_symtab_t *symtab, bool writeable);
+fileexist(const cfg_obj_t *obj, const dns_name_t *zonename,
+         const char *viewname, const char *typename, isc_symtab_t *symtab,
+         bool writeable, isc_mem_t *mctx);
 
 static isc_result_t
 keydirexist(const cfg_obj_t *zcgf, const char *optname, dns_name_t *zname,
@@ -4186,7 +4189,9 @@ isccfg_check_zoneconf(const cfg_obj_t *zconfig, const cfg_obj_t *voptions,
                            ztype == CFG_ZONE_MIRROR || ddns ||
                            has_dnssecpolicy))
                {
-                       tresult = fileexist(fileobj, files, true);
+                       tresult = fileexist(fileobj, zname, viewname,
+                                           dns_zonetype_name(ztype), files,
+                                           true, mctx);
                        if (tresult != ISC_R_SUCCESS && result == ISC_R_SUCCESS)
                        {
                                result = tresult;
@@ -4195,7 +4200,9 @@ isccfg_check_zoneconf(const cfg_obj_t *zconfig, const cfg_obj_t *voptions,
                           (ztype == CFG_ZONE_PRIMARY ||
                            ztype == CFG_ZONE_HINT))
                {
-                       tresult = fileexist(fileobj, files, false);
+                       tresult = fileexist(fileobj, zname, viewname,
+                                           dns_zonetype_name(ztype), files,
+                                           false, mctx);
                        if (tresult != ISC_R_SUCCESS && result == ISC_R_SUCCESS)
                        {
                                result = tresult;
@@ -4396,20 +4403,33 @@ typedef enum symtab_file_type {
 } symtab_file_type_t;
 
 static isc_result_t
-fileexist(const cfg_obj_t *obj, isc_symtab_t *symtab, bool writeable) {
+fileexist(const cfg_obj_t *obj, const dns_name_t *zonename,
+         const char *viewname, const char *typename, isc_symtab_t *symtab,
+         bool writeable, isc_mem_t *mctx) {
        isc_result_t result_ro, result_w;
        isc_symvalue_t symvalue_ro, symvalue_w;
-       unsigned int line;
-       const char *file;
+       const char *filename = cfg_obj_asstring(obj);
+       char buf[PATH_MAX];
+
+       /*
+        * If the filename contains '$' or "%' characters, it's probably
+        * a template file and should not be tested for uniqueness.
+        */
+       if (strpbrk(filename, "%$") != NULL) {
+               isc_buffer_t b;
+               isc_buffer_init(&b, buf, sizeof(buf));
+               dns_zone_expandzonefile(&b, filename, zonename, viewname,
+                                       typename);
+               filename = buf;
+       }
 
        /*
         * Since symtab doesn't let us query the file type, we need to query
         * twice. Once per type.
         */
-       result_ro = isc_symtab_lookup(symtab, cfg_obj_asstring(obj), READ_ONLY,
+       result_ro = isc_symtab_lookup(symtab, filename, READ_ONLY,
                                      &symvalue_ro);
-       result_w = isc_symtab_lookup(symtab, cfg_obj_asstring(obj), WRITEABLE,
-                                    &symvalue_w);
+       result_w = isc_symtab_lookup(symtab, filename, WRITEABLE, &symvalue_w);
 
        bool found_read_only = result_ro == ISC_R_SUCCESS;
        bool found_writable = result_w == ISC_R_SUCCESS;
@@ -4423,8 +4443,8 @@ fileexist(const cfg_obj_t *obj, isc_symtab_t *symtab, bool writeable) {
                                                  ? symvalue_ro
                                                  : symvalue_w;
 
-               file = cfg_obj_file(symvalue.as_cpointer);
-               line = cfg_obj_line(symvalue.as_cpointer);
+               const char *file = cfg_obj_file(symvalue.as_cpointer);
+               unsigned int line = cfg_obj_line(symvalue.as_cpointer);
                cfg_obj_log(obj, ISC_LOG_ERROR,
                            "writeable file '%s': already in use: "
                            "%s:%u",
@@ -4439,9 +4459,12 @@ fileexist(const cfg_obj_t *obj, isc_symtab_t *symtab, bool writeable) {
                isc_symvalue_t symvalue =
                        (isc_symvalue_t){ .as_cpointer = obj };
                symtab_file_type_t type = writeable ? WRITEABLE : READ_ONLY;
-               isc_result_t result =
-                       isc_symtab_define(symtab, cfg_obj_asstring(obj), type,
-                                         symvalue, isc_symexists_reject);
+               char *key = isc_mem_strdup(mctx, filename);
+               isc_result_t result = isc_symtab_define(
+                       symtab, key, type, symvalue, isc_symexists_reject);
+               if (result == ISC_R_EXISTS) {
+                       isc_mem_free(mctx, key);
+               }
                return result;
        }
 }
@@ -6205,7 +6228,7 @@ isccfg_check_namedconf(const cfg_obj_t *config, unsigned int flags,
         * case sensitive. This will prevent people using FOO.DB and foo.db
         * on case sensitive file systems but that shouldn't be a major issue.
         */
-       isc_symtab_create(mctx, NULL, NULL, false, &files);
+       isc_symtab_create(mctx, freekey, mctx, false, &files);
        isc_symtab_create(mctx, freekey, mctx, false, &keydirs);
        isc_symtab_create(mctx, freekey, mctx, true, &inview);