CVE-2023-4154: Unimplement the original DirSync behaviour without LDAP_DIRSYNC_OBJECT...

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>

diff --git a/selftest/knownfail.d/dirsync b/selftest/knownfail.d/dirsync
index db098549a08dfd9c22c7ab72bba2cf5bc42f457c..fcf4d469d6e9d5431bd3ac97be3ed33097e9f6b4 100644 (file)
--- 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 b3c463741c822f69ac3a0d89ffc6f9ea36db153c..fbb75790095b2555cb5da57d5a44caa20b30255f 100644 (file)
--- 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;
                }