[PATCH] Internal DNS, RPC server & samba-tool tests & fixes
Bob Campbell
bobcampbell at catalyst.net.nz
Thu Dec 8 23:29:15 UTC 2016
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
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
-------------- next part --------------
>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