The Samba-Bugzilla – Attachment 15984 Details for
Bug 14310
Can't use MMC DNS Manager with Samba 4.12
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
backported patch for 4.12 (only)
dnsproperty-4.12.patch (text/plain), 16.01 KB, created by
Andrew Bartlett
on 2020-05-15 08:38:58 UTC
(
hide
)
Description:
backported patch for 4.12 (only)
Filename:
MIME Type:
Creator:
Andrew Bartlett
Created:
2020-05-15 08:38:58 UTC
Size:
16.01 KB
patch
obsolete
>From ec23e960b0d22cab68116df36cbd9316b5c264c4 Mon Sep 17 00:00:00 2001 >From: Andrew Bartlett <abartlet@samba.org> >Date: Thu, 14 May 2020 10:21:19 +1200 >Subject: [PATCH 1/3] librpc/idl: Add dnsp_DnsProperty_short > >This will be used by a test and the DNS server code to parse short dnsProperty >records which come from Windows servers. > >This example is from the value that caused Samba to fail as it >can not be parsed as a normal dnsp_DnsProperty > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=14310 > >Signed-off-by: Andrew Bartlett <abartlet@samba.org> >Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz> >(backported to 4.12 from commit 87bf1d687fe7b48a7b6d511dfc7f5414db16119c) > >[abartlet@samba.org: resolve conflict due to less ndrdump tests in 4.12] >--- > librpc/idl/dnsp.idl | 16 ++++++++++++++++ > python/samba/tests/blackbox/ndrdump.py | 21 +++++++++++++++++++++ > 2 files changed, 37 insertions(+) > >diff --git a/librpc/idl/dnsp.idl b/librpc/idl/dnsp.idl >index 814d573cddf..2fb45a217a4 100644 >--- a/librpc/idl/dnsp.idl >+++ b/librpc/idl/dnsp.idl >@@ -260,4 +260,20 @@ interface dnsp > [switch_is(id)] dnsPropertyData data; > uint32 name; > } dnsp_DnsProperty; >+ >+ /* >+ * this is the format for the dnsProperty attribute in the DNS >+ * partitions in AD when the wDataLength is 0. This is an >+ * invalid format seen from some Windows servers in the same >+ * domain. >+ */ >+ typedef [flag(NDR_NOALIGN),public] struct { >+ [range(0, 0), value(0)] uint32 wDataLength; >+ uint32 namelength; >+ [value(0)] uint32 flag; >+ [value(1)] uint32 version; >+ dns_property_id id; >+ [switch_is(DSPROPERTY_ZONE_EMPTY)] dnsPropertyData data; >+ uint32 name; >+ } dnsp_DnsProperty_short; > } >diff --git a/python/samba/tests/blackbox/ndrdump.py b/python/samba/tests/blackbox/ndrdump.py >index b3c837819b1..6795aed41b7 100644 >--- a/python/samba/tests/blackbox/ndrdump.py >+++ b/python/samba/tests/blackbox/ndrdump.py >@@ -437,3 +437,24 @@ dump OK > self.fail(e) > > self.assertEqual(actual, expected) >+ >+ def test_ndrdump_short_dnsProperty(self): >+ expected = b'''pull returned Success >+ dnsp_DnsProperty_short: struct dnsp_DnsProperty_short >+ wDataLength : 0x00000000 (0) >+ namelength : 0x00000000 (0) >+ flag : 0x00000000 (0) >+ version : 0x00000001 (1) >+ id : DSPROPERTY_ZONE_NS_SERVERS_DA (146) >+ data : union dnsPropertyData(case 0) >+ name : 0x00000000 (0) >+dump OK >+''' >+ command = ( >+ "ndrdump dnsp dnsp_DnsProperty_short struct --base64-input " >+ "--input AAAAAAAAAAAAAAAAAQAAAJIAAAAAAAAA") >+ try: >+ actual = self.check_output(command) >+ except BlackboxProcessError as e: >+ self.fail(e) >+ self.assertEqual(actual, expected) >-- >2.17.1 > > >From af3257452a68e3a859fa38f84139b3eeb7dcf9ca Mon Sep 17 00:00:00 2001 >From: Andrew Bartlett <abartlet@samba.org> >Date: Thu, 14 May 2020 10:19:45 +1200 >Subject: [PATCH 2/3] selftest: Add test for handling of "short" dnsProperty > records > >These have been known to be given by Windows DCs that share the same domain >as while invalid, they are not format-checked inbound when set by the DNS >Manager MMC applet over the dnsserver pipe to Windows. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=14310 > >Signed-off-by: Andrew Bartlett <abartlet@samba.org> >Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz> >(cherry picked from commit 6eb2a48f5a998b82bb071ef42d00d2f34a2b0ed8) >--- > python/samba/tests/dns.py | 51 +++++++++++++++++++++++++++++++++++++++ > selftest/knownfail.d/dns | 12 +++++++++ > 2 files changed, 63 insertions(+) > >diff --git a/python/samba/tests/dns.py b/python/samba/tests/dns.py >index bc05076c615..d06cf0b0b19 100644 >--- a/python/samba/tests/dns.py >+++ b/python/samba/tests/dns.py >@@ -1702,6 +1702,57 @@ class TestZones(DNSTest): > self.assert_dns_opcode_equals(response, dns.DNS_OPCODE_QUERY) > self.assertEquals(response.ancount, 0) > >+ def set_dnsProperty_zero_length(self, dnsproperty_id): >+ records = self.samdb.search(base=self.zone_dn, scope=ldb.SCOPE_BASE, >+ expression="(&(objectClass=dnsZone)" + >+ "(name={0}))".format(self.zone), >+ attrs=["dNSProperty"]) >+ self.assertEqual(len(records), 1) >+ props = [ndr_unpack(dnsp.DnsProperty, r) >+ for r in records[0].get('dNSProperty')] >+ new_props = [ndr.ndr_pack(p) for p in props if p.id == dnsproperty_id] >+ >+ zero_length_p = dnsp.DnsProperty_short() >+ zero_length_p.id = dnsproperty_id >+ zero_length_p.namelength = 1 >+ zero_length_p.name = 1 >+ new_props += [ndr.ndr_pack(zero_length_p)] >+ >+ dn = records[0].dn >+ update_dict = {'dn': dn, 'dnsProperty': new_props} >+ self.samdb.modify(ldb.Message.from_dict(self.samdb, >+ update_dict, >+ ldb.FLAG_MOD_REPLACE)) >+ >+ def test_update_while_dnsProperty_zero_length(self): >+ self.create_zone(self.zone) >+ self.set_dnsProperty_zero_length(dnsp.DSPROPERTY_ZONE_ALLOW_UPDATE) >+ rec = self.dns_update_record('dnspropertytest', ['test txt']) >+ self.assertNotEqual(rec.dwTimeStamp, 0) >+ >+ def test_enum_zones_while_dnsProperty_zero_length(self): >+ self.create_zone(self.zone) >+ self.set_dnsProperty_zero_length(dnsp.DSPROPERTY_ZONE_ALLOW_UPDATE) >+ client_version = dnsserver.DNS_CLIENT_VERSION_LONGHORN >+ request_filter = dnsserver.DNS_ZONE_REQUEST_PRIMARY >+ tid = dnsserver.DNSSRV_TYPEID_DWORD >+ typeid, res = self.rpc_conn.DnssrvComplexOperation2(client_version, >+ 0, >+ self.server_ip, >+ None, >+ 'EnumZones', >+ tid, >+ request_filter) >+ >+ def test_rpc_zone_update_while_dnsProperty_zero_length(self): >+ self.create_zone(self.zone) >+ self.set_dnsProperty_zero_length(dnsp.DSPROPERTY_ZONE_ALLOW_UPDATE) >+ self.set_params(zone=self.zone, AllowUpdate=dnsp.DNS_ZONE_UPDATE_SECURE) >+ >+ def test_rpc_zone_update_while_other_dnsProperty_zero_length(self): >+ self.create_zone(self.zone) >+ self.set_dnsProperty_zero_length(dnsp.DSPROPERTY_ZONE_MASTER_SERVERS_DA) >+ self.set_params(zone=self.zone, AllowUpdate=dnsp.DNS_ZONE_UPDATE_SECURE) > > class TestRPCRoundtrip(DNSTest): > def setUp(self): >diff --git a/selftest/knownfail.d/dns b/selftest/knownfail.d/dns >index 70a719a818a..3e7e8c65de2 100644 >--- a/selftest/knownfail.d/dns >+++ b/selftest/knownfail.d/dns >@@ -88,3 +88,15 @@ samba.tests.dns.__main__.TestSimpleQueries.test_one_SOA_query\(rodc:local\) > ^samba.tests.dns.__main__.TestComplexQueries.test_cname_limit\(rodc:local\) > ^samba.tests.dns.__main__.TestComplexQueries.test_cname_any_query\(vampire_dc:local\) > ^samba.tests.dns.__main__.TestComplexQueries.test_cname_any_query\(rodc:local\) >+ >+# Tests for the dnsProperty parse issue >+^samba.tests.dns.__main__.TestZones.test_rpc_zone_update_while_dnsProperty_zero_length\(fl2003dc:local\) >+^samba.tests.dns.__main__.TestZones.test_rpc_zone_update_while_other_dnsProperty_zero_length\(fl2003dc:local\) >+^samba.tests.dns.__main__.TestZones.test_update_while_dnsProperty_zero_length\(fl2003dc:local\) >+^samba.tests.dns.__main__.TestZones.test_enum_zones_while_dnsProperty_zero_length\(rodc:local\) >+^samba.tests.dns.__main__.TestZones.test_rpc_zone_update_while_dnsProperty_zero_length\(rodc:local\) >+^samba.tests.dns.__main__.TestZones.test_rpc_zone_update_while_other_dnsProperty_zero_length\(rodc:local\) >+^samba.tests.dns.__main__.TestZones.test_update_while_dnsProperty_zero_length\(rodc:local\) >+^samba.tests.dns.__main__.TestZones.test_rpc_zone_update_while_dnsProperty_zero_length\(vampire_dc:local\) >+^samba.tests.dns.__main__.TestZones.test_rpc_zone_update_while_other_dnsProperty_zero_length\(vampire_dc:local\) >+^samba.tests.dns.__main__.TestZones.test_update_while_dnsProperty_zero_length\(vampire_dc:local\) >\ No newline at end of file >-- >2.17.1 > > >From fbbedbc99c9a7143a2631d444bf94043d825d4bf Mon Sep 17 00:00:00 2001 >From: Andrew Bartlett <abartlet@samba.org> >Date: Wed, 13 May 2020 12:01:05 +1200 >Subject: [PATCH 3/3] s4/rpc_server/dnsserver: Allow parsing of dnsProperty to > fail gracefully > >On (eg) the > >DC=_msdcs.X.Y,CN=MicrosoftDNS,DC=ForestDnsZones,DC=X,DC=Y > >record, in domains that have had a Microsoft Windows DC an attribute: > >dNSProperty:: AAAAAAAAAAAAAAAAAQAAAJIAAAAAAAAA > >000000 00 00 00 00 00 00 00 00 00 00 00 00 01 00 00 00 >................< >000010 92 00 00 00 00 00 00 00 >........< >000018 > >We, until samba 4.12, would parse this as: > >pull returned Success > dnsp_DnsProperty: struct dnsp_DnsProperty > wDataLength : 0x00000000 (0) > namelength : 0x00000000 (0) > flag : 0x00000000 (0) > version : 0x00000001 (1) > id : DSPROPERTY_ZONE_NS_SERVERS_DA (146) > data : union dnsPropertyData(case 0) > name : 0x00000000 (0) >dump OK > >However, the wDataLength is 0. There is not anything in >[MS-DNSP] 2.3.2.1 dnsProperty to describe any special behaviour >for when the id suggests that there is a value, but wDataLength is 0. > >https://docs.microsoft.com/en-us/openspecs/windows_protocols/ms-dnsp/445c7843-e4a1-4222-8c0f-630c230a4c80 > >We now fail to parse it, because we expect an entry with id DSPROPERTY_ZONE_NS_SERVERS_DA >to therefore have a valid DNS_ADDR_ARRAY (section 2.2.3.2.3). > >As context we changed it in our commit fee5c6a4247aeac71318186bbff7708d25de5912 >because of bug https://bugzilla.samba.org/show_bug.cgi?id=14206 >which was due to the artificial environment of the fuzzer. > >Microsoft advises that Windows also fails to parse this, but >instead of failing the operation, the value is ignored. > >Reported by Alex MacCuish. Many thanks for your assistance in >tracking down the issue. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=14310 >RN: Can't use DNS functionality after a Windows DC has been in domain > >Signed-off-by: Andrew Bartlett <abartlet@samba.org> >Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz> > >Autobuild-User(master): Andrew Bartlett <abartlet@samba.org> >Autobuild-Date(master): Fri May 15 07:29:17 UTC 2020 on sn-devel-184 > >(cherry picked from commit 004e7a1fee766102de302e83f4dc5f4d977aef32) >--- > selftest/knownfail.d/dns | 7 +-- > source4/dns_server/dnsserver_common.c | 9 +++- > source4/rpc_server/dnsserver/dnsdb.c | 72 ++++++++++++++++++++++++--- > 3 files changed, 74 insertions(+), 14 deletions(-) > >diff --git a/selftest/knownfail.d/dns b/selftest/knownfail.d/dns >index 3e7e8c65de2..fd1a78e9b5e 100644 >--- a/selftest/knownfail.d/dns >+++ b/selftest/knownfail.d/dns >@@ -89,14 +89,9 @@ samba.tests.dns.__main__.TestSimpleQueries.test_one_SOA_query\(rodc:local\) > ^samba.tests.dns.__main__.TestComplexQueries.test_cname_any_query\(vampire_dc:local\) > ^samba.tests.dns.__main__.TestComplexQueries.test_cname_any_query\(rodc:local\) > >-# Tests for the dnsProperty parse issue >-^samba.tests.dns.__main__.TestZones.test_rpc_zone_update_while_dnsProperty_zero_length\(fl2003dc:local\) >-^samba.tests.dns.__main__.TestZones.test_rpc_zone_update_while_other_dnsProperty_zero_length\(fl2003dc:local\) >-^samba.tests.dns.__main__.TestZones.test_update_while_dnsProperty_zero_length\(fl2003dc:local\) >+# Tests for the dnsProperty parse issue do not pass here, but do against fl2003dc > ^samba.tests.dns.__main__.TestZones.test_enum_zones_while_dnsProperty_zero_length\(rodc:local\) > ^samba.tests.dns.__main__.TestZones.test_rpc_zone_update_while_dnsProperty_zero_length\(rodc:local\) > ^samba.tests.dns.__main__.TestZones.test_rpc_zone_update_while_other_dnsProperty_zero_length\(rodc:local\) > ^samba.tests.dns.__main__.TestZones.test_update_while_dnsProperty_zero_length\(rodc:local\) >-^samba.tests.dns.__main__.TestZones.test_rpc_zone_update_while_dnsProperty_zero_length\(vampire_dc:local\) >-^samba.tests.dns.__main__.TestZones.test_rpc_zone_update_while_other_dnsProperty_zero_length\(vampire_dc:local\) > ^samba.tests.dns.__main__.TestZones.test_update_while_dnsProperty_zero_length\(vampire_dc:local\) >\ No newline at end of file >diff --git a/source4/dns_server/dnsserver_common.c b/source4/dns_server/dnsserver_common.c >index 420d141202e..8635d075be8 100644 >--- a/source4/dns_server/dnsserver_common.c >+++ b/source4/dns_server/dnsserver_common.c >@@ -901,7 +901,14 @@ WERROR dns_get_zone_properties(struct ldb_context *samdb, > prop, > (ndr_pull_flags_fn_t)ndr_pull_dnsp_DnsProperty); > if (!NDR_ERR_CODE_IS_SUCCESS(err)) { >- return DNS_ERR(SERVER_FAILURE); >+ /* >+ * If we can't pull it, then there is no valid >+ * data to load into the zone, so ignore this >+ * as Micosoft does. Windows can load an >+ * invalid property with a zero length into >+ * the dnsProperty attribute. >+ */ >+ continue; > } > > valid_property = >diff --git a/source4/rpc_server/dnsserver/dnsdb.c b/source4/rpc_server/dnsserver/dnsdb.c >index 9ee50d8ff39..798c869efe5 100644 >--- a/source4/rpc_server/dnsserver/dnsdb.c >+++ b/source4/rpc_server/dnsserver/dnsdb.c >@@ -156,7 +156,24 @@ struct dnsserver_zone *dnsserver_db_enumerate_zones(TALLOC_CTX *mem_ctx, > (ndr_pull_flags_fn_t) > ndr_pull_dnsp_DnsProperty); > if (!NDR_ERR_CODE_IS_SUCCESS(err)){ >- goto failed; >+ /* >+ * Per Microsoft we must >+ * ignore invalid data here >+ * and continue as a Windows >+ * server can put in a >+ * structure with an invalid >+ * length. >+ * >+ * We can safely fill in an >+ * extra empty property here >+ * because >+ * dns_zoneinfo_load_zone_property() >+ * just ignores >+ * DSPROPERTY_ZONE_EMPTY >+ */ >+ ZERO_STRUCT(props[j]); >+ props[j].id = DSPROPERTY_ZONE_EMPTY; >+ continue; > } > } > z->tmp_props = props; >@@ -812,12 +829,53 @@ WERROR dnsserver_db_do_reset_dword(struct ldb_context *samdb, > prop, > (ndr_pull_flags_fn_t)ndr_pull_dnsp_DnsProperty); > if (!NDR_ERR_CODE_IS_SUCCESS(err)){ >- DBG_ERR("dnsserver: couldn't PULL dns property id " >- "%d in zone %s\n", >- prop->id, >- ldb_dn_get_linearized(z->zone_dn)); >- TALLOC_FREE(tmp_ctx); >- return WERR_INTERNAL_DB_ERROR; >+ /* >+ * If we can't pull it then try again parsing >+ * it again. A Windows server in the domain >+ * will permit the addition of an invalidly >+ * formed property with a 0 length and cause a >+ * failure here >+ */ >+ struct dnsp_DnsProperty_short >+ *short_property >+ = talloc_zero(element, >+ struct dnsp_DnsProperty_short); >+ if (short_property == NULL) { >+ TALLOC_FREE(tmp_ctx); >+ return WERR_NOT_ENOUGH_MEMORY; >+ } >+ err = ndr_pull_struct_blob_all( >+ &(element->values[i]), >+ tmp_ctx, >+ short_property, >+ (ndr_pull_flags_fn_t)ndr_pull_dnsp_DnsProperty_short); >+ if (!NDR_ERR_CODE_IS_SUCCESS(err)) { >+ /* >+ * Unknown invalid data should be >+ * ignored and logged to match Windows >+ * behaviour >+ */ >+ DBG_NOTICE("dnsserver: couldn't PULL " >+ "dnsProperty value#%d in " >+ "zone %s while trying to " >+ "reset id %d\n", >+ i, >+ ldb_dn_get_linearized(z->zone_dn), >+ prop_id); >+ continue; >+ } >+ >+ /* >+ * Initialise the parts of the property not >+ * overwritten by value() in the IDL for >+ * re-push >+ */ >+ *prop = (struct dnsp_DnsProperty){ >+ .namelength = short_property->namelength, >+ .id = short_property->id, >+ .name = short_property->name >+ /* .data will be filled in below */ >+ }; > } > > if (prop->id == prop_id) { >-- >2.17.1 >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Raw
Flags:
abartlet
:
review?
(
gary
)
gary
:
review+
Actions:
View
Attachments on
bug 14310
:
15838
| 15984