Attachment 15758 Details for Bug 14258 – patch for master to fix this issue (with tests!)

[patch] patch for master to fix this issue (with tests!)

abartlet-dns-scavanage-dNSTombstoned.patch (text/plain), 5.52 KB, created by Andrew Bartlett on 2020-01-31 03:10:27 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Andrew Bartlett

Created: 2020-01-31 03:10:27 UTC

Size: 5.52 KB

patch

obsolete

>From af606d18a57da2969045ec632d59c25b900e1f84 Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Thu, 30 Jan 2020 16:41:39 +1300
>Subject: [PATCH 1/2] dsdb: Correctly handle memory in objectclass_attrs
>
>el->values is caller-provided memory that should be thought of as constant,
>it should not be assumed to be a talloc context.
>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14258
>---
> .../dsdb/samdb/ldb_modules/objectclass_attrs.c  | 17 ++++++++++++++++-
> 1 file changed, 16 insertions(+), 1 deletion(-)
>
>diff --git a/source4/dsdb/samdb/ldb_modules/objectclass_attrs.c b/source4/dsdb/samdb/ldb_modules/objectclass_attrs.c
>index e5f86d260f4..0b9725e2767 100644
>--- a/source4/dsdb/samdb/ldb_modules/objectclass_attrs.c
>+++ b/source4/dsdb/samdb/ldb_modules/objectclass_attrs.c
>@@ -133,7 +133,16 @@ static int oc_auto_normalise(struct ldb_context *ldb, const struct dsdb_attribut
> 	for (i=0; i<el->num_values; i++) {
> 		struct ldb_val v;
> 		int ret;
>-		ret = attr->ldb_schema_attribute->syntax->canonicalise_fn(ldb, el->values, &el->values[i], &v);
>+		/*
>+		 * We use msg->elements (owned by this module due to
>+		 * ldb_msg_copy_shallow()) as a memory context and
>+		 * then steal from there to the right spot if we don't
>+		 * free it.
>+		 */
>+		ret = attr->ldb_schema_attribute->syntax->canonicalise_fn(ldb,
>+									  msg->elements,
>+									  &el->values[i],
>+									  &v);
> 		if (ret != LDB_SUCCESS) {
> 			return ret;
> 		}
>@@ -156,6 +165,12 @@ static int oc_auto_normalise(struct ldb_context *ldb, const struct dsdb_attribut
> 		}
> 
> 		el->values[i] = v;
>+
>+		/*
>+		 * By now el->values is a talloc pointer under
>+		 * msg->elements and may now be used
>+		 */
>+		talloc_steal(el->values, v.data);
> 	}
> 	return LDB_SUCCESS;
> }
>-- 
>2.17.1
>
>
>From 882e233853109e33161344b16cc1deba9d5db335 Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Thu, 30 Jan 2020 16:44:05 +1300
>Subject: [PATCH 2/2] selftest: Test behaviour of DNS scavenge with an existing
> dNSTombstoned value
>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14258
>---
> python/samba/tests/dns.py | 39 +++++++++++++++++++++++++++++++++++++++
> 1 file changed, 39 insertions(+)
>
>diff --git a/python/samba/tests/dns.py b/python/samba/tests/dns.py
>index 1dd1f549a33..bc05076c615 100644
>--- a/python/samba/tests/dns.py
>+++ b/python/samba/tests/dns.py
>@@ -1523,26 +1523,51 @@ class TestZones(DNSTest):
>         name, txt = 'agingtest', ['test txt']
>         name2, txt2 = 'agingtest2', ['test txt2']
>         name3, txt3 = 'agingtest3', ['test txt3']
>+        name4, txt4 = 'agingtest4', ['test txt4']
>+        name5, txt5 = 'agingtest5', ['test txt5']
>         self.dns_update_record(name, txt)
>         self.dns_update_record(name2, txt)
>         self.dns_update_record(name2, txt2)
>         self.dns_update_record(name3, txt)
>         self.dns_update_record(name3, txt2)
>+
>+        # Create a tomb stoned record.
>+        self.dns_update_record(name4, txt4)
>+        self.dns_tombstone(name4, txt4, self.zone)
>+        records = self.ldap_get_records(name4)
>+        self.assertTrue("dNSTombstoned" in records[0])
>+        self.assertEqual(records[0]["dNSTombstoned"][0], b"TRUE")
>+
>+        # Create an un-tombstoned record, with dnsTombstoned: FALSE
>+        self.dns_update_record(name5, txt5)
>+        self.dns_tombstone(name5, txt5, self.zone)
>+        self.dns_update_record(name5, txt5)
>+        records = self.ldap_get_records(name5)
>+        self.assertTrue("dNSTombstoned" in records[0])
>+        self.assertEqual(records[0]["dNSTombstoned"][0], b"FALSE")
>+
>         last_add = self.dns_update_record(name3, txt3)
> 
>         def mod_ts(rec):
>             self.assertTrue(rec.dwTimeStamp > 0)
>             if rec.data.str == txt:
>                 rec.dwTimeStamp -= interval * 5
>+
>+        def mod_ts_all(rec):
>+            rec.dwTimeStamp -= interval * 5
>         self.ldap_modify_dnsrecs(name, mod_ts)
>         self.ldap_modify_dnsrecs(name2, mod_ts)
>         self.ldap_modify_dnsrecs(name3, mod_ts)
>+        self.ldap_modify_dnsrecs(name5, mod_ts_all)
>         self.assertTrue(callable(getattr(dsdb, '_scavenge_dns_records', None)))
>         dsdb._scavenge_dns_records(self.samdb)
> 
>         recs = self.ldap_get_dns_records(name)
>         self.assertEqual(len(recs), 1)
>         self.assertEqual(recs[0].wType, dnsp.DNS_TYPE_TOMBSTONE)
>+        records = self.ldap_get_records(name)
>+        self.assertTrue("dNSTombstoned" in records[0])
>+        self.assertEqual(records[0]["dNSTombstoned"][0], b"TRUE")
> 
>         recs = self.ldap_get_dns_records(name2)
>         self.assertEqual(len(recs), 1)
>@@ -1556,6 +1581,20 @@ class TestZones(DNSTest):
>         self.assertEqual(recs[0].wType, dnsp.DNS_TYPE_TXT)
>         self.assertEqual(recs[1].wType, dnsp.DNS_TYPE_TXT)
> 
>+        recs = self.ldap_get_dns_records(name4)
>+        self.assertEqual(len(recs), 1)
>+        self.assertEqual(recs[0].wType, dnsp.DNS_TYPE_TOMBSTONE)
>+        records = self.ldap_get_records(name4)
>+        self.assertTrue("dNSTombstoned" in records[0])
>+        self.assertEqual(records[0]["dNSTombstoned"][0], b"TRUE")
>+
>+        recs = self.ldap_get_dns_records(name5)
>+        self.assertEqual(len(recs), 1)
>+        self.assertEqual(recs[0].wType, dnsp.DNS_TYPE_TOMBSTONE)
>+        records = self.ldap_get_records(name5)
>+        self.assertTrue("dNSTombstoned" in records[0])
>+        self.assertEqual(records[0]["dNSTombstoned"][0], b"TRUE")
>+
>         for make_it_work in [False, True]:
>             inc = -1 if make_it_work else 1
> 
>-- 
>2.17.1
>

Actions: View

Attachments on bug 14258: 15758 | 15803 | 15804 | 15805