[PATCH] Internal DNS, RPC server & samba-tool tests & fixes
Jeremy Allison
jra at samba.org
Thu Dec 8 23:52:56 UTC 2016
On Fri, Dec 09, 2016 at 12:29:15PM +1300, Bob Campbell wrote:
> Hi Jeremy, Andrew;
>
> Thanks for the comments; I've fixed the patches up per both of your
> comments and attached.
>
> The lengths are now explicitly checked to be >0. The intention of the
> check using len-1 is to remove dots at the end, which obviously doesn't
> apply for empty strings.
>
> Jeremy - is the way I now handle talloc errors okay?
Thanks Bob that looks much better ! I'm OK with
the C changes now.
Reviewed-by: Jeremy Allison <jra at samba.org>
> On 09/12/16 11:31, Jeremy Allison wrote:
> > On Fri, Dec 09, 2016 at 10:44:05AM +1300, Bob Campbell wrote:
> >> Hi,
> >>
> >> Here are a bunch of patches to test "samba-tool dns", extend testing for
> >> DNS over RPC directly, and fix some problems found through the tests.
> >>
> >> *samba-tool issues found:*
> >>
> >> - There is unnecessary input validation, which means that DNS records
> >> marked with DNS_RANK_NONE can't be deleted, even though they can on the
> >> RPC server. Since we've now proved input validation on the RPC server,
> >> we don't need this validation, so we removed it where possible.
> >>
> >> - Error catching is generic (it just catches runtime errors and gives
> >> the same message), so we made it more specific and changes some messages
> >> to allow for easier debugging.
> >>
> >> *backend issues found:*
> >>
> >> - There is no validation of DNS names over RPC on Samba, but there is on
> >> Windows. We've fixed this by adding a simple check whenever doing
> >> anything to a record except deleting it (as we don't want deletes to
> >> fail against existing entries, even if their DN is invalid). We also
> >> added this check on DNS, and when converting a string to an ldb_dn
> >> structure.
> >>
> >> - ACLs on some zones, in particular default ones, are weird and
> >> inconsistent with Windows in places, but this is mostly because the
> >> default owners are different. We've marked these tests as knownfails,
> >> because we're not sure that there's actually a problem here.
> >>
> >> - DNS records in the _msdcs zone wouldn't have the dNSProperty attribute
> >> (which may have been a root cause of entries with DNS_RANK_NONE).
> >>
> >> These patches are applied at:
> >> http://git.catalyst.net.nz/gw?p=samba.git;a=shortlog;h=refs/heads/dnscmdtest
> >>
> >> Please review.
> > Not commenting much on the python (sorry, I'm still learning), but
> > a few C comments follow inline.
> >
> > Cheers,
> >
> > Jeremy.
> >
> >> From 020c77cc6bf279d969641e8af533107d7b94e3b7 Mon Sep 17 00:00:00 2001
> >> From: Bob Campbell <bobcampbell at catalyst.net.nz>
> >> Date: Tue, 6 Dec 2016 15:34:23 +1300
> >> Subject: [PATCH 07/12] dnsserver: add dns name checking
> >>
> >> This may also prevent deletion of existing corrupted records through
> >> DNS, but should be resolvable through RPC, or at worst LDAP.
> >>
> >> Signed-off-by: Bob Campbell <bobcampbell at catalyst.net.nz>
> >> Peer-programmed-with: Garming Sam <garming at catalyst.net.nz>
> >> Reviewed-by: Garming Sam <garming at catalyst.net.nz>
> >> ---
> >> selftest/knownfail | 2 -
> >> source4/dns_server/dnsserver_common.c | 87 ++++++++++++++++++++++
> >> source4/dns_server/dnsserver_common.h | 4 +-
> >> source4/rpc_server/dnsserver/dnsdata.c | 122 ++++++++++++++++++++++++-------
> >> source4/rpc_server/dnsserver/dnsdb.c | 33 ++++++---
> >> source4/rpc_server/dnsserver/dnsserver.h | 4 +-
> >> source4/rpc_server/wscript_build | 2 +-
> >> 7 files changed, 212 insertions(+), 42 deletions(-)
> >>
> >> diff --git a/selftest/knownfail b/selftest/knownfail
> >> index 7141f43..97ec6ef 100644
> >> --- a/selftest/knownfail
> >> +++ b/selftest/knownfail
> >> @@ -309,5 +309,3 @@
> >> ^samba.tests.dcerpc.dnsserver.samba.tests.dcerpc.dnsserver.DnsserverTests.test_add_duplicate_different_type.*
> >> ^samba.tests.dcerpc.dnsserver.samba.tests.dcerpc.dnsserver.DnsserverTests.test_rank_none.*
> >> ^samba.tests.dcerpc.dnsserver.samba.tests.dcerpc.dnsserver.DnsserverTests.test_security_descriptor.*
> >> -^samba.tests.dcerpc.dnsserver.samba.tests.dcerpc.dnsserver.DnsserverTests.test_reject_invalid_commands
> >> -^samba.tests.samba_tool.dnscmd.samba.tests.samba_tool.dnscmd.DnsCmdTestCase.test_reject_invalid_commands
> >> diff --git a/source4/dns_server/dnsserver_common.c b/source4/dns_server/dnsserver_common.c
> >> index db42bcb..e6534c3 100644
> >> --- a/source4/dns_server/dnsserver_common.c
> >> +++ b/source4/dns_server/dnsserver_common.c
> >> @@ -214,6 +214,87 @@ static int rec_cmp(const struct dnsp_DnssrvRpcRecord *r1,
> >> return r1->dwTimeStamp - r2->dwTimeStamp;
> >> }
> >>
> >> +/*
> >> + * Check for valid DNS names. These are names which are non-empty, do not
> >> + * start with a dot and do not have any empty segments.
> >> + */
> >> +WERROR dns_name_check(TALLOC_CTX *mem_ctx, int len, const char *name)
> > len here should be size_t, not int. It comes from a strlen() which
> > returns size_t.
> >
> >> +{
> >> + int i;
> > i should also be size_t.
> >
> >> + if (len == 0) {
> >> + return WERR_DS_INVALID_DN_SYNTAX;
> >> + }
> >> +
> >> + for (i = 0; i < len - 1; i++) {
> >> + if (name[i] == '.' && name[i+1] == '.') {
> >> + return WERR_DS_INVALID_DN_SYNTAX;
> >> + }
> >> + }
> >> +
> >> + if (len > 1 && name[0] == '.') {
> >> + return WERR_DS_INVALID_DN_SYNTAX;
> >> + }
> >> +
> >> + return WERR_OK;
> >> +}
> >> +
> >> +static WERROR check_name_list(TALLOC_CTX *mem_ctx, uint16_t rec_count,
> >> + struct dnsp_DnssrvRpcRecord *records)
> >> +{
> >> + WERROR werr;
> >> + int i, len;
> > i should be uint16_t, as that's the type of rec_count.
> > len should be size_t as it comes from strlen().
> >
> >> + struct dnsp_DnssrvRpcRecord record;
> >> +
> >> + werr = WERR_OK;
> >> + for(i=0; i<rec_count; i++){
> >> + record = records[i];
> >> +
> >> + switch (record.wType) {
> >> +
> >> + case DNS_TYPE_NS:
> >> + len = strlen(record.data.ns);
> >> + werr = dns_name_check(mem_ctx, len, record.data.ns);
> >> + break;
> >> + case DNS_TYPE_CNAME:
> >> + len = strlen(record.data.cname);
> >> + werr = dns_name_check(mem_ctx, len, record.data.cname);
> >> + break;
> >> + case DNS_TYPE_SOA:
> >> + len = strlen(record.data.soa.mname);
> >> + werr = dns_name_check(mem_ctx, len, record.data.soa.mname);
> >> + if (!W_ERROR_IS_OK(werr)) {
> >> + break;
> >> + }
> >> + len = strlen(record.data.soa.rname);
> >> + werr = dns_name_check(mem_ctx, len, record.data.soa.rname);
> >> + break;
> >> + case DNS_TYPE_PTR:
> >> + len = strlen(record.data.ptr);
> >> + werr = dns_name_check(mem_ctx, len, record.data.ptr);
> >> + break;
> >> + case DNS_TYPE_MX:
> >> + len = strlen(record.data.mx.nameTarget);
> >> + werr = dns_name_check(mem_ctx, len, record.data.mx.nameTarget);
> >> + break;
> >> + case DNS_TYPE_SRV:
> >> + len = strlen(record.data.srv.nameTarget);
> >> + werr = dns_name_check(mem_ctx, len,
> >> + record.data.srv.nameTarget);
> >> + break;
> >> + // In the default case, the record doesn't have a DN, so it must be ok.
> > Use C style comments, not C++.
> >
> >> + default:
> >> + break;
> >> + }
> >> +
> >> + if (!W_ERROR_IS_OK(werr)) {
> >> + return werr;
> >> + }
> >> + }
> >> +
> >> + return WERR_OK;
> >> +}
> >> +
> >> WERROR dns_common_replace(struct ldb_context *samdb,
> >> TALLOC_CTX *mem_ctx,
> >> struct ldb_dn *dn,
> >> @@ -225,6 +306,7 @@ WERROR dns_common_replace(struct ldb_context *samdb,
> >> struct ldb_message_element *el;
> >> uint16_t i;
> >> int ret;
> >> + WERROR werr;
> >> struct ldb_message *msg = NULL;
> >> bool was_tombstoned = false;
> >> bool become_tombstoned = false;
> >> @@ -234,6 +316,11 @@ WERROR dns_common_replace(struct ldb_context *samdb,
> >>
> >> msg->dn = dn;
> >>
> >> + werr = check_name_list(mem_ctx, rec_count, records);
> >> + if (!W_ERROR_IS_OK(werr)) {
> >> + return werr;
> >> + }
> >> +
> >> ret = ldb_msg_add_empty(msg, "dnsRecord", LDB_FLAG_MOD_REPLACE, &el);
> >> if (ret != LDB_SUCCESS) {
> >> return DNS_ERR(SERVER_FAILURE);
> >> diff --git a/source4/dns_server/dnsserver_common.h b/source4/dns_server/dnsserver_common.h
> >> index ad91f61..bea3019 100644
> >> --- a/source4/dns_server/dnsserver_common.h
> >> +++ b/source4/dns_server/dnsserver_common.h
> >> @@ -46,7 +46,9 @@ WERROR dns_common_lookup(struct ldb_context *samdb,
> >> struct dnsp_DnssrvRpcRecord **records,
> >> uint16_t *num_records,
> >> bool *tombstoned);
> >> -
> >> +WERROR dns_name_check(TALLOC_CTX *mem_ctx,
> >> + int len,
> >> + const char *name);
> >> WERROR dns_common_replace(struct ldb_context *samdb,
> >> TALLOC_CTX *mem_ctx,
> >> struct ldb_dn *dn,
> >> diff --git a/source4/rpc_server/dnsserver/dnsdata.c b/source4/rpc_server/dnsserver/dnsdata.c
> >> index ccea0d7..c2ca56e 100644
> >> --- a/source4/rpc_server/dnsserver/dnsdata.c
> >> +++ b/source4/rpc_server/dnsserver/dnsdata.c
> >> @@ -21,6 +21,7 @@
> >>
> >> #include "includes.h"
> >> #include "dnsserver.h"
> >> +#include "dns_server/dnsserver_common.h"
> >> #include "lib/replace/system/network.h"
> >> #include "librpc/gen_ndr/ndr_dnsp.h"
> >> #include "librpc/gen_ndr/ndr_dnsserver.h"
> >> @@ -412,15 +413,17 @@ void dnsp_to_dns_copy(TALLOC_CTX *mem_ctx, struct dnsp_DnssrvRpcRecord *dnsp,
> >>
> >> }
> >>
> >> -
> >> -struct dnsp_DnssrvRpcRecord *dns_to_dnsp_copy(TALLOC_CTX *mem_ctx, struct DNS_RPC_RECORD *dns)
> >> +WERROR dns_to_dnsp_convert(TALLOC_CTX *mem_ctx, struct DNS_RPC_RECORD *dns,
> >> + struct dnsp_DnssrvRpcRecord **out_dnsp, bool check_name)
> >> {
> >> + WERROR res;
> >> int i, len;
> >> - struct dnsp_DnssrvRpcRecord *dnsp;
> >> + const char *name;
> >> + struct dnsp_DnssrvRpcRecord *dnsp = NULL;
> >>
> >> dnsp = talloc_zero(mem_ctx, struct dnsp_DnssrvRpcRecord);
> >> if (dnsp == NULL) {
> >> - return NULL;
> >> + return WERR_NOT_ENOUGH_MEMORY;
> >> }
> >>
> >> dnsp->wDataLength = dns->wDataLength;
> >> @@ -442,21 +445,41 @@ struct dnsp_DnssrvRpcRecord *dns_to_dnsp_copy(TALLOC_CTX *mem_ctx, struct DNS_RP
> >> break;
> >>
> >> case DNS_TYPE_NS:
> >> + name = dns->data.name.str;
> >> len = dns->data.name.len;
> > Where does dns->data.name.len come from ?
> > If it comes from a remote source then there
> > must be range checks around any arithmetic
> > (len - 1) etc.
> >
> >> - if (dns->data.name.str[len-1] == '.') {
> >> - dnsp->data.ns = talloc_strndup(mem_ctx, dns->data.name.str, len-1);
> >> +
> >> + if (check_name) {
> >> + res = dns_name_check(mem_ctx, len, name);
> >> + if (!W_ERROR_IS_OK(res)) {
> >> + return res;
> >> + }
> >> + }
> >> +
> >> + if (name[len-1] == '.') {
> >> + dnsp->data.ns = talloc_strndup(mem_ctx, name, len-1);
> >> } else {
> >> - dnsp->data.ns = talloc_strdup(mem_ctx, dns->data.name.str);
> >> + dnsp->data.ns = talloc_strdup(mem_ctx, name);
> >> }
> > Error check on talloc returns please.
> >
> >> +
> >> break;
> >>
> >> case DNS_TYPE_CNAME:
> >> + name = dns->data.name.str;
> >> len = dns->data.name.len;
> >> - if (dns->data.name.str[len-1] == '.') {
> >> - dnsp->data.cname = talloc_strndup(mem_ctx, dns->data.name.str, len-1);
> >> +
> >> + if (check_name) {
> >> + res = dns_name_check(mem_ctx, len, name);
> >> + if (!W_ERROR_IS_OK(res)) {
> >> + return res;
> >> + }
> >> + }
> >> +
> >> + if (name[len-1] == '.') {
> >> + dnsp->data.cname = talloc_strndup(mem_ctx, name, len-1);
> >> } else {
> >> - dnsp->data.cname = talloc_strdup(mem_ctx, dns->data.name.str);
> >> + dnsp->data.cname = talloc_strdup(mem_ctx, name);
> >> }
> > Error check on talloc returns please.
> >
> >> +
> >> break;
> >>
> >> case DNS_TYPE_SOA:
> >> @@ -466,33 +489,71 @@ struct dnsp_DnssrvRpcRecord *dns_to_dnsp_copy(TALLOC_CTX *mem_ctx, struct DNS_RP
> >> dnsp->data.soa.expire = dns->data.soa.dwExpire;
> >> dnsp->data.soa.minimum = dns->data.soa.dwMinimumTtl;
> >>
> >> + name = dns->data.soa.NamePrimaryServer.str;
> >> len = dns->data.soa.NamePrimaryServer.len;
> >> - if (dns->data.soa.NamePrimaryServer.str[len-1] == '.') {
> >> - dnsp->data.soa.mname = talloc_strndup(mem_ctx, dns->data.soa.NamePrimaryServer.str, len-1);
> >> +
> >> + if (check_name) {
> >> + res = dns_name_check(mem_ctx, len, name);
> >> + if (!W_ERROR_IS_OK(res)) {
> >> + return res;
> >> + }
> >> + }
> >> +
> >> + if (name[len-1] == '.') {
> >> + dnsp->data.soa.mname = talloc_strndup(mem_ctx, name, len-1);
> >> } else {
> >> - dnsp->data.soa.mname = talloc_strdup(mem_ctx, dns->data.soa.NamePrimaryServer.str);
> >> + dnsp->data.soa.mname = talloc_strdup(mem_ctx, name);
> > Error check on talloc returns please.
> >
> >> }
> >>
> >> + name = dns->data.soa.ZoneAdministratorEmail.str;
> >> len = dns->data.soa.ZoneAdministratorEmail.len;
> >> - if (dns->data.soa.ZoneAdministratorEmail.str[len-1] == '.') {
> >> - dnsp->data.soa.rname = talloc_strndup(mem_ctx, dns->data.soa.ZoneAdministratorEmail.str, len-1);
> >> +
> >> + res = dns_name_check(mem_ctx, len, name);
> >> + if (!W_ERROR_IS_OK(res)) {
> >> + return res;
> >> + }
> >> +
> >> + if (name[len-1] == '.') {
> >> + dnsp->data.soa.rname = talloc_strndup(mem_ctx, name, len-1);
> >> } else {
> >> - dnsp->data.soa.rname = talloc_strdup(mem_ctx, dns->data.soa.ZoneAdministratorEmail.str);
> >> + dnsp->data.soa.rname = talloc_strdup(mem_ctx, name);
> > Error check on talloc returns please.
> >
> >> }
> >> +
> >> break;
> >>
> >> case DNS_TYPE_PTR:
> >> - dnsp->data.ptr = talloc_strdup(mem_ctx, dns->data.ptr.str);
> >> + name = dns->data.ptr.str;
> >> + len = dns->data.ptr.len;
> >> +
> >> + if (check_name) {
> >> + res = dns_name_check(mem_ctx, len, name);
> >> + if (!W_ERROR_IS_OK(res)) {
> >> + return res;
> >> + }
> >> + }
> >> +
> >> + dnsp->data.ptr = talloc_strdup(mem_ctx, name);
> > Error check on talloc returns please.
> >
> >> break;
> >>
> >> case DNS_TYPE_MX:
> >> dnsp->data.mx.wPriority = dns->data.mx.wPreference;
> >> +
> >> + name = dns->data.mx.nameExchange.str;
> >> len = dns->data.mx.nameExchange.len;
> >> - if (dns->data.mx.nameExchange.str[len-1] == '.') {
> >> - dnsp->data.mx.nameTarget = talloc_strndup(mem_ctx, dns->data.mx.nameExchange.str, len-1);
> >> +
> >> + if (check_name) {
> >> + res = dns_name_check(mem_ctx, len, name);
> >> + if (!W_ERROR_IS_OK(res)) {
> >> + return res;
> >> + }
> >> + }
> >> +
> >> + if (name[len-1] == '.') {
> >> + dnsp->data.mx.nameTarget = talloc_strndup(mem_ctx, name, len-1);
> >> } else {
> >> - dnsp->data.mx.nameTarget = talloc_strdup(mem_ctx, dns->data.mx.nameExchange.str);
> >> + dnsp->data.mx.nameTarget = talloc_strdup(mem_ctx, name);
> > Error check on talloc returns please.
> >> }
> >> +
> >> break;
> >>
> >> case DNS_TYPE_TXT:
> >> @@ -512,24 +573,33 @@ struct dnsp_DnssrvRpcRecord *dns_to_dnsp_copy(TALLOC_CTX *mem_ctx, struct DNS_RP
> >> dnsp->data.srv.wWeight = dns->data.srv.wWeight;
> >> dnsp->data.srv.wPort = dns->data.srv.wPort;
> >>
> >> + name = dns->data.srv.nameTarget.str;
> >> len = dns->data.srv.nameTarget.len;
> >> - if (dns->data.srv.nameTarget.str[len-1] == '.') {
> >> - dnsp->data.srv.nameTarget = talloc_strndup(mem_ctx, dns->data.srv.nameTarget.str, len-1);
> >> +
> >> + if (check_name) {
> >> + res = dns_name_check(mem_ctx, len, name);
> >> + if (!W_ERROR_IS_OK(res)) {
> >> + return res;
> >> + }
> >> + }
> >> +
> >> + if (name[len-1] == '.') {
> >> + dnsp->data.srv.nameTarget = talloc_strndup(mem_ctx, name, len-1);
> >> } else {
> >> - dnsp->data.srv.nameTarget = talloc_strdup(mem_ctx, dns->data.srv.nameTarget.str);
> >> + dnsp->data.srv.nameTarget = talloc_strdup(mem_ctx, name);
> > Error check on talloc returns please.
> >> }
> >> +
> >> break;
> >>
> >> default:
> >> memcpy(&dnsp->data, &dns->data, sizeof(union dnsRecordData));
> >> DEBUG(0, ("dnsserver: Found Unhandled DNS record type=%d", dns->wType));
> >> -
> >> }
> >>
> >> - return dnsp;
> >> + *out_dnsp = dnsp;
> >> + return WERR_OK;
> >> }
> >>
> >> -
> >> /* Intialize tree with given name as the root */
> >> static struct dns_tree *dns_tree_init(TALLOC_CTX *mem_ctx, const char *name, void *data)
> >> {
> >> diff --git a/source4/rpc_server/dnsserver/dnsdb.c b/source4/rpc_server/dnsserver/dnsdb.c
> >> index 949a8b9..da37878 100644
> >> --- a/source4/rpc_server/dnsserver/dnsdb.c
> >> +++ b/source4/rpc_server/dnsserver/dnsdb.c
> >> @@ -397,17 +397,20 @@ WERROR dnsserver_db_add_record(TALLOC_CTX *mem_ctx,
> >> {
> >> const char * const attrs[] = { "dnsRecord", "dNSTombstoned", NULL };
> >> struct ldb_result *res;
> >> - struct dnsp_DnssrvRpcRecord *rec;
> >> + struct dnsp_DnssrvRpcRecord *rec = NULL;
> >> struct ldb_message_element *el;
> >> struct ldb_dn *dn;
> >> enum ndr_err_code ndr_err;
> >> NTTIME t;
> >> int ret, i;
> >> int serial;
> >> + WERROR werr;
> >> bool was_tombstoned = false;
> >>
> >> - rec = dns_to_dnsp_copy(mem_ctx, add_record);
> >> - W_ERROR_HAVE_NO_MEMORY(rec);
> >> + werr = dns_to_dnsp_convert(mem_ctx, add_record, &rec, true);
> >> + if (!W_ERROR_IS_OK(werr)) {
> >> + return werr;
> >> + }
> >>
> >> /* Set the correct rank for the record. */
> >> if (z->zoneinfo->dwZoneType == DNS_ZONE_TYPE_PRIMARY) {
> >> @@ -514,18 +517,23 @@ WERROR dnsserver_db_update_record(TALLOC_CTX *mem_ctx,
> >> {
> >> const char * const attrs[] = { "dnsRecord", NULL };
> >> struct ldb_result *res;
> >> - struct dnsp_DnssrvRpcRecord *arec, *drec;
> >> + struct dnsp_DnssrvRpcRecord *arec = NULL, *drec = NULL;
> >> struct ldb_message_element *el;
> >> enum ndr_err_code ndr_err;
> >> NTTIME t;
> >> int ret, i;
> >> int serial;
> >> + WERROR werr;
> >>
> >> - arec = dns_to_dnsp_copy(mem_ctx, add_record);
> >> - W_ERROR_HAVE_NO_MEMORY(arec);
> >> + werr = dns_to_dnsp_convert(mem_ctx, add_record, &arec, true);
> >> + if (!W_ERROR_IS_OK(werr)) {
> >> + return werr;
> >> + }
> >>
> >> - drec = dns_to_dnsp_copy(mem_ctx, del_record);
> >> - W_ERROR_HAVE_NO_MEMORY(drec);
> >> + werr = dns_to_dnsp_convert(mem_ctx, del_record, &drec, true);
> >> + if (!W_ERROR_IS_OK(werr)) {
> >> + return werr;
> >> + }
> >>
> >> unix_to_nt_time(&t, time(NULL));
> >> t /= 10*1000*1000;
> >> @@ -616,19 +624,22 @@ WERROR dnsserver_db_delete_record(TALLOC_CTX *mem_ctx,
> >> {
> >> const char * const attrs[] = { "dnsRecord", NULL };
> >> struct ldb_result *res;
> >> - struct dnsp_DnssrvRpcRecord *rec;
> >> + struct dnsp_DnssrvRpcRecord *rec = NULL;
> >> struct ldb_message_element *el;
> >> enum ndr_err_code ndr_err;
> >> int ret, i;
> >> int serial;
> >> + WERROR werr;
> >>
> >> serial = dnsserver_update_soa(mem_ctx, samdb, z);
> >> if (serial < 0) {
> >> return WERR_INTERNAL_DB_ERROR;
> >> }
> >>
> >> - rec = dns_to_dnsp_copy(mem_ctx, del_record);
> >> - W_ERROR_HAVE_NO_MEMORY(rec);
> >> + werr = dns_to_dnsp_convert(mem_ctx, del_record, &rec, false);
> >> + if (!W_ERROR_IS_OK(werr)) {
> >> + return werr;
> >> + }
> >>
> >> ret = ldb_search(samdb, mem_ctx, &res, z->zone_dn, LDB_SCOPE_ONELEVEL, attrs,
> >> "(&(objectClass=dnsNode)(name=%s))", name);
> >> diff --git a/source4/rpc_server/dnsserver/dnsserver.h b/source4/rpc_server/dnsserver/dnsserver.h
> >> index cfe6d4e..0b08f08 100644
> >> --- a/source4/rpc_server/dnsserver/dnsserver.h
> >> +++ b/source4/rpc_server/dnsserver/dnsserver.h
> >> @@ -193,7 +193,9 @@ bool dns_record_match(struct dnsp_DnssrvRpcRecord *rec1, struct dnsp_DnssrvRpcRe
> >>
> >> void dnsp_to_dns_copy(TALLOC_CTX *mem_ctx, struct dnsp_DnssrvRpcRecord *dnsp,
> >> struct DNS_RPC_RECORD *dns);
> >> -struct dnsp_DnssrvRpcRecord *dns_to_dnsp_copy(TALLOC_CTX *mem_ctx, struct DNS_RPC_RECORD *dns);
> >> +WERROR dns_to_dnsp_convert(TALLOC_CTX *mem_ctx, struct DNS_RPC_RECORD *dns,
> >> + struct dnsp_DnssrvRpcRecord **out_dnsp,
> >> + bool check_name);
> >>
> >> struct dns_tree *dns_build_tree(TALLOC_CTX *mem_ctx, const char *name, struct ldb_result *res);
> >> WERROR dns_fill_records_array(TALLOC_CTX *mem_ctx, struct dnsserver_zone *z,
> >> diff --git a/source4/rpc_server/wscript_build b/source4/rpc_server/wscript_build
> >> index ed7dbf6..f1b1f19 100755
> >> --- a/source4/rpc_server/wscript_build
> >> +++ b/source4/rpc_server/wscript_build
> >> @@ -155,7 +155,7 @@ bld.SAMBA_MODULE('dcerpc_dnsserver',
> >> source='dnsserver/dcerpc_dnsserver.c dnsserver/dnsutils.c dnsserver/dnsdata.c dnsserver/dnsdb.c',
> >> subsystem='dcerpc_server',
> >> init_function='dcerpc_server_dnsserver_init',
> >> - deps='DCERPC_COMMON'
> >> + deps='DCERPC_COMMON dnsserver_common'
> >> )
> >>
> >>
> >> --
> >> 1.9.1
>
> From f1152f1824aa82edb55864896e18c289602d2bcf Mon Sep 17 00:00:00 2001
> From: Bob Campbell <bobcampbell at catalyst.net.nz>
> Date: Mon, 28 Nov 2016 14:30:43 +1300
> Subject: [PATCH 01/12] python/netcmd: print traceback through self.errf
>
> Signed-off-by: Bob Campbell <bobcampbell at catalyst.net.nz>
> Reviewed-by: Garming Sam <garming at catalyst.net.nz>
> ---
> python/samba/netcmd/__init__.py | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/python/samba/netcmd/__init__.py b/python/samba/netcmd/__init__.py
> index 1debae8..e91bebd 100644
> --- a/python/samba/netcmd/__init__.py
> +++ b/python/samba/netcmd/__init__.py
> @@ -120,7 +120,7 @@ class Command(object):
> force_traceback = True
>
> if force_traceback or samba.get_debug_level() >= 3:
> - traceback.print_tb(etraceback)
> + traceback.print_tb(etraceback, file=self.errf)
>
> def _create_parser(self, prog, epilog=None):
> parser = optparse.OptionParser(
> --
> 1.9.1
>
>
> From bf34fa1c8ec7b170bcecd9373de58a65db2af8f9 Mon Sep 17 00:00:00 2001
> From: Bob Campbell <bobcampbell at catalyst.net.nz>
> Date: Mon, 21 Nov 2016 16:22:46 +1300
> Subject: [PATCH 02/12] python/tests: add tests for samba-tool dns
>
> Signed-off-by: Bob Campbell <bobcampbell at catalyst.net.nz>
> Reviewed-by: Garming Sam <garming at catalyst.net.nz>
> ---
> python/samba/tests/samba_tool/dnscmd.py | 557 ++++++++++++++++++++++++++++++++
> selftest/knownfail | 4 +
> source4/selftest/tests.py | 1 +
> 3 files changed, 562 insertions(+)
> create mode 100644 python/samba/tests/samba_tool/dnscmd.py
>
> diff --git a/python/samba/tests/samba_tool/dnscmd.py b/python/samba/tests/samba_tool/dnscmd.py
> new file mode 100644
> index 0000000..c1cb7df
> --- /dev/null
> +++ b/python/samba/tests/samba_tool/dnscmd.py
> @@ -0,0 +1,557 @@
> +# Unix SMB/CIFS implementation.
> +# Copyright (C) Andrew Bartlett <abartlet at catalyst.net.nz>
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program. If not, see <http://www.gnu.org/licenses/>.
> +#
> +
> +import os
> +import ldb
> +
> +from samba.auth import system_session
> +from samba.samdb import SamDB
> +from samba.ndr import ndr_unpack, ndr_pack
> +from samba.dcerpc import dnsp
> +from samba.tests.samba_tool.base import SambaToolCmdTest
> +
> +class DnsCmdTestCase(SambaToolCmdTest):
> + def setUp(self):
> + super(DnsCmdTestCase, self).setUp()
> +
> + self.dburl = "ldap://%s" % os.environ["SERVER"]
> + self.creds_string = "-U%s%%%s" % (os.environ["DC_USERNAME"],
> + os.environ["DC_PASSWORD"])
> +
> + self.samdb = self.getSamDB("-H", self.dburl, self.creds_string)
> + self.config_dn = str(self.samdb.get_config_basedn())
> +
> + self.testip = "192.168.0.193"
> + self.testip2 = "192.168.0.194"
> +
> + self.addZone()
> +
> + # Note: SOA types don't work (and shouldn't), as we only have one zone per DNS record.
> +
> + good_dns = ["SAMDOM.EXAMPLE.COM",
> + "1.EXAMPLE.COM",
> + "%sEXAMPLE.COM" % ("1."*100),
> + "EXAMPLE",
> + "\n.COM",
> + "!@#$%^&*()_",
> + "HIGH\xFFBYTE",
> + "@.EXAMPLE.COM",
> + "."]
> + bad_dns = ["...",
> + ".EXAMPLE.COM",
> + ".EXAMPLE.",
> + "",
> + "SAMDOM..EXAMPLE.COM"]
> +
> + good_mx = ["SAMDOM.EXAMPLE.COM 65530"]
> + bad_mx = ["SAMDOM.EXAMPLE.COM -1",
> + "SAMDOM.EXAMPLE.COM",
> + " ",
> + "SAMDOM.EXAMPLE.COM 1 1",
> + "SAMDOM.EXAMPLE.COM SAMDOM.EXAMPLE.COM"]
> +
> + good_srv = ["SAMDOM.EXAMPLE.COM 65530 65530 65530"]
> + bad_srv = ["SAMDOM.EXAMPLE.COM 0 65536 0",
> + "SAMDOM.EXAMPLE.COM 0 0 65536",
> + "SAMDOM.EXAMPLE.COM 65536 0 0" ]
> +
> + for bad_dn in bad_dns:
> + bad_mx.append("%s 1" % bad_dn)
> + bad_srv.append("%s 0 0 0" % bad_dn)
> + for good_dn in good_dns:
> + good_mx.append("%s 1" % good_dn)
> + good_srv.append("%s 0 0 0" % good_dn)
> +
> + self.good_records = {
> + "A":["192.168.0.1", "255.255.255.255"],
> + "AAAA":["1234:5678:9ABC:DEF0:0000:0000:0000:0000",
> + "0000:0000:0000:0000:0000:0000:0000:0000",
> + "1234:5678:9ABC:DEF0:1234:5678:9ABC:DEF0",
> + "1234:1234:1234::",
> + "1234:5678:9ABC:DEF0::",
> + "0000:0000::0000",
> + "1234::5678:9ABC:0000:0000:0000:0000",
> + "::1",
> + "::",
> + "1:1:1:1:1:1:1:1"],
> + "PTR":good_dns,
> + "CNAME":good_dns,
> + "NS":good_dns,
> + "MX":good_mx,
> + "SRV":good_srv,
> + "TXT":["text", "", "@#!", "\n"]
> + }
> +
> + self.bad_records = {
> + "A":["192.168.0.500",
> + "255.255.255.255/32"],
> + "AAAA":["GGGG:1234:5678:9ABC:0000:0000:0000:0000",
> + "0000:0000:0000:0000:0000:0000:0000:0000/1",
> + "AAAA:AAAA:AAAA:AAAA:G000:0000:0000:1234",
> + "1234:5678:9ABC:DEF0:1234:5678:9ABC:DEF0:1234",
> + "1234:5678:9ABC:DEF0:1234:5678:9ABC",
> + "1111::1111::1111"],
> + "PTR":bad_dns,
> + "CNAME":bad_dns,
> + "NS":bad_dns,
> + "MX":bad_mx,
> + "SRV":bad_srv
> + }
> +
> + def tearDown(self):
> + self.deleteZone()
> + super(DnsCmdTestCase, self).tearDown()
> +
> + def resetZone(self):
> + self.deleteZone()
> + self.addZone()
> +
> + def addZone(self):
> + self.zone = "zone"
> + result, out, err = self.runsubcmd("dns",
> + "zonecreate",
> + os.environ["SERVER"],
> + self.zone,
> + self.creds_string)
> + self.assertCmdSuccess(result, out, err)
> +
> + def deleteZone(self):
> + result, out, err = self.runsubcmd("dns",
> + "zonedelete",
> + os.environ["SERVER"],
> + self.zone,
> + self.creds_string)
> + self.assertCmdSuccess(result, out, err)
> +
> + def get_record_from_db(self, zone_name, record_name):
> + 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 zone_name in str(zone.dn):
> + zone_dn = zone.dn
> + break
> +
> + records = self.samdb.search(base=zone_dn, scope=ldb.SCOPE_SUBTREE,
> + expression="(objectClass=dnsNode)",
> + attrs=["dnsRecord"])
> +
> + for old_packed_record in records:
> + if record_name in str(old_packed_record.dn):
> + return (old_packed_record.dn,
> + ndr_unpack(dnsp.DnssrvRpcRecord,
> + old_packed_record["dnsRecord"][0]))
> +
> + def test_rank_none(self):
> + record_str = "192.168.50.50"
> + record_type_str = "A"
> +
> + result, out, err = self.runsubcmd("dns", "add", os.environ["SERVER"],
> + self.zone, "testrecord", record_type_str,
> + record_str, self.creds_string)
> + self.assertCmdSuccess(result, out, err,
> + "Failed to add record '%s' with type %s."
> + % (record_str, record_type_str))
> +
> + dn, record = self.get_record_from_db(self.zone, "testrecord")
> + record.rank = 0 # DNS_RANK_NONE
> + res = self.samdb.dns_replace_by_dn(dn, [record])
> + if res is not None:
> + self.fail("Unable to update dns record to have DNS_RANK_NONE.")
> +
> + errors = []
> +
> + # The record should still exist
> + result, out, err = self.runsubcmd("dns", "query", os.environ["SERVER"],
> + self.zone, "testrecord", record_type_str,
> + self.creds_string)
> + try:
> + self.assertCmdSuccess(result, out, err,
> + "Failed to query for a record" \
> + "which had DNS_RANK_NONE.")
> + self.assertTrue("testrecord" in out and record_str in out,
> + "Query for a record which had DNS_RANK_NONE" \
> + "succeeded but produced no resulting records.")
> + except AssertionError, e:
> + # Windows produces no resulting records
> + pass
> +
> + # We should not be able to add a duplicate
> + result, out, err = self.runsubcmd("dns", "add", os.environ["SERVER"],
> + self.zone, "testrecord", record_type_str,
> + record_str, self.creds_string)
> + try:
> + self.assertCmdFail(result, "Successfully added duplicate record" \
> + "of one which had DNS_RANK_NONE.")
> + except AssertionError, e:
> + errors.append(e)
> +
> + # We should be able to delete it
> + result, out, err = self.runsubcmd("dns", "delete", os.environ["SERVER"],
> + self.zone, "testrecord", record_type_str,
> + record_str, self.creds_string)
> + try:
> + self.assertCmdSuccess(result, out, err, "Failed to delete record" \
> + "which had DNS_RANK_NONE.")
> + except AssertionError, e:
> + errors.append(e)
> +
> + # Now the record should not exist
> + result, out, err = self.runsubcmd("dns", "query", os.environ["SERVER"],
> + self.zone, "testrecord",
> + record_type_str, self.creds_string)
> + try:
> + self.assertCmdFail(result, "Successfully queried for deleted record" \
> + "which had DNS_RANK_NONE.")
> + except AssertionError, e:
> + errors.append(e)
> +
> + if len(errors) > 0:
> + err_str = "Failed appropriate behaviour with DNS_RANK_NONE:"
> + for error in errors:
> + err_str = err_str + "\n" + str(error)
> + raise AssertionError(err_str)
> +
> + def test_accept_valid_commands(self):
> + """
> + For all good records, attempt to add, query and delete them.
> + """
> + num_failures = 0
> + failure_msgs = []
> + for dnstype in self.good_records:
> + for record in self.good_records[dnstype]:
> + try:
> + result, out, err = self.runsubcmd("dns", "add",
> + os.environ["SERVER"],
> + self.zone, "testrecord",
> + dnstype, record,
> + self.creds_string)
> + self.assertCmdSuccess(result, out, err, "Failed to add" \
> + "record %s with type %s."
> + % (record, dnstype))
> +
> + result, out, err = self.runsubcmd("dns", "query",
> + os.environ["SERVER"],
> + self.zone, "testrecord",
> + dnstype,
> + self.creds_string)
> + self.assertCmdSuccess(result, out, err, "Failed to query" \
> + "record %s with qualifier %s."
> + % (record, dnstype))
> +
> + result, out, err = self.runsubcmd("dns", "delete",
> + os.environ["SERVER"],
> + self.zone, "testrecord",
> + dnstype, record,
> + self.creds_string)
> + self.assertCmdSuccess(result, out, err, "Failed to remove" \
> + "record %s with type %s."
> + % (record, dnstype))
> + except AssertionError as e:
> + num_failures = num_failures + 1
> + failure_msgs.append(e)
> +
> + if num_failures > 0:
> + for msg in failure_msgs:
> + print(msg)
> + self.fail("Failed to accept valid commands. %d total failures." \
> + "Errors above." % num_failures)
> +
> + def test_reject_invalid_commands(self):
> + """
> + For all bad records, attempt to add them and update to them,
> + making sure that both operations fail.
> + """
> + num_failures = 0
> + failure_msgs = []
> +
> + # Add invalid records and make sure they fail to be added
> + for dnstype in self.bad_records:
> + for record in self.bad_records[dnstype]:
> + try:
> + result, out, err = self.runsubcmd("dns", "add",
> + os.environ["SERVER"],
> + self.zone, "testrecord",
> + dnstype, record,
> + self.creds_string)
> + self.assertCmdFail(result, "Successfully added invalid" \
> + "record '%s' of type '%s'."
> + % (record, dnstype))
> + except AssertionError as e:
> + num_failures = num_failures + 1
> + failure_msgs.append(e)
> + self.resetZone()
> + try:
> + result, out, err = self.runsubcmd("dns", "delete",
> + os.environ["SERVER"],
> + self.zone, "testrecord",
> + dnstype, record,
> + self.creds_string)
> + self.assertCmdFail(result, "Successfully deleted invalid" \
> + "record '%s' of type '%s' which" \
> + "shouldn't exist." % (record, dnstype))
> + except AssertionError as e:
> + num_failures = num_failures + 1
> + failure_msgs.append(e)
> + self.resetZone()
> +
> + # Update valid records to invalid ones and make sure they
> + # fail to be updated
> + for dnstype in self.bad_records:
> + for bad_record in self.bad_records[dnstype]:
> + good_record = self.good_records[dnstype][0]
> +
> + try:
> + result, out, err = self.runsubcmd("dns", "add",
> + os.environ["SERVER"],
> + self.zone, "testrecord",
> + dnstype, good_record,
> + self.creds_string)
> + self.assertCmdSuccess(result, out, err, "Failed to add " \
> + "record '%s' with type %s."
> + % (record, dnstype))
> +
> + result, out, err = self.runsubcmd("dns", "update",
> + os.environ["SERVER"],
> + self.zone, "testrecord",
> + dnstype, good_record,
> + bad_record,
> + self.creds_string)
> + self.assertCmdFail(result, "Successfully updated valid " \
> + "record '%s' of type '%s' to invalid " \
> + "record '%s' of the same type."
> + % (good_record, dnstype, bad_record))
> +
> + result, out, err = self.runsubcmd("dns", "delete",
> + os.environ["SERVER"],
> + self.zone, "testrecord",
> + dnstype, good_record,
> + self.creds_string)
> + self.assertCmdSuccess(result, out, err, "Could not delete " \
> + "valid record '%s' of type '%s'."
> + % (good_record, dnstype))
> + except AssertionError as e:
> + num_failures = num_failures + 1
> + failure_msgs.append(e)
> + self.resetZone()
> +
> + if num_failures > 0:
> + for msg in failure_msgs:
> + print(msg)
> + self.fail("Failed to reject invalid commands. %d total failures. " \
> + "Errors above." % num_failures)
> +
> + def test_update_invalid_type(self):
> + """
> + Make sure that a record can't be updated to one of a different type.
> + """
> + for dnstype1 in self.good_records:
> + record1 = self.good_records[dnstype1][0]
> + result, out, err = self.runsubcmd("dns", "add",
> + os.environ["SERVER"],
> + self.zone, "testrecord",
> + dnstype1, record1,
> + self.creds_string)
> + self.assertCmdSuccess(result, out, err, "Failed to add " \
> + "record %s with type %s."
> + % (record1, dnstype1))
> +
> + for dnstype2 in self.good_records:
> + record2 = self.good_records[dnstype2][0]
> +
> + # Make sure that record2 isn't a valid entry of dnstype1.
> + # For example, any A-type will also be a valid TXT-type.
> + result, out, err = self.runsubcmd("dns", "add",
> + os.environ["SERVER"],
> + self.zone, "testrecord",
> + dnstype1, record2,
> + self.creds_string)
> + try:
> + self.assertCmdFail(result)
> + except AssertionError:
> + continue # Don't check this one, because record2 _is_ a valid entry of dnstype1.
> +
> + # Check both ways: Give the current type and try to update,
> + # and give the new type and try to update.
> + result, out, err = self.runsubcmd("dns", "update",
> + os.environ["SERVER"],
> + self.zone, "testrecord",
> + dnstype1, record1,
> + record2, self.creds_string)
> + self.assertCmdFail(result, "Successfully updated record '%s' " \
> + "to '%s', even though the latter is of " \
> + "type '%s' where '%s' was expected."
> + % (record1, record2, dnstype2, dnstype1))
> +
> + result, out, err = self.runsubcmd("dns", "update",
> + os.environ["SERVER"],
> + self.zone, "testrecord",
> + dnstype2, record1, record2,
> + self.creds_string)
> + self.assertCmdFail(result, "Successfully updated record " \
> + "'%s' to '%s', even though the former " \
> + "is of type '%s' where '%s' was expected."
> + % (record1, record2, dnstype1, dnstype2))
> +
> +
> + def test_update_valid_type(self):
> + for dnstype in self.good_records:
> + for record in self.good_records[dnstype]:
> + result, out, err = self.runsubcmd("dns", "add",
> + os.environ["SERVER"],
> + self.zone, "testrecord",
> + dnstype, record,
> + self.creds_string)
> + self.assertCmdSuccess(result, out, err, "Failed to add " \
> + "record %s with type %s."
> + % (record, dnstype))
> +
> + # Update the record to be the same.
> + result, out, err = self.runsubcmd("dns", "update",
> + os.environ["SERVER"],
> + self.zone, "testrecord",
> + dnstype, record, record,
> + self.creds_string)
> + self.assertCmdFail(result, "Successfully updated record " \
> + "'%s' to be exactly the same." % record)
> +
> + result, out, err = self.runsubcmd("dns", "delete",
> + os.environ["SERVER"],
> + self.zone, "testrecord",
> + dnstype, record,
> + self.creds_string)
> + self.assertCmdSuccess(result, out, err, "Could not delete " \
> + "valid record '%s' of type '%s'."
> + % (record, dnstype))
> +
> + for record in self.good_records["SRV"]:
> + result, out, err = self.runsubcmd("dns", "add",
> + os.environ["SERVER"],
> + self.zone, "testrecord",
> + "SRV", record,
> + self.creds_string)
> + self.assertCmdSuccess(result, out, err, "Failed to add " \
> + "record %s with type 'SRV'." % record)
> +
> + split = record.split(' ')
> + new_bit = str(int(split[3]) + 1)
> + new_record = '%s %s %s %s' % (split[0], split[1], split[2], new_bit)
> +
> + result, out, err = self.runsubcmd("dns", "update",
> + os.environ["SERVER"],
> + self.zone, "testrecord",
> + "SRV", record,
> + new_record, self.creds_string)
> + self.assertCmdSuccess(result, out, err, "Failed to update record " \
> + "'%s' of type '%s' to '%s'."
> + % (record, "SRV", new_record))
> +
> + result, out, err = self.runsubcmd("dns", "query",
> + os.environ["SERVER"],
> + self.zone, "testrecord",
> + "SRV", self.creds_string)
> + self.assertCmdSuccess(result, out, err, "Failed to query for " \
> + "record '%s' of type '%s'."
> + % (new_record, "SRV"))
> +
> + result, out, err = self.runsubcmd("dns", "delete",
> + os.environ["SERVER"],
> + self.zone, "testrecord",
> + "SRV", new_record,
> + self.creds_string)
> + self.assertCmdSuccess(result, out, err, "Could not delete " \
> + "valid record '%s' of type '%s'."
> + % (new_record, "SRV"))
> +
> + # Since 'dns update' takes the current value as a parameter, make sure
> + # we can't enter the wrong current value for a given record.
> + for dnstype in self.good_records:
> + if len(self.good_records[dnstype]) < 3:
> + continue # Not enough records of this type to do this test
> +
> + used_record = self.good_records[dnstype][0]
> + unused_record = self.good_records[dnstype][1]
> + new_record = self.good_records[dnstype][2]
> +
> + result, out, err = self.runsubcmd("dns", "add",
> + os.environ["SERVER"],
> + self.zone, "testrecord",
> + dnstype, used_record,
> + self.creds_string)
> + self.assertCmdSuccess(result, out, err, "Failed to add record %s " \
> + "with type %s." % (used_record, dnstype))
> +
> + result, out, err = self.runsubcmd("dns", "update",
> + os.environ["SERVER"],
> + self.zone, "testrecord",
> + dnstype, unused_record,
> + new_record,
> + self.creds_string)
> + self.assertCmdFail(result, "Successfully updated record '%s' " \
> + "from '%s' to '%s', even though the given " \
> + "source record is incorrect."
> + % (used_record, unused_record, new_record))
> +
> + def test_invalid_types(self):
> + result, out, err = self.runsubcmd("dns", "add",
> + os.environ["SERVER"],
> + self.zone, "testrecord",
> + "SOA", "test",
> + self.creds_string)
> + self.assertCmdFail(result, "Successfully added record of type SOA, " \
> + "when this type should not be available.")
> + self.assertTrue("type SOA is not supported" in err,
> + "Invalid error message '%s' when attempting to " \
> + "add record of type SOA." % err)
> +
> + def test_query_deleted_record(self):
> + self.runsubcmd("dns", "add", os.environ["SERVER"], self.zone,
> + "testrecord", "A", self.testip, self.creds_string)
> + self.runsubcmd("dns", "delete", os.environ["SERVER"], self.zone,
> + "testrecord", "A", self.testip, self.creds_string)
> +
> + result, out, err = self.runsubcmd("dns", "query",
> + os.environ["SERVER"],
> + self.zone, "testrecord",
> + "A", self.creds_string)
> + self.assertCmdFail(result)
> +
> + def test_remove_deleted_record(self):
> + self.runsubcmd("dns", "add", os.environ["SERVER"], self.zone,
> + "testrecord", "A", self.testip, self.creds_string)
> + self.runsubcmd("dns", "delete", os.environ["SERVER"], self.zone,
> + "testrecord", "A", self.testip, self.creds_string)
> +
> + # Attempting to delete a record that has already been deleted or has never existed should fail
> + result, out, err = self.runsubcmd("dns", "delete",
> + os.environ["SERVER"],
> + self.zone, "testrecord",
> + "A", self.testip, self.creds_string)
> + self.assertCmdFail(result)
> + result, out, err = self.runsubcmd("dns", "query",
> + os.environ["SERVER"],
> + self.zone, "testrecord",
> + "A", self.creds_string)
> + self.assertCmdFail(result)
> + result, out, err = self.runsubcmd("dns", "delete",
> + os.environ["SERVER"],
> + self.zone, "testrecord2",
> + "A", self.testip, self.creds_string)
> + self.assertCmdFail(result)
> diff --git a/selftest/knownfail b/selftest/knownfail
> index 38b5f51..80974bb 100644
> --- a/selftest/knownfail
> +++ b/selftest/knownfail
> @@ -306,3 +306,7 @@
> ^samba4.rpc.echo.*on.*with.object.echo.sinkdata.*nt4_dc
> ^samba4.rpc.echo.*on.*with.object.echo.addone.*nt4_dc
> ^samba4.rpc.echo.*on.*ncacn_ip_tcp.*with.object.*nt4_dc
> +^samba.tests.samba_tool.dnscmd.samba.tests.samba_tool.dnscmd.DnsCmdTestCase.test_accept_valid_commands
> +^samba.tests.samba_tool.dnscmd.samba.tests.samba_tool.dnscmd.DnsCmdTestCase.test_rank_none
> +^samba.tests.samba_tool.dnscmd.samba.tests.samba_tool.dnscmd.DnsCmdTestCase.test_reject_invalid_commands
> +^samba.tests.samba_tool.dnscmd.samba.tests.samba_tool.dnscmd.DnsCmdTestCase.test_update_valid_type
> diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py
> index 87acaf5..ec07106 100755
> --- a/source4/selftest/tests.py
> +++ b/source4/selftest/tests.py
> @@ -578,6 +578,7 @@ planpythontestsuite("ad_dc_ntvfs:local", "samba.tests.samba_tool.group")
> planpythontestsuite("ad_dc:local", "samba.tests.samba_tool.ntacl")
>
> planpythontestsuite("ad_dc:local", "samba.tests.samba_tool.sites")
> +planpythontestsuite("ad_dc:local", "samba.tests.samba_tool.dnscmd")
>
> planpythontestsuite("ad_dc_ntvfs:local", "samba.tests.dcerpc.rpcecho")
> planoldpythontestsuite("ad_dc_ntvfs:local", "samba.tests.dcerpc.registry", extra_args=['-U"$USERNAME%$PASSWORD"'])
> --
> 1.9.1
>
>
> From 1ea6ea5ef0a028b87e66c21c0dbc0c55a6af9d7b Mon Sep 17 00:00:00 2001
> From: Bob Campbell <bobcampbell at catalyst.net.nz>
> Date: Wed, 30 Nov 2016 09:19:31 +1300
> Subject: [PATCH 03/12] python/tests: expand tests for dns server over rpc
>
> Signed-off-by: Bob Campbell <bobcampbell at catalyst.net.nz>
> Reviewed-by: Garming Sam <garming at catalyst.net.nz>
> ---
> python/samba/tests/dcerpc/dnsserver.py | 724 +++++++++++++++++++++++++++++----
> selftest/knownfail | 3 +
> 2 files changed, 656 insertions(+), 71 deletions(-)
>
> diff --git a/python/samba/tests/dcerpc/dnsserver.py b/python/samba/tests/dcerpc/dnsserver.py
> index 7229877..46df5e3 100644
> --- a/python/samba/tests/dcerpc/dnsserver.py
> +++ b/python/samba/tests/dcerpc/dnsserver.py
> @@ -17,20 +17,642 @@
>
> """Tests for samba.dcerpc.dnsserver"""
>
> +import os
> +import ldb
> +
> +from samba.auth import system_session
> +from samba.samdb import SamDB
> +from samba.ndr import ndr_unpack, ndr_pack
> from samba.dcerpc import dnsp, dnsserver
> from samba.tests import RpcInterfaceTestCase, env_get_var_value
> -from samba.netcmd.dns import ARecord, NSRecord
> +from samba.netcmd.dns import ARecord, AAAARecord, PTRRecord, CNameRecord, NSRecord, MXRecord, SRVRecord, TXTRecord
>
> class DnsserverTests(RpcInterfaceTestCase):
>
> + @classmethod
> + def setUpClass(cls):
> + good_dns = ["SAMDOM.EXAMPLE.COM",
> + "1.EXAMPLE.COM",
> + "%sEXAMPLE.COM" % ("1."*100),
> + "EXAMPLE",
> + "\n.COM",
> + "!@#$%^&*()_",
> + "HIGH\xFFBYTE",
> + "@.EXAMPLE.COM",
> + "."]
> + bad_dns = ["...",
> + ".EXAMPLE.COM",
> + ".EXAMPLE.",
> + "",
> + "SAMDOM..EXAMPLE.COM"]
> +
> + good_mx = ["SAMDOM.EXAMPLE.COM 65535"]
> + bad_mx = []
> +
> + good_srv = ["SAMDOM.EXAMPLE.COM 65535 65535 65535"]
> + bad_srv = []
> +
> + for bad_dn in bad_dns:
> + bad_mx.append("%s 1" % bad_dn)
> + bad_srv.append("%s 0 0 0" % bad_dn)
> + for good_dn in good_dns:
> + good_mx.append("%s 1" % good_dn)
> + good_srv.append("%s 0 0 0" % good_dn)
> +
> + cls.good_records = {
> + "A": ["192.168.0.1",
> + "255.255.255.255"],
> + "AAAA": ["1234:5678:9ABC:DEF0:0000:0000:0000:0000",
> + "0000:0000:0000:0000:0000:0000:0000:0000",
> + "1234:5678:9ABC:DEF0:1234:5678:9ABC:DEF0",
> + "1234:1234:1234::",
> + "1234:1234:1234:1234:1234::",
> + "1234:5678:9ABC:DEF0::",
> + "0000:0000::0000",
> + "1234::5678:9ABC:0000:0000:0000:0000",
> + "::1",
> + "::",
> + "1:1:1:1:1:1:1:1"],
> + "PTR": good_dns,
> + "CNAME": good_dns,
> + "NS": good_dns,
> + "MX": good_mx,
> + "SRV": good_srv,
> + "TXT": ["text", "", "@#!", "\n"]
> + }
> +
> + cls.bad_records = {
> + "A": ["192.168.0.500",
> + "255.255.255.255/32"],
> + "AAAA": ["GGGG:1234:5678:9ABC:0000:0000:0000:0000",
> + "0000:0000:0000:0000:0000:0000:0000:0000/1",
> + "AAAA:AAAA:AAAA:AAAA:G000:0000:0000:1234",
> + "1234:5678:9ABC:DEF0:1234:5678:9ABC:DEF0:1234",
> + "1234:5678:9ABC:DEF0:1234:5678:9ABC",
> + "1111::1111::1111"],
> + "PTR": bad_dns,
> + "CNAME": bad_dns,
> + "NS": bad_dns,
> + "MX": bad_mx,
> + "SRV": bad_srv
> + }
> +
> + # Because we use uint16_t for these numbers, we can't
> + # actually create these records.
> + invalid_mx = ["SAMDOM.EXAMPLE.COM -1",
> + "SAMDOM.EXAMPLE.COM 65536",
> + "%s 1" % "A"*256]
> + invalid_srv = ["SAMDOM.EXAMPLE.COM 0 65536 0",
> + "SAMDOM.EXAMPLE.COM 0 0 65536",
> + "SAMDOM.EXAMPLE.COM 65536 0 0"]
> + cls.invalid_records = {
> + "MX": invalid_mx,
> + "SRV": invalid_srv
> + }
> +
> def setUp(self):
> super(DnsserverTests, self).setUp()
> - self.server = env_get_var_value("SERVER_IP")
> + self.server = os.environ["DC_SERVER"]
> self.zone = env_get_var_value("REALM").lower()
> self.conn = dnsserver.dnsserver("ncacn_ip_tcp:%s[sign]" % (self.server),
> self.get_loadparm(),
> self.get_credentials())
>
> + self.samdb = SamDB(url="ldap://%s" % os.environ["DC_SERVER_IP"],
> + lp = self.get_loadparm(),
> + session_info=system_session(),
> + credentials=self.get_credentials())
> +
> +
> + self.custom_zone = "zone"
> + zone_create_info = dnsserver.DNS_RPC_ZONE_CREATE_INFO_LONGHORN()
> + zone_create_info.pszZoneName = self.custom_zone
> + zone_create_info.dwZoneType = dnsp.DNS_ZONE_TYPE_PRIMARY
> + zone_create_info.fAging = 0
> + zone_create_info.fDsIntegrated = 1
> + zone_create_info.fLoadExisting = 1
> + zone_create_info.dwDpFlags = dnsserver.DNS_DP_DOMAIN_DEFAULT
> +
> + self.conn.DnssrvOperation2(dnsserver.DNS_CLIENT_VERSION_LONGHORN,
> + 0,
> + self.server,
> + None,
> + 0,
> + 'ZoneCreate',
> + dnsserver.DNSSRV_TYPEID_ZONE_CREATE,
> + zone_create_info)
> +
> + def tearDown(self):
> + self.conn.DnssrvOperation2(dnsserver.DNS_CLIENT_VERSION_LONGHORN,
> + 0,
> + self.server,
> + self.custom_zone,
> + 0,
> + 'DeleteZoneFromDs',
> + dnsserver.DNSSRV_TYPEID_NULL,
> + None)
> + super(DnsserverTests, self).tearDown()
> +
> + # This test fails against Samba (but passes against Windows),
> + # because Samba does not return the record when we enum records.
> + # Records can be given DNS_RANK_NONE when the zone they are in
> + # does not have DNS_ZONE_TYPE_PRIMARY. Since such records can be
> + # deleted, however, we do not consider this urgent to fix and
> + # so this test is a knownfail.
> + def test_rank_none(self):
> + """
> + See what happens when we set a record's rank to
> + DNS_RANK_NONE.
> + """
> +
> + record_str = "192.168.50.50"
> + record_type_str = "A"
> + self.add_record(self.custom_zone, "testrecord", record_type_str, record_str)
> +
> + dn, record = self.get_record_from_db(self.custom_zone, "testrecord")
> + record.rank = 0 # DNS_RANK_NONE
> + res = self.samdb.dns_replace_by_dn(dn, [record])
> + if res is not None:
> + self.fail("Unable to update dns record to have DNS_RANK_NONE.")
> +
> + self.assert_num_records(self.custom_zone, "testrecord", record_type_str)
> + self.add_record(self.custom_zone, "testrecord", record_type_str, record_str, assertion=False)
> + self.delete_record(self.custom_zone, "testrecord", record_type_str, record_str)
> + self.assert_num_records(self.custom_zone, "testrecord", record_type_str, 0)
> +
> + def test_dns_tombstoned(self):
> + """
> + See what happens when we set a record to be tombstoned.
> + """
> +
> + record_str = "192.168.50.50"
> + record_type_str = "A"
> + self.add_record(self.custom_zone, "testrecord", record_type_str, record_str)
> +
> + dn, record = self.get_record_from_db(self.custom_zone, "testrecord")
> + record.wType = dnsp.DNS_TYPE_TOMBSTONE
> + res = self.samdb.dns_replace_by_dn(dn, [record])
> + if res is not None:
> + self.fail("Unable to update dns record to be tombstoned.")
> +
> + self.assert_num_records(self.custom_zone, "testrecord", record_type_str)
> + self.delete_record(self.custom_zone, "testrecord", record_type_str, record_str)
> + self.assert_num_records(self.custom_zone, "testrecord", record_type_str, 0)
> +
> + def get_record_from_db(self, zone_name, record_name):
> + """
> + Returns (dn of record, record)
> + """
> +
> + zones = self.samdb.search(base="DC=DomainDnsZones,%s" % self.samdb.get_default_basedn(), scope=ldb.SCOPE_SUBTREE,
> + expression="(objectClass=dnsZone)",
> + attrs=["cn"])
> +
> + zone_dn = None
> + for zone in zones:
> + if zone_name in str(zone.dn):
> + zone_dn = zone.dn
> + break
> +
> + if zone_dn is None:
> + raise AssertionError("Couldn't find zone '%s'." % zone_name)
> +
> + records = self.samdb.search(base=zone_dn, scope=ldb.SCOPE_SUBTREE,
> + expression="(objectClass=dnsNode)",
> + attrs=["dnsRecord"])
> +
> + for old_packed_record in records:
> + if record_name in str(old_packed_record.dn):
> + return (old_packed_record.dn, ndr_unpack(dnsp.DnssrvRpcRecord, old_packed_record["dnsRecord"][0]))
> +
> + def test_duplicate_matching(self):
> + """
> + Make sure that records which should be distinct from each other or duplicate
> + to each other behave as expected.
> + """
> +
> + distinct_dns = [("SAMDOM.EXAMPLE.COM",
> + "SAMDOM.EXAMPLE.CO",
> + "EXAMPLE.COM", "SAMDOM.EXAMPLE")]
> + duplicate_dns = [("SAMDOM.EXAMPLE.COM", "samdom.example.com", "SAMDOM.example.COM"),
> + ("EXAMPLE.", "EXAMPLE")]
> +
> + # Every tuple has entries which should be considered duplicate to one another.
> + duplicates = {
> + "AAAA": [("AAAA::", "aaaa::"),
> + ("AAAA::", "AAAA:0000::"),
> + ("AAAA::", "AAAA:0000:0000:0000:0000:0000:0000:0000"),
> + ("AAAA::", "AAAA:0:0:0:0:0:0:0"),
> + ("0123::", "123::"),
> + ("::", "::0", "0000:0000:0000:0000:0000:0000:0000:0000")],
> + }
> +
> + # Every tuple has entries which should be considered distinct from one another.
> + distinct = {
> + "A": [("192.168.1.0", "192.168.1.1", "192.168.2.0", "192.169.1.0", "193.168.1.0")],
> + "AAAA": [("AAAA::1234:5678:9ABC", "::AAAA:1234:5678:9ABC"),
> + ("1000::", "::1000"),
> + ("::1", "::11", "::1111"),
> + ("1234::", "0234::")],
> + "SRV": [("SAMDOM.EXAMPLE.COM 1 1 1", "SAMDOM.EXAMPLE.COM 1 1 0", "SAMDOM.EXAMPLE.COM 1 0 1",
> + "SAMDOM.EXAMPLE.COM 0 1 1", "SAMDOM.EXAMPLE.COM 2 1 0", "SAMDOM.EXAMPLE.COM 2 2 2")],
> + "MX": [("SAMDOM.EXAMPLE.COM 1", "SAMDOM.EXAMPLE.COM 0")],
> + "TXT": [("A RECORD", "B RECORD", "a record")]
> + }
> +
> + for record_type_str in ("PTR", "CNAME", "NS"):
> + distinct[record_type_str] = distinct_dns
> + duplicates[record_type_str] = duplicate_dns
> +
> + for record_type_str in duplicates:
> + for duplicate_tuple in duplicates[record_type_str]:
> + # Attempt to add duplicates and make sure that all after the first fails
> + self.add_record(self.custom_zone, "testrecord", record_type_str, duplicate_tuple[0])
> + for record in duplicate_tuple:
> + self.add_record(self.custom_zone, "testrecord", record_type_str, record, assertion=False)
> + self.assert_num_records(self.custom_zone, "testrecord", record_type_str)
> + self.delete_record(self.custom_zone, "testrecord", record_type_str, duplicate_tuple[0])
> +
> + # Repeatedly: add the first duplicate, and attempt to remove all of the others, making sure this succeeds
> + for record in duplicate_tuple:
> + self.add_record(self.custom_zone, "testrecord", record_type_str, duplicate_tuple[0])
> + self.delete_record(self.custom_zone, "testrecord", record_type_str, record)
> +
> + for record_type_str in distinct:
> + for distinct_tuple in distinct[record_type_str]:
> + # Attempt to add distinct and make sure that they all succeed within a tuple
> + i = 0
> + for record in distinct_tuple:
> + i = i + 1
> + try:
> + self.add_record(self.custom_zone, "testrecord", record_type_str, record)
> + # All records should have been added.
> + self.assert_num_records(self.custom_zone, "testrecord", record_type_str, expected_num=i)
> + except AssertionError as e:
> + raise AssertionError("Failed to add %s, which should be distinct from all others in the set. "
> + "Original error: %s\nDistinct set: %s." % (record, e, distinct_tuple))
> + for record in distinct_tuple:
> + self.delete_record(self.custom_zone, "testrecord", record_type_str, record)
> + # CNAMEs should not have been added, since they conflict.
> + if record_type_str == 'CNAME':
> + continue
> +
> + # Add the first distinct and attempt to remove all of the others, making sure this fails
> + # Windows fails this test. This is probably due to weird tombstoning behavior.
> + self.add_record(self.custom_zone, "testrecord", record_type_str, distinct_tuple[0])
> + for record in distinct_tuple:
> + if record == distinct_tuple[0]:
> + continue
> + try:
> + self.delete_record(self.custom_zone, "testrecord", record_type_str, record, assertion=False)
> + except AssertionError as e:
> + raise AssertionError("Managed to remove %s by attempting to remove %s. Original error: %s"
> + % (distinct_tuple[0], record, e))
> + self.delete_record(self.custom_zone, "testrecord", record_type_str, distinct_tuple[0])
> +
> + def test_accept_valid_commands(self):
> + """
> + Make sure that we can add, update and delete a variety
> + of valid records.
> + """
> + for record_type_str in self.good_records:
> + for record_str in self.good_records[record_type_str]:
> + self.add_record(self.custom_zone, "testrecord", record_type_str, record_str)
> + self.assert_num_records(self.custom_zone, "testrecord", record_type_str)
> + self.delete_record(self.custom_zone, "testrecord", record_type_str, record_str)
> +
> + def test_reject_invalid_commands(self):
> + """
> + Make sure that we can't add a variety of invalid records,
> + and that we can't update valid records to invalid ones.
> + """
> + num_failures = 0
> + for record_type_str in self.bad_records:
> + for record_str in self.bad_records[record_type_str]:
> + # Attempt to add the bad record, which should fail. Then, attempt to query for and delete
> + # it. Since it shouldn't exist, these should fail too.
> + try:
> + self.add_record(self.custom_zone, "testrecord", record_type_str, record_str, assertion=False)
> + self.assert_num_records(self.custom_zone, "testrecord", record_type_str, expected_num=0)
> + self.delete_record(self.custom_zone, "testrecord", record_type_str, record_str, assertion=False)
> + except AssertionError as e:
> + print e
> + num_failures = num_failures + 1
> +
> + # Also try to update valid records to invalid ones, making sure this fails
> + for record_type_str in self.bad_records:
> + for record_str in self.bad_records[record_type_str]:
> + good_record_str = self.good_records[record_type_str][0]
> + self.add_record(self.custom_zone, "testrecord", record_type_str, good_record_str)
> + try:
> + self.add_record(self.custom_zone, "testrecord", record_type_str, record_str, assertion=False)
> + except AssertionError as e:
> + print e
> + num_failures = num_failures + 1
> + self.delete_record(self.custom_zone, "testrecord", record_type_str, good_record_str)
> +
> + self.assertTrue(num_failures == 0, "Failed to reject invalid commands. Total failures: %d." % num_failures)
> +
> + def test_add_duplicate_different_type(self):
> + """
> + Attempt to add some values which have the same name as
> + existing ones, just a different type.
> + """
> + num_failures = 0
> + for record_type_str_1 in self.good_records:
> + record1 = self.good_records[record_type_str_1][0]
> + self.add_record(self.custom_zone, "testrecord", record_type_str_1, record1)
> + for record_type_str_2 in self.good_records:
> + if record_type_str_1 == record_type_str_2:
> + continue
> +
> + record2 = self.good_records[record_type_str_2][0]
> +
> + has_a = record_type_str_1 == 'A' or record_type_str_2 == 'A'
> + has_aaaa = record_type_str_1 == 'AAAA' or record_type_str_2 == 'AAAA'
> + has_cname = record_type_str_1 == 'CNAME' or record_type_str_2 == 'CNAME'
> + has_ptr = record_type_str_1 == 'PTR' or record_type_str_2 == 'PTR'
> + has_mx = record_type_str_1 == 'MX' or record_type_str_2 == 'MX'
> + has_srv = record_type_str_1 == 'SRV' or record_type_str_2 == 'SRV'
> + has_txt = record_type_str_1 == 'TXT' or record_type_str_2 == 'TXT'
> +
> + # If we attempt to add any record except A or AAAA when we already have an NS record,
> + # the add should fail.
> + add_error_ok = False
> + if record_type_str_1 == 'NS' and not has_a and not has_aaaa:
> + add_error_ok = True
> + # If we attempt to add a CNAME when an A, PTR or MX record exists, the add should fail.
> + if record_type_str_2 == 'CNAME' and (has_ptr or has_mx or has_a or has_aaaa):
> + add_error_ok = True
> + # If we have a CNAME, adding an A, AAAA, SRV or TXT record should fail.
> + # If we have an A, AAAA, SRV or TXT record, adding a CNAME should fail.
> + if has_cname and (has_a or has_aaaa or has_srv or has_txt):
> + add_error_ok = True
> +
> + try:
> + self.add_record(self.custom_zone, "testrecord", record_type_str_2, record2)
> + if add_error_ok:
> + num_failures = num_failures + 1
> + print("Expected error when adding %s while a %s existed."
> + % (record_type_str_2, record_type_str_1))
> + except AssertionError as e:
> + if not add_error_ok:
> + num_failures = num_failures + 1
> + print("Didn't expect error when adding %s while a %s existed."
> + % (record_type_str_2, record_type_str_1))
> +
> + if not add_error_ok:
> + # In the "normal" case, we expect the add to work and us to have one of each type of record afterwards.
> + expected_num_type_1 = 1
> + expected_num_type_2 = 1
> +
> + # If we have an MX record, a PTR record should replace it when added.
> + # If we have a PTR record, an MX record should replace it when added.
> + if has_ptr and has_mx:
> + expected_num_type_1 = 0
> +
> + # If we have a CNAME, SRV or TXT record, a PTR or MX record should replace it when added.
> + if (has_cname or has_srv or has_txt) and (record_type_str_2 == 'PTR' or record_type_str_2 == 'MX'):
> + expected_num_type_1 = 0
> +
> + if (record_type_str_1 == 'NS' and (has_a or has_aaaa)):
> + expected_num_type_2 = 0
> +
> + try:
> + self.assert_num_records(self.custom_zone, "testrecord", record_type_str_1, expected_num=expected_num_type_1)
> + except AssertionError as e:
> + num_failures = num_failures + 1
> + print("Expected %s %s records after adding a %s record and a %s record already existed."
> + % (expected_num_type_1, record_type_str_1, record_type_str_2, record_type_str_1))
> + try:
> + self.assert_num_records(self.custom_zone, "testrecord", record_type_str_2, expected_num=expected_num_type_2)
> + except AssertionError as e:
> + num_failures = num_failures + 1
> + print("Expected %s %s records after adding a %s record and a %s record already existed."
> + % (expected_num_type_2, record_type_str_2, record_type_str_2, record_type_str_1))
> +
> + try:
> + self.delete_record(self.custom_zone, "testrecord", record_type_str_2, record2)
> + except AssertionError as e:
> + pass
> +
> + self.delete_record(self.custom_zone, "testrecord", record_type_str_1, record1)
> +
> + self.assertTrue(num_failures == 0, "Failed collision and replacement behavior. Total failures: %d." % num_failures)
> +
> + # Windows fails this test in the same way we do.
> + def _test_cname(self):
> + """
> + Test some special properties of CNAME records.
> + """
> +
> + # RFC 1912: When there is a CNAME record, there must not be any other records with the same alias
> + cname_record = self.good_records["CNAME"][1]
> + self.add_record(self.custom_zone, "testrecord", "CNAME", cname_record)
> +
> + for record_type_str in self.good_records:
> + other_record = self.good_records[record_type_str][0]
> + self.add_record(self.custom_zone, "testrecord", record_type_str, other_record, assertion=False)
> + self.assert_num_records(self.custom_zone, "testrecord", record_type_str, expected_num=0)
> +
> + # RFC 2181: MX & NS records must not be allowed to point to a CNAME alias
> + mx_record = "testrecord 1"
> + ns_record = "testrecord"
> +
> + self.add_record(self.custom_zone, "mxrec", "MX", mx_record, assertion=False)
> + self.add_record(self.custom_zone, "nsrec", "NS", ns_record, assertion=False)
> +
> + self.delete_record(self.custom_zone, "testrecord", "CNAME", cname_record)
> +
> + def test_add_duplicate_value(self):
> + """
> + Make sure that we can't add duplicate values of any type.
> + """
> + for record_type_str in self.good_records:
> + record = self.good_records[record_type_str][0]
> +
> + self.add_record(self.custom_zone, "testrecord", record_type_str, record)
> + self.add_record(self.custom_zone, "testrecord", record_type_str, record, assertion=False)
> + self.assert_num_records(self.custom_zone, "testrecord", record_type_str)
> + self.delete_record(self.custom_zone, "testrecord", record_type_str, record)
> +
> + def test_add_similar_value(self):
> + """
> + Attempt to add values with the same name and type in the same
> + zone. This should work, and should result in both values
> + existing (except with some types).
> + """
> + for record_type_str in self.good_records:
> + for i in range(1, len(self.good_records[record_type_str])):
> + record1 = self.good_records[record_type_str][i-1]
> + record2 = self.good_records[record_type_str][i]
> +
> + if record_type_str == 'CNAME':
> + continue
> + # We expect CNAME records to override one another, as
> + # an alias can only map to one CNAME record.
> + # Also, on Windows, when the empty string is added and
> + # another record is added afterwards, the empty string
> + # will be silently overriden by the new one, so it
> + # fails this test for the empty string.
> + expected_num = 1 if record_type_str == 'CNAME' else 2
> +
> + self.add_record(self.custom_zone, "testrecord", record_type_str, record1)
> + self.add_record(self.custom_zone, "testrecord", record_type_str, record2)
> + self.assert_num_records(self.custom_zone, "testrecord", record_type_str, expected_num=expected_num)
> + self.delete_record(self.custom_zone, "testrecord", record_type_str, record1)
> + self.delete_record(self.custom_zone, "testrecord", record_type_str, record2)
> +
> + def assert_record(self, zone, name, record_type_str, expected_record_str,
> + assertion=True, client_version=dnsserver.DNS_CLIENT_VERSION_LONGHORN):
> + """
> + Asserts whether or not the given record with the given type exists in the
> + given zone.
> + """
> + try:
> + _, result = self.query_records(zone, name, record_type_str)
> + except RuntimeError as e:
> + if assertion:
> + raise AssertionError("Record '%s' of type '%s' was not present when it should have been."
> + % (expected_record_str, record_type_str))
> + else:
> + return
> +
> + found = False
> + for record in result.rec[0].records:
> + if record.data == expected_record_str:
> + found = True
> + break
> +
> + if found and not assertion:
> + raise AssertionError("Record '%s' of type '%s' was present when it shouldn't have been." % (expected_record_str, record_type_str))
> + elif not found and assertion:
> + raise AssertionError("Record '%s' of type '%s' was not present when it should have been." % (expected_record_str, record_type_str))
> +
> + def assert_num_records(self, zone, name, record_type_str, expected_num=1,
> + client_version=dnsserver.DNS_CLIENT_VERSION_LONGHORN):
> + """
> + Asserts that there are a given amount of records with the given type in
> + the given zone.
> + """
> + try:
> + _, result = self.query_records(zone, name, record_type_str)
> + num_results = len(result.rec[0].records)
> + if not num_results == expected_num:
> + raise AssertionError("There were %d records of type '%s' with the name '%s' when %d were expected."
> + % (num_results, record_type_str, name, expected_num))
> + except RuntimeError:
> + if not expected_num == 0:
> + raise AssertionError("There were no records of type '%s' with the name '%s' when %d were expected."
> + % (record_type_str, name, expected_num))
> +
> + def query_records(self, zone, name, record_type_str, client_version=dnsserver.DNS_CLIENT_VERSION_LONGHORN):
> + return self.conn.DnssrvEnumRecords2(client_version,
> + 0,
> + self.server,
> + zone,
> + name,
> + None,
> + self.record_type_int(record_type_str),
> + dnsserver.DNS_RPC_VIEW_AUTHORITY_DATA | dnsserver.DNS_RPC_VIEW_NO_CHILDREN,
> + None,
> + None)
> +
> + def record_obj_from_str(self, record_type_str, record_str):
> + if record_type_str == 'A':
> + return ARecord(record_str)
> + elif record_type_str == 'AAAA':
> + return AAAARecord(record_str)
> + elif record_type_str == 'PTR':
> + return PTRRecord(record_str)
> + elif record_type_str == 'CNAME':
> + return CNameRecord(record_str)
> + elif record_type_str == 'NS':
> + return NSRecord(record_str)
> + elif record_type_str == 'MX':
> + split = record_str.split(' ')
> + return MXRecord(split[0], int(split[1]))
> + elif record_type_str == 'SRV':
> + split = record_str.split(' ')
> + target = split[0]
> + port = int(split[1])
> + priority = int(split[2])
> + weight = int(split[3])
> + return SRVRecord(target, port, priority, weight)
> + elif record_type_str == 'TXT':
> + return TXTRecord(record_str)
> +
> + def record_type_int(self, record_type_str):
> + if record_type_str == 'A':
> + return dnsp.DNS_TYPE_A
> + elif record_type_str == 'AAAA':
> + return dnsp.DNS_TYPE_AAAA
> + elif record_type_str == 'PTR':
> + return dnsp.DNS_TYPE_PTR
> + elif record_type_str == 'CNAME':
> + return dnsp.DNS_TYPE_CNAME
> + elif record_type_str == 'NS':
> + return dnsp.DNS_TYPE_NS
> + elif record_type_str == 'MX':
> + return dnsp.DNS_TYPE_MX
> + elif record_type_str == 'SRV':
> + return dnsp.DNS_TYPE_SRV
> + elif record_type_str == 'TXT':
> + return dnsp.DNS_TYPE_TXT
> +
> + def add_record(self, zone, name, record_type_str, record_str,
> + assertion=True, client_version=dnsserver.DNS_CLIENT_VERSION_LONGHORN):
> + """
> + Attempts to add a map from the given name to a record of the given type,
> + in the given zone.
> + Also asserts whether or not the add was successful.
> + This can also update existing records if they have the same name.
> + """
> + record = self.record_obj_from_str(record_type_str, record_str)
> + add_rec_buf = dnsserver.DNS_RPC_RECORD_BUF()
> + add_rec_buf.rec = record
> +
> + try:
> + self.conn.DnssrvUpdateRecord2(client_version,
> + 0,
> + self.server,
> + zone,
> + name,
> + add_rec_buf,
> + None)
> + if not assertion:
> + raise AssertionError("Successfully added record '%s' of type '%s', which should have failed."
> + % (record_str, record_type_str))
> + except RuntimeError as e:
> + if assertion:
> + raise AssertionError("Failed to add record '%s' of type '%s', which should have succeeded. Error was '%s'."
> + % (record_str, record_type_str, str(e)))
> +
> + def delete_record(self, zone, name, record_type_str, record_str,
> + assertion=True, client_version=dnsserver.DNS_CLIENT_VERSION_LONGHORN):
> + """
> + Attempts to delete a record with the given name, record and record type
> + from the given zone.
> + Also asserts whether or not the deletion was successful.
> + """
> + record = self.record_obj_from_str(record_type_str, record_str)
> + del_rec_buf = dnsserver.DNS_RPC_RECORD_BUF()
> + del_rec_buf.rec = record
> +
> + try:
> + self.conn.DnssrvUpdateRecord2(client_version,
> + 0,
> + self.server,
> + zone,
> + name,
> + None,
> + del_rec_buf)
> + if not assertion:
> + raise AssertionError("Successfully deleted record '%s' of type '%s', which should have failed." % (record_str, record_type_str))
> + except RuntimeError as e:
> + if assertion:
> + raise AssertionError("Failed to delete record '%s' of type '%s', which should have succeeded. Error was '%s'." % (record_str, record_type_str, str(e)))
> +
> def test_query2(self):
> typeid, result = self.conn.DnssrvQuery2(dnsserver.DNS_CLIENT_VERSION_W2K,
> 0,
> @@ -76,13 +698,13 @@ class DnsserverTests(RpcInterfaceTestCase):
>
> request_filter = (dnsserver.DNS_ZONE_REQUEST_REVERSE |
> dnsserver.DNS_ZONE_REQUEST_PRIMARY)
> - typeid, zones = self.conn.DnssrvComplexOperation2(client_version,
> - 0,
> - self.server,
> - None,
> - 'EnumZones',
> - dnsserver.DNSSRV_TYPEID_DWORD,
> - request_filter)
> + _, zones = self.conn.DnssrvComplexOperation2(client_version,
> + 0,
> + self.server,
> + None,
> + 'EnumZones',
> + dnsserver.DNSSRV_TYPEID_DWORD,
> + request_filter)
> self.assertEquals(1, zones.dwZoneCount)
>
> # Delete zone
> @@ -109,6 +731,7 @@ class DnsserverTests(RpcInterfaceTestCase):
> client_version = dnsserver.DNS_CLIENT_VERSION_LONGHORN
> request_filter = (dnsserver.DNS_ZONE_REQUEST_FORWARD |
> dnsserver.DNS_ZONE_REQUEST_PRIMARY)
> +
> typeid, zones = self.conn.DnssrvComplexOperation2(client_version,
> 0,
> self.server,
> @@ -117,7 +740,7 @@ class DnsserverTests(RpcInterfaceTestCase):
> dnsserver.DNSSRV_TYPEID_DWORD,
> request_filter)
> self.assertEquals(dnsserver.DNSSRV_TYPEID_ZONE_LIST, typeid)
> - self.assertEquals(2, zones.dwZoneCount)
> + self.assertEquals(3, zones.dwZoneCount)
>
> request_filter = (dnsserver.DNS_ZONE_REQUEST_REVERSE |
> dnsserver.DNS_ZONE_REQUEST_PRIMARY)
> @@ -131,25 +754,23 @@ class DnsserverTests(RpcInterfaceTestCase):
> self.assertEquals(dnsserver.DNSSRV_TYPEID_ZONE_LIST, typeid)
> self.assertEquals(0, zones.dwZoneCount)
>
> -
> def test_enumrecords2(self):
> client_version = dnsserver.DNS_CLIENT_VERSION_LONGHORN
> record_type = dnsp.DNS_TYPE_NS
> select_flags = (dnsserver.DNS_RPC_VIEW_ROOT_HINT_DATA |
> dnsserver.DNS_RPC_VIEW_ADDITIONAL_DATA)
> - buflen, roothints = self.conn.DnssrvEnumRecords2(client_version,
> - 0,
> - self.server,
> - '..RootHints',
> - '.',
> - None,
> - record_type,
> - select_flags,
> - None,
> - None)
> + _, roothints = self.conn.DnssrvEnumRecords2(client_version,
> + 0,
> + self.server,
> + '..RootHints',
> + '.',
> + None,
> + record_type,
> + select_flags,
> + None,
> + None)
> self.assertEquals(14, roothints.count) # 1 NS + 13 A records (a-m)
>
> -
> def test_updaterecords2(self):
> client_version = dnsserver.DNS_CLIENT_VERSION_LONGHORN
> record_type = dnsp.DNS_TYPE_A
> @@ -170,16 +791,16 @@ class DnsserverTests(RpcInterfaceTestCase):
> add_rec_buf,
> None)
>
> - buflen, result = self.conn.DnssrvEnumRecords2(client_version,
> - 0,
> - self.server,
> - self.zone,
> - name,
> - None,
> - record_type,
> - select_flags,
> - None,
> - None)
> + _, result = self.conn.DnssrvEnumRecords2(client_version,
> + 0,
> + self.server,
> + self.zone,
> + name,
> + None,
> + record_type,
> + select_flags,
> + None,
> + None)
> self.assertEquals(1, result.count)
> self.assertEquals(1, result.rec[0].wRecordCount)
> self.assertEquals(dnsp.DNS_TYPE_A, result.rec[0].records[0].wType)
> @@ -235,42 +856,3 @@ class DnsserverTests(RpcInterfaceTestCase):
> select_flags,
> None,
> None)
> -
> - def test_updaterecords2_soa(self):
> - client_version = dnsserver.DNS_CLIENT_VERSION_LONGHORN
> - record_type = dnsp.DNS_TYPE_NS
> - select_flags = (dnsserver.DNS_RPC_VIEW_AUTHORITY_DATA |
> - dnsserver.DNS_RPC_VIEW_NO_CHILDREN)
> -
> - nameserver = 'ns.example.local'
> - rec = NSRecord(nameserver)
> -
> - # Add record
> - add_rec_buf = dnsserver.DNS_RPC_RECORD_BUF()
> - add_rec_buf.rec = rec
> - self.conn.DnssrvUpdateRecord2(client_version,
> - 0,
> - self.server,
> - self.zone,
> - '.',
> - add_rec_buf,
> - None)
> -
> - buflen, result = self.conn.DnssrvEnumRecords2(client_version,
> - 0,
> - self.server,
> - self.zone,
> - '@',
> - None,
> - record_type,
> - select_flags,
> - None,
> - None)
> - self.assertEquals(1, result.count)
> - self.assertEquals(2, result.rec[0].wRecordCount)
> - match = False
> - for i in range(2):
> - self.assertEquals(dnsp.DNS_TYPE_NS, result.rec[0].records[i].wType)
> - if result.rec[0].records[i].data.str.rstrip('.') == nameserver:
> - match = True
> - self.assertEquals(match, True)
> diff --git a/selftest/knownfail b/selftest/knownfail
> index 80974bb..7c4e093 100644
> --- a/selftest/knownfail
> +++ b/selftest/knownfail
> @@ -310,3 +310,6 @@
> ^samba.tests.samba_tool.dnscmd.samba.tests.samba_tool.dnscmd.DnsCmdTestCase.test_rank_none
> ^samba.tests.samba_tool.dnscmd.samba.tests.samba_tool.dnscmd.DnsCmdTestCase.test_reject_invalid_commands
> ^samba.tests.samba_tool.dnscmd.samba.tests.samba_tool.dnscmd.DnsCmdTestCase.test_update_valid_type
> +^samba.tests.dcerpc.dnsserver.samba.tests.dcerpc.dnsserver.DnsserverTests.test_add_duplicate_different_type.*
> +^samba.tests.dcerpc.dnsserver.samba.tests.dcerpc.dnsserver.DnsserverTests.test_rank_none.*
> +^samba.tests.dcerpc.dnsserver.samba.tests.dcerpc.dnsserver.DnsserverTests.test_reject_invalid_commands
> --
> 1.9.1
>
>
> From 97eb8d94c051ec156a4c273b54e24ad183e73830 Mon Sep 17 00:00:00 2001
> From: Bob Campbell <bobcampbell at catalyst.net.nz>
> Date: Fri, 25 Nov 2016 16:29:31 +1300
> Subject: [PATCH 04/12] samba-tool/dns: reword error messages and make error
> catching specific
>
> Signed-off-by: Bob Campbell <bobcampbell at catalyst.net.nz>
> Reviewed-by: Garming Sam <garming at catalyst.net.nz>
> ---
> python/samba/netcmd/dns.py | 105 +++++++++++++++++++++++++++++++--------------
> 1 file changed, 72 insertions(+), 33 deletions(-)
>
> diff --git a/python/samba/netcmd/dns.py b/python/samba/netcmd/dns.py
> index 7cedffc..f0d98a7 100644
> --- a/python/samba/netcmd/dns.py
> +++ b/python/samba/netcmd/dns.py
> @@ -17,6 +17,7 @@
> #
>
> import samba.getopt as options
> +from samba import WERRORError
> from struct import pack
> from socket import inet_ntoa
> from socket import inet_ntop
> @@ -622,13 +623,16 @@ def dns_record_match(dns_conn, server, zone, name, record_type, data):
> buflen, res = dns_conn.DnssrvEnumRecords2(
> dnsserver.DNS_CLIENT_VERSION_LONGHORN, 0, server, zone, name, None,
> record_type, select_flags, None, None)
> - except RuntimeError, e:
> - return None
> + except WERRORError as e:
> + if e.args[1] == 'WERR_DNS_ERROR_NAME_DOES_NOT_EXIST':
> + # Either the zone doesn't exist, or there were no records.
> + # We can't differentiate the two.
> + return None
> + raise e
>
> if not res or res.count == 0:
> return None
>
> - rec_match = None
> for rec in res.rec[0].records:
> if rec.wType != record_type:
> continue
> @@ -678,10 +682,9 @@ def dns_record_match(dns_conn, server, zone, name, record_type, data):
> (rec.data.str[i].str == urec.data.str[i].str)
>
> if found:
> - rec_match = rec
> - break
> + return rec
>
> - return rec_match
> + return None
>
>
> class cmd_serverinfo(Command):
> @@ -892,9 +895,15 @@ class cmd_zonecreate(Command):
> name_and_param.pszNodeName = 'AllowUpdate'
> name_and_param.dwParam = dnsp.DNS_ZONE_UPDATE_SECURE
>
> - res = dns_conn.DnssrvOperation2(client_version, 0, server, zone,
> - 0, 'ResetDwordProperty', typeid,
> - name_and_param)
> + try:
> + res = dns_conn.DnssrvOperation2(client_version, 0, server, zone,
> + 0, 'ResetDwordProperty', typeid,
> + name_and_param)
> + except WERRORError as e:
> + if e.args[1] == 'WERR_DNS_ERROR_ZONE_ALREADY_EXISTS':
> + self.outf.write('Zone already exists.')
> + raise e
> +
> self.outf.write('Zone %s created successfully\n' % zone)
>
>
> @@ -919,11 +928,17 @@ class cmd_zonedelete(Command):
> dns_conn = dns_connect(server, self.lp, self.creds)
>
> zone = zone.lower()
> - res = dns_conn.DnssrvOperation2(dnsserver.DNS_CLIENT_VERSION_LONGHORN,
> - 0, server, zone, 0, 'DeleteZoneFromDs',
> - dnsserver.DNSSRV_TYPEID_NULL,
> - None)
> - self.outf.write('Zone %s delete successfully\n' % zone)
> + try:
> + res = dns_conn.DnssrvOperation2(dnsserver.DNS_CLIENT_VERSION_LONGHORN,
> + 0, server, zone, 0, 'DeleteZoneFromDs',
> + dnsserver.DNSSRV_TYPEID_NULL,
> + None)
> + except WERRORError as e:
> + if e.args[1] == 'WERR_DNS_ERROR_ZONE_DOES_NOT_EXIST':
> + self.outf.write('Zone does not exist and so could not be deleted.')
> + raise e
> +
> + self.outf.write('Zone %s deleted successfully\n' % zone)
>
>
> class cmd_query(Command):
> @@ -993,9 +1008,15 @@ class cmd_query(Command):
> self.creds = credopts.get_credentials(self.lp)
> dns_conn = dns_connect(server, self.lp, self.creds)
>
> - buflen, res = dns_conn.DnssrvEnumRecords2(
> + try:
> + buflen, res = dns_conn.DnssrvEnumRecords2(
> dnsserver.DNS_CLIENT_VERSION_LONGHORN, 0, server, zone, name,
> None, record_type, select_flags, None, None)
> + except WERRORError as e:
> + if e.args[1] == 'WERR_DNS_ERROR_NAME_DOES_NOT_EXIST':
> + self.outf.write('Record or zone does not exist.')
> + raise e
> +
> print_dnsrecords(self.outf, res)
>
>
> @@ -1073,8 +1094,14 @@ class cmd_add_record(Command):
> add_rec_buf = dnsserver.DNS_RPC_RECORD_BUF()
> add_rec_buf.rec = rec
>
> - dns_conn.DnssrvUpdateRecord2(dnsserver.DNS_CLIENT_VERSION_LONGHORN,
> - 0, server, zone, name, add_rec_buf, None)
> + try:
> + dns_conn.DnssrvUpdateRecord2(dnsserver.DNS_CLIENT_VERSION_LONGHORN,
> + 0, server, zone, name, add_rec_buf, None)
> + except WERRORError as e:
> + if e.args[1] == 'WERR_DNS_ERROR_NAME_DOES_NOT_EXIST':
> + self.outf.write('Zone does not exist; record could not be added.\n')
> + raise e
> +
> self.outf.write('Record added successfully\n')
>
>
> @@ -1119,7 +1146,7 @@ class cmd_update_record(Command):
> rec_match = dns_record_match(dns_conn, server, zone, name, record_type,
> olddata)
> if not rec_match:
> - raise CommandError('Record does not exist')
> + raise CommandError('Record or zone does not exist.')
>
> # Copy properties from existing record to new record
> rec.dwFlags = rec_match.dwFlags
> @@ -1133,13 +1160,19 @@ class cmd_update_record(Command):
> del_rec_buf = dnsserver.DNS_RPC_RECORD_BUF()
> del_rec_buf.rec = rec_match
>
> - dns_conn.DnssrvUpdateRecord2(dnsserver.DNS_CLIENT_VERSION_LONGHORN,
> - 0,
> - server,
> - zone,
> - name,
> - add_rec_buf,
> - del_rec_buf)
> + try:
> + dns_conn.DnssrvUpdateRecord2(dnsserver.DNS_CLIENT_VERSION_LONGHORN,
> + 0,
> + server,
> + zone,
> + name,
> + add_rec_buf,
> + del_rec_buf)
> + except WERRORError as e:
> + if e.args[1] == 'WERR_DNS_ERROR_NAME_DOES_NOT_EXIST':
> + self.outf.write('Zone does not exist; record could not be updated.\n')
> + raise e
> +
> self.outf.write('Record updated successfully\n')
>
>
> @@ -1180,18 +1213,24 @@ class cmd_delete_record(Command):
>
> rec_match = dns_record_match(dns_conn, server, zone, name, record_type, data)
> if not rec_match:
> - raise CommandError('Record does not exist')
> + raise CommandError('Record or zone does not exist.')
>
> del_rec_buf = dnsserver.DNS_RPC_RECORD_BUF()
> del_rec_buf.rec = rec_match
>
> - dns_conn.DnssrvUpdateRecord2(dnsserver.DNS_CLIENT_VERSION_LONGHORN,
> - 0,
> - server,
> - zone,
> - name,
> - None,
> - del_rec_buf)
> + try:
> + dns_conn.DnssrvUpdateRecord2(dnsserver.DNS_CLIENT_VERSION_LONGHORN,
> + 0,
> + server,
> + zone,
> + name,
> + None,
> + del_rec_buf)
> + except WERRORError as e:
> + if e.args[1] == 'WERR_DNS_ERROR_NAME_DOES_NOT_EXIST':
> + self.outf.write('Zone does not exist; record could not be deleted.\n')
> + raise e
> +
> self.outf.write('Record deleted successfully\n')
>
>
> --
> 1.9.1
>
>
> From 412c6812db0629e09aa8571470245fcdd679a10a Mon Sep 17 00:00:00 2001
> From: Bob Campbell <bobcampbell at catalyst.net.nz>
> Date: Mon, 28 Nov 2016 11:12:18 +1300
> Subject: [PATCH 05/12] samba-tool/dns: remove use of dns_record_match from add
> and delete
>
> Signed-off-by: Bob Campbell <bobcampbell at catalyst.net.nz>
> Reviewed-by: Garming Sam <garming at catalyst.net.nz>
> ---
> python/samba/netcmd/dns.py | 12 ++----------
> selftest/knownfail | 5 +----
> 2 files changed, 3 insertions(+), 14 deletions(-)
>
> diff --git a/python/samba/netcmd/dns.py b/python/samba/netcmd/dns.py
> index f0d98a7..fe5a293 100644
> --- a/python/samba/netcmd/dns.py
> +++ b/python/samba/netcmd/dns.py
> @@ -1086,11 +1086,6 @@ class cmd_add_record(Command):
> self.creds = credopts.get_credentials(self.lp)
> dns_conn = dns_connect(server, self.lp, self.creds)
>
> - rec_match = dns_record_match(dns_conn, server, zone, name, record_type,
> - data)
> - if rec_match is not None:
> - raise CommandError('Record already exists')
> -
> add_rec_buf = dnsserver.DNS_RPC_RECORD_BUF()
> add_rec_buf.rec = rec
>
> @@ -1206,17 +1201,14 @@ class cmd_delete_record(Command):
> raise CommandError('Deleting record of type %s is not supported' % rtype)
>
> record_type = dns_type_flag(rtype)
> + rec = data_to_dns_record(record_type, data)
>
> self.lp = sambaopts.get_loadparm()
> self.creds = credopts.get_credentials(self.lp)
> dns_conn = dns_connect(server, self.lp, self.creds)
>
> - rec_match = dns_record_match(dns_conn, server, zone, name, record_type, data)
> - if not rec_match:
> - raise CommandError('Record or zone does not exist.')
> -
> del_rec_buf = dnsserver.DNS_RPC_RECORD_BUF()
> - del_rec_buf.rec = rec_match
> + del_rec_buf.rec = rec
>
> try:
> dns_conn.DnssrvUpdateRecord2(dnsserver.DNS_CLIENT_VERSION_LONGHORN,
> diff --git a/selftest/knownfail b/selftest/knownfail
> index 7c4e093..6fa60a3 100644
> --- a/selftest/knownfail
> +++ b/selftest/knownfail
> @@ -306,10 +306,7 @@
> ^samba4.rpc.echo.*on.*with.object.echo.sinkdata.*nt4_dc
> ^samba4.rpc.echo.*on.*with.object.echo.addone.*nt4_dc
> ^samba4.rpc.echo.*on.*ncacn_ip_tcp.*with.object.*nt4_dc
> -^samba.tests.samba_tool.dnscmd.samba.tests.samba_tool.dnscmd.DnsCmdTestCase.test_accept_valid_commands
> -^samba.tests.samba_tool.dnscmd.samba.tests.samba_tool.dnscmd.DnsCmdTestCase.test_rank_none
> -^samba.tests.samba_tool.dnscmd.samba.tests.samba_tool.dnscmd.DnsCmdTestCase.test_reject_invalid_commands
> -^samba.tests.samba_tool.dnscmd.samba.tests.samba_tool.dnscmd.DnsCmdTestCase.test_update_valid_type
> ^samba.tests.dcerpc.dnsserver.samba.tests.dcerpc.dnsserver.DnsserverTests.test_add_duplicate_different_type.*
> ^samba.tests.dcerpc.dnsserver.samba.tests.dcerpc.dnsserver.DnsserverTests.test_rank_none.*
> ^samba.tests.dcerpc.dnsserver.samba.tests.dcerpc.dnsserver.DnsserverTests.test_reject_invalid_commands
> +^samba.tests.samba_tool.dnscmd.samba.tests.samba_tool.dnscmd.DnsCmdTestCase.test_reject_invalid_commands
> --
> 1.9.1
>
>
> From cc0b4120cedf04044fe44ca2120cbd008e7c9506 Mon Sep 17 00:00:00 2001
> From: Garming Sam <garming at catalyst.net.nz>
> Date: Tue, 6 Dec 2016 11:00:17 +1300
> Subject: [PATCH 06/12] tests/dnsserver: Check security descriptors
>
> These tests discover that there are some discrepancies between Windows and Samba.
> Although there are failures, they do not appear to be critical, however
> some of the SD differences will be important for 2012 support.
>
> Signed-off-by: Garming Sam <garming at catalyst.net.nz>
> Peer-programmed-with: Bob Campbell <bobcampbell at catalyst.net.nz>
> ---
> python/samba/tests/dcerpc/dnsserver.py | 203 ++++++++++++++++++++++++++++++++-
> selftest/knownfail | 1 +
> 2 files changed, 203 insertions(+), 1 deletion(-)
>
> diff --git a/python/samba/tests/dcerpc/dnsserver.py b/python/samba/tests/dcerpc/dnsserver.py
> index 46df5e3..df80c0d 100644
> --- a/python/samba/tests/dcerpc/dnsserver.py
> +++ b/python/samba/tests/dcerpc/dnsserver.py
> @@ -23,9 +23,10 @@ import ldb
> from samba.auth import system_session
> from samba.samdb import SamDB
> from samba.ndr import ndr_unpack, ndr_pack
> -from samba.dcerpc import dnsp, dnsserver
> +from samba.dcerpc import dnsp, dnsserver, security
> from samba.tests import RpcInterfaceTestCase, env_get_var_value
> from samba.netcmd.dns import ARecord, AAAARecord, PTRRecord, CNameRecord, NSRecord, MXRecord, SRVRecord, TXTRecord
> +from samba import sd_utils, descriptor
>
> class DnsserverTests(RpcInterfaceTestCase):
>
> @@ -856,3 +857,203 @@ class DnsserverTests(RpcInterfaceTestCase):
> select_flags,
> None,
> None)
> +
> + # The following tests do not pass against Samba because the owner and
> + # group are not consistent with Windows, as well as some ACEs.
> + #
> + # The following ACE are also required for 2012R2:
> + #
> + # (OA;CIIO;WP;ea1b7b93-5e48-46d5-bc6c-4df4fda78a35;bf967a86-0de6-11d0-a285-00aa003049e2;PS)
> + # (OA;OICI;RPWP;3f78c3e5-f79a-46bd-a0b8-9d18116ddc79;;PS)"
> + #
> + # [TPM + Allowed-To-Act-On-Behalf-Of-Other-Identity]
> + def test_security_descriptor_msdcs_zone(self):
> + """
> + Make sure that security descriptors of the msdcs zone is
> + as expected.
> + """
> +
> + zones = self.samdb.search(base="DC=ForestDnsZones,%s" % self.samdb.get_default_basedn(),
> + scope=ldb.SCOPE_SUBTREE,
> + expression="(&(objectClass=dnsZone)(name=_msdcs*))",
> + attrs=["nTSecurityDescriptor", "objectClass"])
> + self.assertEqual(len(zones), 1)
> + self.assertTrue("nTSecurityDescriptor" in zones[0])
> + tmp = zones[0]["nTSecurityDescriptor"][0]
> + utils = sd_utils.SDUtils(self.samdb)
> + sd = ndr_unpack(security.descriptor, tmp)
> +
> + domain_sid = security.dom_sid(self.samdb.get_domain_sid())
> +
> + res = self.samdb.search(base=self.samdb.get_default_basedn(), scope=ldb.SCOPE_SUBTREE,
> + expression="(sAMAccountName=DnsAdmins)",
> + attrs=["objectSid"])
> +
> + dns_admin = str(ndr_unpack(security.dom_sid, res[0]['objectSid'][0]))
> +
> + packed_sd = descriptor.sddl2binary("O:SYG:BA" \
> + "D:AI(A;;RPWPCRCCDCLCLORCWOWDSDDTSW;;;DA)" \
> + "(A;;CC;;;AU)" \
> + "(A;;RPLCLORC;;;WD)" \
> + "(A;;RPWPCRCCDCLCLORCWOWDSDDTSW;;;SY)" \
> + "(A;CI;RPWPCRCCDCLCRCWOWDSDDTSW;;;ED)",
> + domain_sid, {"DnsAdmins": dns_admin})
> + expected_sd = descriptor.get_clean_sd(ndr_unpack(security.descriptor, packed_sd))
> +
> + diff = descriptor.get_diff_sds(expected_sd, sd, domain_sid)
> + self.assertEqual(diff, '', "SD of msdcs zone different to expected.\n"
> + "Difference was:\n%s\nExpected: %s\nGot: %s" %
> + (diff, expected_sd.as_sddl(utils.domain_sid),
> + sd.as_sddl(utils.domain_sid)))
> +
> + def test_security_descriptor_forest_zone(self):
> + """
> + Make sure that security descriptors of forest dns zones are
> + as expected.
> + """
> + forest_zone = "test_forest_zone"
> + zone_create_info = dnsserver.DNS_RPC_ZONE_CREATE_INFO_LONGHORN()
> + zone_create_info.dwZoneType = dnsp.DNS_ZONE_TYPE_PRIMARY
> + zone_create_info.fAging = 0
> + zone_create_info.fDsIntegrated = 1
> + zone_create_info.fLoadExisting = 1
> +
> + zone_create_info.pszZoneName = forest_zone
> + zone_create_info.dwDpFlags = dnsserver.DNS_DP_FOREST_DEFAULT
> +
> + self.conn.DnssrvOperation2(dnsserver.DNS_CLIENT_VERSION_LONGHORN,
> + 0,
> + self.server,
> + None,
> + 0,
> + 'ZoneCreate',
> + dnsserver.DNSSRV_TYPEID_ZONE_CREATE,
> + zone_create_info)
> +
> + partition_dn = self.samdb.get_default_basedn()
> + partition_dn.add_child("DC=ForestDnsZones")
> + zones = self.samdb.search(base=partition_dn, scope=ldb.SCOPE_SUBTREE,
> + expression="(name=%s)" % forest_zone,
> + attrs=["nTSecurityDescriptor"])
> + self.assertEqual(len(zones), 1)
> + current_dn = zones[0].dn
> + self.assertTrue("nTSecurityDescriptor" in zones[0])
> + tmp = zones[0]["nTSecurityDescriptor"][0]
> + utils = sd_utils.SDUtils(self.samdb)
> + sd = ndr_unpack(security.descriptor, tmp)
> +
> + domain_sid = security.dom_sid(self.samdb.get_domain_sid())
> +
> + res = self.samdb.search(base=self.samdb.get_default_basedn(),
> + scope=ldb.SCOPE_SUBTREE,
> + expression="(sAMAccountName=DnsAdmins)",
> + attrs=["objectSid"])
> +
> + dns_admin = str(ndr_unpack(security.dom_sid, res[0]['objectSid'][0]))
> +
> + packed_sd = descriptor.sddl2binary("O:DAG:DA" \
> + "D:AI(A;;RPWPCRCCDCLCLORCWOWDSDDTSW;;;DA)" \
> + "(A;;CC;;;AU)" \
> + "(A;;RPLCLORC;;;WD)" \
> + "(A;;RPWPCRCCDCLCLORCWOWDSDDTSW;;;SY)" \
> + "(A;CI;RPWPCRCCDCLCRCWOWDSDDTSW;;;ED)",
> + domain_sid, {"DnsAdmins": dns_admin})
> + expected_sd = descriptor.get_clean_sd(ndr_unpack(security.descriptor, packed_sd))
> +
> + packed_msdns = descriptor.get_dns_forest_microsoft_dns_descriptor(domain_sid,
> + {"DnsAdmins": dns_admin})
> + expected_msdns_sd = descriptor.get_clean_sd(ndr_unpack(security.descriptor, packed_msdns))
> +
> + packed_part_sd = descriptor.get_dns_partition_descriptor(domain_sid)
> + expected_part_sd = descriptor.get_clean_sd(ndr_unpack(security.descriptor,
> + packed_part_sd))
> + try:
> + msdns_dn = ldb.Dn(self.samdb, "CN=MicrosoftDNS,%s" % str(partition_dn))
> + security_desc_dict = [(current_dn.get_linearized(), expected_sd),
> + (msdns_dn.get_linearized(), expected_msdns_sd),
> + (partition_dn.get_linearized(), expected_part_sd)]
> +
> + for (key, sec_desc) in security_desc_dict:
> + zones = self.samdb.search(base=key, scope=ldb.SCOPE_BASE,
> + attrs=["nTSecurityDescriptor"])
> + self.assertTrue("nTSecurityDescriptor" in zones[0])
> + tmp = zones[0]["nTSecurityDescriptor"][0]
> + utils = sd_utils.SDUtils(self.samdb)
> +
> + sd = ndr_unpack(security.descriptor, tmp)
> + diff = descriptor.get_diff_sds(sec_desc, sd, domain_sid)
> +
> + self.assertEqual(diff, '', "Security descriptor of forest DNS zone with DN '%s' different to expected. Difference was:\n%s\nExpected: %s\nGot: %s"
> + % (key, diff, sec_desc.as_sddl(utils.domain_sid), sd.as_sddl(utils.domain_sid)))
> +
> + finally:
> + self.conn.DnssrvOperation2(dnsserver.DNS_CLIENT_VERSION_LONGHORN,
> + 0,
> + self.server,
> + forest_zone,
> + 0,
> + 'DeleteZoneFromDs',
> + dnsserver.DNSSRV_TYPEID_NULL,
> + None)
> +
> + def test_security_descriptor_domain_zone(self):
> + """
> + Make sure that security descriptors of domain dns zones are
> + as expected.
> + """
> +
> + partition_dn = self.samdb.get_default_basedn()
> + partition_dn.add_child("DC=DomainDnsZones")
> + zones = self.samdb.search(base=partition_dn, scope=ldb.SCOPE_SUBTREE,
> + expression="(name=%s)" % self.custom_zone,
> + attrs=["nTSecurityDescriptor"])
> + self.assertEqual(len(zones), 1)
> + current_dn = zones[0].dn
> + self.assertTrue("nTSecurityDescriptor" in zones[0])
> + tmp = zones[0]["nTSecurityDescriptor"][0]
> + utils = sd_utils.SDUtils(self.samdb)
> + sd = ndr_unpack(security.descriptor, tmp)
> + sddl = sd.as_sddl(utils.domain_sid)
> +
> + domain_sid = security.dom_sid(self.samdb.get_domain_sid())
> +
> + res = self.samdb.search(base=self.samdb.get_default_basedn(), scope=ldb.SCOPE_SUBTREE,
> + expression="(sAMAccountName=DnsAdmins)",
> + attrs=["objectSid"])
> +
> + dns_admin = str(ndr_unpack(security.dom_sid, res[0]['objectSid'][0]))
> +
> + packed_sd = descriptor.sddl2binary("O:DAG:DA" \
> + "D:AI(A;;RPWPCRCCDCLCLORCWOWDSDDTSW;;;DA)" \
> + "(A;;CC;;;AU)" \
> + "(A;;RPLCLORC;;;WD)" \
> + "(A;;RPWPCRCCDCLCLORCWOWDSDDTSW;;;SY)" \
> + "(A;CI;RPWPCRCCDCLCRCWOWDSDDTSW;;;ED)",
> + domain_sid, {"DnsAdmins": dns_admin})
> + expected_sd = descriptor.get_clean_sd(ndr_unpack(security.descriptor, packed_sd))
> +
> + packed_msdns = descriptor.get_dns_domain_microsoft_dns_descriptor(domain_sid,
> + {"DnsAdmins": dns_admin})
> + expected_msdns_sd = descriptor.get_clean_sd(ndr_unpack(security.descriptor, packed_msdns))
> +
> + packed_part_sd = descriptor.get_dns_partition_descriptor(domain_sid)
> + expected_part_sd = descriptor.get_clean_sd(ndr_unpack(security.descriptor,
> + packed_part_sd))
> +
> + msdns_dn = ldb.Dn(self.samdb, "CN=MicrosoftDNS,%s" % str(partition_dn))
> + security_desc_dict = [(current_dn.get_linearized(), expected_sd),
> + (msdns_dn.get_linearized(), expected_msdns_sd),
> + (partition_dn.get_linearized(), expected_part_sd)]
> +
> + for (key, sec_desc) in security_desc_dict:
> + zones = self.samdb.search(base=key, scope=ldb.SCOPE_BASE,
> + attrs=["nTSecurityDescriptor"])
> + self.assertTrue("nTSecurityDescriptor" in zones[0])
> + tmp = zones[0]["nTSecurityDescriptor"][0]
> + utils = sd_utils.SDUtils(self.samdb)
> +
> + sd = ndr_unpack(security.descriptor, tmp)
> + diff = descriptor.get_diff_sds(sec_desc, sd, domain_sid)
> +
> + self.assertEqual(diff, '', "Security descriptor of domain DNS zone with DN '%s' different to expected. Difference was:\n%s\nExpected: %s\nGot: %s"
> + % (key, diff, sec_desc.as_sddl(utils.domain_sid), sd.as_sddl(utils.domain_sid)))
> diff --git a/selftest/knownfail b/selftest/knownfail
> index 6fa60a3..7141f43 100644
> --- a/selftest/knownfail
> +++ b/selftest/knownfail
> @@ -308,5 +308,6 @@
> ^samba4.rpc.echo.*on.*ncacn_ip_tcp.*with.object.*nt4_dc
> ^samba.tests.dcerpc.dnsserver.samba.tests.dcerpc.dnsserver.DnsserverTests.test_add_duplicate_different_type.*
> ^samba.tests.dcerpc.dnsserver.samba.tests.dcerpc.dnsserver.DnsserverTests.test_rank_none.*
> +^samba.tests.dcerpc.dnsserver.samba.tests.dcerpc.dnsserver.DnsserverTests.test_security_descriptor.*
> ^samba.tests.dcerpc.dnsserver.samba.tests.dcerpc.dnsserver.DnsserverTests.test_reject_invalid_commands
> ^samba.tests.samba_tool.dnscmd.samba.tests.samba_tool.dnscmd.DnsCmdTestCase.test_reject_invalid_commands
> --
> 1.9.1
>
>
> From 29132ab02bd0a564ff590466c1a738f649067b59 Mon Sep 17 00:00:00 2001
> From: Bob Campbell <bobcampbell at catalyst.net.nz>
> Date: Tue, 6 Dec 2016 15:34:23 +1300
> Subject: [PATCH 07/12] dnsserver: add dns name checking
>
> This may also prevent deletion of existing corrupted records through
> DNS, but should be resolvable through RPC, or at worst LDAP.
>
> Signed-off-by: Bob Campbell <bobcampbell at catalyst.net.nz>
> Peer-programmed-with: Garming Sam <garming at catalyst.net.nz>
> Reviewed-by: Garming Sam <garming at catalyst.net.nz>
> ---
> selftest/knownfail | 2 -
> source4/dns_server/dnsserver_common.c | 91 +++++++++++++++
> source4/dns_server/dnsserver_common.h | 4 +-
> source4/rpc_server/dnsserver/dnsdata.c | 191 ++++++++++++++++++++++++++-----
> source4/rpc_server/dnsserver/dnsdb.c | 33 ++++--
> source4/rpc_server/dnsserver/dnsserver.h | 4 +-
> source4/rpc_server/wscript_build | 2 +-
> 7 files changed, 283 insertions(+), 44 deletions(-)
>
> diff --git a/selftest/knownfail b/selftest/knownfail
> index 7141f43..97ec6ef 100644
> --- a/selftest/knownfail
> +++ b/selftest/knownfail
> @@ -309,5 +309,3 @@
> ^samba.tests.dcerpc.dnsserver.samba.tests.dcerpc.dnsserver.DnsserverTests.test_add_duplicate_different_type.*
> ^samba.tests.dcerpc.dnsserver.samba.tests.dcerpc.dnsserver.DnsserverTests.test_rank_none.*
> ^samba.tests.dcerpc.dnsserver.samba.tests.dcerpc.dnsserver.DnsserverTests.test_security_descriptor.*
> -^samba.tests.dcerpc.dnsserver.samba.tests.dcerpc.dnsserver.DnsserverTests.test_reject_invalid_commands
> -^samba.tests.samba_tool.dnscmd.samba.tests.samba_tool.dnscmd.DnsCmdTestCase.test_reject_invalid_commands
> diff --git a/source4/dns_server/dnsserver_common.c b/source4/dns_server/dnsserver_common.c
> index db42bcb..1b7900e 100644
> --- a/source4/dns_server/dnsserver_common.c
> +++ b/source4/dns_server/dnsserver_common.c
> @@ -214,6 +214,91 @@ static int rec_cmp(const struct dnsp_DnssrvRpcRecord *r1,
> return r1->dwTimeStamp - r2->dwTimeStamp;
> }
>
> +/*
> + * Check for valid DNS names. These are names which are non-empty, do not
> + * start with a dot and do not have any empty segments.
> + */
> +WERROR dns_name_check(TALLOC_CTX *mem_ctx, size_t len, const char *name)
> +{
> + size_t i;
> +
> + if (len == 0) {
> + return WERR_DS_INVALID_DN_SYNTAX;
> + }
> +
> + for (i = 0; i < len - 1; i++) {
> + if (name[i] == '.' && name[i+1] == '.') {
> + return WERR_DS_INVALID_DN_SYNTAX;
> + }
> + }
> +
> + if (len > 1 && name[0] == '.') {
> + return WERR_DS_INVALID_DN_SYNTAX;
> + }
> +
> + return WERR_OK;
> +}
> +
> +static WERROR check_name_list(TALLOC_CTX *mem_ctx, uint16_t rec_count,
> + struct dnsp_DnssrvRpcRecord *records)
> +{
> + WERROR werr;
> + uint16_t i;
> + size_t len;
> + struct dnsp_DnssrvRpcRecord record;
> +
> + werr = WERR_OK;
> + for (i = 0; i < rec_count; i++) {
> + record = records[i];
> +
> + switch (record.wType) {
> +
> + case DNS_TYPE_NS:
> + len = strlen(record.data.ns);
> + werr = dns_name_check(mem_ctx, len, record.data.ns);
> + break;
> + case DNS_TYPE_CNAME:
> + len = strlen(record.data.cname);
> + werr = dns_name_check(mem_ctx, len, record.data.cname);
> + break;
> + case DNS_TYPE_SOA:
> + len = strlen(record.data.soa.mname);
> + werr = dns_name_check(mem_ctx, len, record.data.soa.mname);
> + if (!W_ERROR_IS_OK(werr)) {
> + break;
> + }
> + len = strlen(record.data.soa.rname);
> + werr = dns_name_check(mem_ctx, len, record.data.soa.rname);
> + break;
> + case DNS_TYPE_PTR:
> + len = strlen(record.data.ptr);
> + werr = dns_name_check(mem_ctx, len, record.data.ptr);
> + break;
> + case DNS_TYPE_MX:
> + len = strlen(record.data.mx.nameTarget);
> + werr = dns_name_check(mem_ctx, len, record.data.mx.nameTarget);
> + break;
> + case DNS_TYPE_SRV:
> + len = strlen(record.data.srv.nameTarget);
> + werr = dns_name_check(mem_ctx, len,
> + record.data.srv.nameTarget);
> + break;
> + /*
> + * In the default case, the record doesn't have a DN, so it
> + * must be ok.
> + */
> + default:
> + break;
> + }
> +
> + if (!W_ERROR_IS_OK(werr)) {
> + return werr;
> + }
> + }
> +
> + return WERR_OK;
> +}
> +
> WERROR dns_common_replace(struct ldb_context *samdb,
> TALLOC_CTX *mem_ctx,
> struct ldb_dn *dn,
> @@ -225,6 +310,7 @@ WERROR dns_common_replace(struct ldb_context *samdb,
> struct ldb_message_element *el;
> uint16_t i;
> int ret;
> + WERROR werr;
> struct ldb_message *msg = NULL;
> bool was_tombstoned = false;
> bool become_tombstoned = false;
> @@ -234,6 +320,11 @@ WERROR dns_common_replace(struct ldb_context *samdb,
>
> msg->dn = dn;
>
> + werr = check_name_list(mem_ctx, rec_count, records);
> + if (!W_ERROR_IS_OK(werr)) {
> + return werr;
> + }
> +
> ret = ldb_msg_add_empty(msg, "dnsRecord", LDB_FLAG_MOD_REPLACE, &el);
> if (ret != LDB_SUCCESS) {
> return DNS_ERR(SERVER_FAILURE);
> diff --git a/source4/dns_server/dnsserver_common.h b/source4/dns_server/dnsserver_common.h
> index ad91f61..57d5d9f 100644
> --- a/source4/dns_server/dnsserver_common.h
> +++ b/source4/dns_server/dnsserver_common.h
> @@ -46,7 +46,9 @@ WERROR dns_common_lookup(struct ldb_context *samdb,
> struct dnsp_DnssrvRpcRecord **records,
> uint16_t *num_records,
> bool *tombstoned);
> -
> +WERROR dns_name_check(TALLOC_CTX *mem_ctx,
> + size_t len,
> + const char *name);
> WERROR dns_common_replace(struct ldb_context *samdb,
> TALLOC_CTX *mem_ctx,
> struct ldb_dn *dn,
> diff --git a/source4/rpc_server/dnsserver/dnsdata.c b/source4/rpc_server/dnsserver/dnsdata.c
> index ccea0d7..9b3c9f9 100644
> --- a/source4/rpc_server/dnsserver/dnsdata.c
> +++ b/source4/rpc_server/dnsserver/dnsdata.c
> @@ -21,6 +21,7 @@
>
> #include "includes.h"
> #include "dnsserver.h"
> +#include "dns_server/dnsserver_common.h"
> #include "lib/replace/system/network.h"
> #include "librpc/gen_ndr/ndr_dnsp.h"
> #include "librpc/gen_ndr/ndr_dnsserver.h"
> @@ -412,15 +413,18 @@ void dnsp_to_dns_copy(TALLOC_CTX *mem_ctx, struct dnsp_DnssrvRpcRecord *dnsp,
>
> }
>
> -
> -struct dnsp_DnssrvRpcRecord *dns_to_dnsp_copy(TALLOC_CTX *mem_ctx, struct DNS_RPC_RECORD *dns)
> +WERROR dns_to_dnsp_convert(TALLOC_CTX *mem_ctx, struct DNS_RPC_RECORD *dns,
> + struct dnsp_DnssrvRpcRecord **out_dnsp, bool check_name)
> {
> + WERROR res;
> int i, len;
> - struct dnsp_DnssrvRpcRecord *dnsp;
> + const char *name;
> + char *talloc_res = NULL;
> + struct dnsp_DnssrvRpcRecord *dnsp = NULL;
>
> dnsp = talloc_zero(mem_ctx, struct dnsp_DnssrvRpcRecord);
> if (dnsp == NULL) {
> - return NULL;
> + return WERR_NOT_ENOUGH_MEMORY;
> }
>
> dnsp->wDataLength = dns->wDataLength;
> @@ -438,25 +442,65 @@ struct dnsp_DnssrvRpcRecord *dns_to_dnsp_copy(TALLOC_CTX *mem_ctx, struct DNS_RP
> break;
>
> case DNS_TYPE_A:
> - dnsp->data.ipv4 = talloc_strdup(mem_ctx, dns->data.ipv4);
> + talloc_res = talloc_strdup(mem_ctx, dns->data.ipv4);
> + if (talloc_res == NULL) {
> + goto fail_nomemory;
> + }
> + dnsp->data.ipv4 = talloc_res;
> break;
>
> case DNS_TYPE_NS:
> + name = dns->data.name.str;
> len = dns->data.name.len;
> - if (dns->data.name.str[len-1] == '.') {
> - dnsp->data.ns = talloc_strndup(mem_ctx, dns->data.name.str, len-1);
> +
> + if (check_name) {
> + res = dns_name_check(mem_ctx, len, name);
> + if (!W_ERROR_IS_OK(res)) {
> + return res;
> + }
> + }
> +
> + if (len > 0 && name[len-1] == '.') {
> + talloc_res = talloc_strndup(mem_ctx, name, len-1);
> + if (talloc_res == NULL) {
> + goto fail_nomemory;
> + }
> + dnsp->data.ns = talloc_res;
> } else {
> - dnsp->data.ns = talloc_strdup(mem_ctx, dns->data.name.str);
> + talloc_res = talloc_strdup(mem_ctx, name);
> + if (talloc_res == NULL) {
> + goto fail_nomemory;
> + }
> + dnsp->data.ns = talloc_res;
> }
> +
> break;
>
> case DNS_TYPE_CNAME:
> + name = dns->data.name.str;
> len = dns->data.name.len;
> - if (dns->data.name.str[len-1] == '.') {
> - dnsp->data.cname = talloc_strndup(mem_ctx, dns->data.name.str, len-1);
> +
> + if (check_name) {
> + res = dns_name_check(mem_ctx, len, name);
> + if (!W_ERROR_IS_OK(res)) {
> + return res;
> + }
> + }
> +
> + if (len > 0 && name[len-1] == '.') {
> + talloc_res = talloc_strndup(mem_ctx, name, len-1);
> + if (talloc_res == NULL) {
> + goto fail_nomemory;
> + }
> + dnsp->data.cname = talloc_res;
> } else {
> - dnsp->data.cname = talloc_strdup(mem_ctx, dns->data.name.str);
> + talloc_res = talloc_strdup(mem_ctx, name);
> + if (talloc_res == NULL) {
> + goto fail_nomemory;
> + }
> + dnsp->data.cname = talloc_res;
> }
> +
> break;
>
> case DNS_TYPE_SOA:
> @@ -466,40 +510,111 @@ struct dnsp_DnssrvRpcRecord *dns_to_dnsp_copy(TALLOC_CTX *mem_ctx, struct DNS_RP
> dnsp->data.soa.expire = dns->data.soa.dwExpire;
> dnsp->data.soa.minimum = dns->data.soa.dwMinimumTtl;
>
> + name = dns->data.soa.NamePrimaryServer.str;
> len = dns->data.soa.NamePrimaryServer.len;
> - if (dns->data.soa.NamePrimaryServer.str[len-1] == '.') {
> - dnsp->data.soa.mname = talloc_strndup(mem_ctx, dns->data.soa.NamePrimaryServer.str, len-1);
> +
> + if (check_name) {
> + res = dns_name_check(mem_ctx, len, name);
> + if (!W_ERROR_IS_OK(res)) {
> + return res;
> + }
> + }
> +
> + if (len > 0 && name[len-1] == '.') {
> + talloc_res = talloc_strndup(mem_ctx, name, len-1);
> + if (talloc_res == NULL) {
> + goto fail_nomemory;
> + }
> + dnsp->data.soa.mname = talloc_res;
> } else {
> - dnsp->data.soa.mname = talloc_strdup(mem_ctx, dns->data.soa.NamePrimaryServer.str);
> + talloc_res = talloc_strdup(mem_ctx, name);
> + if (talloc_res == NULL) {
> + goto fail_nomemory;
> + }
> + dnsp->data.soa.mname = talloc_res;
> }
>
> + name = dns->data.soa.ZoneAdministratorEmail.str;
> len = dns->data.soa.ZoneAdministratorEmail.len;
> - if (dns->data.soa.ZoneAdministratorEmail.str[len-1] == '.') {
> - dnsp->data.soa.rname = talloc_strndup(mem_ctx, dns->data.soa.ZoneAdministratorEmail.str, len-1);
> +
> + res = dns_name_check(mem_ctx, len, name);
> + if (!W_ERROR_IS_OK(res)) {
> + return res;
> + }
> +
> + if (len > 0 && name[len-1] == '.') {
> + talloc_res = talloc_strndup(mem_ctx, name, len-1);
> + if (talloc_res == NULL) {
> + goto fail_nomemory;
> + }
> + dnsp->data.soa.rname = talloc_res;
> } else {
> - dnsp->data.soa.rname = talloc_strdup(mem_ctx, dns->data.soa.ZoneAdministratorEmail.str);
> + talloc_res = talloc_strdup(mem_ctx, name);
> + if (talloc_res == NULL) {
> + goto fail_nomemory;
> + }
> + dnsp->data.soa.rname = talloc_res;
> }
> +
> break;
>
> case DNS_TYPE_PTR:
> - dnsp->data.ptr = talloc_strdup(mem_ctx, dns->data.ptr.str);
> + name = dns->data.ptr.str;
> + len = dns->data.ptr.len;
> +
> + if (check_name) {
> + res = dns_name_check(mem_ctx, len, name);
> + if (!W_ERROR_IS_OK(res)) {
> + return res;
> + }
> + }
> +
> + talloc_res = talloc_strdup(mem_ctx, name);
> + if (talloc_res == NULL) {
> + goto fail_nomemory;
> + }
> + dnsp->data.ptr = talloc_res;
> +
> break;
>
> case DNS_TYPE_MX:
> dnsp->data.mx.wPriority = dns->data.mx.wPreference;
> +
> + name = dns->data.mx.nameExchange.str;
> len = dns->data.mx.nameExchange.len;
> - if (dns->data.mx.nameExchange.str[len-1] == '.') {
> - dnsp->data.mx.nameTarget = talloc_strndup(mem_ctx, dns->data.mx.nameExchange.str, len-1);
> +
> + if (check_name) {
> + res = dns_name_check(mem_ctx, len, name);
> + if (!W_ERROR_IS_OK(res)) {
> + return res;
> + }
> + }
> +
> + if (len > 0 && name[len-1] == '.') {
> + talloc_res = talloc_strndup(mem_ctx, name, len-1);
> + if (talloc_res == NULL) {
> + goto fail_nomemory;
> + }
> + dnsp->data.mx.nameTarget = talloc_res;
> } else {
> - dnsp->data.mx.nameTarget = talloc_strdup(mem_ctx, dns->data.mx.nameExchange.str);
> + talloc_res = talloc_strdup(mem_ctx, name);
> + if (talloc_res == NULL) {
> + goto fail_nomemory;
> + }
> + dnsp->data.mx.nameTarget = talloc_res;
> }
> +
> break;
>
> case DNS_TYPE_TXT:
> dnsp->data.txt.count = dns->data.txt.count;
> dnsp->data.txt.str = talloc_array(mem_ctx, const char *, dns->data.txt.count);
> for (i=0; i<dns->data.txt.count; i++) {
> - dnsp->data.txt.str[i] = talloc_strdup(mem_ctx, dns->data.txt.str[i].str);
> + talloc_res = talloc_strdup(mem_ctx, dns->data.txt.str[i].str);
> + if (talloc_res == NULL) {
> + goto fail_nomemory;
> + }
> + dnsp->data.txt.str[i] = talloc_res;
> }
> break;
>
> @@ -512,23 +627,43 @@ struct dnsp_DnssrvRpcRecord *dns_to_dnsp_copy(TALLOC_CTX *mem_ctx, struct DNS_RP
> dnsp->data.srv.wWeight = dns->data.srv.wWeight;
> dnsp->data.srv.wPort = dns->data.srv.wPort;
>
> + name = dns->data.srv.nameTarget.str;
> len = dns->data.srv.nameTarget.len;
> - if (dns->data.srv.nameTarget.str[len-1] == '.') {
> - dnsp->data.srv.nameTarget = talloc_strndup(mem_ctx, dns->data.srv.nameTarget.str, len-1);
> +
> + if (check_name) {
> + res = dns_name_check(mem_ctx, len, name);
> + if (!W_ERROR_IS_OK(res)) {
> + return res;
> + }
> + }
> +
> + if (len > 0 && name[len-1] == '.') {
> + talloc_res = talloc_strndup(mem_ctx, name, len-1);
> + if (talloc_res == NULL) {
> + goto fail_nomemory;
> + }
> + dnsp->data.srv.nameTarget = talloc_res;
> } else {
> - dnsp->data.srv.nameTarget = talloc_strdup(mem_ctx, dns->data.srv.nameTarget.str);
> + talloc_res = talloc_strdup(mem_ctx, name);
> + if (talloc_res == NULL) {
> + goto fail_nomemory;
> + }
> + dnsp->data.srv.nameTarget = talloc_res;
> }
> +
> break;
>
> default:
> memcpy(&dnsp->data, &dns->data, sizeof(union dnsRecordData));
> DEBUG(0, ("dnsserver: Found Unhandled DNS record type=%d", dns->wType));
> -
> }
>
> - return dnsp;
> -}
> + *out_dnsp = dnsp;
> + return WERR_OK;
>
> +fail_nomemory:
> + return WERR_NOT_ENOUGH_MEMORY;
> +}
>
> /* Intialize tree with given name as the root */
> static struct dns_tree *dns_tree_init(TALLOC_CTX *mem_ctx, const char *name, void *data)
> diff --git a/source4/rpc_server/dnsserver/dnsdb.c b/source4/rpc_server/dnsserver/dnsdb.c
> index 949a8b9..da37878 100644
> --- a/source4/rpc_server/dnsserver/dnsdb.c
> +++ b/source4/rpc_server/dnsserver/dnsdb.c
> @@ -397,17 +397,20 @@ WERROR dnsserver_db_add_record(TALLOC_CTX *mem_ctx,
> {
> const char * const attrs[] = { "dnsRecord", "dNSTombstoned", NULL };
> struct ldb_result *res;
> - struct dnsp_DnssrvRpcRecord *rec;
> + struct dnsp_DnssrvRpcRecord *rec = NULL;
> struct ldb_message_element *el;
> struct ldb_dn *dn;
> enum ndr_err_code ndr_err;
> NTTIME t;
> int ret, i;
> int serial;
> + WERROR werr;
> bool was_tombstoned = false;
>
> - rec = dns_to_dnsp_copy(mem_ctx, add_record);
> - W_ERROR_HAVE_NO_MEMORY(rec);
> + werr = dns_to_dnsp_convert(mem_ctx, add_record, &rec, true);
> + if (!W_ERROR_IS_OK(werr)) {
> + return werr;
> + }
>
> /* Set the correct rank for the record. */
> if (z->zoneinfo->dwZoneType == DNS_ZONE_TYPE_PRIMARY) {
> @@ -514,18 +517,23 @@ WERROR dnsserver_db_update_record(TALLOC_CTX *mem_ctx,
> {
> const char * const attrs[] = { "dnsRecord", NULL };
> struct ldb_result *res;
> - struct dnsp_DnssrvRpcRecord *arec, *drec;
> + struct dnsp_DnssrvRpcRecord *arec = NULL, *drec = NULL;
> struct ldb_message_element *el;
> enum ndr_err_code ndr_err;
> NTTIME t;
> int ret, i;
> int serial;
> + WERROR werr;
>
> - arec = dns_to_dnsp_copy(mem_ctx, add_record);
> - W_ERROR_HAVE_NO_MEMORY(arec);
> + werr = dns_to_dnsp_convert(mem_ctx, add_record, &arec, true);
> + if (!W_ERROR_IS_OK(werr)) {
> + return werr;
> + }
>
> - drec = dns_to_dnsp_copy(mem_ctx, del_record);
> - W_ERROR_HAVE_NO_MEMORY(drec);
> + werr = dns_to_dnsp_convert(mem_ctx, del_record, &drec, true);
> + if (!W_ERROR_IS_OK(werr)) {
> + return werr;
> + }
>
> unix_to_nt_time(&t, time(NULL));
> t /= 10*1000*1000;
> @@ -616,19 +624,22 @@ WERROR dnsserver_db_delete_record(TALLOC_CTX *mem_ctx,
> {
> const char * const attrs[] = { "dnsRecord", NULL };
> struct ldb_result *res;
> - struct dnsp_DnssrvRpcRecord *rec;
> + struct dnsp_DnssrvRpcRecord *rec = NULL;
> struct ldb_message_element *el;
> enum ndr_err_code ndr_err;
> int ret, i;
> int serial;
> + WERROR werr;
>
> serial = dnsserver_update_soa(mem_ctx, samdb, z);
> if (serial < 0) {
> return WERR_INTERNAL_DB_ERROR;
> }
>
> - rec = dns_to_dnsp_copy(mem_ctx, del_record);
> - W_ERROR_HAVE_NO_MEMORY(rec);
> + werr = dns_to_dnsp_convert(mem_ctx, del_record, &rec, false);
> + if (!W_ERROR_IS_OK(werr)) {
> + return werr;
> + }
>
> ret = ldb_search(samdb, mem_ctx, &res, z->zone_dn, LDB_SCOPE_ONELEVEL, attrs,
> "(&(objectClass=dnsNode)(name=%s))", name);
> diff --git a/source4/rpc_server/dnsserver/dnsserver.h b/source4/rpc_server/dnsserver/dnsserver.h
> index cfe6d4e..0b08f08 100644
> --- a/source4/rpc_server/dnsserver/dnsserver.h
> +++ b/source4/rpc_server/dnsserver/dnsserver.h
> @@ -193,7 +193,9 @@ bool dns_record_match(struct dnsp_DnssrvRpcRecord *rec1, struct dnsp_DnssrvRpcRe
>
> void dnsp_to_dns_copy(TALLOC_CTX *mem_ctx, struct dnsp_DnssrvRpcRecord *dnsp,
> struct DNS_RPC_RECORD *dns);
> -struct dnsp_DnssrvRpcRecord *dns_to_dnsp_copy(TALLOC_CTX *mem_ctx, struct DNS_RPC_RECORD *dns);
> +WERROR dns_to_dnsp_convert(TALLOC_CTX *mem_ctx, struct DNS_RPC_RECORD *dns,
> + struct dnsp_DnssrvRpcRecord **out_dnsp,
> + bool check_name);
>
> struct dns_tree *dns_build_tree(TALLOC_CTX *mem_ctx, const char *name, struct ldb_result *res);
> WERROR dns_fill_records_array(TALLOC_CTX *mem_ctx, struct dnsserver_zone *z,
> diff --git a/source4/rpc_server/wscript_build b/source4/rpc_server/wscript_build
> index ed7dbf6..f1b1f19 100755
> --- a/source4/rpc_server/wscript_build
> +++ b/source4/rpc_server/wscript_build
> @@ -155,7 +155,7 @@ bld.SAMBA_MODULE('dcerpc_dnsserver',
> source='dnsserver/dcerpc_dnsserver.c dnsserver/dnsutils.c dnsserver/dnsdata.c dnsserver/dnsdb.c',
> subsystem='dcerpc_server',
> init_function='dcerpc_server_dnsserver_init',
> - deps='DCERPC_COMMON'
> + deps='DCERPC_COMMON dnsserver_common'
> )
>
>
> --
> 1.9.1
>
>
> From 3ab972005bfe6839011a6a6c236fdcfea8f6f811 Mon Sep 17 00:00:00 2001
> From: Garming Sam <garming at catalyst.net.nz>
> Date: Wed, 7 Dec 2016 14:25:18 +1300
> Subject: [PATCH 08/12] provision/dns: add dNSProperty attr when adding msdcs
> record
>
> Signed-off-by: Garming Sam <garming at catalyst.net.nz>
> ---
> python/samba/provision/sambadns.py | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/python/samba/provision/sambadns.py b/python/samba/provision/sambadns.py
> index df4673b..c959538 100644
> --- a/python/samba/provision/sambadns.py
> +++ b/python/samba/provision/sambadns.py
> @@ -491,7 +491,10 @@ def add_msdcs_record(samdb, forestdn, prefix, dnsforest):
> # DC=_msdcs.<DNSFOREST>,CN=MicrosoftDNS,<PREFIX>,<FORESTDN>
> msg = ldb.Message(ldb.Dn(samdb, "DC=_msdcs.%s,CN=MicrosoftDNS,%s,%s" %
> (dnsforest, prefix, forestdn)))
> + props = []
> + props.append(ndr_pack(TypeProperty()))
> msg["objectClass"] = ["top", "dnsZone"]
> + msg["dNSProperty"] = ldb.MessageElement(props, ldb.FLAG_MOD_ADD, "dNSProperty")
> samdb.add(msg)
>
>
> --
> 1.9.1
>
>
> From 6e5a3b3d19fd3e71feccf0fb8f9da2595ba16c0d Mon Sep 17 00:00:00 2001
> From: Garming Sam <garming at catalyst.net.nz>
> Date: Wed, 7 Dec 2016 14:25:35 +1300
> Subject: [PATCH 09/12] python/tests: fix typo to use correct var
>
> Signed-off-by: Garming Sam <garming at catalyst.net.nz>
> Reviewed-by: Bob Campbell <bobcampbell at catalyst.net.nz>
> ---
> python/samba/tests/dns.py | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/python/samba/tests/dns.py b/python/samba/tests/dns.py
> index babf898..79db4de 100644
> --- a/python/samba/tests/dns.py
> +++ b/python/samba/tests/dns.py
> @@ -894,7 +894,7 @@ class TestComplexQueries(DNSTest):
>
> self.assertEquals(response.answers[1].rr_type, dns.DNS_QTYPE_CNAME)
> self.assertEquals(response.answers[1].name, name2)
> - self.assertEquals(response.answers[1].rdata, name3)
> + self.assertEquals(response.answers[1].rdata, name0)
>
> class TestInvalidQueries(DNSTest):
>
> --
> 1.9.1
>
>
> From 9cd3840b9e856a4adeef0f17c2d42e70493b87ea Mon Sep 17 00:00:00 2001
> From: Bob Campbell <bobcampbell at catalyst.net.nz>
> Date: Wed, 7 Dec 2016 15:00:25 +1300
> Subject: [PATCH 10/12] python/tests: expand samba-tool dns tests
>
> These new tests concern collisions and lock in current Samba behaviour.
>
> They do not pass against Windows Server 2012R2. See dnsserver.py tests
> for the tests consistent with Windows behaviour.
>
> Signed-off-by: Bob Campbell <bobcampbell at catalyst.net.nz>
> Reviewed-by: Garming Sam <garming at catalyst.net.nz>
> ---
> python/samba/tests/samba_tool/dnscmd.py | 104 ++++++++++++++++++++++++++++++++
> 1 file changed, 104 insertions(+)
>
> diff --git a/python/samba/tests/samba_tool/dnscmd.py b/python/samba/tests/samba_tool/dnscmd.py
> index c1cb7df..3a369d9 100644
> --- a/python/samba/tests/samba_tool/dnscmd.py
> +++ b/python/samba/tests/samba_tool/dnscmd.py
> @@ -521,6 +521,81 @@ class DnsCmdTestCase(SambaToolCmdTest):
> "Invalid error message '%s' when attempting to " \
> "add record of type SOA." % err)
>
> + def test_add_overlapping_different_type(self):
> + """
> + Make sure that we can add an entry with the same name as an existing one but a different type.
> + """
> +
> + i = 0
> + for dnstype1 in self.good_records:
> + record1 = self.good_records[dnstype1][0]
> + for dnstype2 in self.good_records:
> + # Only do some subset of dns types, otherwise it takes a long time.
> + i += 1
> + if i % 4 != 0:
> + continue
> +
> + if dnstype1 == dnstype2:
> + continue
> +
> + record2 = self.good_records[dnstype2][0]
> +
> + result, out, err = self.runsubcmd("dns", "add",
> + os.environ["SERVER"],
> + self.zone, "testrecord",
> + dnstype1, record1,
> + self.creds_string)
> + self.assertCmdSuccess(result, out, err, "Failed to add record " \
> + "'%s' of type '%s'." % (record1, dnstype1))
> +
> + result, out, err = self.runsubcmd("dns", "add",
> + os.environ["SERVER"],
> + self.zone, "testrecord",
> + dnstype2, record2,
> + self.creds_string)
> + self.assertCmdSuccess(result, out, err, "Failed to add record " \
> + "'%s' of type '%s' when a record '%s' " \
> + "of type '%s' with the same name exists."
> + % (record1, dnstype1, record2, dnstype2))
> +
> + result, out, err = self.runsubcmd("dns", "query",
> + os.environ["SERVER"],
> + self.zone, "testrecord",
> + dnstype1, self.creds_string)
> + self.assertCmdSuccess(result, out, err, "Failed to query for " \
> + "record '%s' of type '%s' when a new " \
> + "record '%s' of type '%s' with the same " \
> + "name was added."
> + % (record1, dnstype1, record2, dnstype2))
> +
> + result, out, err = self.runsubcmd("dns", "query",
> + os.environ["SERVER"],
> + self.zone, "testrecord",
> + dnstype2, self.creds_string)
> + self.assertCmdSuccess(result, out, err, "Failed to query " \
> + "record '%s' of type '%s' which should " \
> + "have been added with the same name as " \
> + "record '%s' of type '%s'."
> + % (record2, dnstype2, record1, dnstype1))
> +
> + result, out, err = self.runsubcmd("dns", "delete",
> + os.environ["SERVER"],
> + self.zone, "testrecord",
> + dnstype1, record1,
> + self.creds_string)
> + self.assertCmdSuccess(result, out, err, "Failed to delete " \
> + "record '%s' of type '%s'."
> + % (record1, dnstype1))
> +
> + result, out, err = self.runsubcmd("dns", "delete",
> + os.environ["SERVER"],
> + self.zone, "testrecord",
> + dnstype2, record2,
> + self.creds_string)
> + self.assertCmdSuccess(result, out, err, "Failed to delete " \
> + "record '%s' of type '%s'."
> + % (record2, dnstype2))
> +
> def test_query_deleted_record(self):
> self.runsubcmd("dns", "add", os.environ["SERVER"], self.zone,
> "testrecord", "A", self.testip, self.creds_string)
> @@ -533,6 +608,35 @@ class DnsCmdTestCase(SambaToolCmdTest):
> "A", self.creds_string)
> self.assertCmdFail(result)
>
> + def test_add_duplicate_record(self):
> + for record_type in self.good_records:
> + result, out, err = self.runsubcmd("dns", "add",
> + os.environ["SERVER"],
> + self.zone, "testrecord",
> + record_type,
> + self.good_records[record_type][0],
> + self.creds_string)
> + self.assertCmdSuccess(result, out, err)
> + result, out, err = self.runsubcmd("dns", "add",
> + os.environ["SERVER"],
> + self.zone, "testrecord",
> + record_type,
> + self.good_records[record_type][0],
> + self.creds_string)
> + self.assertCmdFail(result)
> + result, out, err = self.runsubcmd("dns", "query",
> + os.environ["SERVER"],
> + self.zone, "testrecord",
> + record_type, self.creds_string)
> + self.assertCmdSuccess(result, out, err)
> + result, out, err = self.runsubcmd("dns", "delete",
> + os.environ["SERVER"],
> + self.zone, "testrecord",
> + record_type,
> + self.good_records[record_type][0],
> + self.creds_string)
> + self.assertCmdSuccess(result, out, err)
> +
> def test_remove_deleted_record(self):
> self.runsubcmd("dns", "add", os.environ["SERVER"], self.zone,
> "testrecord", "A", self.testip, self.creds_string)
> --
> 1.9.1
>
>
> From cd474b6a112ba860bcc75bcc61083cb5191afcfe Mon Sep 17 00:00:00 2001
> From: Bob Campbell <bobcampbell at catalyst.net.nz>
> Date: Wed, 7 Dec 2016 15:33:06 +1300
> Subject: [PATCH 11/12] dnsserver_common: Add name check in name2dn
>
> Fills in the missing TODO. Note that this may also prevent deletion of
> existing corrupted records, but should be resolvable through RPC, or at
> worst LDAP.
>
> Signed-off-by: Bob Campbell <bobcampbell at catalyst.net.nz>
> Peer-programmed-with: Garming Sam <garming at catalyst.net.nz>
> Reviewed-by: Garming Sam <garming at catalyst.net.nz>
> ---
> source4/dns_server/dnsserver_common.c | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/source4/dns_server/dnsserver_common.c b/source4/dns_server/dnsserver_common.c
> index 1b7900e..945e646 100644
> --- a/source4/dns_server/dnsserver_common.c
> +++ b/source4/dns_server/dnsserver_common.c
> @@ -491,13 +491,12 @@ WERROR dns_common_name2dn(struct ldb_context *samdb,
> struct ldb_dn *dn;
> const struct dns_server_zone *z;
> size_t host_part_len = 0;
> + WERROR werr;
>
> if (name == NULL) {
> return DNS_ERR(FORMAT_ERROR);
> }
>
> - /*TODO: Check if 'name' is a valid DNS name */
> -
> if (strcmp(name, "") == 0) {
> base = ldb_get_default_basedn(samdb);
> dn = ldb_dn_copy(mem_ctx, base);
> @@ -506,6 +505,12 @@ WERROR dns_common_name2dn(struct ldb_context *samdb,
> return WERR_OK;
> }
>
> + /* Check non-empty names */
> + werr = dns_name_check(mem_ctx, strlen(name), name);
> + if (!W_ERROR_IS_OK(werr)) {
> + return werr;
> + }
> +
> for (z = zones; z != NULL; z = z->next) {
> bool match;
>
> --
> 1.9.1
>
>
> From 937eb4ed68fff52e75f136c1700753a8b5917685 Mon Sep 17 00:00:00 2001
> From: Garming Sam <garming at catalyst.net.nz>
> Date: Wed, 7 Dec 2016 16:42:38 +1300
> Subject: [PATCH 12/12] tests/dns: Check you cannot add empty CNAME
>
> This exercises the dns_check_name case in the DNS server. Directly
> attempting to add an invalid name with leading . or double .. cannot be
> done due to ndr_pull_component forcing the check on the client side
> (leading to a CNAME name of NUL and unexpected data of the actual name).
>
> Signed-off-by: Garming Sam <garming at catalyst.net.nz>
> Peer-programmed-by: Bob Campbell <bobcampbell at catalyst.net.nz>
> ---
> python/samba/tests/dns.py | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/python/samba/tests/dns.py b/python/samba/tests/dns.py
> index 79db4de..66198b5 100644
> --- a/python/samba/tests/dns.py
> +++ b/python/samba/tests/dns.py
> @@ -865,6 +865,15 @@ class TestComplexQueries(DNSTest):
> self.assertEquals(response.answers[2].rdata,
> self.server_ip)
>
> + def test_invalid_empty_cname(self):
> + name0 = "cnamedotprefix0.%s" % self.get_dns_domain()
> + try:
> + self.make_dns_update(name0, "", dns.DNS_QTYPE_CNAME)
> + except AssertionError as e:
> + pass
> + else:
> + self.fail("Successfully added empty CNAME, which is invalid.")
> +
> def test_cname_two_chain_not_matching_qtype(self):
> name0 = "cnamechain0.%s" % self.get_dns_domain()
> name1 = "cnamechain1.%s" % self.get_dns_domain()
> --
> 1.9.1
>
More information about the samba-technical
mailing list