110 lines
5.2 KiB
Diff
110 lines
5.2 KiB
Diff
From 4b3da3a97d1cbfd17a4eef466eb3bc1fc4887a34 Mon Sep 17 00:00:00 2001
|
|
From: Andrew Bartlett <abartlet@samba.org>
|
|
Date: Tue, 8 Aug 2023 17:58:27 +1200
|
|
Subject: [PATCH 24/28] CVE-2023-4154: Unimplement the original DirSync
|
|
behaviour without LDAP_DIRSYNC_OBJECT_SECURITY
|
|
|
|
This makes LDAP_DIRSYNC_OBJECT_SECURITY the only behaviour provided by
|
|
Samba.
|
|
|
|
Having a second access control system withing the LDAP stack is unsafe
|
|
and this layer is incomplete.
|
|
|
|
The current system gives all accounts that have been given the
|
|
GUID_DRS_GET_CHANGES extended right SYSTEM access. Currently in Samba
|
|
this equates to full access to passwords as well as "RODC Filtered
|
|
attributes" (often used with confidential attributes).
|
|
|
|
Rather than attempting to correctly filter for secrets (passwords) and
|
|
these filtered attributes, as well as preventing search expressions for
|
|
both, we leave this complexity to the acl_read module which has this
|
|
facility already well tested.
|
|
|
|
The implication is that callers will only see and filter by attribute
|
|
in DirSync that they could without DirSync.
|
|
|
|
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15424
|
|
|
|
Signed-off-by: Andrew Bartlett <abartlet@samba.org>
|
|
|
|
Conflict: NA
|
|
Reference: https://download.samba.org/pub/samba/patches/security/samba-4.18.8-security-2023-10-10.patch
|
|
[PATCH 24/28] CVE-2023-4154: Unimplement the original DirSync
|
|
behaviour without LDAP_DIRSYNC_OBJECT_SECURITY
|
|
---
|
|
selftest/knownfail.d/dirsync | 3 +--
|
|
source4/dsdb/samdb/ldb_modules/dirsync.c | 22 +++++++++++-----------
|
|
2 files changed, 12 insertions(+), 13 deletions(-)
|
|
|
|
diff --git a/selftest/knownfail.d/dirsync b/selftest/knownfail.d/dirsync
|
|
index db098549a08..fcf4d469d6e 100644
|
|
--- a/selftest/knownfail.d/dirsync
|
|
+++ b/selftest/knownfail.d/dirsync
|
|
@@ -1,12 +1,11 @@
|
|
^samba4.ldap.dirsync.python\(.*\).__main__.ConfidentialDirsyncTests.test_dirsync_OBJECT_SECURITY_insist_on_empty_element\(.*\)
|
|
^samba4.ldap.dirsync.python\(.*\).__main__.ConfidentialDirsyncTests.test_dirsync_unicodePwd_OBJ_SEC_insist_on_empty_element\(.*\)
|
|
-^samba4.ldap.dirsync.python\(.*\).__main__.ConfidentialDirsyncTests.test_dirsync_unicodePwd_with_GET_CHANGES\(.*\)
|
|
^samba4.ldap.dirsync.python\(.*\).__main__.ConfidentialDirsyncTests.test_dirsync_unicodePwd_with_GET_CHANGES_OBJ_SEC_insist_on_empty_element\(.*\)
|
|
^samba4.ldap.dirsync.python\(.*\).__main__.ConfidentialDirsyncTests.test_dirsync_unicodePwd_with_GET_CHANGES_insist_on_empty_element\(.*\)
|
|
^samba4.ldap.dirsync.python\(.*\).__main__.ConfidentialDirsyncTests.test_dirsync_with_GET_CHANGES_OBJECT_SECURITY_insist_on_empty_element\(.*\)
|
|
+^samba4.ldap.dirsync.python\(.*\).__main__.ConfidentialDirsyncTests.test_dirsync_with_GET_CHANGES\(.*\)
|
|
^samba4.ldap.dirsync.python\(.*\).__main__.ConfidentialFilteredDirsyncTests.test_dirsync_OBJECT_SECURITY_insist_on_empty_element\(.*\)
|
|
^samba4.ldap.dirsync.python\(.*\).__main__.ConfidentialFilteredDirsyncTests.test_dirsync_OBJECT_SECURITY_with_GET_CHANGES_insist_on_empty_element\(.*\)
|
|
-^samba4.ldap.dirsync.python\(.*\).__main__.ConfidentialFilteredDirsyncTests.test_dirsync_with_GET_CHANGES\(.*\)
|
|
^samba4.ldap.dirsync.python\(.*\).__main__.ConfidentialFilteredDirsyncTests.test_dirsync_with_GET_CHANGES_attr\(.*\)
|
|
^samba4.ldap.dirsync.python\(.*\).__main__.ConfidentialFilteredDirsyncTests.test_dirsync_with_GET_CHANGES_insist_on_empty_element\(.*\)
|
|
^samba4.ldap.dirsync.python\(.*\).__main__.FilteredDirsyncTests.test_dirsync_with_GET_CHANGES\(.*\)
|
|
diff --git a/source4/dsdb/samdb/ldb_modules/dirsync.c b/source4/dsdb/samdb/ldb_modules/dirsync.c
|
|
index b3c463741c8..fbb75790095 100644
|
|
--- a/source4/dsdb/samdb/ldb_modules/dirsync.c
|
|
+++ b/source4/dsdb/samdb/ldb_modules/dirsync.c
|
|
@@ -56,7 +56,6 @@ struct dirsync_context {
|
|
bool linkIncrVal;
|
|
bool localonly;
|
|
bool partial;
|
|
- bool assystem;
|
|
int functional_level;
|
|
const struct GUID *our_invocation_id;
|
|
const struct dsdb_schema *schema;
|
|
@@ -872,10 +871,6 @@ static int dirsync_search_callback(struct ldb_request *req, struct ldb_reply *ar
|
|
DSDB_SEARCH_SHOW_DELETED |
|
|
DSDB_SEARCH_SHOW_EXTENDED_DN;
|
|
|
|
- if (dsc->assystem) {
|
|
- flags = flags | DSDB_FLAG_AS_SYSTEM;
|
|
- }
|
|
-
|
|
ret = dsdb_module_search_tree(dsc->module, dsc, &res,
|
|
dn, LDB_SCOPE_BASE,
|
|
req->op.search.tree,
|
|
@@ -1102,16 +1097,21 @@ static int dirsync_ldb_search(struct ldb_module *module, struct ldb_request *req
|
|
return LDB_ERR_OPERATIONS_ERROR;
|
|
}
|
|
objectclass = dsdb_get_structural_oc_from_msg(schema, acl_res->msgs[0]);
|
|
+
|
|
+ /*
|
|
+ * While we never use the answer to this for access
|
|
+ * control (after CVE-2023-4154), we return a
|
|
+ * different error message depending on if the user
|
|
+ * was granted GUID_DRS_GET_CHANGES to provide a closer
|
|
+ * emulation and keep some tests passing.
|
|
+ *
|
|
+ * (Samba's ACL logic is not well suited to redacting
|
|
+ * only the secret and RODC filtered attributes).
|
|
+ */
|
|
ret = acl_check_extended_right(dsc, module, req, objectclass,
|
|
sd, acl_user_token(module),
|
|
GUID_DRS_GET_CHANGES, SEC_ADS_CONTROL_ACCESS, sid);
|
|
|
|
- if (ret == LDB_ERR_INSUFFICIENT_ACCESS_RIGHTS) {
|
|
- return ret;
|
|
- }
|
|
- dsc->assystem = true;
|
|
- ret = ldb_request_add_control(req, LDB_CONTROL_AS_SYSTEM_OID, false, NULL);
|
|
-
|
|
if (ret != LDB_SUCCESS) {
|
|
return ret;
|
|
}
|
|
--
|
|
2.34.1
|