From bf596c14c2462b9a15ea738ef4f32b3abb8b63d1 Mon Sep 17 00:00:00 2001
From: Aaron Haslett <aaronhaslett@catalyst.net.nz>
Date: Tue, 23 Oct 2018 17:25:51 +1300
Subject: [PATCH 01/17] CVE-2018-14629 dns: CNAME loop prevention using counter

Count number of answers generated by internal DNS query routine and stop at
20 to match Microsoft's loop prevention mechanism.

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

Signed-off-by: Aaron Haslett <aaronhaslett@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Garming Sam <garming@catalyst.net.nz>
---
 python/samba/tests/dns.py      | 22 ++++++++++++++++++++++
 selftest/knownfail.d/dns       |  6 ++++++
 source4/dns_server/dns_query.c |  6 ++++++
 3 files changed, 34 insertions(+)

diff --git a/python/samba/tests/dns.py b/python/samba/tests/dns.py
index 6771e3bb8c4..3e6306e2be8 100644
--- a/python/samba/tests/dns.py
+++ b/python/samba/tests/dns.py
@@ -844,6 +844,28 @@ class TestComplexQueries(DNSTest):
         self.assertEquals(response.answers[1].name, name2)
         self.assertEquals(response.answers[1].rdata, name0)
 
+    def test_cname_loop(self):
+        cname1 = "cnamelooptestrec." + self.get_dns_domain()
+        cname2 = "cnamelooptestrec2." + self.get_dns_domain()
+        cname3 = "cnamelooptestrec3." + self.get_dns_domain()
+        self.make_dns_update(cname1, cname2, dnsp.DNS_TYPE_CNAME)
+        self.make_dns_update(cname2, cname3, dnsp.DNS_TYPE_CNAME)
+        self.make_dns_update(cname3, cname1, dnsp.DNS_TYPE_CNAME)
+
+        p = self.make_name_packet(dns.DNS_OPCODE_QUERY)
+        questions = []
+
+        q = self.make_name_question(cname1,
+                                    dns.DNS_QTYPE_A,
+                                    dns.DNS_QCLASS_IN)
+        questions.append(q)
+        self.finish_name_packet(p, questions)
+
+        (response, response_packet) =\
+            self.dns_transaction_udp(p, host=self.server_ip)
+
+        max_recursion_depth = 20
+        self.assertEquals(len(response.answers), max_recursion_depth)
 
 class TestInvalidQueries(DNSTest):
     def setUp(self):
diff --git a/selftest/knownfail.d/dns b/selftest/knownfail.d/dns
index a5176654cc2..a248432aafa 100644
--- a/selftest/knownfail.d/dns
+++ b/selftest/knownfail.d/dns
@@ -69,3 +69,9 @@ samba.tests.dns.__main__.TestSimpleQueries.test_qtype_all_query\(rodc:local\)
 
 # The SOA override should not pass against the RODC, it must not overstamp
 samba.tests.dns.__main__.TestSimpleQueries.test_one_SOA_query\(rodc:local\)
+
+#
+# rodc and vampire_dc require signed dns updates, so the test setup
+# fails, but the test does run on fl2003dc
+^samba.tests.dns.__main__.TestComplexQueries.test_cname_loop\(rodc:local\)
+^samba.tests.dns.__main__.TestComplexQueries.test_cname_loop\(vampire_dc:local\)
diff --git a/source4/dns_server/dns_query.c b/source4/dns_server/dns_query.c
index 923f7233eb9..65faeac3b6a 100644
--- a/source4/dns_server/dns_query.c
+++ b/source4/dns_server/dns_query.c
@@ -40,6 +40,7 @@
 
 #undef DBGC_CLASS
 #define DBGC_CLASS DBGC_DNS
+#define MAX_Q_RECURSION_DEPTH 20
 
 struct forwarder_string {
 	const char *forwarder;
@@ -419,6 +420,11 @@ static struct tevent_req *handle_dnsrpcrec_send(
 	state->answers = answers;
 	state->nsrecs = nsrecs;
 
+	if (talloc_array_length(*answers) >= MAX_Q_RECURSION_DEPTH) {
+		tevent_req_done(req);
+		return tevent_req_post(req, ev);
+	}
+
 	resolve_cname = ((rec->wType == DNS_TYPE_CNAME) &&
 			 ((question->question_type == DNS_QTYPE_A) ||
 			  (question->question_type == DNS_QTYPE_AAAA)));
-- 
2.17.1


From 6e84215d4aa7ef51096db3b187adbe22cacdd921 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet@samba.org>
Date: Tue, 23 Oct 2018 17:33:46 +1300
Subject: [PATCH 02/17] CVE-2018-16841 heimdal: Fix segfault on PKINIT with
 mis-matching principal

In Heimdal KRB5_KDC_ERR_CLIENT_NAME_MISMATCH is an enum, so we tried to double-free
mem_ctx.

This was introduced in 9a0263a7c316112caf0265237bfb2cfb3a3d370d for the
MIT KDC effort.

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

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Gary Lockyer <gary@catalyst.net.nz>
---
 source4/kdc/db-glue.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/source4/kdc/db-glue.c b/source4/kdc/db-glue.c
index bb428e45cbc..f4f3ba63299 100644
--- a/source4/kdc/db-glue.c
+++ b/source4/kdc/db-glue.c
@@ -2610,10 +2610,10 @@ samba_kdc_check_pkinit_ms_upn_match(krb5_context context,
 	 * comparison */
 	if (!(orig_sid && target_sid && dom_sid_equal(orig_sid, target_sid))) {
 		talloc_free(mem_ctx);
-#ifdef KRB5_KDC_ERR_CLIENT_NAME_MISMATCH /* Heimdal */
-		return KRB5_KDC_ERR_CLIENT_NAME_MISMATCH;
-#elif defined(KRB5KDC_ERR_CLIENT_NAME_MISMATCH) /* MIT */
+#if defined(KRB5KDC_ERR_CLIENT_NAME_MISMATCH) /* MIT */
 		return KRB5KDC_ERR_CLIENT_NAME_MISMATCH;
+#else /* Heimdal (where this is an enum) */
+		return KRB5_KDC_ERR_CLIENT_NAME_MISMATCH;
 #endif
 	}
 
-- 
2.17.1


From 4783b9d6a43287a938b18e15f146e6895b689956 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet@samba.org>
Date: Wed, 24 Oct 2018 15:41:28 +1300
Subject: [PATCH 03/17] CVE-2018-16841 selftest: Check for mismatching
 principal in certficate compared with principal in AS-REQ

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13628
Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Gary Lockyer <gary@catalyst.net.nz>
---
 testprogs/blackbox/test_pkinit_heimdal.sh | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/testprogs/blackbox/test_pkinit_heimdal.sh b/testprogs/blackbox/test_pkinit_heimdal.sh
index 0a13aa293e7..0912e0dbfe8 100755
--- a/testprogs/blackbox/test_pkinit_heimdal.sh
+++ b/testprogs/blackbox/test_pkinit_heimdal.sh
@@ -75,10 +75,18 @@ testit "STEP1 kinit with pkinit (name specified) " $samba4kinit $enctype --reque
 testit "STEP1 kinit renew ticket (name specified)" $samba4kinit --request-pac -R  || failed=`expr $failed + 1`
 test_smbclient "STEP1 Test login with kerberos ccache (name specified)" 'ls' "$unc" -k yes || failed=`expr $failed + 1`
 
+testit_expect_failure "STEP1 kinit with pkinit (wrong name specified) " $samba4kinit $enctype --request-pac --renewable $PKUSER not$USERNAME@$REALM || failed=`expr $failed + 1`
+
+testit_expect_failure "STEP1 kinit with pkinit (wrong name specified 2) " $samba4kinit $enctype --request-pac --renewable $PKUSER $SERVER@$REALM || failed=`expr $failed + 1`
+
 testit "STEP1 kinit with pkinit (enterprise name specified)" $samba4kinit $enctype --request-pac --renewable $PKUSER --enterprise $USERNAME@$REALM || failed=`expr $failed + 1`
 testit "STEP1 kinit renew ticket (enterprise name specified)" $samba4kinit --request-pac -R  || failed=`expr $failed + 1`
 test_smbclient "STEP1 Test login with kerberos ccache (enterprise name specified)" 'ls' "$unc" -k yes || failed=`expr $failed + 1`
 
+testit_expect_failure "STEP1 kinit with pkinit (wrong enterprise name specified) " $samba4kinit $enctype --request-pac --renewable $PKUSER --enterprise not$USERNAME@$REALM || failed=`expr $failed + 1`
+
+testit_expect_failure "STEP1 kinit with pkinit (wrong enterprise name specified 2) " $samba4kinit $enctype --request-pac --renewable $PKUSER --enterprise $SERVER$@$REALM || failed=`expr $failed + 1`
+
 testit "STEP1 kinit with pkinit (enterprise name in cert)" $samba4kinit $enctype --request-pac --renewable $PKUSER --pk-enterprise || failed=`expr $failed + 1`
 testit "STEP1 kinit renew ticket (enterprise name in cert)" $samba4kinit --request-pac -R  || failed=`expr $failed + 1`
 test_smbclient "STEP1 Test login with kerberos ccache (enterprise name in cert)" 'ls' "$unc" -k yes || failed=`expr $failed + 1`
-- 
2.17.1


From f40e1b3b42ce23b574a4c530545ff8170ddc7330 Mon Sep 17 00:00:00 2001
From: Gary Lockyer <gary@catalyst.net.nz>
Date: Tue, 6 Nov 2018 12:10:07 +1300
Subject: [PATCH 04/17] CVE-2018-16852 dcerpc dnsserver: Verification tests

Tests to verify
Bug 13669 - (CVE-2018-16852) NULL
            pointer de-reference in Samba AD DC DNS management

The presence of the ZONE_MASTER_SERVERS property or the
ZONE_SCAVENGING_SERVERS property in a zone record causes the server to
follow a null pointer and terminate.

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

Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Signed-off-by: Gary Lockyer <gary@catalyst.net.nz>
---
 selftest/knownfail.d/bug13669                 |   4 +
 .../tests/rpc_dns_server_dnsutils_test.c      | 304 ++++++++++++++++++
 source4/rpc_server/wscript_build              |  17 +-
 source4/selftest/tests.py                     |   2 +
 4 files changed, 325 insertions(+), 2 deletions(-)
 create mode 100644 selftest/knownfail.d/bug13669
 create mode 100644 source4/rpc_server/tests/rpc_dns_server_dnsutils_test.c

diff --git a/selftest/knownfail.d/bug13669 b/selftest/knownfail.d/bug13669
new file mode 100644
index 00000000000..74c8c130674
--- /dev/null
+++ b/selftest/knownfail.d/bug13669
@@ -0,0 +1,4 @@
+^samba4.dcerpc.dnsserver.dnsutils.test_dnsserver_init_zoneinfo_master_servers_empty
+^samba4.dcerpc.dnsserver.dnsutils.test_dnsserver_init_zoneinfo_master_servers
+^samba4.dcerpc.dnsserver.dnsutils.test_dnsserver_init_zoneinfo_scavenging_servers_empty
+^samba4.dcerpc.dnsserver.dnsutils.test_dnsserver_init_zoneinfo_scavenging_servers
diff --git a/source4/rpc_server/tests/rpc_dns_server_dnsutils_test.c b/source4/rpc_server/tests/rpc_dns_server_dnsutils_test.c
new file mode 100644
index 00000000000..89721135658
--- /dev/null
+++ b/source4/rpc_server/tests/rpc_dns_server_dnsutils_test.c
@@ -0,0 +1,304 @@
+/*
+ * Unit tests for source4/rpc_server/dnsserver/dnsutils.c
+ *
+ *  Copyright (C) Catalyst.NET Ltd 2018
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+/*
+ * from cmocka.c:
+ * These headers or their equivalents should be included prior to
+ * including
+ * this header file.
+ *
+ * #include <stdarg.h>
+ * #include <stddef.h>
+ * #include <setjmp.h>
+ *
+ * This allows test applications to use custom definitions of C standard
+ * library functions and types.
+ *
+ */
+
+#include <stdarg.h>
+#include <stddef.h>
+#include <setjmp.h>
+#include <cmocka.h>
+
+
+#include "../dnsserver/dnsutils.c"
+
+
+/*
+ * Test setting of an empty ZONE_MASTER_SERVERS property
+ */
+static void test_dnsserver_init_zoneinfo_master_servers_empty(void **state)
+{
+	struct dnsserver_zone *zone = NULL;
+	struct dnsserver_serverinfo *serverinfo = NULL;
+	struct dnsserver_zoneinfo *zoneinfo = NULL;
+	struct dnsp_DnsProperty *property = NULL;
+
+	TALLOC_CTX *ctx = talloc_new(NULL);
+
+	/*
+	 * Setup the zone data
+	 */
+	zone = talloc_zero(ctx, struct dnsserver_zone);
+	assert_non_null(zone);
+	zone->name = "test";
+
+	/*
+	 * Set up an empty ZONE_MASTER_SERVERS property
+	 */
+	property = talloc_zero_array(ctx, struct dnsp_DnsProperty, 1);
+	assert_non_null(property);
+	property->id = DSPROPERTY_ZONE_MASTER_SERVERS;
+	property->data.master_servers.addrCount = 0;
+	property->data.master_servers.addr = NULL;
+
+	zone->tmp_props = property;
+	zone->num_props = 1;
+
+
+	/*
+	 * Setup the server info
+	 */
+	serverinfo = talloc_zero(ctx, struct dnsserver_serverinfo);
+	assert_non_null(serverinfo);
+
+	/*
+	 * call dnsserver_init_zoneinfo
+	 */
+	zoneinfo = dnsserver_init_zoneinfo(zone, serverinfo);
+
+	/*
+	 * Check results
+	 */
+	assert_non_null(zoneinfo);
+	assert_non_null(zoneinfo->aipLocalMasters);
+	assert_int_equal(zoneinfo->aipLocalMasters->AddrCount, 0);
+	assert_null(zoneinfo->aipLocalMasters->AddrArray);
+
+	TALLOC_FREE(ctx);
+}
+
+/*
+ * Test setting of a non empty ZONE_MASTER_SERVERS property
+ */
+static void test_dnsserver_init_zoneinfo_master_servers(void **state)
+{
+	struct dnsserver_zone *zone = NULL;
+	struct dnsserver_serverinfo *serverinfo = NULL;
+	struct dnsserver_zoneinfo *zoneinfo = NULL;
+	struct dnsp_DnsProperty *property = NULL;
+
+	TALLOC_CTX *ctx = talloc_new(NULL);
+
+	/*
+	 * Setup the zone data
+	 */
+	zone = talloc_zero(ctx, struct dnsserver_zone);
+	assert_non_null(zone);
+	zone->name = "test";
+
+	/*
+	 * Set up an empty ZONE_MASTER_SERVERS property
+	 */
+	property = talloc_zero_array(ctx, struct dnsp_DnsProperty, 1);
+	assert_non_null(property);
+	property->id = DSPROPERTY_ZONE_MASTER_SERVERS;
+	property->data.master_servers.addrCount = 4;
+	property->data.master_servers.addr =
+		talloc_zero_array(ctx, uint32_t, 4);
+	property->data.master_servers.addr[0] = 1000;
+	property->data.master_servers.addr[1] = 1001;
+	property->data.master_servers.addr[2] = 1002;
+	property->data.master_servers.addr[3] = 1003;
+
+	zone->tmp_props = property;
+	zone->num_props = 1;
+
+
+	/*
+	 * Setup the server info
+	 */
+	serverinfo = talloc_zero(ctx, struct dnsserver_serverinfo);
+	assert_non_null(serverinfo);
+
+	/*
+	 * call dnsserver_init_zoneinfo
+	 */
+	zoneinfo = dnsserver_init_zoneinfo(zone, serverinfo);
+
+	/*
+	 * Check results
+	 */
+	assert_non_null(zoneinfo);
+	assert_non_null(zoneinfo->aipLocalMasters);
+	assert_int_equal(zoneinfo->aipLocalMasters->AddrCount, 4);
+	assert_non_null(zoneinfo->aipLocalMasters->AddrArray);
+	assert_int_equal(zoneinfo->aipLocalMasters->AddrArray[0], 1000);
+	assert_int_equal(zoneinfo->aipLocalMasters->AddrArray[1], 1001);
+	assert_int_equal(zoneinfo->aipLocalMasters->AddrArray[2], 1002);
+	assert_int_equal(zoneinfo->aipLocalMasters->AddrArray[3], 1003);
+
+	/*
+	 * Ensure that we're working with a copy of the property data
+	 * and not a reference.
+	 * The pointers should be different, and we should be able to change
+	 * the values in the property without affecting the zoneinfo data
+	 */
+	assert_true(zoneinfo->aipLocalMasters->AddrArray !=
+		    property->data.master_servers.addr);
+	property->data.master_servers.addr[0] = 0;
+	property->data.master_servers.addr[1] = 1;
+	property->data.master_servers.addr[2] = 2;
+	property->data.master_servers.addr[3] = 3;
+	assert_int_equal(zoneinfo->aipLocalMasters->AddrArray[0], 1000);
+	assert_int_equal(zoneinfo->aipLocalMasters->AddrArray[1], 1001);
+	assert_int_equal(zoneinfo->aipLocalMasters->AddrArray[2], 1002);
+	assert_int_equal(zoneinfo->aipLocalMasters->AddrArray[3], 1003);
+
+	TALLOC_FREE(ctx);
+}
+
+/*
+ * Test setting of an empty ZONE_SCAVENGING_SERVERS property
+ */
+static void test_dnsserver_init_zoneinfo_scavenging_servers_empty(void **state)
+{
+	struct dnsserver_zone *zone = NULL;
+	struct dnsserver_serverinfo *serverinfo = NULL;
+	struct dnsserver_zoneinfo *zoneinfo = NULL;
+	struct dnsp_DnsProperty *property = NULL;
+
+	TALLOC_CTX *ctx = talloc_new(NULL);
+
+	/*
+	 * Setup the zone data
+	 */
+	zone = talloc_zero(ctx, struct dnsserver_zone);
+	assert_non_null(zone);
+	zone->name = "test";
+
+	property = talloc_zero_array(ctx, struct dnsp_DnsProperty, 1);
+	assert_non_null(property);
+	property->id = DSPROPERTY_ZONE_SCAVENGING_SERVERS;
+	property->data.servers.addrCount = 0;
+	property->data.servers.addr = NULL;
+
+	zone->tmp_props = property;
+	zone->num_props = 1;
+
+
+	serverinfo = talloc_zero(ctx, struct dnsserver_serverinfo);
+	assert_non_null(serverinfo);
+
+	zoneinfo = dnsserver_init_zoneinfo(zone, serverinfo);
+
+	assert_non_null(zoneinfo);
+	assert_non_null(zoneinfo->aipScavengeServers);
+	assert_int_equal(zoneinfo->aipScavengeServers->AddrCount, 0);
+	assert_null(zoneinfo->aipScavengeServers->AddrArray);
+
+	TALLOC_FREE(ctx);
+}
+
+/*
+ * Test setting of a non empty ZONE_SCAVENGING_SERVERS property
+ */
+static void test_dnsserver_init_zoneinfo_scavenging_servers(void **state)
+{
+	struct dnsserver_zone *zone = NULL;
+	struct dnsserver_serverinfo *serverinfo = NULL;
+	struct dnsserver_zoneinfo *zoneinfo = NULL;
+	struct dnsp_DnsProperty *property = NULL;
+
+	TALLOC_CTX *ctx = talloc_new(NULL);
+
+	/*
+	 * Setup the zone data
+	 */
+	zone = talloc_zero(ctx, struct dnsserver_zone);
+	assert_non_null(zone);
+	zone->name = "test";
+
+	property = talloc_zero_array(ctx, struct dnsp_DnsProperty, 1);
+	assert_non_null(property);
+	property->id = DSPROPERTY_ZONE_SCAVENGING_SERVERS;
+	property->data.servers.addrCount = 4;
+	property->data.servers.addr = talloc_zero_array(ctx, uint32_t, 4);
+	property->data.servers.addr[0] = 1000;
+	property->data.servers.addr[1] = 1001;
+	property->data.servers.addr[2] = 1002;
+	property->data.servers.addr[3] = 1003;
+
+	zone->tmp_props = property;
+	zone->num_props = 1;
+
+
+	serverinfo = talloc_zero(ctx, struct dnsserver_serverinfo);
+	assert_non_null(serverinfo);
+
+	zoneinfo = dnsserver_init_zoneinfo(zone, serverinfo);
+
+	assert_non_null(zoneinfo);
+	assert_non_null(zoneinfo->aipScavengeServers);
+	assert_int_equal(zoneinfo->aipScavengeServers->AddrCount, 4);
+	assert_non_null(zoneinfo->aipScavengeServers->AddrArray);
+	assert_non_null(zoneinfo->aipScavengeServers->AddrArray);
+	assert_int_equal(zoneinfo->aipScavengeServers->AddrArray[0], 1000);
+	assert_int_equal(zoneinfo->aipScavengeServers->AddrArray[1], 1001);
+	assert_int_equal(zoneinfo->aipScavengeServers->AddrArray[2], 1002);
+	assert_int_equal(zoneinfo->aipScavengeServers->AddrArray[3], 1003);
+
+	/*
+	 * Ensure that we're working with a copy of the property data
+	 * and not a reference.
+	 * The pointers should be different, and we should be able to change
+	 * the values in the property without affecting the zoneinfo data
+	 */
+	assert_true(zoneinfo->aipScavengeServers->AddrArray !=
+		    property->data.servers.addr);
+	property->data.servers.addr[0] = 0;
+	property->data.servers.addr[1] = 1;
+	property->data.servers.addr[2] = 2;
+	property->data.servers.addr[3] = 3;
+	assert_int_equal(zoneinfo->aipScavengeServers->AddrArray[0], 1000);
+	assert_int_equal(zoneinfo->aipScavengeServers->AddrArray[1], 1001);
+	assert_int_equal(zoneinfo->aipScavengeServers->AddrArray[2], 1002);
+	assert_int_equal(zoneinfo->aipScavengeServers->AddrArray[3], 1003);
+
+
+	TALLOC_FREE(ctx);
+}
+int main(int argc, const char **argv)
+{
+	const struct CMUnitTest tests[] = {
+		cmocka_unit_test(
+		    test_dnsserver_init_zoneinfo_master_servers_empty),
+		cmocka_unit_test(
+		    test_dnsserver_init_zoneinfo_master_servers),
+		cmocka_unit_test(
+		    test_dnsserver_init_zoneinfo_scavenging_servers_empty),
+		cmocka_unit_test(
+		    test_dnsserver_init_zoneinfo_scavenging_servers),
+	};
+
+	cmocka_set_message_output(CM_OUTPUT_SUBUNIT);
+	return cmocka_run_group_tests(tests, NULL, NULL);
+}
diff --git a/source4/rpc_server/wscript_build b/source4/rpc_server/wscript_build
index 8e05eb8a0c3..14b68c7ce0f 100644
--- a/source4/rpc_server/wscript_build
+++ b/source4/rpc_server/wscript_build
@@ -3,7 +3,7 @@
 bld.SAMBA_SUBSYSTEM('DCERPC_SHARE',
 	source='common/share_info.c',
 	autoproto='common/share.h',
-	deps='ldb',
+	deps='ldb share',
 	enabled=bld.CONFIG_SET('WITH_NTVFS_FILESERVER'),
 	)
 
@@ -163,7 +163,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 dnsserver_common'
+    deps='DCERPC_COMMON dnsserver_common netif'
     )
 
 
@@ -176,3 +176,16 @@ bld.SAMBA_MODULE('service_dcerpc',
 	deps='dcerpc_server'
 	)
 
+if bld.CONFIG_GET('ENABLE_SELFTEST'):
+    bld.SAMBA_BINARY(
+        'test_rpc_dns_server_dnsutils',
+        source='tests/rpc_dns_server_dnsutils_test.c',
+        deps='''
+            dnsserver_common
+            DCERPC_COMMON
+            cmocka
+            talloc
+        ''',
+        install=False,
+        enabled=bld.AD_DC_BUILD_IS_ENABLED()
+    )
diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py
index 9dec0adb05c..18b2c1162b0 100755
--- a/source4/selftest/tests.py
+++ b/source4/selftest/tests.py
@@ -1164,3 +1164,5 @@ plantestsuite("samba4.dsdb.samdb.ldb_modules.audit_util", "none",
                   [os.path.join(bindir(), "test_audit_util")])
 plantestsuite("samba4.dsdb.samdb.ldb_modules.audit_log", "none",
                   [os.path.join(bindir(), "test_audit_log")])
+plantestsuite("samba4.dcerpc.dnsserver.dnsutils", "none",
+              [os.path.join(bindir(), "test_rpc_dns_server_dnsutils")])
-- 
2.17.1


From 05f867db81f118215445f2c49eda4b9c3451d14a Mon Sep 17 00:00:00 2001
From: Gary Lockyer <gary@catalyst.net.nz>
Date: Tue, 6 Nov 2018 12:16:30 +1300
Subject: [PATCH 05/17] CVE-2018-16852 dcerpc dnsserver: Ensure properties are
 handled correctly

Fixes for
Bug 13669 - (CVE-2018-16852) NULL
            pointer de-reference in Samba AD DC DNS management

The presence of the ZONE_MASTER_SERVERS property or the
ZONE_SCAVENGING_SERVERS property in a zone record causes the server to
follow a null pointer and terminate.

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

Signed-off-by: Gary Lockyer <gary@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
---
 selftest/knownfail.d/bug13669           |  4 --
 source4/rpc_server/dnsserver/dnsutils.c | 64 +++++++++++++++++++++----
 2 files changed, 56 insertions(+), 12 deletions(-)
 delete mode 100644 selftest/knownfail.d/bug13669

diff --git a/selftest/knownfail.d/bug13669 b/selftest/knownfail.d/bug13669
deleted file mode 100644
index 74c8c130674..00000000000
--- a/selftest/knownfail.d/bug13669
+++ /dev/null
@@ -1,4 +0,0 @@
-^samba4.dcerpc.dnsserver.dnsutils.test_dnsserver_init_zoneinfo_master_servers_empty
-^samba4.dcerpc.dnsserver.dnsutils.test_dnsserver_init_zoneinfo_master_servers
-^samba4.dcerpc.dnsserver.dnsutils.test_dnsserver_init_zoneinfo_scavenging_servers_empty
-^samba4.dcerpc.dnsserver.dnsutils.test_dnsserver_init_zoneinfo_scavenging_servers
diff --git a/source4/rpc_server/dnsserver/dnsutils.c b/source4/rpc_server/dnsserver/dnsutils.c
index a1c749074af..e4055c99e74 100644
--- a/source4/rpc_server/dnsserver/dnsutils.c
+++ b/source4/rpc_server/dnsserver/dnsutils.c
@@ -209,6 +209,46 @@ struct dnsserver_serverinfo *dnsserver_init_serverinfo(TALLOC_CTX *mem_ctx,
 }
 
 
+/*
+ * Helper function to copy a dnsp_ip4_array struct to an IP4_ARRAY struct.
+ * The new structure and it's data are allocated on the supplied talloc context
+ */
+static struct IP4_ARRAY *copy_ip4_array(
+	TALLOC_CTX *ctx,
+	const char *name,
+	struct dnsp_ip4_array array) {
+
+	struct IP4_ARRAY *ip4_array = NULL;
+	unsigned int i;
+
+	ip4_array = talloc_zero(ctx, struct IP4_ARRAY);
+	if (ip4_array == NULL) {
+		DBG_ERR("Out of memory copying property [%s]\n",
+			name);
+		return NULL;
+	}
+
+	ip4_array->AddrCount = array.addrCount;
+	if (ip4_array->AddrCount == 0) {
+		return ip4_array;
+	}
+
+	ip4_array->AddrArray = talloc_array(ip4_array, uint32_t,
+					    ip4_array->AddrCount);
+	if (ip4_array->AddrArray == NULL) {
+		TALLOC_FREE(ip4_array);
+		DBG_ERR("Out of memory copying property [%s] values\n",
+			name);
+		return NULL;
+	}
+
+	for (i = 0; i < ip4_array->AddrCount; i++) {
+		ip4_array->AddrArray[i] = array.addr[i];
+	}
+
+	return ip4_array;
+}
+
 struct dnsserver_zoneinfo *dnsserver_init_zoneinfo(struct dnsserver_zone *zone,
 						struct dnsserver_serverinfo *serverinfo)
 {
@@ -309,20 +349,28 @@ struct dnsserver_zoneinfo *dnsserver_init_zoneinfo(struct dnsserver_zone *zone,
 				prop->aging_enabled;
 			break;
 		case DSPROPERTY_ZONE_SCAVENGING_SERVERS:
-			zoneinfo->aipScavengeServers->AddrCount =
-				prop->servers.addrCount;
-			zoneinfo->aipScavengeServers->AddrArray =
-				prop->servers.addr;
+			zoneinfo->aipScavengeServers =
+				copy_ip4_array(zoneinfo,
+					       "ZONE_SCAVENGING_SERVERS",
+					       prop->servers);
+			if (zoneinfo->aipScavengeServers == NULL) {
+				TALLOC_FREE(zoneinfo);
+				return NULL;
+			}
 			break;
 		case DSPROPERTY_ZONE_AGING_ENABLED_TIME:
 			zoneinfo->dwAvailForScavengeTime =
 				prop->next_scavenging_cycle_hours;
 			break;
 		case DSPROPERTY_ZONE_MASTER_SERVERS:
-			zoneinfo->aipLocalMasters->AddrCount =
-				prop->master_servers.addrCount;
-			zoneinfo->aipLocalMasters->AddrArray =
-				prop->master_servers.addr;
+			zoneinfo->aipLocalMasters =
+				copy_ip4_array(zoneinfo,
+					       "ZONE_MASTER_SERVERS",
+					       prop->master_servers);
+			if (zoneinfo->aipLocalMasters == NULL) {
+				TALLOC_FREE(zoneinfo);
+				return NULL;
+			}
 			break;
 		case DSPROPERTY_ZONE_EMPTY:
 		case DSPROPERTY_ZONE_SECURE_TIME:
-- 
2.17.1


From c78ca8b9b48a19e71f4d6ddd2e300f282fb0b247 Mon Sep 17 00:00:00 2001
From: Gary Lockyer <gary@catalyst.net.nz>
Date: Wed, 7 Nov 2018 15:08:04 +1300
Subject: [PATCH 06/17] CVE-2018-16852 dcerpc dnsserver: refactor common
 properties handling

dnsserver_common.c and dnsutils.c both share similar code to process
zone properties.  This patch extracts the common code and moves it to
dnsserver_common.c.

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

Signed-off-by: Gary Lockyer <gary@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
---
 source4/dns_server/dnsserver_common.c   | 129 +++++++++++++++++-------
 source4/dns_server/dnsserver_common.h   |   3 +
 source4/rpc_server/dnsserver/dnsutils.c | 107 ++------------------
 3 files changed, 104 insertions(+), 135 deletions(-)

diff --git a/source4/dns_server/dnsserver_common.c b/source4/dns_server/dnsserver_common.c
index bbbfe920f4e..cc24a6c1b52 100644
--- a/source4/dns_server/dnsserver_common.c
+++ b/source4/dns_server/dnsserver_common.c
@@ -742,6 +742,94 @@ bool dns_name_is_static(struct dnsp_DnssrvRpcRecord *records,
 	return false;
 }
 
+/*
+ * Helper function to copy a dnsp_ip4_array struct to an IP4_ARRAY struct.
+ * The new structure and it's data are allocated on the supplied talloc context
+ */
+static struct IP4_ARRAY *copy_ip4_array(TALLOC_CTX *ctx,
+					const char *name,
+					struct dnsp_ip4_array array)
+{
+
+	struct IP4_ARRAY *ip4_array = NULL;
+	unsigned int i;
+
+	ip4_array = talloc_zero(ctx, struct IP4_ARRAY);
+	if (ip4_array == NULL) {
+		DBG_ERR("Out of memory copying property [%s]\n", name);
+		return NULL;
+	}
+
+	ip4_array->AddrCount = array.addrCount;
+	if (ip4_array->AddrCount == 0) {
+		return ip4_array;
+	}
+
+	ip4_array->AddrArray =
+	    talloc_array(ip4_array, uint32_t, ip4_array->AddrCount);
+	if (ip4_array->AddrArray == NULL) {
+		TALLOC_FREE(ip4_array);
+		DBG_ERR("Out of memory copying property [%s] values\n", name);
+		return NULL;
+	}
+
+	for (i = 0; i < ip4_array->AddrCount; i++) {
+		ip4_array->AddrArray[i] = array.addr[i];
+	}
+
+	return ip4_array;
+}
+
+bool dns_zoneinfo_load_zone_property(struct dnsserver_zoneinfo *zoneinfo,
+				     struct dnsp_DnsProperty *prop)
+{
+	switch (prop->id) {
+	case DSPROPERTY_ZONE_TYPE:
+		zoneinfo->dwZoneType = prop->data.zone_type;
+		break;
+	case DSPROPERTY_ZONE_ALLOW_UPDATE:
+		zoneinfo->fAllowUpdate = prop->data.allow_update_flag;
+		break;
+	case DSPROPERTY_ZONE_NOREFRESH_INTERVAL:
+		zoneinfo->dwNoRefreshInterval = prop->data.norefresh_hours;
+		break;
+	case DSPROPERTY_ZONE_REFRESH_INTERVAL:
+		zoneinfo->dwRefreshInterval = prop->data.refresh_hours;
+		break;
+	case DSPROPERTY_ZONE_AGING_STATE:
+		zoneinfo->fAging = prop->data.aging_enabled;
+		break;
+	case DSPROPERTY_ZONE_SCAVENGING_SERVERS:
+		zoneinfo->aipScavengeServers = copy_ip4_array(
+		    zoneinfo, "ZONE_SCAVENGING_SERVERS", prop->data.servers);
+		if (zoneinfo->aipScavengeServers == NULL) {
+			return false;
+		}
+		break;
+	case DSPROPERTY_ZONE_AGING_ENABLED_TIME:
+		zoneinfo->dwAvailForScavengeTime =
+		    prop->data.next_scavenging_cycle_hours;
+		break;
+	case DSPROPERTY_ZONE_MASTER_SERVERS:
+		zoneinfo->aipLocalMasters = copy_ip4_array(
+		    zoneinfo, "ZONE_MASTER_SERVERS", prop->data.master_servers);
+		if (zoneinfo->aipLocalMasters == NULL) {
+			return false;
+		}
+		break;
+	case DSPROPERTY_ZONE_EMPTY:
+	case DSPROPERTY_ZONE_SECURE_TIME:
+	case DSPROPERTY_ZONE_DELETED_FROM_HOSTNAME:
+	case DSPROPERTY_ZONE_AUTO_NS_SERVERS:
+	case DSPROPERTY_ZONE_DCPROMO_CONVERT:
+	case DSPROPERTY_ZONE_SCAVENGING_SERVERS_DA:
+	case DSPROPERTY_ZONE_MASTER_SERVERS_DA:
+	case DSPROPERTY_ZONE_NS_SERVERS_DA:
+	case DSPROPERTY_ZONE_NODE_DBFLAGS:
+		break;
+	}
+	return true;
+}
 WERROR dns_get_zone_properties(struct ldb_context *samdb,
 			       TALLOC_CTX *mem_ctx,
 			       struct ldb_dn *zone_dn,
@@ -774,6 +862,7 @@ WERROR dns_get_zone_properties(struct ldb_context *samdb,
 	}
 
 	for (i = 0; i < element->num_values; i++) {
+		bool valid_property;
 		prop = talloc_zero(mem_ctx, struct dnsp_DnsProperty);
 		if (prop == NULL) {
 			return WERR_NOT_ENOUGH_MEMORY;
@@ -787,42 +876,10 @@ WERROR dns_get_zone_properties(struct ldb_context *samdb,
 			return DNS_ERR(SERVER_FAILURE);
 		}
 
-		switch (prop->id) {
-		case DSPROPERTY_ZONE_AGING_STATE:
-			zoneinfo->fAging = prop->data.aging_enabled;
-			break;
-		case DSPROPERTY_ZONE_NOREFRESH_INTERVAL:
-			zoneinfo->dwNoRefreshInterval =
-			    prop->data.norefresh_hours;
-			break;
-		case DSPROPERTY_ZONE_REFRESH_INTERVAL:
-			zoneinfo->dwRefreshInterval = prop->data.refresh_hours;
-			break;
-		case DSPROPERTY_ZONE_ALLOW_UPDATE:
-			zoneinfo->fAllowUpdate = prop->data.allow_update_flag;
-			break;
-		case DSPROPERTY_ZONE_AGING_ENABLED_TIME:
-			zoneinfo->dwAvailForScavengeTime =
-			    prop->data.next_scavenging_cycle_hours;
-			break;
-		case DSPROPERTY_ZONE_SCAVENGING_SERVERS:
-			zoneinfo->aipScavengeServers->AddrCount =
-			    prop->data.servers.addrCount;
-			zoneinfo->aipScavengeServers->AddrArray =
-			    prop->data.servers.addr;
-			break;
-		case DSPROPERTY_ZONE_EMPTY:
-		case DSPROPERTY_ZONE_TYPE:
-		case DSPROPERTY_ZONE_SECURE_TIME:
-		case DSPROPERTY_ZONE_DELETED_FROM_HOSTNAME:
-		case DSPROPERTY_ZONE_MASTER_SERVERS:
-		case DSPROPERTY_ZONE_AUTO_NS_SERVERS:
-		case DSPROPERTY_ZONE_DCPROMO_CONVERT:
-		case DSPROPERTY_ZONE_SCAVENGING_SERVERS_DA:
-		case DSPROPERTY_ZONE_MASTER_SERVERS_DA:
-		case DSPROPERTY_ZONE_NS_SERVERS_DA:
-		case DSPROPERTY_ZONE_NODE_DBFLAGS:
-			break;
+		valid_property =
+		    dns_zoneinfo_load_zone_property(zoneinfo, prop);
+		if (!valid_property) {
+			return DNS_ERR(SERVER_FAILURE);
 		}
 	}
 
diff --git a/source4/dns_server/dnsserver_common.h b/source4/dns_server/dnsserver_common.h
index 380f61b8dbc..60ecde4fa91 100644
--- a/source4/dns_server/dnsserver_common.h
+++ b/source4/dns_server/dnsserver_common.h
@@ -87,4 +87,7 @@ NTSTATUS dns_common_zones(struct ldb_context *samdb,
 			  TALLOC_CTX *mem_ctx,
 			  struct ldb_dn *base_dn,
 			  struct dns_server_zone **zones_ret);
+
+bool dns_zoneinfo_load_zone_property(struct dnsserver_zoneinfo *zoneinfo,
+				     struct dnsp_DnsProperty *prop);
 #endif /* __DNSSERVER_COMMON_H__ */
diff --git a/source4/rpc_server/dnsserver/dnsutils.c b/source4/rpc_server/dnsserver/dnsutils.c
index e4055c99e74..bb52a1797a9 100644
--- a/source4/rpc_server/dnsserver/dnsutils.c
+++ b/source4/rpc_server/dnsserver/dnsutils.c
@@ -25,6 +25,7 @@
 #include "dsdb/samdb/samdb.h"
 #include "lib/socket/netif.h"
 #include "lib/util/util_net.h"
+#include "dnsserver_common.h"
 
 static struct DNS_ADDR_ARRAY *fill_dns_addr_array(TALLOC_CTX *mem_ctx,
 					   struct loadparm_context *lp_ctx,
@@ -208,47 +209,6 @@ struct dnsserver_serverinfo *dnsserver_init_serverinfo(TALLOC_CTX *mem_ctx,
 	return serverinfo;
 }
 
-
-/*
- * Helper function to copy a dnsp_ip4_array struct to an IP4_ARRAY struct.
- * The new structure and it's data are allocated on the supplied talloc context
- */
-static struct IP4_ARRAY *copy_ip4_array(
-	TALLOC_CTX *ctx,
-	const char *name,
-	struct dnsp_ip4_array array) {
-
-	struct IP4_ARRAY *ip4_array = NULL;
-	unsigned int i;
-
-	ip4_array = talloc_zero(ctx, struct IP4_ARRAY);
-	if (ip4_array == NULL) {
-		DBG_ERR("Out of memory copying property [%s]\n",
-			name);
-		return NULL;
-	}
-
-	ip4_array->AddrCount = array.addrCount;
-	if (ip4_array->AddrCount == 0) {
-		return ip4_array;
-	}
-
-	ip4_array->AddrArray = talloc_array(ip4_array, uint32_t,
-					    ip4_array->AddrCount);
-	if (ip4_array->AddrArray == NULL) {
-		TALLOC_FREE(ip4_array);
-		DBG_ERR("Out of memory copying property [%s] values\n",
-			name);
-		return NULL;
-	}
-
-	for (i = 0; i < ip4_array->AddrCount; i++) {
-		ip4_array->AddrArray[i] = array.addr[i];
-	}
-
-	return ip4_array;
-}
-
 struct dnsserver_zoneinfo *dnsserver_init_zoneinfo(struct dnsserver_zone *zone,
 						struct dnsserver_serverinfo *serverinfo)
 {
@@ -257,8 +217,7 @@ struct dnsserver_zoneinfo *dnsserver_init_zoneinfo(struct dnsserver_zone *zone,
 	const char *revzone = "in-addr.arpa";
 	const char *revzone6 = "ip6.arpa";
 	int len1, len2;
-	union dnsPropertyData *prop = NULL;
-	int i=0;
+	unsigned int i = 0;
 
 	zoneinfo = talloc_zero(zone, struct dnsserver_zoneinfo);
 	if (zoneinfo == NULL) {
@@ -326,62 +285,12 @@ struct dnsserver_zoneinfo *dnsserver_init_zoneinfo(struct dnsserver_zone *zone,
 	zoneinfo->dwLastXfrResult = 0;
 
 	for(i=0; i<zone->num_props; i++){
-		prop=&(zone->tmp_props[i].data);
-		switch (zone->tmp_props[i].id) {
-		case DSPROPERTY_ZONE_TYPE:
-			zoneinfo->dwZoneType =
-				prop->zone_type;
-			break;
-		case DSPROPERTY_ZONE_ALLOW_UPDATE:
-			zoneinfo->fAllowUpdate =
-				prop->allow_update_flag;
-			break;
-		case DSPROPERTY_ZONE_NOREFRESH_INTERVAL:
-			zoneinfo->dwNoRefreshInterval =
-				prop->norefresh_hours;
-			break;
-		case DSPROPERTY_ZONE_REFRESH_INTERVAL:
-			zoneinfo->dwRefreshInterval =
-				prop->refresh_hours;
-			break;
-		case DSPROPERTY_ZONE_AGING_STATE:
-			zoneinfo->fAging =
-				prop->aging_enabled;
-			break;
-		case DSPROPERTY_ZONE_SCAVENGING_SERVERS:
-			zoneinfo->aipScavengeServers =
-				copy_ip4_array(zoneinfo,
-					       "ZONE_SCAVENGING_SERVERS",
-					       prop->servers);
-			if (zoneinfo->aipScavengeServers == NULL) {
-				TALLOC_FREE(zoneinfo);
-				return NULL;
-			}
-			break;
-		case DSPROPERTY_ZONE_AGING_ENABLED_TIME:
-			zoneinfo->dwAvailForScavengeTime =
-				prop->next_scavenging_cycle_hours;
-			break;
-		case DSPROPERTY_ZONE_MASTER_SERVERS:
-			zoneinfo->aipLocalMasters =
-				copy_ip4_array(zoneinfo,
-					       "ZONE_MASTER_SERVERS",
-					       prop->master_servers);
-			if (zoneinfo->aipLocalMasters == NULL) {
-				TALLOC_FREE(zoneinfo);
-				return NULL;
-			}
-			break;
-		case DSPROPERTY_ZONE_EMPTY:
-		case DSPROPERTY_ZONE_SECURE_TIME:
-		case DSPROPERTY_ZONE_DELETED_FROM_HOSTNAME:
-		case DSPROPERTY_ZONE_AUTO_NS_SERVERS:
-		case DSPROPERTY_ZONE_DCPROMO_CONVERT:
-		case DSPROPERTY_ZONE_SCAVENGING_SERVERS_DA:
-		case DSPROPERTY_ZONE_MASTER_SERVERS_DA:
-		case DSPROPERTY_ZONE_NS_SERVERS_DA:
-		case DSPROPERTY_ZONE_NODE_DBFLAGS:
-			break;
+		bool valid_property;
+		valid_property = dns_zoneinfo_load_zone_property(
+		    zoneinfo, &zone->tmp_props[i]);
+		if (!valid_property) {
+			TALLOC_FREE(zoneinfo);
+			return NULL;
 		}
 	}
 
-- 
2.17.1


From f33f52c366f7cf140f470de44579dcb7eb832629 Mon Sep 17 00:00:00 2001
From: Garming Sam <garming@catalyst.net.nz>
Date: Mon, 5 Nov 2018 16:18:18 +1300
Subject: [PATCH 07/17] CVE-2018-16851 ldap_server: Check ret before
 manipulating blob

In the case of hitting the talloc ~256MB limit, this causes a crash in
the server.

Note that you would actually need to load >256MB of data into the LDAP.
Although there is some generated/hidden data which would help you reach that
limit (descriptors and RMD blobs).

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

Signed-off-by: Garming Sam <garming@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
---
 source4/ldap_server/ldap_server.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/source4/ldap_server/ldap_server.c b/source4/ldap_server/ldap_server.c
index b5251e3623e..bc2f54bc146 100644
--- a/source4/ldap_server/ldap_server.c
+++ b/source4/ldap_server/ldap_server.c
@@ -690,13 +690,13 @@ static void ldapsrv_call_writev_start(struct ldapsrv_call *call)
 		ret = data_blob_append(call, &blob, b.data, b.length);
 		data_blob_free(&b);
 
-		talloc_set_name_const(blob.data, "Outgoing, encoded LDAP packet");
-
 		if (!ret) {
 			ldapsrv_terminate_connection(conn, "data_blob_append failed");
 			return;
 		}
 
+		talloc_set_name_const(blob.data, "Outgoing, encoded LDAP packet");
+
 		DLIST_REMOVE(call->replies, call->replies);
 	}
 
-- 
2.17.1


From 4aabfecd290cd2769376abf7f170e832becc4112 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet@samba.org>
Date: Tue, 6 Nov 2018 13:32:05 +1300
Subject: [PATCH 08/17] CVE-2018-16853 build: The Samba AD DC, when build with
 MIT Kerberos is experimental

This matches https://wiki.samba.org/index.php/Running_a_Samba_AD_DC_with_MIT_Kerberos_KDC

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

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Gary Lockyer <gary@catalyst.net.nz>
---
 wscript | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/wscript b/wscript
index 19fc6d12118..7c265e7befb 100644
--- a/wscript
+++ b/wscript
@@ -56,6 +56,14 @@ def set_options(opt):
                    help='build Samba with system MIT Kerberos. ' +
                         'You may specify list of paths where Kerberos is installed (e.g. /usr/local /usr/kerberos) to search krb5-config',
                    action='callback', callback=system_mitkrb5_callback, dest='with_system_mitkrb5', default=False)
+
+    opt.add_option('--with-experimental-mit-ad-dc',
+                   help='Enable the experimental MIT Kerberos-backed AD DC.  ' +
+                   'Note that security patches are not issued for this configuration',
+                   action='store_true',
+                   dest='with_experimental_mit_ad_dc',
+                   default=False)
+
     opt.add_option('--with-system-mitkdc',
                    help=('Specify the path to the krb5kdc binary from MIT Kerberos'),
                    type="string",
@@ -210,7 +218,16 @@ def configure(conf):
         conf.DEFINE('AD_DC_BUILD_IS_ENABLED', 1)
 
     if Options.options.with_system_mitkrb5:
+        if not Options.options.with_experimental_mit_ad_dc and \
+           not Options.options.without_ad_dc:
+            raise Utils.WafError('The MIT Kerberos build of Samba as an AD DC ' +
+                                 'is experimental. Therefore '
+                                 '--with-system-mitkrb5 requires either ' +
+                                 '--with-experimental-mit-ad-dc or ' +
+                                 '--without-ad-dc')
+
         conf.PROCESS_SEPARATE_RULE('system_mitkrb5')
+
     if not (Options.options.without_ad_dc or Options.options.with_system_mitkrb5):
         conf.DEFINE('AD_DC_BUILD_IS_ENABLED', 1)
 
-- 
2.17.1


From 862d4909eccd18942e3de8e8b0dc6e1594ec27f1 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet@samba.org>
Date: Sun, 2 Sep 2018 17:34:03 +1200
Subject: [PATCH 09/17] CVE-2018-16857 selftest: Prepare to allow override of
 lockout duration in password_lockout tests

This will make it easier to avoid flapping tests.

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Gary Lockyer <gary@catalyst.net.nz>
(cherry picked from commit a740a6131c967f9640b19a6964fd5d6f85ce853a)

Backported as a dependency for:
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13683
---
 source4/dsdb/tests/python/password_lockout.py      |  9 ++++-----
 source4/dsdb/tests/python/password_lockout_base.py | 11 +++++++++--
 2 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/source4/dsdb/tests/python/password_lockout.py b/source4/dsdb/tests/python/password_lockout.py
index ec6cf13fe66..e817e656a2a 100755
--- a/source4/dsdb/tests/python/password_lockout.py
+++ b/source4/dsdb/tests/python/password_lockout.py
@@ -616,15 +616,14 @@ userPassword: thatsAcomplPASS2XYZ
                                                           initial_lastlogon_relation='greater')
 
     def use_pso_lockout_settings(self, creds):
+
         # create a PSO with the lockout settings the test cases normally expect
+        #
+        # Some test cases sleep() for self.account_lockout_duration
         pso = PasswordSettings("lockout-PSO", self.ldb, lockout_attempts=3,
-                               lockout_duration=3)
+                               lockout_duration=self.account_lockout_duration)
         self.addCleanup(self.ldb.delete, pso.dn)
 
-        # the test cases should sleep() for the PSO's lockoutDuration/obsvWindow
-        self.account_lockout_duration = 3
-        self.lockout_observation_window = 3
-
         userdn = "cn=%s,cn=users,%s" % (creds.get_username(), self.base_dn)
         pso.apply_to(userdn)
 
diff --git a/source4/dsdb/tests/python/password_lockout_base.py b/source4/dsdb/tests/python/password_lockout_base.py
index 4a320684702..9d82e088bb8 100644
--- a/source4/dsdb/tests/python/password_lockout_base.py
+++ b/source4/dsdb/tests/python/password_lockout_base.py
@@ -323,8 +323,15 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
 """)
 
         self.base_dn = self.ldb.domain_dn()
-        self.account_lockout_duration = 3
-        self.lockout_observation_window = 3
+
+        #
+        # Some test cases sleep() for self.account_lockout_duration
+        # so allow it to be controlled via the subclass
+        #
+        if not hasattr(self, 'account_lockout_duration'):
+            self.account_lockout_duration = 3
+        if not hasattr(self, 'lockout_observation_window'):
+            self.lockout_observation_window = 3
         self.update_lockout_settings(threshold=3,
                                      duration=self.account_lockout_duration,
                                      observation_window=self.lockout_observation_window)
-- 
2.17.1


From 31198d39a76474d55c3d391e04d76758ee115d8e Mon Sep 17 00:00:00 2001
From: Joe Guo <joeg@catalyst.net.nz>
Date: Mon, 30 Jul 2018 18:21:29 +1200
Subject: [PATCH 10/17] CVE-2018-16857 PEP8: fix E305: expected 2 blank lines
 after class or function definition, found 1

Signed-off-by: Joe Guo <joeg@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>

Partial backport of commit 115f2a71b88 (only password_lockout.py
change) as a dependency for:
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13683
---
 source4/dsdb/tests/python/password_lockout.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/source4/dsdb/tests/python/password_lockout.py b/source4/dsdb/tests/python/password_lockout.py
index e817e656a2a..d8710866f39 100755
--- a/source4/dsdb/tests/python/password_lockout.py
+++ b/source4/dsdb/tests/python/password_lockout.py
@@ -1400,6 +1400,7 @@ userPassword: """ + userpass + """
         self._test_samr_password_change(self.lockout1ntlm_creds,
                                         other_creds=self.lockout2ntlm_creds)
 
+
 host_url = "ldap://%s" % host
 
 TestProgram(module=__name__, opts=subunitopts)
-- 
2.17.1


From 4d0fd1a421ad4a3ca19ed954ee91fcc36413b017 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet@samba.org>
Date: Sun, 2 Sep 2018 18:03:06 +1200
Subject: [PATCH 11/17] CVE-2018-16857 selftest: Split up password_lockout into
 tests with and without a call to sleep()

This means we can have a long observation window for many of the tests and
so make them much more reliable.  Many of these cause frustrating flapping
failures in our CI systems.

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Gary Lockyer <gary@catalyst.net.nz>

Autobuild-User(master): Andrew Bartlett <abartlet@samba.org>
Autobuild-Date(master): Mon Sep  3 06:14:55 CEST 2018 on sn-devel-144

(cherry picked from commit 74357bf347348d3a8b7483c58e5250e98f7e8810)
Backported as a dependency for:
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13683
---
 source4/dsdb/tests/python/password_lockout.py | 299 +++++++++---------
 1 file changed, 157 insertions(+), 142 deletions(-)

diff --git a/source4/dsdb/tests/python/password_lockout.py b/source4/dsdb/tests/python/password_lockout.py
index d8710866f39..0022dee21ba 100755
--- a/source4/dsdb/tests/python/password_lockout.py
+++ b/source4/dsdb/tests/python/password_lockout.py
@@ -88,6 +88,42 @@ class PasswordTests(password_lockout_base.BasePasswordTestCase):
         self.lockout2ntlm_ldb = self._readd_user(self.lockout2ntlm_creds,
                                                  lockOutObservationWindow=self.lockout_observation_window)
 
+
+    def use_pso_lockout_settings(self, creds):
+
+        # create a PSO with the lockout settings the test cases normally expect
+        #
+        # Some test cases sleep() for self.account_lockout_duration
+        pso = PasswordSettings("lockout-PSO", self.ldb, lockout_attempts=3,
+                               lockout_duration=self.account_lockout_duration)
+        self.addCleanup(self.ldb.delete, pso.dn)
+
+        userdn = "cn=%s,cn=users,%s" % (creds.get_username(), self.base_dn)
+        pso.apply_to(userdn)
+
+        # update the global lockout settings to be wildly different to what
+        # the test cases normally expect
+        self.update_lockout_settings(threshold=10, duration=600,
+                                     observation_window=600)
+
+    def _reset_samr(self, res):
+
+        # Now reset the lockout, by removing ACB_AUTOLOCK (which removes the lock, despite being a generated attribute)
+        samr_user = self._open_samr_user(res)
+        acb_info = self.samr.QueryUserInfo(samr_user, 16)
+        acb_info.acct_flags &= ~samr.ACB_AUTOLOCK
+        self.samr.SetUserInfo(samr_user, 16, acb_info)
+        self.samr.Close(samr_user)
+
+
+class PasswordTestsWithoutSleep(PasswordTests):
+    def setUp(self):
+        # The tests in this class do not sleep, so we can have a
+        # longer window and not flap on slower hosts
+        self.account_lockout_duration = 30
+        self.lockout_observation_window = 30
+        super(PasswordTestsWithoutSleep, self).setUp()
+
     def _reset_ldap_lockoutTime(self, res):
         self.ldb.modify_ldif("""
 dn: """ + str(res[0].dn) + """
@@ -615,22 +651,130 @@ userPassword: thatsAcomplPASS2XYZ
                                                           "samr",
                                                           initial_lastlogon_relation='greater')
 
-    def use_pso_lockout_settings(self, creds):
+    def test_multiple_logon_krb5(self):
+        self._test_multiple_logon(self.lockout1krb5_creds)
 
-        # create a PSO with the lockout settings the test cases normally expect
-        #
-        # Some test cases sleep() for self.account_lockout_duration
-        pso = PasswordSettings("lockout-PSO", self.ldb, lockout_attempts=3,
-                               lockout_duration=self.account_lockout_duration)
-        self.addCleanup(self.ldb.delete, pso.dn)
+    def test_multiple_logon_ntlm(self):
+        self._test_multiple_logon(self.lockout1ntlm_creds)
 
-        userdn = "cn=%s,cn=users,%s" % (creds.get_username(), self.base_dn)
-        pso.apply_to(userdn)
+    def _test_samr_password_change(self, creds, other_creds, lockout_threshold=3):
+        """Tests user lockout by using bad password in SAMR password_change"""
 
-        # update the global lockout settings to be wildly different to what
-        # the test cases normally expect
-        self.update_lockout_settings(threshold=10, duration=600,
-                                     observation_window=600)
+        # create a connection for SAMR using another user's credentials
+        lp = self.get_loadparm()
+        net = Net(other_creds, lp, server=self.host)
+
+        # work out the initial account values for this user
+        username = creds.get_username()
+        userdn = "cn=%s,cn=users,%s" % (username, self.base_dn)
+        res = self._check_account(userdn,
+                                  badPwdCount=0,
+                                  badPasswordTime=("greater", 0),
+                                  badPwdCountOnly=True)
+        badPasswordTime = int(res[0]["badPasswordTime"][0])
+        logonCount = int(res[0]["logonCount"][0])
+        lastLogon = int(res[0]["lastLogon"][0])
+        lastLogonTimestamp = int(res[0]["lastLogonTimestamp"][0])
+
+        # prove we can change the user password (using the correct password)
+        new_password = "thatsAcomplPASS2"
+        net.change_password(newpassword=new_password.encode('utf-8'),
+                            username=username,
+                            oldpassword=creds.get_password())
+        creds.set_password(new_password)
+
+        # try entering 'x' many bad passwords in a row to lock the user out
+        new_password = "thatsAcomplPASS3"
+        for i in range(lockout_threshold):
+            badPwdCount = i + 1
+            try:
+                print("Trying bad password, attempt #%u" % badPwdCount)
+                net.change_password(newpassword=new_password.encode('utf-8'),
+                                    username=creds.get_username(),
+                                    oldpassword="bad-password")
+                self.fail("Invalid SAMR change_password accepted")
+            except NTSTATUSError as e:
+                enum = ctypes.c_uint32(e[0]).value
+                self.assertEquals(enum, ntstatus.NT_STATUS_WRONG_PASSWORD)
+
+            # check the status of the account is updated after each bad attempt
+            account_flags = 0
+            lockoutTime = None
+            if badPwdCount >= lockout_threshold:
+                account_flags = dsdb.UF_LOCKOUT
+                lockoutTime = ("greater", badPasswordTime)
+
+            res = self._check_account(userdn,
+                                      badPwdCount=badPwdCount,
+                                      badPasswordTime=("greater", badPasswordTime),
+                                      logonCount=logonCount,
+                                      lastLogon=lastLogon,
+                                      lastLogonTimestamp=lastLogonTimestamp,
+                                      lockoutTime=lockoutTime,
+                                      userAccountControl=dsdb.UF_NORMAL_ACCOUNT,
+                                      msDSUserAccountControlComputed=account_flags)
+            badPasswordTime = int(res[0]["badPasswordTime"][0])
+
+        # the user is now locked out
+        lockoutTime = int(res[0]["lockoutTime"][0])
+
+        # check the user remains locked out regardless of whether they use a
+        # good or a bad password now
+        for password in (creds.get_password(), "bad-password"):
+            try:
+                print("Trying password %s" % password)
+                net.change_password(newpassword=new_password.encode('utf-8'),
+                                    username=creds.get_username(),
+                                    oldpassword=password)
+                self.fail("Invalid SAMR change_password accepted")
+            except NTSTATUSError as e:
+                enum = ctypes.c_uint32(e[0]).value
+                self.assertEquals(enum, ntstatus.NT_STATUS_ACCOUNT_LOCKED_OUT)
+
+            res = self._check_account(userdn,
+                                      badPwdCount=lockout_threshold,
+                                      badPasswordTime=badPasswordTime,
+                                      logonCount=logonCount,
+                                      lastLogon=lastLogon,
+                                      lastLogonTimestamp=lastLogonTimestamp,
+                                      lockoutTime=lockoutTime,
+                                      userAccountControl=dsdb.UF_NORMAL_ACCOUNT,
+                                      msDSUserAccountControlComputed=dsdb.UF_LOCKOUT)
+
+        # reset the user account lockout
+        self._reset_samr(res)
+
+        # check bad password counts are reset
+        res = self._check_account(userdn,
+                                  badPwdCount=0,
+                                  badPasswordTime=badPasswordTime,
+                                  logonCount=logonCount,
+                                  lockoutTime=0,
+                                  lastLogon=lastLogon,
+                                  lastLogonTimestamp=lastLogonTimestamp,
+                                  userAccountControl=dsdb.UF_NORMAL_ACCOUNT,
+                                  msDSUserAccountControlComputed=0)
+
+        # check we can change the user password successfully now
+        net.change_password(newpassword=new_password.encode('utf-8'),
+                            username=username,
+                            oldpassword=creds.get_password())
+        creds.set_password(new_password)
+
+    def test_samr_change_password(self):
+        self._test_samr_password_change(self.lockout1ntlm_creds,
+                                        other_creds=self.lockout2ntlm_creds)
+
+    # same as above, but use a PSO to enforce the lockout
+    def test_pso_samr_change_password(self):
+        self.use_pso_lockout_settings(self.lockout1ntlm_creds)
+        self._test_samr_password_change(self.lockout1ntlm_creds,
+                                        other_creds=self.lockout2ntlm_creds)
+
+
+class PasswordTestsWithSleep(PasswordTests):
+    def setUp(self):
+        super(PasswordTestsWithSleep, self).setUp()
 
     def _test_unicodePwd_lockout_with_clear_change(self, creds, other_ldb,
                                                    initial_logoncount_relation=None):
@@ -1065,12 +1209,6 @@ unicodePwd:: """ + base64.b64encode(new_utf16).decode('utf8') + """
         self.use_pso_lockout_settings(self.lockout1ntlm_creds)
         self._test_login_lockout(self.lockout1ntlm_creds)
 
-    def test_multiple_logon_krb5(self):
-        self._test_multiple_logon(self.lockout1krb5_creds)
-
-    def test_multiple_logon_ntlm(self):
-        self._test_multiple_logon(self.lockout1ntlm_creds)
-
     def _testing_add_user(self, creds, lockOutObservationWindow=0):
         username = creds.get_username()
         userpass = creds.get_password()
@@ -1251,15 +1389,6 @@ userPassword: """ + userpass + """
                                   msDSUserAccountControlComputed=0)
         return ldb
 
-    def _reset_samr(self, res):
-
-        # Now reset the lockout, by removing ACB_AUTOLOCK (which removes the lock, despite being a generated attribute)
-        samr_user = self._open_samr_user(res)
-        acb_info = self.samr.QueryUserInfo(samr_user, 16)
-        acb_info.acct_flags &= ~samr.ACB_AUTOLOCK
-        self.samr.SetUserInfo(samr_user, 16, acb_info)
-        self.samr.Close(samr_user)
-
     def test_lockout_observation_window(self):
         lockout3krb5_creds = self.insta_creds(self.template_creds,
                                               username="lockout3krb5",
@@ -1286,120 +1415,6 @@ userPassword: """ + userpass + """
         self._testing_add_user(lockout4ntlm_creds,
                                lockOutObservationWindow=self.lockout_observation_window)
 
-    def _test_samr_password_change(self, creds, other_creds, lockout_threshold=3):
-        """Tests user lockout by using bad password in SAMR password_change"""
-
-        # create a connection for SAMR using another user's credentials
-        lp = self.get_loadparm()
-        net = Net(other_creds, lp, server=self.host)
-
-        # work out the initial account values for this user
-        username = creds.get_username()
-        userdn = "cn=%s,cn=users,%s" % (username, self.base_dn)
-        res = self._check_account(userdn,
-                                  badPwdCount=0,
-                                  badPasswordTime=("greater", 0),
-                                  badPwdCountOnly=True)
-        badPasswordTime = int(res[0]["badPasswordTime"][0])
-        logonCount = int(res[0]["logonCount"][0])
-        lastLogon = int(res[0]["lastLogon"][0])
-        lastLogonTimestamp = int(res[0]["lastLogonTimestamp"][0])
-
-        # prove we can change the user password (using the correct password)
-        new_password = "thatsAcomplPASS2"
-        net.change_password(newpassword=new_password.encode('utf-8'),
-                            username=username,
-                            oldpassword=creds.get_password())
-        creds.set_password(new_password)
-
-        # try entering 'x' many bad passwords in a row to lock the user out
-        new_password = "thatsAcomplPASS3"
-        for i in range(lockout_threshold):
-            badPwdCount = i + 1
-            try:
-                print("Trying bad password, attempt #%u" % badPwdCount)
-                net.change_password(newpassword=new_password.encode('utf-8'),
-                                    username=creds.get_username(),
-                                    oldpassword="bad-password")
-                self.fail("Invalid SAMR change_password accepted")
-            except NTSTATUSError as e:
-                enum = ctypes.c_uint32(e[0]).value
-                self.assertEquals(enum, ntstatus.NT_STATUS_WRONG_PASSWORD)
-
-            # check the status of the account is updated after each bad attempt
-            account_flags = 0
-            lockoutTime = None
-            if badPwdCount >= lockout_threshold:
-                account_flags = dsdb.UF_LOCKOUT
-                lockoutTime = ("greater", badPasswordTime)
-
-            res = self._check_account(userdn,
-                                      badPwdCount=badPwdCount,
-                                      badPasswordTime=("greater", badPasswordTime),
-                                      logonCount=logonCount,
-                                      lastLogon=lastLogon,
-                                      lastLogonTimestamp=lastLogonTimestamp,
-                                      lockoutTime=lockoutTime,
-                                      userAccountControl=dsdb.UF_NORMAL_ACCOUNT,
-                                      msDSUserAccountControlComputed=account_flags)
-            badPasswordTime = int(res[0]["badPasswordTime"][0])
-
-        # the user is now locked out
-        lockoutTime = int(res[0]["lockoutTime"][0])
-
-        # check the user remains locked out regardless of whether they use a
-        # good or a bad password now
-        for password in (creds.get_password(), "bad-password"):
-            try:
-                print("Trying password %s" % password)
-                net.change_password(newpassword=new_password.encode('utf-8'),
-                                    username=creds.get_username(),
-                                    oldpassword=password)
-                self.fail("Invalid SAMR change_password accepted")
-            except NTSTATUSError as e:
-                enum = ctypes.c_uint32(e[0]).value
-                self.assertEquals(enum, ntstatus.NT_STATUS_ACCOUNT_LOCKED_OUT)
-
-            res = self._check_account(userdn,
-                                      badPwdCount=lockout_threshold,
-                                      badPasswordTime=badPasswordTime,
-                                      logonCount=logonCount,
-                                      lastLogon=lastLogon,
-                                      lastLogonTimestamp=lastLogonTimestamp,
-                                      lockoutTime=lockoutTime,
-                                      userAccountControl=dsdb.UF_NORMAL_ACCOUNT,
-                                      msDSUserAccountControlComputed=dsdb.UF_LOCKOUT)
-
-        # reset the user account lockout
-        self._reset_samr(res)
-
-        # check bad password counts are reset
-        res = self._check_account(userdn,
-                                  badPwdCount=0,
-                                  badPasswordTime=badPasswordTime,
-                                  logonCount=logonCount,
-                                  lockoutTime=0,
-                                  lastLogon=lastLogon,
-                                  lastLogonTimestamp=lastLogonTimestamp,
-                                  userAccountControl=dsdb.UF_NORMAL_ACCOUNT,
-                                  msDSUserAccountControlComputed=0)
-
-        # check we can change the user password successfully now
-        net.change_password(newpassword=new_password.encode('utf-8'),
-                            username=username,
-                            oldpassword=creds.get_password())
-        creds.set_password(new_password)
-
-    def test_samr_change_password(self):
-        self._test_samr_password_change(self.lockout1ntlm_creds,
-                                        other_creds=self.lockout2ntlm_creds)
-
-    # same as above, but use a PSO to enforce the lockout
-    def test_pso_samr_change_password(self):
-        self.use_pso_lockout_settings(self.lockout1ntlm_creds)
-        self._test_samr_password_change(self.lockout1ntlm_creds,
-                                        other_creds=self.lockout2ntlm_creds)
-
 
 host_url = "ldap://%s" % host
 
-- 
2.17.1


From fe8e05a9ea8185325ff87ac73ef0106a85cd662a Mon Sep 17 00:00:00 2001
From: Joe Guo <joeg@catalyst.net.nz>
Date: Mon, 30 Jul 2018 18:15:34 +1200
Subject: [PATCH 12/17] CVE-2018-16857 PEP8: fix E127: continuation line
 over-indented for visual indent

Signed-off-by: Joe Guo <joeg@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>

Partial backport of commit bbb9f57603d (only password_lockout_base.py
change) as a dependency for:
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13683
---
 .../tests/python/password_lockout_base.py     | 36 +++++++++----------
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/source4/dsdb/tests/python/password_lockout_base.py b/source4/dsdb/tests/python/password_lockout_base.py
index 9d82e088bb8..1b408c75166 100644
--- a/source4/dsdb/tests/python/password_lockout_base.py
+++ b/source4/dsdb/tests/python/password_lockout_base.py
@@ -390,7 +390,7 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
                                   lastLogon=("greater", 0),
                                   lastLogonTimestamp=("greater", 0),
                                   userAccountControl=
-                                    dsdb.UF_NORMAL_ACCOUNT,
+                                  dsdb.UF_NORMAL_ACCOUNT,
                                   msDSUserAccountControlComputed=0)
         badPasswordTime = int(res[0]["badPasswordTime"][0])
         logonCount = int(res[0]["logonCount"][0])
@@ -421,7 +421,7 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
                                   lastLogon=lastLogon,
                                   lastLogonTimestamp=lastLogonTimestamp,
                                   userAccountControl=
-                                    dsdb.UF_NORMAL_ACCOUNT,
+                                  dsdb.UF_NORMAL_ACCOUNT,
                                   msDSUserAccountControlComputed=0,
                                   msg='lastlogontimestamp with wrong password')
         badPasswordTime = int(res[0]["badPasswordTime"][0])
@@ -440,7 +440,7 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
                                   lastLogon=('greater', lastLogon),
                                   lastLogonTimestamp=lastLogonTimestamp,
                                   userAccountControl=
-                                    dsdb.UF_NORMAL_ACCOUNT,
+                                  dsdb.UF_NORMAL_ACCOUNT,
                                   msDSUserAccountControlComputed=0,
                                   msg='LLTimestamp is updated to lastlogon')
 
@@ -461,7 +461,7 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
                                   lastLogon=lastLogon,
                                   lastLogonTimestamp=lastLogonTimestamp,
                                   userAccountControl=
-                                    dsdb.UF_NORMAL_ACCOUNT,
+                                  dsdb.UF_NORMAL_ACCOUNT,
                                   msDSUserAccountControlComputed=0)
         badPasswordTime = int(res[0]["badPasswordTime"][0])
 
@@ -483,7 +483,7 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
                                   lastLogon=lastLogon,
                                   lastLogonTimestamp=lastLogonTimestamp,
                                   userAccountControl=
-                                    dsdb.UF_NORMAL_ACCOUNT,
+                                  dsdb.UF_NORMAL_ACCOUNT,
                                   msDSUserAccountControlComputed=0)
         badPasswordTime = int(res[0]["badPasswordTime"][0])
 
@@ -508,7 +508,7 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
                                   lastLogonTimestamp=lastLogonTimestamp,
                                   lockoutTime=("greater", badPasswordTime),
                                   userAccountControl=
-                                    dsdb.UF_NORMAL_ACCOUNT,
+                                  dsdb.UF_NORMAL_ACCOUNT,
                                   msDSUserAccountControlComputed=dsdb.UF_LOCKOUT)
         badPasswordTime = int(res[0]["badPasswordTime"][0])
         lockoutTime = int(res[0]["lockoutTime"][0])
@@ -530,7 +530,7 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
                                   lastLogonTimestamp=lastLogonTimestamp,
                                   lockoutTime=lockoutTime,
                                   userAccountControl=
-                                    dsdb.UF_NORMAL_ACCOUNT,
+                                  dsdb.UF_NORMAL_ACCOUNT,
                                   msDSUserAccountControlComputed=dsdb.UF_LOCKOUT)
 
         # The wrong password
@@ -550,7 +550,7 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
                                   lastLogonTimestamp=lastLogonTimestamp,
                                   lockoutTime=lockoutTime,
                                   userAccountControl=
-                                    dsdb.UF_NORMAL_ACCOUNT,
+                                  dsdb.UF_NORMAL_ACCOUNT,
                                   msDSUserAccountControlComputed=dsdb.UF_LOCKOUT)
 
         # The correct password, but we are locked out
@@ -570,7 +570,7 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
                                   lastLogonTimestamp=lastLogonTimestamp,
                                   lockoutTime=lockoutTime,
                                   userAccountControl=
-                                    dsdb.UF_NORMAL_ACCOUNT,
+                                  dsdb.UF_NORMAL_ACCOUNT,
                                   msDSUserAccountControlComputed=dsdb.UF_LOCKOUT)
 
         # wait for the lockout to end
@@ -585,7 +585,7 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
                                   lastLogon=lastLogon,
                                   lastLogonTimestamp=lastLogonTimestamp,
                                   userAccountControl=
-                                    dsdb.UF_NORMAL_ACCOUNT,
+                                  dsdb.UF_NORMAL_ACCOUNT,
                                   msDSUserAccountControlComputed=0)
 
         # The correct password after letting the timeout expire
@@ -605,7 +605,7 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
                                   lastLogonTimestamp=lastLogonTimestamp,
                                   lockoutTime=0,
                                   userAccountControl=
-                                    dsdb.UF_NORMAL_ACCOUNT,
+                                  dsdb.UF_NORMAL_ACCOUNT,
                                   msDSUserAccountControlComputed=0,
                                   msg="lastLogon is way off")
 
@@ -629,7 +629,7 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
                                   lastLogon=lastLogon,
                                   lastLogonTimestamp=lastLogonTimestamp,
                                   userAccountControl=
-                                    dsdb.UF_NORMAL_ACCOUNT,
+                                  dsdb.UF_NORMAL_ACCOUNT,
                                   msDSUserAccountControlComputed=0)
         badPasswordTime = int(res[0]["badPasswordTime"][0])
 
@@ -650,7 +650,7 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
                                   lastLogon=lastLogon,
                                   lastLogonTimestamp=lastLogonTimestamp,
                                   userAccountControl=
-                                    dsdb.UF_NORMAL_ACCOUNT,
+                                  dsdb.UF_NORMAL_ACCOUNT,
                                   msDSUserAccountControlComputed=0)
         badPasswordTime = int(res[0]["badPasswordTime"][0])
 
@@ -664,7 +664,7 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
                                   lastLogon=lastLogon,
                                   lastLogonTimestamp=lastLogonTimestamp,
                                   userAccountControl=
-                                    dsdb.UF_NORMAL_ACCOUNT,
+                                  dsdb.UF_NORMAL_ACCOUNT,
                                   msDSUserAccountControlComputed=0)
 
         # The wrong password
@@ -684,7 +684,7 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
                                   lastLogon=lastLogon,
                                   lastLogonTimestamp=lastLogonTimestamp,
                                   userAccountControl=
-                                    dsdb.UF_NORMAL_ACCOUNT,
+                                  dsdb.UF_NORMAL_ACCOUNT,
                                   msDSUserAccountControlComputed=0)
         badPasswordTime = int(res[0]["badPasswordTime"][0])
 
@@ -700,7 +700,7 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
                                   lastLogon=("greater", lastLogon),
                                   lastLogonTimestamp=lastLogonTimestamp,
                                   userAccountControl=
-                                    dsdb.UF_NORMAL_ACCOUNT,
+                                  dsdb.UF_NORMAL_ACCOUNT,
                                   msDSUserAccountControlComputed=0)
 
     def _test_multiple_logon(self, creds):
@@ -734,7 +734,7 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
                                   lastLogon=("greater", 0),
                                   lastLogonTimestamp=("greater", 0),
                                   userAccountControl=
-                                    dsdb.UF_NORMAL_ACCOUNT,
+                                  dsdb.UF_NORMAL_ACCOUNT,
                                   msDSUserAccountControlComputed=0)
         badPasswordTime = int(res[0]["badPasswordTime"][0])
         logonCount = int(res[0]["logonCount"][0])
@@ -774,5 +774,5 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
                                   lastLogon=(lastlogon_relation, lastLogon),
                                   lastLogonTimestamp=lastLogonTimestamp,
                                   userAccountControl=
-                                    dsdb.UF_NORMAL_ACCOUNT,
+                                  dsdb.UF_NORMAL_ACCOUNT,
                                   msDSUserAccountControlComputed=0)
-- 
2.17.1


From 9cb6b4e9131afac71a39a2f6a3c142723cb6ca19 Mon Sep 17 00:00:00 2001
From: Joe Guo <joeg@catalyst.net.nz>
Date: Mon, 30 Jul 2018 18:19:21 +1200
Subject: [PATCH 13/17] CVE-2018-16857 PEP8: fix E251: unexpected spaces around
 keyword / parameter equals

Signed-off-by: Joe Guo <joeg@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>

Partial backport of commit 1ccc36b4010cd63 (only password_lockout_base.py
change) as a dependency for:
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13683
---
 .../tests/python/password_lockout_base.py     | 60 +++++++------------
 1 file changed, 20 insertions(+), 40 deletions(-)

diff --git a/source4/dsdb/tests/python/password_lockout_base.py b/source4/dsdb/tests/python/password_lockout_base.py
index 1b408c75166..48a74018624 100644
--- a/source4/dsdb/tests/python/password_lockout_base.py
+++ b/source4/dsdb/tests/python/password_lockout_base.py
@@ -93,8 +93,7 @@ class BasePasswordTestCase(PasswordTestCase):
                             logonCount=0,
                             lastLogon=0,
                             lastLogonTimestamp=("absent", None),
-                            userAccountControl=
-                            dsdb.UF_NORMAL_ACCOUNT,
+                            userAccountControl=dsdb.UF_NORMAL_ACCOUNT,
                             msDSUserAccountControlComputed=0)
 
     def _check_account(self, dn,
@@ -389,8 +388,7 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
                                   logonCount=(logoncount_relation, 0),
                                   lastLogon=("greater", 0),
                                   lastLogonTimestamp=("greater", 0),
-                                  userAccountControl=
-                                  dsdb.UF_NORMAL_ACCOUNT,
+                                  userAccountControl=dsdb.UF_NORMAL_ACCOUNT,
                                   msDSUserAccountControlComputed=0)
         badPasswordTime = int(res[0]["badPasswordTime"][0])
         logonCount = int(res[0]["logonCount"][0])
@@ -420,8 +418,7 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
                                   logonCount=logonCount,
                                   lastLogon=lastLogon,
                                   lastLogonTimestamp=lastLogonTimestamp,
-                                  userAccountControl=
-                                  dsdb.UF_NORMAL_ACCOUNT,
+                                  userAccountControl=dsdb.UF_NORMAL_ACCOUNT,
                                   msDSUserAccountControlComputed=0,
                                   msg='lastlogontimestamp with wrong password')
         badPasswordTime = int(res[0]["badPasswordTime"][0])
@@ -439,8 +436,7 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
                                   logonCount=(logoncount_relation, logonCount),
                                   lastLogon=('greater', lastLogon),
                                   lastLogonTimestamp=lastLogonTimestamp,
-                                  userAccountControl=
-                                  dsdb.UF_NORMAL_ACCOUNT,
+                                  userAccountControl=dsdb.UF_NORMAL_ACCOUNT,
                                   msDSUserAccountControlComputed=0,
                                   msg='LLTimestamp is updated to lastlogon')
 
@@ -460,8 +456,7 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
                                   logonCount=logonCount,
                                   lastLogon=lastLogon,
                                   lastLogonTimestamp=lastLogonTimestamp,
-                                  userAccountControl=
-                                  dsdb.UF_NORMAL_ACCOUNT,
+                                  userAccountControl=dsdb.UF_NORMAL_ACCOUNT,
                                   msDSUserAccountControlComputed=0)
         badPasswordTime = int(res[0]["badPasswordTime"][0])
 
@@ -482,8 +477,7 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
                                   logonCount=logonCount,
                                   lastLogon=lastLogon,
                                   lastLogonTimestamp=lastLogonTimestamp,
-                                  userAccountControl=
-                                  dsdb.UF_NORMAL_ACCOUNT,
+                                  userAccountControl=dsdb.UF_NORMAL_ACCOUNT,
                                   msDSUserAccountControlComputed=0)
         badPasswordTime = int(res[0]["badPasswordTime"][0])
 
@@ -507,8 +501,7 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
                                   lastLogon=lastLogon,
                                   lastLogonTimestamp=lastLogonTimestamp,
                                   lockoutTime=("greater", badPasswordTime),
-                                  userAccountControl=
-                                  dsdb.UF_NORMAL_ACCOUNT,
+                                  userAccountControl=dsdb.UF_NORMAL_ACCOUNT,
                                   msDSUserAccountControlComputed=dsdb.UF_LOCKOUT)
         badPasswordTime = int(res[0]["badPasswordTime"][0])
         lockoutTime = int(res[0]["lockoutTime"][0])
@@ -529,8 +522,7 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
                                   lastLogon=lastLogon,
                                   lastLogonTimestamp=lastLogonTimestamp,
                                   lockoutTime=lockoutTime,
-                                  userAccountControl=
-                                  dsdb.UF_NORMAL_ACCOUNT,
+                                  userAccountControl=dsdb.UF_NORMAL_ACCOUNT,
                                   msDSUserAccountControlComputed=dsdb.UF_LOCKOUT)
 
         # The wrong password
@@ -549,8 +541,7 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
                                   lastLogon=lastLogon,
                                   lastLogonTimestamp=lastLogonTimestamp,
                                   lockoutTime=lockoutTime,
-                                  userAccountControl=
-                                  dsdb.UF_NORMAL_ACCOUNT,
+                                  userAccountControl=dsdb.UF_NORMAL_ACCOUNT,
                                   msDSUserAccountControlComputed=dsdb.UF_LOCKOUT)
 
         # The correct password, but we are locked out
@@ -569,8 +560,7 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
                                   lastLogon=lastLogon,
                                   lastLogonTimestamp=lastLogonTimestamp,
                                   lockoutTime=lockoutTime,
-                                  userAccountControl=
-                                  dsdb.UF_NORMAL_ACCOUNT,
+                                  userAccountControl=dsdb.UF_NORMAL_ACCOUNT,
                                   msDSUserAccountControlComputed=dsdb.UF_LOCKOUT)
 
         # wait for the lockout to end
@@ -584,8 +574,7 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
                                   lockoutTime=lockoutTime,
                                   lastLogon=lastLogon,
                                   lastLogonTimestamp=lastLogonTimestamp,
-                                  userAccountControl=
-                                  dsdb.UF_NORMAL_ACCOUNT,
+                                  userAccountControl=dsdb.UF_NORMAL_ACCOUNT,
                                   msDSUserAccountControlComputed=0)
 
         # The correct password after letting the timeout expire
@@ -604,8 +593,7 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
                                   lastLogon=(lastlogon_relation, lastLogon),
                                   lastLogonTimestamp=lastLogonTimestamp,
                                   lockoutTime=0,
-                                  userAccountControl=
-                                  dsdb.UF_NORMAL_ACCOUNT,
+                                  userAccountControl=dsdb.UF_NORMAL_ACCOUNT,
                                   msDSUserAccountControlComputed=0,
                                   msg="lastLogon is way off")
 
@@ -628,8 +616,7 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
                                   lockoutTime=0,
                                   lastLogon=lastLogon,
                                   lastLogonTimestamp=lastLogonTimestamp,
-                                  userAccountControl=
-                                  dsdb.UF_NORMAL_ACCOUNT,
+                                  userAccountControl=dsdb.UF_NORMAL_ACCOUNT,
                                   msDSUserAccountControlComputed=0)
         badPasswordTime = int(res[0]["badPasswordTime"][0])
 
@@ -649,8 +636,7 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
                                   lockoutTime=0,
                                   lastLogon=lastLogon,
                                   lastLogonTimestamp=lastLogonTimestamp,
-                                  userAccountControl=
-                                  dsdb.UF_NORMAL_ACCOUNT,
+                                  userAccountControl=dsdb.UF_NORMAL_ACCOUNT,
                                   msDSUserAccountControlComputed=0)
         badPasswordTime = int(res[0]["badPasswordTime"][0])
 
@@ -663,8 +649,7 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
                                   lockoutTime=0,
                                   lastLogon=lastLogon,
                                   lastLogonTimestamp=lastLogonTimestamp,
-                                  userAccountControl=
-                                  dsdb.UF_NORMAL_ACCOUNT,
+                                  userAccountControl=dsdb.UF_NORMAL_ACCOUNT,
                                   msDSUserAccountControlComputed=0)
 
         # The wrong password
@@ -683,8 +668,7 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
                                   lockoutTime=0,
                                   lastLogon=lastLogon,
                                   lastLogonTimestamp=lastLogonTimestamp,
-                                  userAccountControl=
-                                  dsdb.UF_NORMAL_ACCOUNT,
+                                  userAccountControl=dsdb.UF_NORMAL_ACCOUNT,
                                   msDSUserAccountControlComputed=0)
         badPasswordTime = int(res[0]["badPasswordTime"][0])
 
@@ -699,8 +683,7 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
                                   lockoutTime=0,
                                   lastLogon=("greater", lastLogon),
                                   lastLogonTimestamp=lastLogonTimestamp,
-                                  userAccountControl=
-                                  dsdb.UF_NORMAL_ACCOUNT,
+                                  userAccountControl=dsdb.UF_NORMAL_ACCOUNT,
                                   msDSUserAccountControlComputed=0)
 
     def _test_multiple_logon(self, creds):
@@ -733,8 +716,7 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
                                   logonCount=(logoncount_relation, 0),
                                   lastLogon=("greater", 0),
                                   lastLogonTimestamp=("greater", 0),
-                                  userAccountControl=
-                                  dsdb.UF_NORMAL_ACCOUNT,
+                                  userAccountControl=dsdb.UF_NORMAL_ACCOUNT,
                                   msDSUserAccountControlComputed=0)
         badPasswordTime = int(res[0]["badPasswordTime"][0])
         logonCount = int(res[0]["logonCount"][0])
@@ -754,8 +736,7 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
                                   logonCount=(logoncount_relation, logonCount),
                                   lastLogon=(lastlogon_relation, lastLogon),
                                   lastLogonTimestamp=lastLogonTimestamp,
-                                  userAccountControl=
-                                  dsdb.UF_NORMAL_ACCOUNT,
+                                  userAccountControl=dsdb.UF_NORMAL_ACCOUNT,
                                   msDSUserAccountControlComputed=0,
                                   msg=("second logon, firstlogon was %s" %
                                        firstLogon))
@@ -773,6 +754,5 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
                                   logonCount=(logoncount_relation, logonCount),
                                   lastLogon=(lastlogon_relation, lastLogon),
                                   lastLogonTimestamp=lastLogonTimestamp,
-                                  userAccountControl=
-                                  dsdb.UF_NORMAL_ACCOUNT,
+                                  userAccountControl=dsdb.UF_NORMAL_ACCOUNT,
                                   msDSUserAccountControlComputed=0)
-- 
2.17.1


From ec9cc4ed5a05490297cde3fcaac50eeeaaca8469 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale@catalyst.net.nz>
Date: Tue, 13 Nov 2018 11:49:56 +1300
Subject: [PATCH 14/17] CVE-2018-16857 tests: Sanity-check password lockout
 works with default values

Sanity-check that when we use the default lockOutObservationWindow that
user lockout actually works.

The easiest way to do this is to reuse the _test_login_lockout()
test-case, but stop at the point where we wait for the lockout duration
to expire (because we don't want the test to wait 30 mins).

This highlights a problem currently where the default values don't work.

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

Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
---
 selftest/knownfail.d/password_lockout         |  4 +++
 source4/dsdb/tests/python/password_lockout.py | 30 +++++++++++++++++++
 .../tests/python/password_lockout_base.py     |  6 +++-
 3 files changed, 39 insertions(+), 1 deletion(-)
 create mode 100644 selftest/knownfail.d/password_lockout

diff --git a/selftest/knownfail.d/password_lockout b/selftest/knownfail.d/password_lockout
new file mode 100644
index 00000000000..305bcbdef25
--- /dev/null
+++ b/selftest/knownfail.d/password_lockout
@@ -0,0 +1,4 @@
+samba4.ldap.password_lockout.python\(ad_dc_ntvfs\).__main__.PasswordTestsWithDefaults.test_pso_login_lockout_krb5\(ad_dc_ntvfs\)
+samba4.ldap.password_lockout.python\(ad_dc_ntvfs\).__main__.PasswordTestsWithDefaults.test_pso_login_lockout_ntlm\(ad_dc_ntvfs\)
+samba4.ldap.password_lockout.python\(ad_dc_ntvfs\).__main__.PasswordTestsWithDefaults.test_login_lockout_ntlm\(ad_dc_ntvfs\)
+samba4.ldap.password_lockout.python\(ad_dc_ntvfs\).__main__.PasswordTestsWithDefaults.test_login_lockout_krb5\(ad_dc_ntvfs\)
diff --git a/source4/dsdb/tests/python/password_lockout.py b/source4/dsdb/tests/python/password_lockout.py
index 0022dee21ba..b09a732e179 100755
--- a/source4/dsdb/tests/python/password_lockout.py
+++ b/source4/dsdb/tests/python/password_lockout.py
@@ -1415,6 +1415,36 @@ userPassword: """ + userpass + """
         self._testing_add_user(lockout4ntlm_creds,
                                lockOutObservationWindow=self.lockout_observation_window)
 
+class PasswordTestsWithDefaults(PasswordTests):
+    def setUp(self):
+        # The tests in this class do not sleep, so we can use the default
+        # timeout windows here
+        self.account_lockout_duration = 30 * 60
+        self.lockout_observation_window = 30 * 60
+        super(PasswordTestsWithDefaults, self).setUp()
+
+    # sanity-check that user lockout works with the default settings (we just
+    # check the user is locked out - we don't wait for the lockout to expire)
+    def test_login_lockout_krb5(self):
+        self._test_login_lockout(self.lockout1krb5_creds,
+                                 wait_lockout_duration=False)
+
+    def test_login_lockout_ntlm(self):
+        self._test_login_lockout(self.lockout1ntlm_creds,
+                                 wait_lockout_duration=False)
+
+    # Repeat the login lockout tests using PSOs
+    def test_pso_login_lockout_krb5(self):
+        """Check the PSO lockout settings get applied to the user correctly"""
+        self.use_pso_lockout_settings(self.lockout1krb5_creds)
+        self._test_login_lockout(self.lockout1krb5_creds,
+                                 wait_lockout_duration=False)
+
+    def test_pso_login_lockout_ntlm(self):
+        """Check the PSO lockout settings get applied to the user correctly"""
+        self.use_pso_lockout_settings(self.lockout1ntlm_creds)
+        self._test_login_lockout(self.lockout1ntlm_creds,
+                                 wait_lockout_duration=False)
 
 host_url = "ldap://%s" % host
 
diff --git a/source4/dsdb/tests/python/password_lockout_base.py b/source4/dsdb/tests/python/password_lockout_base.py
index 48a74018624..e8ac46dcd97 100644
--- a/source4/dsdb/tests/python/password_lockout_base.py
+++ b/source4/dsdb/tests/python/password_lockout_base.py
@@ -365,7 +365,7 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
     def tearDown(self):
         super(BasePasswordTestCase, self).tearDown()
 
-    def _test_login_lockout(self, creds):
+    def _test_login_lockout(self, creds, wait_lockout_duration=True):
         username = creds.get_username()
         userpass = creds.get_password()
         userdn = "cn=%s,cn=users,%s" % (username, self.base_dn)
@@ -563,6 +563,10 @@ lockoutThreshold: """ + str(lockoutThreshold) + """
                                   userAccountControl=dsdb.UF_NORMAL_ACCOUNT,
                                   msDSUserAccountControlComputed=dsdb.UF_LOCKOUT)
 
+        # if we're just checking the user gets locked out, we can stop here
+        if not wait_lockout_duration:
+            return
+
         # wait for the lockout to end
         time.sleep(self.account_lockout_duration + 1)
         print(self.account_lockout_duration + 1)
-- 
2.17.1


From 4f86beeaf3408383385ee99a74520a805dd63c0f Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale@catalyst.net.nz>
Date: Tue, 13 Nov 2018 12:24:16 +1300
Subject: [PATCH 15/17] CVE-2018-16857 dsdb/util: Correctly treat
 lockOutObservationWindow as 64-bit int

Commit 442a38c918ae1666b35 refactored some code into a new
get_lockout_observation_window() function. However, in moving the code,
an ldb_msg_find_attr_as_int64() inadvertently got converted to a
ldb_msg_find_attr_as_int().

ldb_msg_find_attr_as_int() will only work for values up to -2147483648
(about 3.5 minutes in MS timestamp form). Unfortunately, the automated
tests used a low enough timeout that they still worked, however,
password lockout would not work with the Samba default settings.

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

Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
---
 selftest/knownfail.d/password_lockout |  2 --
 source4/dsdb/common/util.c            | 10 +++++-----
 2 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/selftest/knownfail.d/password_lockout b/selftest/knownfail.d/password_lockout
index 305bcbdef25..a4e37a84c21 100644
--- a/selftest/knownfail.d/password_lockout
+++ b/selftest/knownfail.d/password_lockout
@@ -1,4 +1,2 @@
 samba4.ldap.password_lockout.python\(ad_dc_ntvfs\).__main__.PasswordTestsWithDefaults.test_pso_login_lockout_krb5\(ad_dc_ntvfs\)
 samba4.ldap.password_lockout.python\(ad_dc_ntvfs\).__main__.PasswordTestsWithDefaults.test_pso_login_lockout_ntlm\(ad_dc_ntvfs\)
-samba4.ldap.password_lockout.python\(ad_dc_ntvfs\).__main__.PasswordTestsWithDefaults.test_login_lockout_ntlm\(ad_dc_ntvfs\)
-samba4.ldap.password_lockout.python\(ad_dc_ntvfs\).__main__.PasswordTestsWithDefaults.test_login_lockout_krb5\(ad_dc_ntvfs\)
diff --git a/source4/dsdb/common/util.c b/source4/dsdb/common/util.c
index 193fa2ae653..438a29e1773 100644
--- a/source4/dsdb/common/util.c
+++ b/source4/dsdb/common/util.c
@@ -5400,12 +5400,12 @@ static int64_t get_lockout_observation_window(struct ldb_message *domain_msg,
 					      struct ldb_message *pso_msg)
 {
 	if (pso_msg != NULL) {
-		return ldb_msg_find_attr_as_int(pso_msg,
-						"msDS-LockoutObservationWindow",
-						 0);
+		return ldb_msg_find_attr_as_int64(pso_msg,
+						  "msDS-LockoutObservationWindow",
+						   0);
 	} else {
-		return ldb_msg_find_attr_as_int(domain_msg,
-						"lockOutObservationWindow", 0);
+		return ldb_msg_find_attr_as_int64(domain_msg,
+						  "lockOutObservationWindow", 0);
 	}
 }
 
-- 
2.17.1


From d12b02c78842786969557b9be7c953e9594d90dd Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale@catalyst.net.nz>
Date: Tue, 13 Nov 2018 13:19:04 +1300
Subject: [PATCH 16/17] CVE-2018-16857 dsdb/util: Fix lockOutObservationWindow
 for PSOs

Fix a remaining place where we were trying to read the
msDS-LockoutObservationWindow as an int instead of an int64.

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

Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
---
 selftest/knownfail.d/password_lockout | 2 --
 source4/dsdb/common/util.c            | 6 +++---
 2 files changed, 3 insertions(+), 5 deletions(-)
 delete mode 100644 selftest/knownfail.d/password_lockout

diff --git a/selftest/knownfail.d/password_lockout b/selftest/knownfail.d/password_lockout
deleted file mode 100644
index a4e37a84c21..00000000000
--- a/selftest/knownfail.d/password_lockout
+++ /dev/null
@@ -1,2 +0,0 @@
-samba4.ldap.password_lockout.python\(ad_dc_ntvfs\).__main__.PasswordTestsWithDefaults.test_pso_login_lockout_krb5\(ad_dc_ntvfs\)
-samba4.ldap.password_lockout.python\(ad_dc_ntvfs\).__main__.PasswordTestsWithDefaults.test_pso_login_lockout_ntlm\(ad_dc_ntvfs\)
diff --git a/source4/dsdb/common/util.c b/source4/dsdb/common/util.c
index 438a29e1773..8d863f85a29 100644
--- a/source4/dsdb/common/util.c
+++ b/source4/dsdb/common/util.c
@@ -5361,9 +5361,9 @@ int samdb_result_effective_badPwdCount(struct ldb_context *sam_ldb,
 
 	if (res != NULL) {
 		lockOutObservationWindow =
-			ldb_msg_find_attr_as_int(res->msgs[0],
-						 "msDS-LockoutObservationWindow",
-						  0);
+			ldb_msg_find_attr_as_int64(res->msgs[0],
+						   "msDS-LockoutObservationWindow",
+						    0);
 		talloc_free(res);
 	} else {
 
-- 
2.17.1


From 60b2cd50f4d0554cc5ca8c53b2d1fa89e56a6d06 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale@catalyst.net.nz>
Date: Tue, 13 Nov 2018 13:22:41 +1300
Subject: [PATCH 17/17] CVE-2018-16857 dsdb/util: Add better default
 lockOutObservationWindow

Clearly the lockOutObservationWindow value is important, and using a
default value of zero doesn't work very well.

This patch adds a better default value (the domain default setting of 30
minutes).

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

Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
---
 source4/dsdb/common/util.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/source4/dsdb/common/util.c b/source4/dsdb/common/util.c
index 8d863f85a29..18f700370a3 100644
--- a/source4/dsdb/common/util.c
+++ b/source4/dsdb/common/util.c
@@ -56,6 +56,9 @@
  */
 #include "dsdb/samdb/ldb_modules/util.h"
 
+/* default is 30 minutes: -1e7 * 30 * 60 */
+#define DEFAULT_OBSERVATION_WINDOW              -18000000000
+
 /*
   search the sam for the specified attributes in a specific domain, filter on
   objectSid being in domain_sid.
@@ -5363,7 +5366,7 @@ int samdb_result_effective_badPwdCount(struct ldb_context *sam_ldb,
 		lockOutObservationWindow =
 			ldb_msg_find_attr_as_int64(res->msgs[0],
 						   "msDS-LockoutObservationWindow",
-						    0);
+						    DEFAULT_OBSERVATION_WINDOW);
 		talloc_free(res);
 	} else {
 
@@ -5402,10 +5405,11 @@ static int64_t get_lockout_observation_window(struct ldb_message *domain_msg,
 	if (pso_msg != NULL) {
 		return ldb_msg_find_attr_as_int64(pso_msg,
 						  "msDS-LockoutObservationWindow",
-						   0);
+						   DEFAULT_OBSERVATION_WINDOW);
 	} else {
 		return ldb_msg_find_attr_as_int64(domain_msg,
-						  "lockOutObservationWindow", 0);
+						  "lockOutObservationWindow",
+						   DEFAULT_OBSERVATION_WINDOW);
 	}
 }
 
-- 
2.17.1