Attachment 14657 Details for Bug 13683 – backported patch for 4.9 with CVE (v3)

[patch] backported patch for 4.9 with CVE (v3)

CVE-2018-16857-v4-9.patch (text/plain), 55.37 KB, created by Tim Beale on 2018-11-14 04:35:06 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Tim Beale

Created: 2018-11-14 04:35:06 UTC

Size: 55.37 KB

patch

obsolete

>From 5c29528f3d587c38a44888fe24d66f5ca21ef3ac Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Sun, 2 Sep 2018 17:34:03 +1200
>Subject: [PATCH 1/9] 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 ec6cf13..e817e65 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 4a32068..9d82e08 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.7.4
>
>
>From f830d2836116f915e3a43c7d3eb99173e1aab9a2 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 2/9] 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 e817e65..d871086 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.7.4
>
>
>From 730e402bfb5de56c27ecfde39a08ea4f1c650c41 Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Sun, 2 Sep 2018 18:03:06 +1200
>Subject: [PATCH 3/9] 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 d871086..0022dee 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.7.4
>
>
>From 0f653c6c0d8d2b934e1fcbe6c20c3f7276d02cc9 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 4/9] 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
>---
> source4/dsdb/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 9d82e08..1b408c7 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.7.4
>
>
>From 651f51e329c2b700423390ce49a206037ac73a71 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 5/9] 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
>---
> source4/dsdb/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 1b408c7..48a7401 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.7.4
>
>
>From 5dde8dc535747a4f11ab58b0102cb576ab2ec93d 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 6/9] 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 ++++++++++++++++++++++
> source4/dsdb/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 0000000..305bcbd
>--- /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 0022dee..b09a732 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 48a7401..e8ac46d 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.7.4
>
>
>From f6253f6cef6a79cfe17ac1ecb99bc2586b8a9f3c 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 7/9] 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 305bcbd..a4e37a8 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 193fa2a..438a29e 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.7.4
>
>
>From 170f0a51d24ddfbe4cd8db8bf4f1b4e0250632d5 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 8/9] 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 a4e37a8..0000000
>--- 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 438a29e..8d863f8 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.7.4
>
>
>From f4d23dba8c1b3edda798c8b196144dcd8016560b 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 9/9] 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 8d863f8..18f7003 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.7.4
>

Flags:

timbeale: review+
dbagnall: review+

Actions: View

Attachments on bug 13683: 14644 | 14645 | 14646 | 14647 | 14654 | 14655 | 14656 | 14657 | 14686