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>

diff --git a/selftest/knownfail b/selftest/knownfail
index 7141f430945ea8dcde72deef269a74104bd09782..97ec6ef1d64df46b6cbf2fead46f10bda1cb3831 100644 (file)
--- 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 db42bcb5981cb9ead5200c3c91d8e58f34e0fbd0..4982d3a37b1331e928b9fcca3167d6b52d04c97b 100644 (file)
--- 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 ad91f617be7bb268901e07e011d2eb490ca6b8c3..57d5d9f3c157b2d5108d43feb3f50b46c8f8d814 100644 (file)
--- 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 ccea0d700a5618b8a318746dafc6c7f2ee76ac04..9b3c9f9d4065279db2ef11ea522c2e47e0fafffc 100644 (file)
--- 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 949a8b9b05e3a02d48c70d87cabcf97b21819471..da37878ce2c0e00b97c22b03c213ba4ef8ee785c 100644 (file)
--- 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 cfe6d4e81a394c2ad4891b51f887ff83d863d8c0..0b08f08fa2dcc2b4ac0e835e8b99ba75474ad015 100644 (file)
--- 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 ed7dbf6b173354d61d17a88e2e30bab47f80cd1d..f1b1f19455a973f9d7d1a4feea6f3686ee95804f 100755 (executable)
--- 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'
     )