[PATCH] Fix ordering issue in dSHeuristics for userPassword public attribute read
Andrew Bartlett
abartlet at samba.org
Wed Apr 11 11:33:29 UTC 2018
Fixing our locking of the module_init() stack fixed this issue and so I
had to fix it properly in a way that could be backported and adjust the
tests.
At no time are private attributes exposed, the whole problem with the
first userPassword non-implementation in AD is that it was a public
attribute to hold something, presumably a crypt() password.
It is essentially like non-shadow NIS until you turn on userPassword
support, then it becomes a gateway to the proper AD password. However
we should to match AD hide the old public value, but we got that wrong
recently.
Please review/push as this is blocking the LMDB effort.
Andrew Bartlett
--
Andrew Bartlett http://samba.org/~abartlet/
Authentication Developer, Samba Team http://samba.org
Samba Developer, Catalyst IT http://catalyst.net.nz/services/samba
-------------- next part --------------
From 9fa7d7b28be0bca9599b1299950f17cdb704b4f0 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Wed, 11 Apr 2018 22:49:31 +1200
Subject: [PATCH 1/2] dsdb: Check for userPassword support after loading the
databases
The net result of this is only that userPassword values (which were
world readable when set) would still be visible after userPassword
started setting the main DB password.
In AD, those values become hidden once the dSHeuristics bit is set,
but Samba lost that when fixing a performance issue with
f26a2845bd42e580ddeaf0eecc9b46b823a0c6bc
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13378
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
selftest/knownfail.d/dsheuristics_userPassword | 1 +
source4/dsdb/samdb/ldb_modules/acl.c | 18 +++++++++++++++---
2 files changed, 16 insertions(+), 3 deletions(-)
create mode 100644 selftest/knownfail.d/dsheuristics_userPassword
diff --git a/selftest/knownfail.d/dsheuristics_userPassword b/selftest/knownfail.d/dsheuristics_userPassword
new file mode 100644
index 00000000000..6981255d9e9
--- /dev/null
+++ b/selftest/knownfail.d/dsheuristics_userPassword
@@ -0,0 +1 @@
+^samba4.ldap.passwords.python\(.*\).__main__.PasswordTests.test_modify_dsheuristics_userPassword
diff --git a/source4/dsdb/samdb/ldb_modules/acl.c b/source4/dsdb/samdb/ldb_modules/acl.c
index d750362c47f..8b1dcbeed51 100644
--- a/source4/dsdb/samdb/ldb_modules/acl.c
+++ b/source4/dsdb/samdb/ldb_modules/acl.c
@@ -108,8 +108,6 @@ static int acl_module_init(struct ldb_module *module)
NULL, "acl", "search", true);
ldb_module_set_private(module, data);
- data->userPassword_support = dsdb_user_password_support(module, module, NULL);
-
mem_ctx = talloc_new(module);
if (!mem_ctx) {
return ldb_oom(ldb);
@@ -180,7 +178,21 @@ static int acl_module_init(struct ldb_module *module)
done:
talloc_free(mem_ctx);
- return ldb_next_init(module);
+ ret = ldb_next_init(module);
+
+ if (ret != LDB_SUCCESS) {
+ return ret;
+ }
+
+ /*
+ * Check this after the modules have be initalised so we
+ * can actually read the backend DB.
+ */
+ data->userPassword_support
+ = dsdb_user_password_support(module,
+ module,
+ NULL);
+ return ret;
}
static int acl_allowedAttributes(struct ldb_module *module,
--
2.14.3
From 420074e5a5ff44bcb6e93998f2434f97227f9a5f Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Wed, 11 Apr 2018 22:47:03 +1200
Subject: [PATCH 2/2] dsdb: check for dSHeuristics more carefully
This check would pass if the dSHeuristics was treated as always being
000000000 for searches which is not enough, we must check for a value
of 000000001 (userPassword enabled).
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13378
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
selftest/knownfail.d/dsheuristics_userPassword | 1 -
source4/dsdb/tests/python/passwords.py | 31 +++++++++++++++++++++-----
2 files changed, 25 insertions(+), 7 deletions(-)
delete mode 100644 selftest/knownfail.d/dsheuristics_userPassword
diff --git a/selftest/knownfail.d/dsheuristics_userPassword b/selftest/knownfail.d/dsheuristics_userPassword
deleted file mode 100644
index 6981255d9e9..00000000000
--- a/selftest/knownfail.d/dsheuristics_userPassword
+++ /dev/null
@@ -1 +0,0 @@
-^samba4.ldap.passwords.python\(.*\).__main__.PasswordTests.test_modify_dsheuristics_userPassword
diff --git a/source4/dsdb/tests/python/passwords.py b/source4/dsdb/tests/python/passwords.py
index c8c2b762a64..bbb8be1d2ca 100755
--- a/source4/dsdb/tests/python/passwords.py
+++ b/source4/dsdb/tests/python/passwords.py
@@ -985,7 +985,8 @@ userPassword: thatsAcomplPASS4
res = ldb1.search("cn=testuser,cn=users," + self.base_dn,
scope=SCOPE_BASE, attrs=["userPassword"])
- # userPassword cannot be read, despite the dsHeuristic setting
+ # userPassword cannot be read, it wasn't set, instead the
+ # password was
self.assertTrue(len(res) == 1)
self.assertFalse("userPassword" in res[0])
@@ -993,7 +994,15 @@ userPassword: thatsAcomplPASS4
ldb2 = SamDB(url=host, session_info=system_session(lp),
credentials=creds, lp=lp)
- # Set userPassword to be unreadable
+ res = ldb2.search("cn=testuser,cn=users," + self.base_dn,
+ scope=SCOPE_BASE, attrs=["userPassword"])
+
+ # Check on the new connection that userPassword was not stored
+ # from ldb1 or is not readable
+ self.assertTrue(len(res) == 1)
+ self.assertFalse("userPassword" in res[0])
+
+ # Set userPassword to be readable
# This setting does not affect this connection
ldb2.set_dsheuristics("000000000")
time.sleep(1)
@@ -1014,11 +1023,10 @@ userPassword: thatsAcomplPASS4
res = ldb2.search("cn=testuser,cn=users," + self.base_dn,
scope=SCOPE_BASE, attrs=["userPassword"])
- # userPassword can be read in this connection
- # This is regardless of the current dsHeuristics setting
+ # Check despite setting it with userPassword support disabled
+ # on this connection it should still not be readable
self.assertTrue(len(res) == 1)
- self.assertTrue("userPassword" in res[0])
- self.assertEquals(res[0]["userPassword"][0], "thatsAcomplPASS2")
+ self.assertFalse("userPassword" in res[0])
# Only password from ldb1 is the user's password
creds2 = Credentials()
@@ -1050,6 +1058,17 @@ userPassword: thatsAcomplPASS4
# Reset the test "dSHeuristics" (reactivate "userPassword" pwd changes)
self.ldb.set_dsheuristics("000000001")
+ ldb4 = SamDB(url=host, session_info=system_session(lp),
+ credentials=creds, lp=lp)
+
+ # Check that userPassword that was stored from ldb2
+ res = ldb4.search("cn=testuser,cn=users," + self.base_dn,
+ scope=SCOPE_BASE, attrs=["userPassword"])
+
+ # userPassword can be not be read
+ self.assertTrue(len(res) == 1)
+ self.assertFalse("userPassword" in res[0])
+
def test_zero_length(self):
# Get the old "minPwdLength"
minPwdLength = self.ldb.get_minPwdLength()
--
2.14.3
More information about the samba-technical
mailing list