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@catalyst.net.nz>
Pair-programmed-with: Garming Sam <garming@catalyst.net.nz>
Reviewed-by: Garming Sam <garming@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>

7 files changed, 283 insertions, 44 deletions

diff --git a/selftest/knownfail b/selftest/knownfail
index 7141f430945..97ec6ef1d64 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 db42bcb5981..4982d3a37b1 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 ad91f617be7..57d5d9f3c15 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 ccea0d700a5..9b3c9f9d406 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 949a8b9b05e..da37878ce2c 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 cfe6d4e81a3..0b08f08fa2d 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 ed7dbf6b173..f1b1f19455a 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'
     )