From 29763b0fa428e58938f6c8644ecf6f7dd4401383 Mon Sep 17 00:00:00 2001 From: Tim Beale Date: Tue, 13 Nov 2018 11:49:56 +1300 Subject: [PATCH 1/4] 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 --- 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 14cf00a..bc0613b 100755 --- a/source4/dsdb/tests/python/password_lockout.py +++ b/source4/dsdb/tests/python/password_lockout.py @@ -1371,6 +1371,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 c2664e9..24b066c 100644 --- a/source4/dsdb/tests/python/password_lockout_base.py +++ b/source4/dsdb/tests/python/password_lockout_base.py @@ -364,7 +364,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) @@ -561,6 +561,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 5a167429d14d0a5af17b313fbdccbd3bd24db5d8 Mon Sep 17 00:00:00 2001 From: Tim Beale Date: Tue, 13 Nov 2018 12:24:16 +1300 Subject: [PATCH 2/4] 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 --- 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 e7b860d..a0bbf48 100644 --- a/source4/dsdb/common/util.c +++ b/source4/dsdb/common/util.c @@ -5408,12 +5408,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 9247ef00f7498d46c4dbefa1867d9b8174d27f61 Mon Sep 17 00:00:00 2001 From: Tim Beale Date: Tue, 13 Nov 2018 13:19:04 +1300 Subject: [PATCH 3/4] 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 --- 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 a0bbf48..9ce2e02 100644 --- a/source4/dsdb/common/util.c +++ b/source4/dsdb/common/util.c @@ -5369,9 +5369,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 61e172fd18eeb1fdbc5cd4c64c84674b6b41db77 Mon Sep 17 00:00:00 2001 From: Tim Beale Date: Tue, 13 Nov 2018 13:22:41 +1300 Subject: [PATCH 4/4] 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 --- 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 9ce2e02..36e4b26 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. @@ -5371,7 +5374,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 { @@ -5410,10 +5413,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