[PATCH] Bug 12451 - We set dwTimeStamp value of dnsRecord attributes is updated incorrectly

Stefan Metzmacher metze at samba.org
Wed Nov 30 06:24:09 UTC 2016


Hi,

here're some patches to fix https://bugzilla.samba.org/show_bug.cgi?id=12451

Please review and push:-)

Thanks!
metze
-------------- next part --------------
From 19b8ba0e92c4024ea1eca2b1746720c48434870f Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 25 Nov 2016 11:50:54 +0100
Subject: [PATCH 1/2] s4:rpc_server/dnsserver: records added via rpc are
 'static'

BUG: https://bugzilla.samba.org/show_bug.cgi?id=12451

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 source4/dns_server/dnsserver_common.h |  2 ++
 source4/rpc_server/dnsserver/dnsdb.c  | 30 ++++++------------------------
 2 files changed, 8 insertions(+), 24 deletions(-)

diff --git a/source4/dns_server/dnsserver_common.h b/source4/dns_server/dnsserver_common.h
index ad91f61..ce13da8 100644
--- a/source4/dns_server/dnsserver_common.h
+++ b/source4/dns_server/dnsserver_common.h
@@ -47,6 +47,8 @@ WERROR dns_common_lookup(struct ldb_context *samdb,
 			 uint16_t *num_records,
 			 bool *tombstoned);
 
+#define DNS_COMMON_TIMESTAMP_STATIC  ((uint32_t)0)
+
 WERROR dns_common_replace(struct ldb_context *samdb,
 			  TALLOC_CTX *mem_ctx,
 			  struct ldb_dn *dn,
diff --git a/source4/rpc_server/dnsserver/dnsdb.c b/source4/rpc_server/dnsserver/dnsdb.c
index 949a8b9..21a5fde 100644
--- a/source4/rpc_server/dnsserver/dnsdb.c
+++ b/source4/rpc_server/dnsserver/dnsdb.c
@@ -28,6 +28,7 @@
 #include "dsdb/samdb/samdb.h"
 #include "libcli/security/security.h"
 #include "dsdb/common/util.h"
+#include "dns_server/dnsserver_common.h"
 
 /* There are only 2 fixed partitions for DNS */
 struct dnsserver_partition *dnsserver_db_enumerate_partitions(TALLOC_CTX *mem_ctx,
@@ -258,11 +259,6 @@ static unsigned int dnsserver_update_soa(TALLOC_CTX *mem_ctx,
 	struct ldb_message_element *el;
 	enum ndr_err_code ndr_err;
 	int ret, i, serial = -1;
-	NTTIME t;
-
-	unix_to_nt_time(&t, time(NULL));
-	t /= 10*1000*1000; /* convert to seconds (NT time is in 100ns units) */
-	t /= 3600;         /* convert to hours */
 
 	ret = ldb_search(samdb, mem_ctx, &res, z->zone_dn, LDB_SCOPE_ONELEVEL, attrs,
 			"(&(objectClass=dnsNode)(name=@))");
@@ -285,7 +281,7 @@ static unsigned int dnsserver_update_soa(TALLOC_CTX *mem_ctx,
 		if (rec.wType == DNS_TYPE_SOA) {
 			serial = rec.data.soa.serial + 1;
 			rec.dwSerial = serial;
-			rec.dwTimeStamp = (uint32_t)t;
+			rec.dwTimeStamp = DNS_COMMON_TIMESTAMP_STATIC;
 			rec.data.soa.serial = serial;
 
 			ndr_err = ndr_push_struct_blob(&el->values[i], mem_ctx, &rec,
@@ -401,7 +397,6 @@ WERROR dnsserver_db_add_record(TALLOC_CTX *mem_ctx,
 	struct ldb_message_element *el;
 	struct ldb_dn *dn;
 	enum ndr_err_code ndr_err;
-	NTTIME t;
 	int ret, i;
 	int serial;
 	bool was_tombstoned = false;
@@ -425,12 +420,8 @@ WERROR dnsserver_db_add_record(TALLOC_CTX *mem_ctx,
 		return WERR_INTERNAL_DB_ERROR;
 	}
 
-	unix_to_nt_time(&t, time(NULL));
-	t /= 10*1000*1000; /* convert to seconds (NT time is in 100ns units) */
-	t /= 3600;         /* convert to hours */
-
 	rec->dwSerial = serial;
-	rec->dwTimeStamp = t;
+	rec->dwTimeStamp = DNS_COMMON_TIMESTAMP_STATIC;
 
 	ret = ldb_search(samdb, mem_ctx, &res, z->zone_dn, LDB_SCOPE_ONELEVEL, attrs,
 			"(&(objectClass=dnsNode)(name=%s))", name);
@@ -517,7 +508,6 @@ WERROR dnsserver_db_update_record(TALLOC_CTX *mem_ctx,
 	struct dnsp_DnssrvRpcRecord *arec, *drec;
 	struct ldb_message_element *el;
 	enum ndr_err_code ndr_err;
-	NTTIME t;
 	int ret, i;
 	int serial;
 
@@ -527,10 +517,7 @@ WERROR dnsserver_db_update_record(TALLOC_CTX *mem_ctx,
 	drec = dns_to_dnsp_copy(mem_ctx, del_record);
 	W_ERROR_HAVE_NO_MEMORY(drec);
 
-	unix_to_nt_time(&t, time(NULL));
-	t /= 10*1000*1000;
-
-	arec->dwTimeStamp = t;
+	arec->dwTimeStamp = DNS_COMMON_TIMESTAMP_STATIC;
 
 	ret = ldb_search(samdb, mem_ctx, &res, z->zone_dn, LDB_SCOPE_ONELEVEL, attrs,
 			"(&(objectClass=dnsNode)(name=%s)(!(dNSTombstoned=TRUE)))", name);
@@ -865,7 +852,6 @@ WERROR dnsserver_db_create_zone(struct ldb_context *samdb,
 	struct dnsp_DnssrvRpcRecord *dns_rec;
 	struct dnsp_soa soa;
 	char *tmpstr, *server_fqdn, *soa_email;
-	NTTIME t;
 
 	/* We only support primary zones for now */
 	if (zone->zoneinfo->dwZoneType != DNS_ZONE_TYPE_PRIMARY) {
@@ -926,10 +912,6 @@ WERROR dnsserver_db_create_zone(struct ldb_context *samdb,
 	W_ERROR_HAVE_NO_MEMORY_AND_FREE(soa_email, tmp_ctx);
 	talloc_free(tmpstr);
 
-	unix_to_nt_time(&t, time(NULL));
-	t /= 10*1000*1000; /* convert to seconds (NT time is in 100ns units) */
-	t /= 3600;         /* convert to hours */
-
 	/* SOA Record - values same as defined in provision/sambadns.py */
 	soa.serial = 1;
 	soa.refresh = 900;
@@ -943,7 +925,7 @@ WERROR dnsserver_db_create_zone(struct ldb_context *samdb,
 	dns_rec[0].rank = DNS_RANK_ZONE;
 	dns_rec[0].dwSerial = soa.serial;
 	dns_rec[0].dwTtlSeconds = 3600;
-	dns_rec[0].dwTimeStamp = (uint32_t)t;
+	dns_rec[0].dwTimeStamp = DNS_COMMON_TIMESTAMP_STATIC;
 	dns_rec[0].data.soa = soa;
 
 	/* NS Record */
@@ -951,7 +933,7 @@ WERROR dnsserver_db_create_zone(struct ldb_context *samdb,
 	dns_rec[1].rank = DNS_RANK_ZONE;
 	dns_rec[1].dwSerial = soa.serial;
 	dns_rec[1].dwTtlSeconds = 3600;
-	dns_rec[1].dwTimeStamp = (uint32_t)t;
+	dns_rec[1].dwTimeStamp = DNS_COMMON_TIMESTAMP_STATIC;
 	dns_rec[1].data.ns = server_fqdn;
 
 	/* Add @ Record */
-- 
1.9.1


From 5a7d5f38d67e6c668f256e54562b6c8d13a6ed4d Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 25 Nov 2016 13:41:27 +0100
Subject: [PATCH 2/2] s4:dns_server: handle dwTimeStamp within
 dns_common_replace()

If we replace an existing record, we keep it static
or dynamic.

If we're at the zone root or don't have any static records
we add new records as dynamic.

In all other cases we create new records static (even for
dynamic updates).

The callers just set dwTimeStamp to
DNS_COMMON_TIMESTAMP_{STATIC,UPDATED,NEW}.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=12451

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 source4/dns_server/dlz_bind9.c        | 23 +++++++++-------
 source4/dns_server/dns_update.c       | 23 +++++++++++++++-
 source4/dns_server/dns_utils.c        |  4 ++-
 source4/dns_server/dnsserver_common.c | 51 ++++++++++++++++++++++++++++++++---
 source4/dns_server/dnsserver_common.h |  3 +++
 source4/dns_server/pydns.c            |  6 +++++
 6 files changed, 96 insertions(+), 14 deletions(-)

diff --git a/source4/dns_server/dlz_bind9.c b/source4/dns_server/dlz_bind9.c
index 4c21a5e..e61b76e 100644
--- a/source4/dns_server/dlz_bind9.c
+++ b/source4/dns_server/dlz_bind9.c
@@ -1546,7 +1546,8 @@ _PUBLIC_ isc_result_t dlz_addrdataset(const char *name, const char *rdatastr, vo
 	uint16_t num_recs = 0;
 	uint16_t first = 0;
 	uint16_t i;
-	NTTIME t;
+	struct timeval tv_now = timeval_current();
+	NTTIME now = timeval_to_nttime(&tv_now);
 	WERROR werr;
 
 	if (state->transaction_token != (void*)version) {
@@ -1559,12 +1560,7 @@ _PUBLIC_ isc_result_t dlz_addrdataset(const char *name, const char *rdatastr, vo
 		return ISC_R_NOMEMORY;
 	}
 
-	unix_to_nt_time(&t, time(NULL));
-	t /= 10*1000*1000; /* convert to seconds (NT time is in 100ns units) */
-	t /= 3600;         /* convert to hours */
-
 	rec->rank        = DNS_RANK_ZONE;
-	rec->dwTimeStamp = (uint32_t)t;
 
 	if (!b9_parse(state, rdatastr, rec)) {
 		state->log(ISC_LOG_INFO, "samba_dlz: failed to parse rdataset '%s'", rdatastr);
@@ -1626,6 +1622,11 @@ _PUBLIC_ isc_result_t dlz_addrdataset(const char *name, const char *rdatastr, vo
 			return ISC_R_NOMEMORY;
 		}
 		num_recs++;
+		rec->dwTimeStamp = DNS_COMMON_TIMESTAMP_NEW;
+	} else if (recs[i].dwTimeStamp == DNS_COMMON_TIMESTAMP_STATIC) {
+		rec->dwTimeStamp = DNS_COMMON_TIMESTAMP_STATIC;
+	} else {
+		rec->dwTimeStamp = DNS_COMMON_TIMESTAMP_UPDATED;
 	}
 
 	recs[i] = *rec;
@@ -1638,7 +1639,7 @@ _PUBLIC_ isc_result_t dlz_addrdataset(const char *name, const char *rdatastr, vo
 	/* modify the record */
 	werr = dns_common_replace(state->samdb, rec, dn,
 				  needs_add,
-				  state->soa_serial,
+				  state->soa_serial, now,
 				  recs, num_recs);
 	b9_reset_session_info(state);
 	if (!W_ERROR_IS_OK(werr)) {
@@ -1667,6 +1668,8 @@ _PUBLIC_ isc_result_t dlz_subrdataset(const char *name, const char *rdatastr, vo
 	struct dnsp_DnssrvRpcRecord *recs = NULL;
 	uint16_t num_recs = 0;
 	uint16_t i;
+	struct timeval tv_now = timeval_current();
+	NTTIME now = timeval_to_nttime(&tv_now);
 	WERROR werr;
 
 	if (state->transaction_token != (void*)version) {
@@ -1721,7 +1724,7 @@ _PUBLIC_ isc_result_t dlz_subrdataset(const char *name, const char *rdatastr, vo
 	/* modify the record */
 	werr = dns_common_replace(state->samdb, rec, dn,
 				  false,/* needs_add */
-				  state->soa_serial,
+				  state->soa_serial, now,
 				  recs, num_recs);
 	b9_reset_session_info(state);
 	if (!W_ERROR_IS_OK(werr)) {
@@ -1752,6 +1755,8 @@ _PUBLIC_ isc_result_t dlz_delrdataset(const char *name, const char *type, void *
 	struct dnsp_DnssrvRpcRecord *recs = NULL;
 	uint16_t num_recs = 0;
 	uint16_t ri = 0;
+	struct timeval tv_now = timeval_current();
+	NTTIME now = timeval_to_nttime(&tv_now);
 	WERROR werr;
 
 	if (state->transaction_token != (void*)version) {
@@ -1805,7 +1810,7 @@ _PUBLIC_ isc_result_t dlz_delrdataset(const char *name, const char *type, void *
 	/* modify the record */
 	werr = dns_common_replace(state->samdb, tmp_ctx, dn,
 				  false,/* needs_add */
-				  state->soa_serial,
+				  state->soa_serial, now,
 				  recs, num_recs);
 	b9_reset_session_info(state);
 	if (!W_ERROR_IS_OK(werr)) {
diff --git a/source4/dns_server/dns_update.c b/source4/dns_server/dns_update.c
index bb41a9c..3b514b0 100644
--- a/source4/dns_server/dns_update.c
+++ b/source4/dns_server/dns_update.c
@@ -429,15 +429,26 @@ static WERROR handle_one_update(struct dns_server *dns,
 
 	if (update->rr_class == zone->question_class) {
 		if (update->rr_type == DNS_QTYPE_CNAME) {
+			uint32_t new_timestamp = DNS_COMMON_TIMESTAMP_NEW;
+
 			/*
 			 * If there is a record in the directory
 			 * that's not a CNAME, ignore update
 			 */
 			for (i = first; i < rcount; i++) {
-				if (recs[i].wType != DNS_TYPE_CNAME) {
+				struct dnsp_DnssrvRpcRecord *r = &recs[i];
+
+				if (r->wType != DNS_TYPE_CNAME) {
 					DEBUG(5, ("Skipping update\n"));
 					return WERR_OK;
 				}
+				if (r->dwTimeStamp == DNS_COMMON_TIMESTAMP_STATIC) {
+					new_timestamp =
+						DNS_COMMON_TIMESTAMP_STATIC;
+				} else {
+					new_timestamp =
+						DNS_COMMON_TIMESTAMP_UPDATED;
+				}
 				break;
 			}
 
@@ -453,6 +464,7 @@ static WERROR handle_one_update(struct dns_server *dns,
 
 			werror = dns_rr_to_dnsp(recs, update, &recs[rcount]);
 			W_ERROR_NOT_OK_RETURN(werror);
+			recs[rcount].dwTimeStamp = new_timestamp;
 			rcount += 1;
 
 			werror = dns_replace_records(dns, mem_ctx, dn,
@@ -529,12 +541,21 @@ static WERROR handle_one_update(struct dns_server *dns,
 
 		werror = dns_rr_to_dnsp(recs, update, &recs[rcount]);
 		W_ERROR_NOT_OK_RETURN(werror);
+		recs[rcount].dwTimeStamp = DNS_COMMON_TIMESTAMP_NEW;
 
 		for (i = first; i < rcount; i++) {
 			if (!dns_records_match(&recs[i], &recs[rcount])) {
 				continue;
 			}
 
+			if (recs[i].dwTimeStamp == DNS_COMMON_TIMESTAMP_STATIC) {
+				recs[rcount].dwTimeStamp =
+					DNS_COMMON_TIMESTAMP_STATIC;
+			} else {
+				recs[rcount].dwTimeStamp =
+					DNS_COMMON_TIMESTAMP_UPDATED;
+			}
+
 			recs[i] = recs[rcount];
 
 			werror = dns_replace_records(dns, mem_ctx, dn,
diff --git a/source4/dns_server/dns_utils.c b/source4/dns_server/dns_utils.c
index c728eaa..3a82430 100644
--- a/source4/dns_server/dns_utils.c
+++ b/source4/dns_server/dns_utils.c
@@ -126,8 +126,10 @@ WERROR dns_replace_records(struct dns_server *dns,
 {
 	/* TODO: Autogenerate this somehow */
 	uint32_t dwSerial = 110;
+	struct timeval tv_now = timeval_current();
+	NTTIME now = timeval_to_nttime(&tv_now);
 	return dns_common_replace(dns->samdb, mem_ctx, dn,
-				  needs_add, dwSerial, records, rec_count);
+				  needs_add, dwSerial, now, records, rec_count);
 }
 
 bool dns_authoritative_for_zone(struct dns_server *dns,
diff --git a/source4/dns_server/dnsserver_common.c b/source4/dns_server/dnsserver_common.c
index db42bcb..b9b6d977 100644
--- a/source4/dns_server/dnsserver_common.c
+++ b/source4/dns_server/dnsserver_common.c
@@ -219,6 +219,7 @@ WERROR dns_common_replace(struct ldb_context *samdb,
 			  struct ldb_dn *dn,
 			  bool needs_add,
 			  uint32_t serial,
+			  NTTIME now,
 			  struct dnsp_DnssrvRpcRecord *records,
 			  uint16_t rec_count)
 {
@@ -228,6 +229,17 @@ WERROR dns_common_replace(struct ldb_context *samdb,
 	struct ldb_message *msg = NULL;
 	bool was_tombstoned = false;
 	bool become_tombstoned = false;
+	uint16_t num_static = 0;
+	uint16_t num_soa = 0;
+	NTTIME t;
+	uint32_t updated_timestamp;
+	uint32_t new_timestamp = 0;
+
+	t = now;
+	t /= 10*1000*1000; /* convert to seconds (NT time is in 100ns units) */
+	t /= 3600;         /* convert to hours */
+
+	updated_timestamp = (uint32_t)t;
 
 	msg = ldb_msg_new(mem_ctx);
 	W_ERROR_HAVE_NO_MEMORY(msg);
@@ -256,6 +268,31 @@ WERROR dns_common_replace(struct ldb_context *samdb,
 	}
 
 	for (i = 0; i < rec_count; i++) {
+		if (records[i].wType == DNS_TYPE_TOMBSTONE) {
+			continue;
+		}
+
+		if (records[i].wType == DNS_TYPE_SOA) {
+			num_soa += 1;
+			continue;
+		}
+
+		if (records[i].dwTimeStamp == DNS_COMMON_TIMESTAMP_STATIC) {
+			num_static += 1;
+			continue;
+		}
+	}
+
+	if (num_soa != 0 || num_static == 0) {
+		/*
+		 * If this is the zone root or
+		 * if there're no existing static records
+		 * we let new record get the current time.
+		 */
+		new_timestamp = updated_timestamp;
+	}
+
+	for (i = 0; i < rec_count; i++) {
 		struct ldb_val *v = &el->values[el->num_values];
 		enum ndr_err_code ndr_err;
 
@@ -267,6 +304,16 @@ WERROR dns_common_replace(struct ldb_context *samdb,
 		}
 
 		records[i].dwSerial = serial;
+
+		switch (records[i].dwTimeStamp) {
+		case DNS_COMMON_TIMESTAMP_NEW:
+			records[i].dwTimeStamp = new_timestamp;
+			break;
+		case DNS_COMMON_TIMESTAMP_UPDATED:
+			records[i].dwTimeStamp = updated_timestamp;
+			break;
+		}
+
 		ndr_err = ndr_push_struct_blob(v, el->values, &records[i],
 				(ndr_push_flags_fn_t)ndr_push_dnsp_DnssrvRpcRecord);
 		if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) {
@@ -298,7 +345,6 @@ WERROR dns_common_replace(struct ldb_context *samdb,
 		struct dnsp_DnssrvRpcRecord tbs;
 		struct ldb_val *v = &el->values[el->num_values];
 		enum ndr_err_code ndr_err;
-		struct timeval tv;
 
 		if (was_tombstoned) {
 			/*
@@ -308,11 +354,10 @@ WERROR dns_common_replace(struct ldb_context *samdb,
 			return WERR_OK;
 		}
 
-		tv = timeval_current();
 		tbs = (struct dnsp_DnssrvRpcRecord) {
 			.wType = DNS_TYPE_TOMBSTONE,
 			.dwSerial = serial,
-			.data.timestamp = timeval_to_nttime(&tv),
+			.data.timestamp = now,
 		};
 
 		ndr_err = ndr_push_struct_blob(v, el->values, &tbs,
diff --git a/source4/dns_server/dnsserver_common.h b/source4/dns_server/dnsserver_common.h
index ce13da8..f9ecc55 100644
--- a/source4/dns_server/dnsserver_common.h
+++ b/source4/dns_server/dnsserver_common.h
@@ -48,12 +48,15 @@ WERROR dns_common_lookup(struct ldb_context *samdb,
 			 bool *tombstoned);
 
 #define DNS_COMMON_TIMESTAMP_STATIC  ((uint32_t)0)
+#define DNS_COMMON_TIMESTAMP_UPDATED ((uint32_t)UINT32_MAX-1)
+#define DNS_COMMON_TIMESTAMP_NEW     ((uint32_t)UINT32_MAX-2)
 
 WERROR dns_common_replace(struct ldb_context *samdb,
 			  TALLOC_CTX *mem_ctx,
 			  struct ldb_dn *dn,
 			  bool needs_add,
 			  uint32_t serial,
+			  NTTIME now,
 			  struct dnsp_DnssrvRpcRecord *records,
 			  uint16_t rec_count);
 bool dns_name_match(const char *zone, const char *name, size_t *host_part_len);
diff --git a/source4/dns_server/pydns.c b/source4/dns_server/pydns.c
index 9842f24..c0be734 100644
--- a/source4/dns_server/pydns.c
+++ b/source4/dns_server/pydns.c
@@ -202,6 +202,8 @@ static PyObject *py_dsdb_dns_replace(PyObject *self, PyObject *args)
 	 * dns_common_replace()
 	 */
 	static const int serial = 110;
+	struct timeval tv_now = timeval_current();
+	NTTIME now = timeval_to_nttime(&tv_now);
 
 	if (!PyArg_ParseTuple(args, "OsO", &py_ldb, &dns_name, &py_dns_records)) {
 		return NULL;
@@ -234,6 +236,7 @@ static PyObject *py_dsdb_dns_replace(PyObject *self, PyObject *args)
 				  dn,
 				  false, /* Not adding a record */
 				  serial,
+				  now,
 				  records,
 				  num_records);
 	if (!W_ERROR_IS_OK(werr)) {
@@ -261,6 +264,8 @@ static PyObject *py_dsdb_dns_replace_by_dn(PyObject *self, PyObject *args)
 	 * dns_common_replace()
 	 */
 	static const int serial = 110;
+	struct timeval tv_now = timeval_current();
+	NTTIME now = timeval_to_nttime(&tv_now);
 
 	if (!PyArg_ParseTuple(args, "OOO", &py_ldb, &py_dn, &py_dns_records)) {
 		return NULL;
@@ -283,6 +288,7 @@ static PyObject *py_dsdb_dns_replace_by_dn(PyObject *self, PyObject *args)
 				  dn,
 				  false, /* Not adding a record */
 				  serial,
+				  now,
 				  records,
 				  num_records);
 	if (!W_ERROR_IS_OK(werr)) {
-- 
1.9.1

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20161130/12d589b2/signature.sig>


More information about the samba-technical mailing list