From d93d32d982d2bed09cb6690740c0a633be027412 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Wed, 26 Aug 2026 13:13:32 +0200 Subject: [PATCH 1/3] python/tests: add tests for samba-tool dns zoneoptions on a zone without dNSProperty values Currently they fail with WERR_INTERNAL_DB_ERROR. BUG: https://bugzilla.samba.org/show_bug.cgi?id=16226 Signed-off-by: Stefan Metzmacher Reviewed-by: Volker Lendecke (cherry picked from commit b179abed6757ece911d58e562a2045edf2225a91) --- python/samba/tests/samba_tool/dnscmd.py | 71 +++++++++++++++---- .../knownfail.d/samba.tests.samba_tool.dnscmd | 1 + 2 files changed, 57 insertions(+), 15 deletions(-) create mode 100644 selftest/knownfail.d/samba.tests.samba_tool.dnscmd diff --git a/python/samba/tests/samba_tool/dnscmd.py b/python/samba/tests/samba_tool/dnscmd.py index 4f144c17c166..97a7f2f6f553 100644 --- a/python/samba/tests/samba_tool/dnscmd.py +++ b/python/samba/tests/samba_tool/dnscmd.py @@ -116,11 +116,12 @@ class DnsCmdTestCase(SambaToolCmdTest): "SRV": bad_srv } - def resetZone(self): + def resetZone(self, clear_properties=False): self.deleteZone() - self.addZone() + self.addZone(clear_properties=clear_properties) - def addZone(self): + def addZone(self, clear_properties=False): + self.zone_dn = None self.zone = "zone" result, out, err = self.runsubcmd("dns", "zonecreate", @@ -128,6 +129,24 @@ class DnsCmdTestCase(SambaToolCmdTest): self.zone, self.creds_string) self.assertCmdSuccess(result, out, err) + zones = self.samdb.search(base="DC=DomainDnsZones,%s" + % self.samdb.get_default_basedn(), + scope=ldb.SCOPE_SUBTREE, + expression="(objectClass=dnsZone)", + attrs=["cn"]) + + for zone in zones: + if self.zone in str(zone.dn): + self.zone_dn = zone.dn + break + self.assertIsNotNone(self.zone_dn) + if clear_properties: + msg_dict = { + "dn": self.zone_dn, + "dNSProperty": [] + } + msg = ldb.Message.from_dict(self.samdb, msg_dict, ldb.FLAG_MOD_DELETE) + self.samdb.modify(msg) def deleteZone(self): result, out, err = self.runsubcmd("dns", @@ -948,26 +967,48 @@ class DnsCmdTestCase(SambaToolCmdTest): self.assertTrue(out != '') def test_zoneoptions_aging(self): - for options, vals, error in ( - (['--aging=1'], {'fAging': 'TRUE'}, False), - (['--aging=0'], {'fAging': 'FALSE'}, False), - (['--aging=-1'], {'fAging': 'FALSE'}, True), - (['--aging=2'], {}, True), - (['--aging=2', '--norefreshinterval=1'], {}, True), + for options, vals, error, clear_properties in ( + (['--aging=1'], {'fAging': 'TRUE'}, False, False), + (['--aging=0'], {'fAging': 'FALSE'}, False, False), + (['--aging=-1'], {'fAging': 'FALSE'}, True, False), + (['--aging=2'], {}, True, False), + (['--aging=2', '--norefreshinterval=1'], {}, True, False), (['--aging=1', '--norefreshinterval=1'], - {'fAging': 'TRUE', 'dwNoRefreshInterval': '1'}, False), + {'fAging': 'TRUE', 'dwNoRefreshInterval': '1'}, False, False), # setting --norefreshinterval=0 results in the server default (['--aging=1', '--norefreshinterval=0'], - {'fAging': 'TRUE', 'dwNoRefreshInterval': '168'}, False), + {'fAging': 'TRUE', 'dwNoRefreshInterval': '168'}, False, False), (['--aging=0', '--norefreshinterval=99', '--refreshinterval=99'], {'fAging': 'FALSE', 'dwNoRefreshInterval': '99', - 'dwRefreshInterval': '99'}, False), + 'dwRefreshInterval': '99'}, False, False), (['--aging=0', '--norefreshinterval=-99', '--refreshinterval=99'], - {}, True), - (['--refreshinterval=9999999'], {}, True), - (['--norefreshinterval=9999999'], {}, True), + {}, True, False), + (['--refreshinterval=9999999'], {}, True, False), + (['--norefreshinterval=9999999'], {}, True, False), + (['--aging=1', '--norefreshinterval=100', '--refreshinterval=200'], + {'fAging': 'TRUE', + 'dwNoRefreshInterval': '100', + 'dwRefreshInterval': '200'}, + False, True), + (['--aging=1'], + {'fAging': 'TRUE', + 'dwNoRefreshInterval': '168', + 'dwRefreshInterval': '168'}, + False, True), + (['--norefreshinterval=100'], + {'fAging': 'FALSE', + 'dwNoRefreshInterval': '100', + 'dwRefreshInterval': '168'}, + False, True), + (['--refreshinterval=200'], + {'fAging': 'FALSE', + 'dwNoRefreshInterval': '168', + 'dwRefreshInterval': '200'}, + False, True), ): + if clear_properties: + self.resetZone(clear_properties=True) result, out, err = self.runsubcmd("dns", "zoneoptions", os.environ["SERVER"], diff --git a/selftest/knownfail.d/samba.tests.samba_tool.dnscmd b/selftest/knownfail.d/samba.tests.samba_tool.dnscmd new file mode 100644 index 000000000000..194d88f17e5e --- /dev/null +++ b/selftest/knownfail.d/samba.tests.samba_tool.dnscmd @@ -0,0 +1 @@ +^samba.tests.samba_tool.dnscmd.samba.tests.samba_tool.dnscmd.DnsCmdTestCase.test_zoneoptions_aging -- 2.43.0 From 1cefcceed7fb9f8969601a5b665869edca2d7c7e Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Wed, 26 Aug 2026 16:37:19 +0200 Subject: [PATCH 2/3] s4:rpc_server/dnsserver: fix talloc hierarchy in dnsserver_db_do_reset_dword() BUG: https://bugzilla.samba.org/show_bug.cgi?id=16226 Signed-off-by: Stefan Metzmacher Reviewed-by: Volker Lendecke (cherry picked from commit 3ae706be815952a20ecfef427ab98f540fa6b8ac) --- source4/rpc_server/dnsserver/dnsdb.c | 32 +++++++++++++--------------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/source4/rpc_server/dnsserver/dnsdb.c b/source4/rpc_server/dnsserver/dnsdb.c index dc32169dcccd..45b59ea7592b 100644 --- a/source4/rpc_server/dnsserver/dnsdb.c +++ b/source4/rpc_server/dnsserver/dnsdb.c @@ -818,7 +818,6 @@ WERROR dnsserver_db_do_reset_dword(struct ldb_context *samdb, struct DNS_RPC_NAME_AND_PARAM *n_p) { struct ldb_message_element *element = NULL; - struct dnsp_DnsProperty *prop = NULL; enum ndr_err_code err; TALLOC_CTX *tmp_ctx = NULL; const char * const attrs[] = {"dNSProperty", NULL}; @@ -871,14 +870,16 @@ WERROR dnsserver_db_do_reset_dword(struct ldb_context *samdb, } for (i = 0; i < element->num_values; i++) { - prop = talloc_zero(element, struct dnsp_DnsProperty); + struct dnsp_DnsProperty *prop = NULL; + + prop = talloc_zero(tmp_ctx, struct dnsp_DnsProperty); if (prop == NULL) { TALLOC_FREE(tmp_ctx); return WERR_NOT_ENOUGH_MEMORY; } err = ndr_pull_struct_blob( &(element->values[i]), - tmp_ctx, + prop, prop, (ndr_pull_flags_fn_t)ndr_pull_dnsp_DnsProperty); if (!NDR_ERR_CODE_IS_SUCCESS(err)){ @@ -889,18 +890,12 @@ WERROR dnsserver_db_do_reset_dword(struct ldb_context *samdb, * formed property with a 0 length and cause a * failure here */ - struct dnsp_DnsProperty_short - *short_property - = talloc_zero(element, - struct dnsp_DnsProperty_short); - if (short_property == NULL) { - TALLOC_FREE(tmp_ctx); - return WERR_NOT_ENOUGH_MEMORY; - } + struct dnsp_DnsProperty_short short_property = {}; + err = ndr_pull_struct_blob_all( &(element->values[i]), - tmp_ctx, - short_property, + prop, + &short_property, (ndr_pull_flags_fn_t)ndr_pull_dnsp_DnsProperty_short); if (!NDR_ERR_CODE_IS_SUCCESS(err)) { /* @@ -915,6 +910,7 @@ WERROR dnsserver_db_do_reset_dword(struct ldb_context *samdb, i, ldb_dn_get_linearized(z->zone_dn), prop_id); + TALLOC_FREE(prop); continue; } @@ -924,9 +920,9 @@ WERROR dnsserver_db_do_reset_dword(struct ldb_context *samdb, * re-push */ *prop = (struct dnsp_DnsProperty){ - .namelength = short_property->namelength, - .id = short_property->id, - .name = short_property->name + .namelength = short_property.namelength, + .id = short_property.id, + .name = short_property.name /* .data will be filled in below */ }; } @@ -949,7 +945,7 @@ WERROR dnsserver_db_do_reset_dword(struct ldb_context *samdb, err = ndr_push_struct_blob( &(element->values[i]), - tmp_ctx, + element->values, prop, (ndr_push_flags_fn_t)ndr_push_dnsp_DnsProperty); if (!NDR_ERR_CODE_IS_SUCCESS(err)){ @@ -961,6 +957,8 @@ WERROR dnsserver_db_do_reset_dword(struct ldb_context *samdb, return WERR_INTERNAL_DB_ERROR; } } + + TALLOC_FREE(prop); } element->flags = LDB_FLAG_MOD_REPLACE; -- 2.43.0 From 8029f3c6135aac446f84496ec1f19db5803422e3 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Wed, 26 Aug 2026 13:12:40 +0200 Subject: [PATCH 3/3] s4:rpc_server/dnsserver: allow dnsserver_db_do_reset_dword() to add new dNSProperty values On a zone without "dNSProperty" values at all, or without a value for the new property, we failed with WERR_INTERNAL_DB_ERROR. If there's no value stored for the property, we just need to add a new value. BUG: https://bugzilla.samba.org/show_bug.cgi?id=16226 Signed-off-by: Stefan Metzmacher Reviewed-by: Volker Lendecke Autobuild-User(master): Stefan Metzmacher Autobuild-Date(master): Thu Aug 27 17:36:53 UTC 2026 on atb-devel-224 (cherry picked from commit 731e541a0ef4fda598a1480e66aff9d26641d9da) --- .../knownfail.d/samba.tests.samba_tool.dnscmd | 1 - source4/rpc_server/dnsserver/dnsdb.c | 127 ++++++++++++++---- 2 files changed, 101 insertions(+), 27 deletions(-) delete mode 100644 selftest/knownfail.d/samba.tests.samba_tool.dnscmd diff --git a/selftest/knownfail.d/samba.tests.samba_tool.dnscmd b/selftest/knownfail.d/samba.tests.samba_tool.dnscmd deleted file mode 100644 index 194d88f17e5e..000000000000 --- a/selftest/knownfail.d/samba.tests.samba_tool.dnscmd +++ /dev/null @@ -1 +0,0 @@ -^samba.tests.samba_tool.dnscmd.samba.tests.samba_tool.dnscmd.DnsCmdTestCase.test_zoneoptions_aging diff --git a/source4/rpc_server/dnsserver/dnsdb.c b/source4/rpc_server/dnsserver/dnsdb.c index 45b59ea7592b..cc28bdd657bf 100644 --- a/source4/rpc_server/dnsserver/dnsdb.c +++ b/source4/rpc_server/dnsserver/dnsdb.c @@ -813,6 +813,43 @@ static bool dnsserver_db_msg_add_dnsproperty(TALLOC_CTX *mem_ctx, return true; } +static enum ndr_err_code dnsserver_push_property( + TALLOC_CTX *mem_ctx, + DATA_BLOB *blob, + struct dnsp_DnsProperty *prop, + const struct DNS_RPC_NAME_AND_PARAM *n_p) +{ + enum ndr_err_code err; + + switch (prop->id) { + case DSPROPERTY_ZONE_AGING_STATE: + prop->data.aging_enabled = n_p->dwParam; + break; + case DSPROPERTY_ZONE_NOREFRESH_INTERVAL: + prop->data.norefresh_hours = n_p->dwParam; + break; + case DSPROPERTY_ZONE_REFRESH_INTERVAL: + prop->data.refresh_hours = n_p->dwParam; + break; + case DSPROPERTY_ZONE_ALLOW_UPDATE: + prop->data.allow_update_flag = n_p->dwParam; + break; + default: + return NDR_ERR_BAD_SWITCH; + } + + err = ndr_push_struct_blob( + blob, + mem_ctx, + prop, + (ndr_push_flags_fn_t)ndr_push_dnsp_DnsProperty); + if (!NDR_ERR_CODE_IS_SUCCESS(err)) { + return err; + } + + return NDR_ERR_SUCCESS; +} + WERROR dnsserver_db_do_reset_dword(struct ldb_context *samdb, struct dnsserver_zone *z, struct DNS_RPC_NAME_AND_PARAM *n_p) @@ -823,6 +860,7 @@ WERROR dnsserver_db_do_reset_dword(struct ldb_context *samdb, const char * const attrs[] = {"dNSProperty", NULL}; struct ldb_result *res = NULL; int i, ret, prop_id; + bool replaced = false; if (strcasecmp(n_p->pszNodeName, "Aging") == 0) { z->zoneinfo->fAging = n_p->dwParam; @@ -863,10 +901,25 @@ WERROR dnsserver_db_do_reset_dword(struct ldb_context *samdb, element = ldb_msg_find_element(res->msgs[0], "dNSProperty"); if (element == NULL) { - DBG_ERR("dnsserver: zone %s has no properties.\n", - ldb_dn_get_linearized(z->zone_dn)); - TALLOC_FREE(tmp_ctx); - return WERR_INTERNAL_DB_ERROR; + /* + * If there are no values yet, + * just create an empty element + * where we can add new values below. + * + * We'll force LDB_FLAG_MOD_REPLACE below, + * so we use 0 here. + */ + ret = ldb_msg_add_empty(res->msgs[0], + "dNSProperty", + 0, + &element); + if (ret != LDB_SUCCESS) { + DBG_ERR("%s: ldb_msg_add_empty(dNSProperty) - %s\n", + ldb_dn_get_linearized(z->zone_dn), + ldb_strerror(ret)); + TALLOC_FREE(tmp_ctx); + return WERR_INTERNAL_DB_ERROR; + } } for (i = 0; i < element->num_values; i++) { @@ -928,39 +981,61 @@ WERROR dnsserver_db_do_reset_dword(struct ldb_context *samdb, } if (prop->id == prop_id) { - switch (prop_id) { - case DSPROPERTY_ZONE_AGING_STATE: - prop->data.aging_enabled = n_p->dwParam; - break; - case DSPROPERTY_ZONE_NOREFRESH_INTERVAL: - prop->data.norefresh_hours = n_p->dwParam; - break; - case DSPROPERTY_ZONE_REFRESH_INTERVAL: - prop->data.refresh_hours = n_p->dwParam; - break; - case DSPROPERTY_ZONE_ALLOW_UPDATE: - prop->data.allow_update_flag = n_p->dwParam; - break; - } - - err = ndr_push_struct_blob( - &(element->values[i]), - element->values, - prop, - (ndr_push_flags_fn_t)ndr_push_dnsp_DnsProperty); + err = dnsserver_push_property(element->values, + &element->values[i], + prop, + n_p); if (!NDR_ERR_CODE_IS_SUCCESS(err)){ DBG_ERR("dnsserver: couldn't PUSH dns prop id " - "%d in zone %s\n", + "%d in zone %s - %s\n", prop->id, - ldb_dn_get_linearized(z->zone_dn)); + ldb_dn_get_linearized(z->zone_dn), + ndr_errstr(err)); TALLOC_FREE(tmp_ctx); return WERR_INTERNAL_DB_ERROR; } + + replaced = true; } TALLOC_FREE(prop); } + /* + * If it's not replaced we need to add a new one + */ + if (!replaced) { + struct dnsp_DnsProperty prop = { + .id = prop_id, + }; + DATA_BLOB blob = {}; + + err = dnsserver_push_property(element, + &blob, + &prop, + n_p); + if (!NDR_ERR_CODE_IS_SUCCESS(err)){ + DBG_ERR("dnsserver: couldn't PUSH dns prop id " + "%d in zone %s - %s\n", + prop.id, + ldb_dn_get_linearized(z->zone_dn), + ndr_errstr(err)); + TALLOC_FREE(tmp_ctx); + return WERR_INTERNAL_DB_ERROR; + } + + ret = ldb_msg_add_steal_value(res->msgs[0], + "dNSProperty", + &blob); + if (ret != LDB_SUCCESS) { + DBG_ERR("%s: ldb_msg_add_steal_value(dNSProperty) - %s\n", + ldb_dn_get_linearized(z->zone_dn), + ldb_strerror(ret)); + TALLOC_FREE(tmp_ctx); + return WERR_INTERNAL_DB_ERROR; + } + } + element->flags = LDB_FLAG_MOD_REPLACE; ret = ldb_modify(samdb, res->msgs[0]); if (ret != LDB_SUCCESS) { -- 2.43.0