[SCM] Samba Shared Repository - branch master updated
Andrew Bartlett
abartlet at samba.org
Fri May 15 07:30:03 UTC 2020
The branch, master has been updated
via 004e7a1fee7 s4/rpc_server/dnsserver: Allow parsing of dnsProperty to fail gracefully
via 6eb2a48f5a9 selftest: Add test for handling of "short" dnsProperty records
via 87bf1d687fe librpc/idl: Add dnsp_DnsProperty_short
via 4e08ea2aa3e selftest: Avoid running the slowest of the "none" tests in samba-o3
from 49951b283d9 smbd: Store share_entries in locking.tdb again
https://git.samba.org/?p=samba.git;a=shortlog;h=master
- Log -----------------------------------------------------------------
commit 004e7a1fee766102de302e83f4dc5f4d977aef32
Author: Andrew Bartlett <abartlet at samba.org>
Date: Wed May 13 12:01:05 2020 +1200
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
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Autobuild-User(master): Andrew Bartlett <abartlet at samba.org>
Autobuild-Date(master): Fri May 15 07:29:17 UTC 2020 on sn-devel-184
commit 6eb2a48f5a998b82bb071ef42d00d2f34a2b0ed8
Author: Andrew Bartlett <abartlet at samba.org>
Date: Thu May 14 10:19:45 2020 +1200
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 at samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
commit 87bf1d687fe7b48a7b6d511dfc7f5414db16119c
Author: Andrew Bartlett <abartlet at samba.org>
Date: Thu May 14 10:21:19 2020 +1200
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 at samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
commit 4e08ea2aa3ed95398c9679aaaa2722aecff77547
Author: Andrew Bartlett <abartlet at samba.org>
Date: Fri May 8 23:28:52 2020 +1200
selftest: Avoid running the slowest of the "none" tests in samba-o3
This job is already quite long and these tests are unlikely
to vary between hosts or under the -O3 compile
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
Reviewed-by: Volker Lendecke <vl at samba.org>
-----------------------------------------------------------------------
Summary of changes:
librpc/idl/dnsp.idl | 16 ++++++++
python/samba/tests/blackbox/ndrdump.py | 21 ++++++++++
python/samba/tests/dns.py | 51 ++++++++++++++++++++++++
script/autobuild.py | 3 +-
selftest/knownfail.d/dns | 7 ++++
selftest/slow-none | 13 ++++++
source4/dns_server/dnsserver_common.c | 9 ++++-
source4/rpc_server/dnsserver/dnsdb.c | 72 ++++++++++++++++++++++++++++++----
8 files changed, 183 insertions(+), 9 deletions(-)
create mode 100644 selftest/slow-none
Changeset truncated at 500 lines:
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 0b72684f270..a33229e4740 100644
--- a/python/samba/tests/blackbox/ndrdump.py
+++ b/python/samba/tests/blackbox/ndrdump.py
@@ -468,3 +468,24 @@ dump OK
# check_output will return bytes
# convert expected to bytes for python 3
self.assertRegex(actual.decode('utf8'), 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)
diff --git a/python/samba/tests/dns.py b/python/samba/tests/dns.py
index 5af24588428..093cae14078 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.assertEqual(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/script/autobuild.py b/script/autobuild.py
index 7a9e57e3b24..a9eb980c7aa 100755
--- a/script/autobuild.py
+++ b/script/autobuild.py
@@ -245,6 +245,7 @@ tasks = {
"schema_dc",
"clusteredmember_smb1",
])),
+ ("test-slow-none", make_test(cmd='make test', TESTS="--include=selftest/slow-none", include_envs=["none"])),
("lcov", LCOV_CMD),
("install", "make install"),
("check-clean-tree", "script/clean-source-tree.sh"),
@@ -563,7 +564,7 @@ tasks = {
("random-sleep", random_sleep(300, 900)),
("configure", "ADDITIONAL_CFLAGS='-O3 -Wp,-D_FORTIFY_SOURCE=2' ./configure.developer --with-selftest-prefix=./bin/ab --abi-check-disable" + samba_configure_params),
("make", "make -j"),
- ("test", make_test(cmd='make test', include_envs=["none"])),
+ ("test", make_test(cmd='make test', TESTS="--exclude=selftest/slow-none", include_envs=["none"])),
("quicktest", make_test(cmd='make quicktest', include_envs=["ad_dc", "ad_dc_smb1", "ad_dc_smb1_done"])),
("lcov", LCOV_CMD),
("install", "make install"),
diff --git a/selftest/knownfail.d/dns b/selftest/knownfail.d/dns
index bf563632472..fee2f2ae322 100644
--- a/selftest/knownfail.d/dns
+++ b/selftest/knownfail.d/dns
@@ -80,3 +80,10 @@ 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 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_update_while_dnsProperty_zero_length\(vampire_dc:local\)
\ No newline at end of file
diff --git a/selftest/slow-none b/selftest/slow-none
new file mode 100644
index 00000000000..2f0ae1e98cc
--- /dev/null
+++ b/selftest/slow-none
@@ -0,0 +1,13 @@
+# This file to have control over where in autobuild the slower "none"
+# tests are running, to avoid really slow tests being run on multiple
+# hosts that host the samba-o3 job.
+^samba.tests.docs
+^ldb.python
+^samba.tests.dsdb_lock
+^samba4.blackbox.upgradeprovision.alpha13
+^samba4.blackbox.upgradeprovision.release-4-0-0
+^samba.tests.domain_backup_offline
+^samba.tests.samba_tool.help
+^samba4.blackbox.schemaupgrade
+^samba4.blackbox.group.py
+^samba4.blackbox.provision.py
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) {
--
Samba Shared Repository
More information about the samba-cvs
mailing list