!177 fix CVE-2023-0225 CVE-2023-0614 CVE-2023-0922

From: @xinghe_1 
Reviewed-by: @kircher 
Signed-off-by: @kircher
This commit is contained in:
openeuler-ci-bot 2023-04-06 02:35:16 +00:00 committed by Gitee
commit 6d347a7bfc
No known key found for this signature in database
GPG Key ID: 173E9B9CA92EEF8F
41 changed files with 8644 additions and 2 deletions

View File

@ -0,0 +1,93 @@
From 0bc08daf4c191c370cb218e9a0ecac51b8c36468 Mon Sep 17 00:00:00 2001
From: Joseph Sutton <josephsutton@catalyst.net.nz>
Date: Thu, 28 Apr 2022 20:34:36 +1200
Subject: [PATCH 1/4] CVE-2023-0225 CVE-2020-25720 s4/dsdb/util: Add functions
for dsHeuristics 28, 29
These are the newly-added AttributeAuthorizationOnLDAPAdd and
BlockOwnerImplicitRights.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14810
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15276
Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
(cherry picked from commit 0af5706b559e89c77123ed174b41fd3d01705aa5)
[abartlet@samba.org This patch is needed for a clean backport of
CVE-2023-0225 as these constants are used in the acl_modify test
even when this behaviour is not itself used.]
Conflict: NA
Reference: https://attachments.samba.org/attachment.cgi?id=17833
---
libds/common/flags.h | 2 ++
source4/dsdb/samdb/ldb_modules/util.c | 40 +++++++++++++++++++++++++++
2 files changed, 42 insertions(+)
diff --git a/libds/common/flags.h b/libds/common/flags.h
index 75e04b0c488..bee1016b294 100644
--- a/libds/common/flags.h
+++ b/libds/common/flags.h
@@ -258,6 +258,8 @@
#define DS_HR_KVNOEMUW2K 0x00000011
#define DS_HR_TWENTIETH_CHAR 0x00000014
+#define DS_HR_ATTR_AUTHZ_ON_LDAP_ADD 0x0000001C
+#define DS_HR_BLOCK_OWNER_IMPLICIT_RIGHTS 0x0000001D
#define DS_HR_THIRTIETH_CHAR 0x0000001E
#define DS_HR_FOURTIETH_CHAR 0x00000028
#define DS_HR_FIFTIETH_CHAR 0x00000032
diff --git a/source4/dsdb/samdb/ldb_modules/util.c b/source4/dsdb/samdb/ldb_modules/util.c
index 9e00aedd09e..c2949f0734d 100644
--- a/source4/dsdb/samdb/ldb_modules/util.c
+++ b/source4/dsdb/samdb/ldb_modules/util.c
@@ -1433,6 +1433,46 @@ bool dsdb_do_list_object(struct ldb_module *module,
return result;
}
+bool dsdb_attribute_authz_on_ldap_add(struct ldb_module *module,
+ TALLOC_CTX *mem_ctx,
+ struct ldb_request *parent)
+{
+ TALLOC_CTX *tmp_ctx = talloc_new(mem_ctx);
+ bool result = false;
+ const struct ldb_val *hr_val = dsdb_module_find_dsheuristics(module,
+ tmp_ctx,
+ parent);
+ if (hr_val != NULL && hr_val->length >= DS_HR_ATTR_AUTHZ_ON_LDAP_ADD) {
+ uint8_t val = hr_val->data[DS_HR_ATTR_AUTHZ_ON_LDAP_ADD - 1];
+ if (val != '0' && val != '2') {
+ result = true;
+ }
+ }
+
+ talloc_free(tmp_ctx);
+ return result;
+}
+
+bool dsdb_block_owner_implicit_rights(struct ldb_module *module,
+ TALLOC_CTX *mem_ctx,
+ struct ldb_request *parent)
+{
+ TALLOC_CTX *tmp_ctx = talloc_new(mem_ctx);
+ bool result = false;
+ const struct ldb_val *hr_val = dsdb_module_find_dsheuristics(module,
+ tmp_ctx,
+ parent);
+ if (hr_val != NULL && hr_val->length >= DS_HR_BLOCK_OWNER_IMPLICIT_RIGHTS) {
+ uint8_t val = hr_val->data[DS_HR_BLOCK_OWNER_IMPLICIT_RIGHTS - 1];
+ if (val != '0' && val != '2') {
+ result = true;
+ }
+ }
+
+ talloc_free(tmp_ctx);
+ return result;
+}
+
/*
show the chain of requests, useful for debugging async requests
*/
--
2.25.1

View File

@ -0,0 +1,174 @@
From 197d1f09158ee0575cd92c461ed12bdbf4efc074 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet@samba.org>
Date: Mon, 13 Mar 2023 14:25:56 +1300
Subject: [PATCH 01/35] CVE-2023-0614 lib/ldb: Avoid allocation and memcpy()
for every wildcard match candidate
The value can be quite large, the allocation will take much
longer than the actual match and is repeated per candidate
record.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15331
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15270
Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Joseph Sutton <josephsutton@catalyst.net.nz>
(cherry picked from commit cad96f59a08192df927fb1df4e9787c7f70991a2)
[abartlet@samba.org Included in the security release as this
makes the new large_ldap.py timeout test more reliable]
Conflict: NA
Reference: https://attachments.samba.org/attachment.cgi?id=17821
---
lib/ldb/common/ldb_match.c | 60 +++++++++++++++++++++++++++++++-------
1 file changed, 50 insertions(+), 10 deletions(-)
diff --git a/lib/ldb/common/ldb_match.c b/lib/ldb/common/ldb_match.c
index 2f4d41f3441..51376871b4c 100644
--- a/lib/ldb/common/ldb_match.c
+++ b/lib/ldb/common/ldb_match.c
@@ -34,6 +34,7 @@
#include "ldb_private.h"
#include "dlinklist.h"
+#include "ldb_handlers.h"
/*
check if the scope matches in a search result
@@ -259,20 +260,42 @@ static int ldb_wildcard_compare(struct ldb_context *ldb,
return LDB_SUCCESS;
}
- if (a->syntax->canonicalise_fn(ldb, ldb, &value, &val) != 0) {
- return LDB_ERR_INVALID_ATTRIBUTE_SYNTAX;
+ /* No need to just copy this value for a binary match */
+ if (a->syntax->canonicalise_fn != ldb_handler_copy) {
+ if (a->syntax->canonicalise_fn(ldb, ldb, &value, &val) != 0) {
+ return LDB_ERR_INVALID_ATTRIBUTE_SYNTAX;
+ }
+
+ /*
+ * Only set save_p if we allocate (call
+ * a->syntax->canonicalise_fn()), as we
+ * talloc_free(save_p) below to clean up
+ */
+ save_p = val.data;
+ } else {
+ val = value;
}
- save_p = val.data;
cnk.data = NULL;
if ( ! tree->u.substring.start_with_wildcard ) {
+ uint8_t *cnk_to_free = NULL;
chunk = tree->u.substring.chunks[c];
- if (a->syntax->canonicalise_fn(ldb, ldb, chunk, &cnk) != 0) goto mismatch;
+ /* No need to just copy this value for a binary match */
+ if (a->syntax->canonicalise_fn != ldb_handler_copy) {
+ if (a->syntax->canonicalise_fn(ldb, ldb, chunk, &cnk) != 0) {
+ goto mismatch;
+ }
+
+ cnk_to_free = cnk.data;
+ } else {
+ cnk = *chunk;
+ }
/* This deals with wildcard prefix searches on binary attributes (eg objectGUID) */
if (cnk.length > val.length) {
+ TALLOC_FREE(cnk_to_free);
goto mismatch;
}
/*
@@ -280,32 +303,47 @@ static int ldb_wildcard_compare(struct ldb_context *ldb,
* we can cope with this.
*/
if (cnk.length == 0) {
+ TALLOC_FREE(cnk_to_free);
+ goto mismatch;
+ }
+
+ if (memcmp((char *)val.data, (char *)cnk.data, cnk.length) != 0) {
+ TALLOC_FREE(cnk_to_free);
goto mismatch;
}
- if (memcmp((char *)val.data, (char *)cnk.data, cnk.length) != 0) goto mismatch;
val.length -= cnk.length;
val.data += cnk.length;
c++;
- talloc_free(cnk.data);
+ TALLOC_FREE(cnk_to_free);
cnk.data = NULL;
}
while (tree->u.substring.chunks[c]) {
uint8_t *p;
+ uint8_t *cnk_to_free = NULL;
chunk = tree->u.substring.chunks[c];
- if(a->syntax->canonicalise_fn(ldb, ldb, chunk, &cnk) != 0) {
- goto mismatch;
+ /* No need to just copy this value for a binary match */
+ if (a->syntax->canonicalise_fn != ldb_handler_copy) {
+ if (a->syntax->canonicalise_fn(ldb, ldb, chunk, &cnk) != 0) {
+ goto mismatch;
+ }
+
+ cnk_to_free = cnk.data;
+ } else {
+ cnk = *chunk;
}
/*
* Empty strings are returned as length 0. Ensure
* we can cope with this.
*/
if (cnk.length == 0) {
+ TALLOC_FREE(cnk_to_free);
goto mismatch;
}
if (cnk.length > val.length) {
+ TALLOC_FREE(cnk_to_free);
goto mismatch;
}
@@ -320,6 +358,8 @@ static int ldb_wildcard_compare(struct ldb_context *ldb,
cmp = memcmp(p,
cnk.data,
cnk.length);
+ TALLOC_FREE(cnk_to_free);
+
if (cmp != 0) {
goto mismatch;
}
@@ -331,15 +371,16 @@ static int ldb_wildcard_compare(struct ldb_context *ldb,
p = memmem((const void *)val.data, val.length,
(const void *)cnk.data, cnk.length);
if (p == NULL) {
+ TALLOC_FREE(cnk_to_free);
goto mismatch;
}
/* move val to the end of the match */
p += cnk.length;
val.length -= (p - val.data);
val.data = p;
+ TALLOC_FREE(cnk_to_free);
}
c++;
- TALLOC_FREE(cnk.data);
}
talloc_free(save_p);
@@ -349,7 +390,6 @@ static int ldb_wildcard_compare(struct ldb_context *ldb,
mismatch:
*matched = false;
talloc_free(save_p);
- talloc_free(cnk.data);
return LDB_SUCCESS;
}
--
2.25.1

View File

@ -0,0 +1,68 @@
From 47f8a529885d321c4f787832d5934757656e8094 Mon Sep 17 00:00:00 2001
From: Joseph Sutton <josephsutton@catalyst.net.nz>
Date: Tue, 6 Sep 2022 19:23:13 +1200
Subject: [PATCH 2/4] CVE-2023-0225 CVE-2020-25720 pydsdb: Add dsHeuristics
constant definitions
We want to be able to use these values in Python tests.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14810
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15276
Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
(cherry picked from commit cc709077822a39227174b91ed2345c2bd603f61f)
[abartlet@samba.org This patch is needed for a clean backport of
CVE-2023-0225 as these constants are used in the acl_modify test
even when this behaviour is not itself used.]
Conflict: NA
Reference: https://attachments.samba.org/attachment.cgi?id=17833
---
source4/dsdb/pydsdb.c | 30 ++++++++++++++++++++++++++++++
1 file changed, 30 insertions(+)
diff --git a/source4/dsdb/pydsdb.c b/source4/dsdb/pydsdb.c
index bcfc7e95478..626d849a561 100644
--- a/source4/dsdb/pydsdb.c
+++ b/source4/dsdb/pydsdb.c
@@ -1665,6 +1665,36 @@ MODULE_INIT_FUNC(dsdb)
ADD_DSDB_FLAG(DS_NTDSDSA_OPT_DISABLE_NTDSCONN_XLATE);
ADD_DSDB_FLAG(DS_NTDSDSA_OPT_DISABLE_SPN_REGISTRATION);
+ /* dsHeuristics character indexes (see MS-ADTS 7.1.1.2.4.1.2) */
+ ADD_DSDB_FLAG(DS_HR_SUPFIRSTLASTANR);
+ ADD_DSDB_FLAG(DS_HR_SUPLASTFIRSTANR);
+ ADD_DSDB_FLAG(DS_HR_DOLISTOBJECT);
+ ADD_DSDB_FLAG(DS_HR_DONICKRES);
+ ADD_DSDB_FLAG(DS_HR_LDAP_USEPERMMOD);
+ ADD_DSDB_FLAG(DS_HR_HIDEDSID);
+ ADD_DSDB_FLAG(DS_HR_BLOCK_ANONYMOUS_OPS);
+ ADD_DSDB_FLAG(DS_HR_ALLOW_ANON_NSPI);
+ ADD_DSDB_FLAG(DS_HR_USER_PASSWORD_SUPPORT);
+ ADD_DSDB_FLAG(DS_HR_TENTH_CHAR);
+ ADD_DSDB_FLAG(DS_HR_SPECIFY_GUID_ON_ADD);
+ ADD_DSDB_FLAG(DS_HR_NO_STANDARD_SD);
+ ADD_DSDB_FLAG(DS_HR_ALLOW_NONSECURE_PWD_OPS);
+ ADD_DSDB_FLAG(DS_HR_NO_PROPAGATE_ON_NOCHANGE);
+ ADD_DSDB_FLAG(DS_HR_COMPUTE_ANR_STATS);
+ ADD_DSDB_FLAG(DS_HR_ADMINSDEXMASK);
+ ADD_DSDB_FLAG(DS_HR_KVNOEMUW2K);
+
+ ADD_DSDB_FLAG(DS_HR_TWENTIETH_CHAR);
+ ADD_DSDB_FLAG(DS_HR_ATTR_AUTHZ_ON_LDAP_ADD);
+ ADD_DSDB_FLAG(DS_HR_BLOCK_OWNER_IMPLICIT_RIGHTS);
+ ADD_DSDB_FLAG(DS_HR_THIRTIETH_CHAR);
+ ADD_DSDB_FLAG(DS_HR_FOURTIETH_CHAR);
+ ADD_DSDB_FLAG(DS_HR_FIFTIETH_CHAR);
+ ADD_DSDB_FLAG(DS_HR_SIXTIETH_CHAR);
+ ADD_DSDB_FLAG(DS_HR_SEVENTIETH_CHAR);
+ ADD_DSDB_FLAG(DS_HR_EIGHTIETH_CHAR);
+ ADD_DSDB_FLAG(DS_HR_NINETIETH_CHAR);
+
ADD_DSDB_FLAG(NTDSCONN_KCC_GC_TOPOLOGY);
ADD_DSDB_FLAG(NTDSCONN_KCC_RING_TOPOLOGY);
ADD_DSDB_FLAG(NTDSCONN_KCC_MINIMIZE_HOPS_TOPOLOGY);
--
2.25.1

View File

@ -0,0 +1,137 @@
From ba8c9eae1392bf21d3e36530eda1780defd22300 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet@samba.org>
Date: Mon, 13 Mar 2023 17:20:00 +1300
Subject: [PATCH 02/35] CVE-2023-0614 selftest: Use setUpClass() to reduce
"make test TESTS=large_ldap" time
This reduces the elapsed time to 6m from 20m on my laptop.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15332
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15270
Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Autobuild-User(master): Andrew Bartlett <abartlet@samba.org>
Autobuild-Date(master): Tue Mar 14 07:16:04 UTC 2023 on atb-devel-224
(cherry picked from commit b4a6c054ec6acefacd22cb7230a783d20cb07c05)
[abartlet@samba.org Included in the security release as this
makes working on the large_ldap test practical by reducing
the elapsed time taken]
Conflict: NA
Reference: https://attachments.samba.org/attachment.cgi?id=17821
---
source4/dsdb/tests/python/large_ldap.py | 69 +++++++++++++------------
1 file changed, 36 insertions(+), 33 deletions(-)
diff --git a/source4/dsdb/tests/python/large_ldap.py b/source4/dsdb/tests/python/large_ldap.py
index 0805119a700..13729052e65 100644
--- a/source4/dsdb/tests/python/large_ldap.py
+++ b/source4/dsdb/tests/python/large_ldap.py
@@ -66,30 +66,32 @@ creds = credopts.get_credentials(lp)
class ManyLDAPTest(samba.tests.TestCase):
- def setUp(self):
- super(ManyLDAPTest, self).setUp()
- self.ldb = SamDB(url, credentials=creds, session_info=system_session(lp), lp=lp)
- self.base_dn = self.ldb.domain_dn()
- self.OU_NAME_MANY="many_ou" + format(random.randint(0, 99999), "05")
- self.ou_dn = ldb.Dn(self.ldb, "ou=" + self.OU_NAME_MANY + "," + str(self.base_dn))
-
- samba.tests.delete_force(self.ldb, self.ou_dn,
+ @classmethod
+ def setUpClass(cls):
+ super().setUpClass()
+ cls.ldb = SamDB(url, credentials=creds, session_info=system_session(lp), lp=lp)
+ cls.base_dn = self.ldb.domain_dn()
+ cls.OU_NAME_MANY="many_ou" + format(random.randint(0, 99999), "05")
+ cls.ou_dn = ldb.Dn(self.ldb, "ou=" + self.OU_NAME_MANY + "," + str(self.base_dn))
+
+ samba.tests.delete_force(cls.ldb, cls.ou_dn,
controls=['tree_delete:1'])
- self.ldb.add({
- "dn": self.ou_dn,
+ cls.ldb.add({
+ "dn": cls.ou_dn,
"objectclass": "organizationalUnit",
- "ou": self.OU_NAME_MANY})
+ "ou": cls.OU_NAME_MANY})
for x in range(2000):
- ou_name = self.OU_NAME_MANY + str(x)
- self.ldb.add({
- "dn": "ou=" + ou_name + "," + str(self.ou_dn),
+ ou_name = cls.OU_NAME_MANY + str(x)
+ cls.ldb.add({
+ "dn": "ou=" + ou_name + "," + str(cls.ou_dn),
"objectclass": "organizationalUnit",
"ou": ou_name})
- def tearDown(self):
- samba.tests.delete_force(self.ldb, self.ou_dn,
+ @classmethod
+ def tearDownClass(cls):
+ samba.tests.delete_force(cls.ldb, self.ou_dn,
controls=['tree_delete:1'])
def test_unindexed_iterator_search(self):
@@ -117,34 +119,35 @@ class ManyLDAPTest(samba.tests.TestCase):
class LargeLDAPTest(samba.tests.TestCase):
- def setUp(self):
- super(LargeLDAPTest, self).setUp()
- self.ldb = SamDB(url, credentials=creds, session_info=system_session(lp), lp=lp)
- self.base_dn = self.ldb.domain_dn()
- self.USER_NAME = "large_user" + format(random.randint(0, 99999), "05") + "-"
- self.OU_NAME="large_user_ou" + format(random.randint(0, 99999), "05")
- self.ou_dn = ldb.Dn(self.ldb, "ou=" + self.OU_NAME + "," + str(self.base_dn))
+ @classmethod
+ def setUpClass(cls):
+ cls.ldb = SamDB(url, credentials=creds, session_info=system_session(lp), lp=lp)
+ cls.base_dn = cls.ldb.domain_dn()
+ cls.USER_NAME = "large_user" + format(random.randint(0, 99999), "05") + "-"
+ cls.OU_NAME="large_user_ou" + format(random.randint(0, 99999), "05")
+ cls.ou_dn = ldb.Dn(cls.ldb, "ou=" + cls.OU_NAME + "," + str(cls.base_dn))
- samba.tests.delete_force(self.ldb, self.ou_dn,
+ samba.tests.delete_force(cls.ldb, cls.ou_dn,
controls=['tree_delete:1'])
- self.ldb.add({
- "dn": self.ou_dn,
+ cls.ldb.add({
+ "dn": cls.ou_dn,
"objectclass": "organizationalUnit",
- "ou": self.OU_NAME})
+ "ou": cls.OU_NAME})
for x in range(200):
- user_name = self.USER_NAME + format(x, "03")
- self.ldb.add({
- "dn": "cn=" + user_name + "," + str(self.ou_dn),
+ user_name = cls.USER_NAME + format(x, "03")
+ cls.ldb.add({
+ "dn": "cn=" + user_name + "," + str(cls.ou_dn),
"objectclass": "user",
"sAMAccountName": user_name,
"jpegPhoto": b'a' * (2 * 1024 * 1024)})
- def tearDown(self):
+ @classmethod
+ def tearDownClass(cls):
# Remake the connection for tear-down (old Samba drops the socket)
- self.ldb = SamDB(url, credentials=creds, session_info=system_session(lp), lp=lp)
- samba.tests.delete_force(self.ldb, self.ou_dn,
+ cls.ldb = SamDB(url, credentials=creds, session_info=system_session(lp), lp=lp)
+ samba.tests.delete_force(cls.ldb, cls.ou_dn,
controls=['tree_delete:1'])
def test_unindexed_iterator_search(self):
--
2.25.1

View File

@ -0,0 +1,291 @@
From 417fda9bfde2f8db315ea0460fb7da6e780b859c Mon Sep 17 00:00:00 2001
From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Date: Wed, 4 Jan 2023 21:37:49 +1300
Subject: [PATCH 3/4] CVE-2023-0225 pytest/acl: test deleting dNSHostName as
unprivileged user
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15276
Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
[abartlet@samba.org The self.set_heuristic(samba.dsdb.DS_HR_ATTR_AUTHZ_ON_LDAP_ADD, b'11')
in the test setUp() is unused in this test but is included as a
clean backport, so the fact that the server does not implement this
is unimportant]
Conflict: NA
Reference: https://attachments.samba.org/attachment.cgi?id=17833
---
selftest/knownfail.d/dns-host-name-deletion | 2 +
source4/dsdb/tests/python/acl_modify.py | 236 ++++++++++++++++++++
source4/selftest/tests.py | 1 +
3 files changed, 239 insertions(+)
create mode 100644 selftest/knownfail.d/dns-host-name-deletion
create mode 100755 source4/dsdb/tests/python/acl_modify.py
diff --git a/selftest/knownfail.d/dns-host-name-deletion b/selftest/knownfail.d/dns-host-name-deletion
new file mode 100644
index 00000000000..ac11619ffc3
--- /dev/null
+++ b/selftest/knownfail.d/dns-host-name-deletion
@@ -0,0 +1,2 @@
+^samba4.ldap.acl_modify.python\(.*\).__main__.AclModifyTests.test_modify_delete_dns_host_name_ldif_unspecified\(.*\)
+^samba4.ldap.acl_modify.python\(.*\).__main__.AclModifyTests.test_modify_delete_dns_host_name_unspecified\(.*\)
diff --git a/source4/dsdb/tests/python/acl_modify.py b/source4/dsdb/tests/python/acl_modify.py
new file mode 100755
index 00000000000..c85748a764f
--- /dev/null
+++ b/source4/dsdb/tests/python/acl_modify.py
@@ -0,0 +1,236 @@
+#!/usr/bin/env python3
+# -*- coding: utf-8 -*-
+
+
+import optparse
+import sys
+sys.path.insert(0, "bin/python")
+import samba
+
+from samba.tests.subunitrun import SubunitOptions, TestProgram
+
+import samba.getopt as options
+
+from ldb import ERR_INSUFFICIENT_ACCESS_RIGHTS
+from ldb import Message, MessageElement, Dn
+from ldb import FLAG_MOD_REPLACE, FLAG_MOD_DELETE
+from samba.dcerpc import security
+
+from samba.auth import system_session
+from samba import gensec, sd_utils
+from samba.samdb import SamDB
+from samba.credentials import Credentials, DONT_USE_KERBEROS
+import samba.tests
+import samba.dsdb
+
+
+parser = optparse.OptionParser("acl.py [options] <host>")
+sambaopts = options.SambaOptions(parser)
+parser.add_option_group(sambaopts)
+parser.add_option_group(options.VersionOptions(parser))
+
+# use command line creds if available
+credopts = options.CredentialsOptions(parser)
+parser.add_option_group(credopts)
+subunitopts = SubunitOptions(parser)
+parser.add_option_group(subunitopts)
+
+opts, args = parser.parse_args()
+
+if len(args) < 1:
+ parser.print_usage()
+ sys.exit(1)
+
+host = args[0]
+if "://" not in host:
+ ldaphost = "ldap://%s" % host
+else:
+ ldaphost = host
+ start = host.rindex("://")
+ host = host.lstrip(start + 3)
+
+lp = sambaopts.get_loadparm()
+creds = credopts.get_credentials(lp)
+creds.set_gensec_features(creds.get_gensec_features() | gensec.FEATURE_SEAL)
+
+#
+# Tests start here
+#
+
+
+class AclTests(samba.tests.TestCase):
+
+ def setUp(self):
+ super(AclTests, self).setUp()
+
+ strict_checking = samba.tests.env_get_var_value('STRICT_CHECKING', allow_missing=True)
+ if strict_checking is None:
+ strict_checking = '1'
+ self.strict_checking = bool(int(strict_checking))
+
+ self.ldb_admin = SamDB(ldaphost, credentials=creds, session_info=system_session(lp), lp=lp)
+ self.base_dn = self.ldb_admin.domain_dn()
+ self.domain_sid = security.dom_sid(self.ldb_admin.get_domain_sid())
+ self.user_pass = "samba123@"
+ self.configuration_dn = self.ldb_admin.get_config_basedn().get_linearized()
+ self.sd_utils = sd_utils.SDUtils(self.ldb_admin)
+ self.addCleanup(self.delete_admin_connection)
+ # used for anonymous login
+ self.creds_tmp = Credentials()
+ self.creds_tmp.set_username("")
+ self.creds_tmp.set_password("")
+ self.creds_tmp.set_domain(creds.get_domain())
+ self.creds_tmp.set_realm(creds.get_realm())
+ self.creds_tmp.set_workstation(creds.get_workstation())
+ print("baseDN: %s" % self.base_dn)
+
+ # set AttributeAuthorizationOnLDAPAdd and BlockOwnerImplicitRights
+ self.set_heuristic(samba.dsdb.DS_HR_ATTR_AUTHZ_ON_LDAP_ADD, b'11')
+
+ def set_heuristic(self, index, values):
+ self.assertGreater(index, 0)
+ self.assertLess(index, 30)
+ self.assertIsInstance(values, bytes)
+
+ # Get the old "dSHeuristics" if it was set
+ dsheuristics = self.ldb_admin.get_dsheuristics()
+ # Reset the "dSHeuristics" as they were before
+ self.addCleanup(self.ldb_admin.set_dsheuristics, dsheuristics)
+ # Set the "dSHeuristics" to activate the correct behaviour
+ default_heuristics = b"000000000100000000020000000003"
+ if dsheuristics is None:
+ dsheuristics = b""
+ dsheuristics += default_heuristics[len(dsheuristics):]
+ dsheuristics = (dsheuristics[:index - 1] +
+ values +
+ dsheuristics[index - 1 + len(values):])
+ self.ldb_admin.set_dsheuristics(dsheuristics)
+
+ def get_user_dn(self, name):
+ return "CN=%s,CN=Users,%s" % (name, self.base_dn)
+
+ def get_ldb_connection(self, target_username, target_password):
+ creds_tmp = Credentials()
+ creds_tmp.set_username(target_username)
+ creds_tmp.set_password(target_password)
+ creds_tmp.set_domain(creds.get_domain())
+ creds_tmp.set_realm(creds.get_realm())
+ creds_tmp.set_workstation(creds.get_workstation())
+ creds_tmp.set_gensec_features(creds_tmp.get_gensec_features()
+ | gensec.FEATURE_SEAL)
+ creds_tmp.set_kerberos_state(DONT_USE_KERBEROS) # kinit is too expensive to use in a tight loop
+ ldb_target = SamDB(url=ldaphost, credentials=creds_tmp, lp=lp)
+ return ldb_target
+
+ # Test if we have any additional groups for users than default ones
+ def assert_user_no_group_member(self, username):
+ res = self.ldb_admin.search(self.base_dn, expression="(distinguishedName=%s)" % self.get_user_dn(username))
+ try:
+ self.assertEqual(res[0]["memberOf"][0], "")
+ except KeyError:
+ pass
+ else:
+ self.fail()
+
+ def delete_admin_connection(self):
+ del self.sd_utils
+ del self.ldb_admin
+
+
+class AclModifyTests(AclTests):
+
+ def setup_computer_with_hostname(self, account_name):
+ ou_dn = f'OU={account_name},{self.base_dn}'
+ dn = f'CN={account_name},{ou_dn}'
+
+ user, password = "mouse", "mus musculus 123!"
+ self.addCleanup(self.ldb_admin.deleteuser, user)
+
+ self.ldb_admin.newuser(user, password)
+ self.ldb_user = self.get_ldb_connection(user, password)
+
+ self.addCleanup(self.ldb_admin.delete, ou_dn,
+ controls=["tree_delete:0"])
+ self.ldb_admin.create_ou(ou_dn)
+
+ self.ldb_admin.add({
+ 'dn': dn,
+ 'objectClass': 'computer',
+ 'sAMAccountName': account_name + '$',
+ })
+
+ host_name = f'{account_name}.{self.ldb_user.domain_dns_name()}'
+
+ m = Message(Dn(self.ldb_admin, dn))
+ m['dNSHostName'] = MessageElement(host_name,
+ FLAG_MOD_REPLACE,
+ 'dNSHostName')
+
+ self.ldb_admin.modify(m)
+ return host_name, dn
+
+ def test_modify_delete_dns_host_name_specified(self):
+ '''Test deleting dNSHostName'''
+ account_name = self.id().rsplit(".", 1)[1][:63]
+ host_name, dn = self.setup_computer_with_hostname(account_name)
+
+ m = Message(Dn(self.ldb_user, dn))
+ m['dNSHostName'] = MessageElement(host_name,
+ FLAG_MOD_DELETE,
+ 'dNSHostName')
+
+ self.assertRaisesLdbError(
+ ERR_INSUFFICIENT_ACCESS_RIGHTS,
+ "User able to delete dNSHostName (with specified name)",
+ self.ldb_user.modify, m)
+
+ def test_modify_delete_dns_host_name_unspecified(self):
+ '''Test deleting dNSHostName'''
+ account_name = self.id().rsplit(".", 1)[1][:63]
+ host_name, dn = self.setup_computer_with_hostname(account_name)
+
+ m = Message(Dn(self.ldb_user, dn))
+ m['dNSHostName'] = MessageElement([],
+ FLAG_MOD_DELETE,
+ 'dNSHostName')
+
+ self.assertRaisesLdbError(
+ ERR_INSUFFICIENT_ACCESS_RIGHTS,
+ "User able to delete dNSHostName (without specified name)",
+ self.ldb_user.modify, m)
+
+ def test_modify_delete_dns_host_name_ldif_specified(self):
+ '''Test deleting dNSHostName'''
+ account_name = self.id().rsplit(".", 1)[1][:63]
+ host_name, dn = self.setup_computer_with_hostname(account_name)
+
+ ldif = f"""
+dn: {dn}
+changetype: modify
+delete: dNSHostName
+dNSHostName: {host_name}
+"""
+ self.assertRaisesLdbError(
+ ERR_INSUFFICIENT_ACCESS_RIGHTS,
+ "User able to delete dNSHostName (with specified name)",
+ self.ldb_user.modify_ldif, ldif)
+
+ def test_modify_delete_dns_host_name_ldif_unspecified(self):
+ '''Test deleting dNSHostName'''
+ account_name = self.id().rsplit(".", 1)[1][:63]
+ host_name, dn = self.setup_computer_with_hostname(account_name)
+
+ ldif = f"""
+dn: {dn}
+changetype: modify
+delete: dNSHostName
+"""
+ self.assertRaisesLdbError(
+ ERR_INSUFFICIENT_ACCESS_RIGHTS,
+ "User able to delete dNSHostName (without specific name)",
+ self.ldb_user.modify_ldif, ldif)
+
+
+ldb = SamDB(ldaphost, credentials=creds, session_info=system_session(lp), lp=lp)
+
+TestProgram(module=__name__, opts=subunitopts)
diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py
index c6bf668aa9c..9ef9600d886 100755
--- a/source4/selftest/tests.py
+++ b/source4/selftest/tests.py
@@ -1417,6 +1417,7 @@ for env in all_fl_envs + ["schema_dc"]:
plantestsuite("samba4.ldap.possibleInferiors.python(%s)" % env, env, [python, os.path.join(samba4srcdir, "dsdb/samdb/ldb_modules/tests/possibleinferiors.py"), "ldap://$SERVER", '-U"$USERNAME%$PASSWORD"', "-W$DOMAIN"])
plantestsuite_loadlist("samba4.ldap.secdesc.python(%s)" % env, env, [python, os.path.join(DSDB_PYTEST_DIR, "sec_descriptor.py"), '$SERVER', '-U"$USERNAME%$PASSWORD"', '--workgroup=$DOMAIN', '$LOADLIST', '$LISTOPT'])
plantestsuite_loadlist("samba4.ldap.acl.python(%s)" % env, env, ["STRICT_CHECKING=0", python, os.path.join(DSDB_PYTEST_DIR, "acl.py"), '$SERVER', '-U"$USERNAME%$PASSWORD"', '--workgroup=$DOMAIN', '$LOADLIST', '$LISTOPT'])
+ plantestsuite_loadlist("samba4.ldap.acl_modify.python(%s)" % env, env, ["STRICT_CHECKING=0", python, os.path.join(DSDB_PYTEST_DIR, "acl_modify.py"), '$SERVER', '-U"$USERNAME%$PASSWORD"', '--workgroup=$DOMAIN', '$LOADLIST', '$LISTOPT'])
for env in all_fl_envs + ["schema_dc", "ad_dc_no_ntlm"]:
if env != "fl2000dc":
--
2.25.1

View File

@ -0,0 +1,77 @@
From e7aa14a5405735234b989bdeba384c7c9249257a Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet@samba.org>
Date: Fri, 3 Mar 2023 10:31:40 +1300
Subject: [PATCH 01/34] CVE-2023-0614 dsdb: Alter timeout test in large_ldap.py
to be slower by matching on large objects
This changes the slow aspect to be the object matching not the filter parsing.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15270
Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Conflict: NA
Reference: https://attachments.samba.org/attachment.cgi?id=17821
---
source4/dsdb/tests/python/large_ldap.py | 17 +++++++++++++++--
1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/source4/dsdb/tests/python/large_ldap.py b/source4/dsdb/tests/python/large_ldap.py
index 0805119a700..f7569607cb2 100644
--- a/source4/dsdb/tests/python/large_ldap.py
+++ b/source4/dsdb/tests/python/large_ldap.py
@@ -32,7 +32,7 @@ from samba.tests.subunitrun import SubunitOptions, TestProgram
import samba.getopt as options
from samba.auth import system_session
-from samba import ldb
+from samba import ldb, sd_utils
from samba.samdb import SamDB
from samba.ndr import ndr_unpack
from samba import gensec
@@ -123,10 +123,13 @@ class LargeLDAPTest(samba.tests.TestCase):
def setUpClass(cls):
cls.ldb = SamDB(url, credentials=creds, session_info=system_session(lp), lp=lp)
cls.base_dn = cls.ldb.domain_dn()
+
+ cls.sd_utils = sd_utils.SDUtils(cls.ldb)
cls.USER_NAME = "large_user" + format(random.randint(0, 99999), "05") + "-"
cls.OU_NAME="large_user_ou" + format(random.randint(0, 99999), "05")
cls.ou_dn = ldb.Dn(cls.ldb, "ou=" + cls.OU_NAME + "," + str(cls.base_dn))
+
samba.tests.delete_force(cls.ldb, cls.ou_dn,
controls=['tree_delete:1'])
@@ -249,6 +252,7 @@ class LargeLDAPTest(samba.tests.TestCase):
self.assertGreater(count, count_jpeg)
def test_timeout(self):
+
policy_dn = ldb.Dn(self.ldb,
'CN=Default Query Policy,CN=Query-Policies,'
'CN=Directory Service,CN=Windows NT,CN=Services,'
@@ -283,9 +286,19 @@ class LargeLDAPTest(samba.tests.TestCase):
session_info=system_session(lp),
lp=lp)
+ for x in range(200):
+ user_name = self.USER_NAME + format(x, "03")
+ ace = "(OD;;RP;{6bc69afa-7bd9-4184-88f5-28762137eb6a};;S-1-%d)" % x
+ dn = ldb.Dn(self.ldb, "cn=" + user_name + "," + str(self.ou_dn))
+
+ # add an ACE that denies access to the above random attr
+ # for a not-existing user. This makes each SD distinct
+ # and so will slow SD parsing.
+ self.sd_utils.dacl_add_ace(dn, ace)
+
# Create a large search expression that will take a long time to
# evaluate.
- expression = '(anr=l)' * 10000
+ expression = f'(jpegPhoto=*X*)' * 1000
expression = f'(|{expression})'
# Perform the LDAP search.
--
2.25.1

View File

@ -0,0 +1,67 @@
From 0d753cc8f2b072175f994ede8b3a541303a8a2d5 Mon Sep 17 00:00:00 2001
From: Joseph Sutton <josephsutton@catalyst.net.nz>
Date: Mon, 9 Jan 2023 11:22:34 +1300
Subject: [PATCH 4/4] CVE-2023-0225 s4-acl: Don't return early if dNSHostName
element has no values
This early return would mistakenly allow an unprivileged user to delete
the dNSHostName attribute by making an LDAP modify request with no
values. We should no longer allow this.
Add or replace operations with no values and no privileges are
disallowed.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15276
Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Conflict: NA
Reference: https://attachments.samba.org/attachment.cgi?id=17833
---
selftest/knownfail.d/dns-host-name-deletion | 2 --
source4/dsdb/samdb/ldb_modules/acl.c | 12 +++++++-----
2 files changed, 7 insertions(+), 7 deletions(-)
delete mode 100644 selftest/knownfail.d/dns-host-name-deletion
diff --git a/selftest/knownfail.d/dns-host-name-deletion b/selftest/knownfail.d/dns-host-name-deletion
deleted file mode 100644
index ac11619ffc3..00000000000
--- a/selftest/knownfail.d/dns-host-name-deletion
+++ /dev/null
@@ -1,2 +0,0 @@
-^samba4.ldap.acl_modify.python\(.*\).__main__.AclModifyTests.test_modify_delete_dns_host_name_ldif_unspecified\(.*\)
-^samba4.ldap.acl_modify.python\(.*\).__main__.AclModifyTests.test_modify_delete_dns_host_name_unspecified\(.*\)
diff --git a/source4/dsdb/samdb/ldb_modules/acl.c b/source4/dsdb/samdb/ldb_modules/acl.c
index 4098ae2d671..b602520ca2b 100644
--- a/source4/dsdb/samdb/ldb_modules/acl.c
+++ b/source4/dsdb/samdb/ldb_modules/acl.c
@@ -900,11 +900,6 @@ static int acl_check_dns_host_name(TALLOC_CTX *mem_ctx,
NULL
};
- if (el->num_values == 0) {
- return LDB_SUCCESS;
- }
- dnsHostName = &el->values[0];
-
tmp_ctx = talloc_new(mem_ctx);
if (tmp_ctx == NULL) {
return ldb_oom(ldb);
@@ -1050,6 +1045,13 @@ static int acl_check_dns_host_name(TALLOC_CTX *mem_ctx,
--account_name_len;
}
+ /* Check for add or replace requests with no value. */
+ if (el->num_values == 0) {
+ talloc_free(tmp_ctx);
+ return ldb_operr(ldb);
+ }
+ dnsHostName = &el->values[0];
+
dnsHostName_str = (const char *)dnsHostName->data;
dns_host_name_len = dnsHostName->length;
--
2.25.1

View File

@ -0,0 +1,79 @@
From ad6945f667329d75174cfb9e90786f811c579355 Mon Sep 17 00:00:00 2001
From: Joseph Sutton <josephsutton@catalyst.net.nz>
Date: Fri, 27 Jan 2023 07:57:27 +1300
Subject: [PATCH 02/34] CVE-2023-0614 libcli/security: Make some parameters
const
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15270
Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
[abartlet@samba.org Updated to add const to sec_access_check_ds()
instead of the sec_access_check_ds_implicit_owner() wrapper
found in 4.18 and later]
Conflict: NA
Reference: https://attachments.samba.org/attachment.cgi?id=17821
---
libcli/security/access_check.c | 10 +++++-----
libcli/security/access_check.h | 2 +-
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/libcli/security/access_check.c b/libcli/security/access_check.c
index f5051b0fa93..7dd3798703c 100644
--- a/libcli/security/access_check.c
+++ b/libcli/security/access_check.c
@@ -394,7 +394,7 @@ NTSTATUS se_file_access_check(const struct security_descriptor *sd,
return NT_STATUS_OK;
}
-static const struct GUID *get_ace_object_type(struct security_ace *ace)
+static const struct GUID *get_ace_object_type(const struct security_ace *ace)
{
if (ace->object.object.flags & SEC_ACE_OBJECT_TYPE_PRESENT) {
return &ace->object.object.type.type;
@@ -412,7 +412,7 @@ static const struct GUID *get_ace_object_type(struct security_ace *ace)
* rights to the object/attribute
* @returns NT_STATUS_OK, unless access was denied
*/
-static NTSTATUS check_object_specific_access(struct security_ace *ace,
+static NTSTATUS check_object_specific_access(const struct security_ace *ace,
struct object_tree *tree,
bool *grant_access)
{
@@ -505,7 +505,7 @@ NTSTATUS sec_access_check_ds(const struct security_descriptor *sd,
uint32_t access_desired,
uint32_t *access_granted,
struct object_tree *tree,
- struct dom_sid *replace_sid)
+ const struct dom_sid *replace_sid)
{
uint32_t i;
uint32_t bits_remaining;
@@ -556,8 +556,8 @@ NTSTATUS sec_access_check_ds(const struct security_descriptor *sd,
/* check each ace in turn. */
for (i=0; bits_remaining && i < sd->dacl->num_aces; i++) {
- struct dom_sid *trustee;
- struct security_ace *ace = &sd->dacl->aces[i];
+ const struct dom_sid *trustee;
+ const struct security_ace *ace = &sd->dacl->aces[i];
NTSTATUS status;
bool grant_access = false;
diff --git a/libcli/security/access_check.h b/libcli/security/access_check.h
index 96e33c6624f..37ca078a24e 100644
--- a/libcli/security/access_check.h
+++ b/libcli/security/access_check.h
@@ -74,7 +74,7 @@ NTSTATUS sec_access_check_ds(const struct security_descriptor *sd,
uint32_t access_desired,
uint32_t *access_granted,
struct object_tree *tree,
- struct dom_sid *replace_sid);
+ const struct dom_sid *replace_sid);
bool insert_in_object_tree(TALLOC_CTX *mem_ctx,
const struct GUID *guid,
--
2.25.1

View File

@ -0,0 +1,86 @@
From 89f882b49d2669ba8b51e9b5de644164f5c1995e Mon Sep 17 00:00:00 2001
From: Joseph Sutton <josephsutton@catalyst.net.nz>
Date: Tue, 7 Feb 2023 09:29:51 +1300
Subject: [PATCH 03/34] CVE-2023-0614 s4:dsdb: Use talloc_get_type_abort() more
consistently
It is better to explicitly abort than to dereference a NULL pointer or
try to read data cast to the wrong type.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15270
Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Conflict: NA
Reference: https://attachments.samba.org/attachment.cgi?id=17821
---
source4/dsdb/samdb/ldb_modules/acl_read.c | 4 ++--
source4/dsdb/samdb/ldb_modules/acl_util.c | 2 +-
source4/dsdb/samdb/ldb_modules/linked_attributes.c | 2 +-
source4/dsdb/samdb/ldb_modules/password_hash.c | 2 +-
4 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/source4/dsdb/samdb/ldb_modules/acl_read.c b/source4/dsdb/samdb/ldb_modules/acl_read.c
index b221dcde445..16a1927183c 100644
--- a/source4/dsdb/samdb/ldb_modules/acl_read.c
+++ b/source4/dsdb/samdb/ldb_modules/acl_read.c
@@ -268,7 +268,7 @@ static int aclread_get_sd_from_ldb_message(struct aclread_context *ac,
struct ldb_message_element *sd_element;
struct ldb_context *ldb = ldb_module_get_ctx(ac->module);
struct aclread_private *private_data
- = talloc_get_type(ldb_module_get_private(ac->module),
+ = talloc_get_type_abort(ldb_module_get_private(ac->module),
struct aclread_private);
enum ndr_err_code ndr_err;
@@ -568,7 +568,7 @@ static int aclread_callback(struct ldb_request *req, struct ldb_reply *ares)
const struct dsdb_class *objectclass;
bool suppress_result = false;
- ac = talloc_get_type(req->context, struct aclread_context);
+ ac = talloc_get_type_abort(req->context, struct aclread_context);
ldb = ldb_module_get_ctx(ac->module);
if (!ares) {
return ldb_module_done(ac->req, NULL, NULL, LDB_ERR_OPERATIONS_ERROR );
diff --git a/source4/dsdb/samdb/ldb_modules/acl_util.c b/source4/dsdb/samdb/ldb_modules/acl_util.c
index 12f00fbff16..367c11d1ba9 100644
--- a/source4/dsdb/samdb/ldb_modules/acl_util.c
+++ b/source4/dsdb/samdb/ldb_modules/acl_util.c
@@ -298,7 +298,7 @@ uint32_t dsdb_request_sd_flags(struct ldb_request *req, bool *explicit)
sd_control = ldb_request_get_control(req, LDB_CONTROL_SD_FLAGS_OID);
if (sd_control != NULL && sd_control->data != NULL) {
- struct ldb_sd_flags_control *sdctr = (struct ldb_sd_flags_control *)sd_control->data;
+ struct ldb_sd_flags_control *sdctr = talloc_get_type_abort(sd_control->data, struct ldb_sd_flags_control);
sd_flags = sdctr->secinfo_flags;
diff --git a/source4/dsdb/samdb/ldb_modules/linked_attributes.c b/source4/dsdb/samdb/ldb_modules/linked_attributes.c
index 5ef075f2037..317df9d3e0e 100644
--- a/source4/dsdb/samdb/ldb_modules/linked_attributes.c
+++ b/source4/dsdb/samdb/ldb_modules/linked_attributes.c
@@ -104,7 +104,7 @@ static int handle_verify_name_control(TALLOC_CTX *ctx, struct ldb_context *ldb,
* If we are a GC let's remove the control,
* if there is a specified GC check that is us.
*/
- struct ldb_verify_name_control *lvnc = (struct ldb_verify_name_control *)control->data;
+ struct ldb_verify_name_control *lvnc = talloc_get_type_abort(control->data, struct ldb_verify_name_control);
if (samdb_is_gc(ldb)) {
/* Because we can't easily talloc a struct ldb_dn*/
struct ldb_dn **dn = talloc_array(ctx, struct ldb_dn *, 1);
diff --git a/source4/dsdb/samdb/ldb_modules/password_hash.c b/source4/dsdb/samdb/ldb_modules/password_hash.c
index b308226a9f9..6a713b86736 100644
--- a/source4/dsdb/samdb/ldb_modules/password_hash.c
+++ b/source4/dsdb/samdb/ldb_modules/password_hash.c
@@ -4066,7 +4066,7 @@ static void ph_apply_controls(struct ph_context *ac)
ctrl = ldb_request_get_control(ac->req,
DSDB_CONTROL_PASSWORD_CHANGE_OLD_PW_CHECKED_OID);
if (ctrl != NULL) {
- ac->change = (struct dsdb_control_password_change *) ctrl->data;
+ ac->change = talloc_get_type_abort(ctrl->data, struct dsdb_control_password_change);
/* Mark the "change" control as uncritical (done) */
ctrl->critical = false;
--
2.25.1

View File

@ -0,0 +1,36 @@
From 003c65be1c6d8f8ea853896a75b315ef8e98cfb3 Mon Sep 17 00:00:00 2001
From: Joseph Sutton <josephsutton@catalyst.net.nz>
Date: Fri, 27 Jan 2023 08:00:32 +1300
Subject: [PATCH 04/34] CVE-2023-0614 s4-acl: Make some parameters const
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15270
Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
[abartlet@samba.org Adapted to code without newer
acl_check_access_on_attribute_implicit_owner name]
Conflict: NA
Reference: https://attachments.samba.org/attachment.cgi?id=17821
---
source4/dsdb/samdb/ldb_modules/acl_util.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/source4/dsdb/samdb/ldb_modules/acl_util.c b/source4/dsdb/samdb/ldb_modules/acl_util.c
index 367c11d1ba9..56aa4bd7531 100644
--- a/source4/dsdb/samdb/ldb_modules/acl_util.c
+++ b/source4/dsdb/samdb/ldb_modules/acl_util.c
@@ -97,8 +97,8 @@ int dsdb_module_check_access_on_dn(struct ldb_module *module,
int acl_check_access_on_attribute(struct ldb_module *module,
TALLOC_CTX *mem_ctx,
- struct security_descriptor *sd,
- struct dom_sid *rp_sid,
+ const struct security_descriptor *sd,
+ const struct dom_sid *rp_sid,
uint32_t access_mask,
const struct dsdb_attribute *attr,
const struct dsdb_class *objectclass)
--
2.25.1

View File

@ -0,0 +1,72 @@
From b01d3ae3261264236504475a26c54ab45dd2175f Mon Sep 17 00:00:00 2001
From: Joseph Sutton <josephsutton@catalyst.net.nz>
Date: Fri, 27 Jan 2023 08:28:36 +1300
Subject: [PATCH 05/34] CVE-2023-0614 ldb: Add functions for handling
inaccessible message elements
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15270
Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Conflict: NA
Reference: https://attachments.samba.org/attachment.cgi?id=17821
---
lib/ldb/common/ldb_msg.c | 26 ++++++++++++++++++++++++++
lib/ldb/include/ldb_module.h | 4 ++++
2 files changed, 30 insertions(+)
diff --git a/lib/ldb/common/ldb_msg.c b/lib/ldb/common/ldb_msg.c
index 9cd7998e21c..cbc7e32b2ba 100644
--- a/lib/ldb/common/ldb_msg.c
+++ b/lib/ldb/common/ldb_msg.c
@@ -795,6 +795,32 @@ int ldb_msg_element_compare_name(struct ldb_message_element *el1,
return ldb_attr_cmp(el1->name, el2->name);
}
+void ldb_msg_element_mark_inaccessible(struct ldb_message_element *el)
+{
+ el->flags |= LDB_FLAG_INTERNAL_INACCESSIBLE_ATTRIBUTE;
+}
+
+bool ldb_msg_element_is_inaccessible(const struct ldb_message_element *el)
+{
+ return (el->flags & LDB_FLAG_INTERNAL_INACCESSIBLE_ATTRIBUTE) != 0;
+}
+
+void ldb_msg_remove_inaccessible(struct ldb_message *msg)
+{
+ unsigned i;
+ unsigned num_del = 0;
+
+ for (i = 0; i < msg->num_elements; ++i) {
+ if (ldb_msg_element_is_inaccessible(&msg->elements[i])) {
+ ++num_del;
+ } else if (num_del) {
+ msg->elements[i - num_del] = msg->elements[i];
+ }
+ }
+
+ msg->num_elements -= num_del;
+}
+
/*
convenience functions to return common types from a message
these return the first value if the attribute is multi-valued
diff --git a/lib/ldb/include/ldb_module.h b/lib/ldb/include/ldb_module.h
index 4c7c85a17f0..8481fd3991a 100644
--- a/lib/ldb/include/ldb_module.h
+++ b/lib/ldb/include/ldb_module.h
@@ -513,6 +513,10 @@ struct ldb_extended_match_rule
int ldb_register_extended_match_rule(struct ldb_context *ldb,
const struct ldb_extended_match_rule *rule);
+void ldb_msg_element_mark_inaccessible(struct ldb_message_element *el);
+bool ldb_msg_element_is_inaccessible(const struct ldb_message_element *el);
+void ldb_msg_remove_inaccessible(struct ldb_message *msg);
+
/*
* these pack/unpack functions are exposed in the library for use by
* ldb tools like ldbdump and for use in tests,
--
2.25.1

View File

@ -0,0 +1,140 @@
From c818c16912f5af248b91f0688c3e57012db89011 Mon Sep 17 00:00:00 2001
From: Joseph Sutton <josephsutton@catalyst.net.nz>
Date: Fri, 27 Jan 2023 08:29:33 +1300
Subject: [PATCH 06/34] CVE-2023-0614 s4-acl: Use ldb functions for handling
inaccessible message elements
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15270
Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Conflict: NA
Reference: https://attachments.samba.org/attachment.cgi?id=17821
---
source4/dsdb/samdb/ldb_modules/acl_read.c | 62 ++++-------------------
1 file changed, 10 insertions(+), 52 deletions(-)
diff --git a/source4/dsdb/samdb/ldb_modules/acl_read.c b/source4/dsdb/samdb/ldb_modules/acl_read.c
index 16a1927183c..8814a816797 100644
--- a/source4/dsdb/samdb/ldb_modules/acl_read.c
+++ b/source4/dsdb/samdb/ldb_modules/acl_read.c
@@ -70,14 +70,6 @@ struct aclread_private {
struct ldb_val sd_cached_blob;
};
-static void aclread_mark_inaccesslible(struct ldb_message_element *el) {
- el->flags |= LDB_FLAG_INTERNAL_INACCESSIBLE_ATTRIBUTE;
-}
-
-static bool aclread_is_inaccessible(struct ldb_message_element *el) {
- return el->flags & LDB_FLAG_INTERNAL_INACCESSIBLE_ATTRIBUTE;
-}
-
/*
* the object has a parent, so we have to check for visibility
*
@@ -557,11 +549,9 @@ static int aclread_callback(struct ldb_request *req, struct ldb_reply *ares)
{
struct ldb_context *ldb;
struct aclread_context *ac;
- struct ldb_message *ret_msg;
struct ldb_message *msg;
int ret;
- size_t num_of_attrs = 0;
- unsigned int i, k = 0;
+ unsigned int i;
struct security_descriptor *sd = NULL;
struct dom_sid *sid = NULL;
TALLOC_CTX *tmp_ctx;
@@ -651,26 +641,26 @@ static int aclread_callback(struct ldb_request *req, struct ldb_reply *ares)
msg->elements[i].name) == 0;
/* these attributes were added to perform access checks and must be removed */
if (is_objectsid && ac->added_objectSid) {
- aclread_mark_inaccesslible(&msg->elements[i]);
+ ldb_msg_element_mark_inaccessible(&msg->elements[i]);
continue;
}
if (is_instancetype && ac->added_instanceType) {
- aclread_mark_inaccesslible(&msg->elements[i]);
+ ldb_msg_element_mark_inaccessible(&msg->elements[i]);
continue;
}
if (is_objectclass && ac->added_objectClass) {
- aclread_mark_inaccesslible(&msg->elements[i]);
+ ldb_msg_element_mark_inaccessible(&msg->elements[i]);
continue;
}
if (is_sd && ac->added_nTSecurityDescriptor) {
- aclread_mark_inaccesslible(&msg->elements[i]);
+ ldb_msg_element_mark_inaccessible(&msg->elements[i]);
continue;
}
access_mask = get_attr_access_mask(attr, ac->sd_flags);
if (access_mask == 0) {
- aclread_mark_inaccesslible(&msg->elements[i]);
+ ldb_msg_element_mark_inaccessible(&msg->elements[i]);
continue;
}
@@ -714,7 +704,7 @@ static int aclread_callback(struct ldb_request *req, struct ldb_reply *ares)
return LDB_SUCCESS;
}
} else {
- aclread_mark_inaccesslible(&msg->elements[i]);
+ ldb_msg_element_mark_inaccessible(&msg->elements[i]);
}
} else if (ret != LDB_SUCCESS) {
ldb_debug_set(ldb, LDB_DEBUG_FATAL,
@@ -757,44 +747,12 @@ static int aclread_callback(struct ldb_request *req, struct ldb_reply *ares)
}
}
- for (i=0; i < msg->num_elements; i++) {
- if (!aclread_is_inaccessible(&msg->elements[i])) {
- num_of_attrs++;
- }
- }
- /*create a new message to return*/
- ret_msg = ldb_msg_new(ac->req);
- ret_msg->dn = msg->dn;
- talloc_steal(ret_msg, msg->dn);
- ret_msg->num_elements = num_of_attrs;
- if (num_of_attrs > 0) {
- ret_msg->elements = talloc_array(ret_msg,
- struct ldb_message_element,
- num_of_attrs);
- if (ret_msg->elements == NULL) {
- return ldb_oom(ldb);
- }
- for (i=0; i < msg->num_elements; i++) {
- bool to_remove = aclread_is_inaccessible(&msg->elements[i]);
- if (!to_remove) {
- ret_msg->elements[k] = msg->elements[i];
- talloc_steal(ret_msg->elements, msg->elements[i].name);
- talloc_steal(ret_msg->elements, msg->elements[i].values);
- k++;
- }
- }
- /*
- * This should not be needed, but some modules
- * may allocate values on the wrong context...
- */
- talloc_steal(ret_msg->elements, msg);
- } else {
- ret_msg->elements = NULL;
- }
+ ldb_msg_remove_inaccessible(msg);
+
talloc_free(tmp_ctx);
ac->num_entries++;
- return ldb_module_send_entry(ac->req, ret_msg, ares->controls);
+ return ldb_module_send_entry(ac->req, msg, ares->controls);
case LDB_REPLY_REFERRAL:
return ldb_module_send_referral(ac->req, ares->referral);
case LDB_REPLY_DONE:
--
2.25.1

View File

@ -0,0 +1,409 @@
From e7445d18badee6c3b1bbee48c689eb2629c31681 Mon Sep 17 00:00:00 2001
From: Joseph Sutton <josephsutton@catalyst.net.nz>
Date: Wed, 15 Feb 2023 12:34:51 +1300
Subject: [PATCH 07/34] CVE-2023-0614 ldb:tests: Ensure ldb_val data is
zero-terminated
If the value of an ldb message element is not zero-terminated, calling
ldb_msg_find_attr_as_string() will cause the function to read off the
end of the buffer in an attempt to verify that the value is
zero-terminated. This can cause unexpected behaviour and make the test
randomly fail.
To avoid this, we must have a terminating null byte that is *not*
counted as part of the length, and so we must calculate the length with
strlen() rather than sizeof.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15270
Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Conflict: NA
Reference: https://attachments.samba.org/attachment.cgi?id=17821
---
lib/ldb/tests/ldb_filter_attrs_test.c | 171 +++++++++++++-------------
1 file changed, 86 insertions(+), 85 deletions(-)
diff --git a/lib/ldb/tests/ldb_filter_attrs_test.c b/lib/ldb/tests/ldb_filter_attrs_test.c
index 7d555e0da2e..442d9c77ed2 100644
--- a/lib/ldb/tests/ldb_filter_attrs_test.c
+++ b/lib/ldb/tests/ldb_filter_attrs_test.c
@@ -36,6 +36,7 @@
#include <stdarg.h>
#include <stddef.h>
#include <stdint.h>
+#include <string.h>
#include <setjmp.h>
#include <cmocka.h>
@@ -96,10 +97,10 @@ static void test_filter_attrs_one_attr_matched(void **state)
const char *attrs[] = {"foo", NULL};
- uint8_t value[] = "The value.......end";
+ char value[] = "The value.......end";
struct ldb_val value_1 = {
- .data = value,
- .length = (sizeof(value))
+ .data = (uint8_t *)value,
+ .length = strlen(value)
};
struct ldb_message_element element_1 = {
.name = "foo",
@@ -130,9 +131,9 @@ static void test_filter_attrs_one_attr_matched(void **state)
assert_string_equal(filtered_msg->elements[0].name, "foo");
assert_int_equal(filtered_msg->elements[0].num_values, 1);
assert_int_equal(filtered_msg->elements[0].values[0].length,
- sizeof(value));
+ strlen(value));
assert_memory_equal(filtered_msg->elements[0].values[0].data,
- value, sizeof(value));
+ value, strlen(value));
}
/*
@@ -148,10 +149,10 @@ static void test_filter_attrs_one_attr_matched_of_many(void **state)
const char *attrs[] = {"foo", "bar", "baz", NULL};
- uint8_t value[] = "The value.......end";
+ char value[] = "The value.......end";
struct ldb_val value_1 = {
- .data = value,
- .length = (sizeof(value))
+ .data = (uint8_t *)value,
+ .length = strlen(value)
};
struct ldb_message_element element_1 = {
.name = "foo",
@@ -182,9 +183,9 @@ static void test_filter_attrs_one_attr_matched_of_many(void **state)
assert_string_equal(filtered_msg->elements[0].name, "foo");
assert_int_equal(filtered_msg->elements[0].num_values, 1);
assert_int_equal(filtered_msg->elements[0].values[0].length,
- sizeof(value));
+ strlen(value));
assert_memory_equal(filtered_msg->elements[0].values[0].data,
- value, sizeof(value));
+ value, strlen(value));
}
/*
@@ -201,15 +202,15 @@ static void test_filter_attrs_two_attr_matched_attrs(void **state)
/* deliberatly the other order */
const char *attrs[] = {"bar", "foo", NULL};
- uint8_t value1[] = "The value.......end";
- uint8_t value2[] = "The value..MUST.end";
+ char value1[] = "The value.......end";
+ char value2[] = "The value..MUST.end";
struct ldb_val value_1 = {
- .data = value1,
- .length = (sizeof(value1))
+ .data = (uint8_t *)value1,
+ .length = strlen(value1)
};
struct ldb_val value_2 = {
- .data = value2,
- .length = (sizeof(value2))
+ .data = (uint8_t *)value2,
+ .length = strlen(value2)
};
/* foo and bar are the other order to in attrs */
@@ -251,15 +252,15 @@ static void test_filter_attrs_two_attr_matched_attrs(void **state)
assert_string_equal(filtered_msg->elements[0].name, "foo");
assert_int_equal(filtered_msg->elements[0].num_values, 1);
assert_int_equal(filtered_msg->elements[0].values[0].length,
- sizeof(value1));
+ strlen(value1));
assert_memory_equal(filtered_msg->elements[0].values[0].data,
- value1, sizeof(value1));
+ value1, strlen(value1));
assert_string_equal(filtered_msg->elements[1].name, "bar");
assert_int_equal(filtered_msg->elements[1].num_values, 1);
assert_int_equal(filtered_msg->elements[1].values[0].length,
- sizeof(value2));
+ strlen(value2));
assert_memory_equal(filtered_msg->elements[1].values[0].data,
- value2, sizeof(value2));
+ value2, strlen(value2));
}
/*
@@ -276,15 +277,15 @@ static void test_filter_attrs_two_attr_matched_one_attr(void **state)
/* deliberatly the other order */
const char *attrs[] = {"bar", NULL};
- uint8_t value1[] = "The value.......end";
- uint8_t value2[] = "The value..MUST.end";
+ char value1[] = "The value.......end";
+ char value2[] = "The value..MUST.end";
struct ldb_val value_1 = {
- .data = value1,
- .length = (sizeof(value1))
+ .data = (uint8_t *)value1,
+ .length = strlen(value1)
};
struct ldb_val value_2 = {
- .data = value2,
- .length = (sizeof(value2))
+ .data = (uint8_t *)value2,
+ .length = strlen(value2)
};
/* foo and bar are the other order to in attrs */
@@ -326,9 +327,9 @@ static void test_filter_attrs_two_attr_matched_one_attr(void **state)
assert_string_equal(filtered_msg->elements[0].name, "bar");
assert_int_equal(filtered_msg->elements[0].num_values, 1);
assert_int_equal(filtered_msg->elements[0].values[0].length,
- sizeof(value2));
+ strlen(value2));
assert_memory_equal(filtered_msg->elements[0].values[0].data,
- value2, sizeof(value2));
+ value2, strlen(value2));
}
/*
@@ -345,15 +346,15 @@ static void test_filter_attrs_two_dup_attr_matched_one_attr(void **state)
/* deliberatly the other order */
const char *attrs[] = {"bar", NULL};
- uint8_t value1[] = "The value.......end";
- uint8_t value2[] = "The value..MUST.end";
+ char value1[] = "The value.......end";
+ char value2[] = "The value..MUST.end";
struct ldb_val value_1 = {
- .data = value1,
- .length = (sizeof(value1))
+ .data = (uint8_t *)value1,
+ .length = strlen(value1)
};
struct ldb_val value_2 = {
- .data = value2,
- .length = (sizeof(value2))
+ .data = (uint8_t *)value2,
+ .length = strlen(value2)
};
/* foo and bar are the other order to in attrs */
@@ -400,15 +401,15 @@ static void test_filter_attrs_two_dup_attr_matched_dup(void **state)
const char *attrs[] = {"bar", "bar", NULL};
- uint8_t value1[] = "The value.......end";
- uint8_t value2[] = "The value..MUST.end";
+ char value1[] = "The value.......end";
+ char value2[] = "The value..MUST.end";
struct ldb_val value_1 = {
- .data = value1,
- .length = (sizeof(value1))
+ .data = (uint8_t *)value1,
+ .length = strlen(value1)
};
struct ldb_val value_2 = {
- .data = value2,
- .length = (sizeof(value2))
+ .data = (uint8_t *)value2,
+ .length = strlen(value2)
};
/* foo and bar are the other order to in attrs */
@@ -445,15 +446,15 @@ static void test_filter_attrs_two_dup_attr_matched_dup(void **state)
assert_string_equal(filtered_msg->elements[0].name, "bar");
assert_int_equal(filtered_msg->elements[0].num_values, 1);
assert_int_equal(filtered_msg->elements[0].values[0].length,
- sizeof(value1));
+ strlen(value1));
assert_memory_equal(filtered_msg->elements[0].values[0].data,
- value1, sizeof(value1));
+ value1, strlen(value1));
assert_string_equal(filtered_msg->elements[1].name, "bar");
assert_int_equal(filtered_msg->elements[1].num_values, 1);
assert_int_equal(filtered_msg->elements[1].values[0].length,
- sizeof(value2));
+ strlen(value2));
assert_memory_equal(filtered_msg->elements[1].values[0].data,
- value2, sizeof(value2));
+ value2, strlen(value2));
}
/*
@@ -469,15 +470,15 @@ static void test_filter_attrs_two_dup_attr_matched_one_of_two(void **state)
const char *attrs[] = {"bar", "foo", NULL};
- uint8_t value1[] = "The value.......end";
- uint8_t value2[] = "The value..MUST.end";
+ char value1[] = "The value.......end";
+ char value2[] = "The value..MUST.end";
struct ldb_val value_1 = {
- .data = value1,
- .length = (sizeof(value1))
+ .data = (uint8_t *)value1,
+ .length = strlen(value1)
};
struct ldb_val value_2 = {
- .data = value2,
- .length = (sizeof(value2))
+ .data = (uint8_t *)value2,
+ .length = strlen(value2)
};
/* foo and bar are the other order to in attrs */
@@ -514,15 +515,15 @@ static void test_filter_attrs_two_dup_attr_matched_one_of_two(void **state)
assert_string_equal(filtered_msg->elements[0].name, "bar");
assert_int_equal(filtered_msg->elements[0].num_values, 1);
assert_int_equal(filtered_msg->elements[0].values[0].length,
- sizeof(value1));
+ strlen(value1));
assert_memory_equal(filtered_msg->elements[0].values[0].data,
- value1, sizeof(value1));
+ value1, strlen(value1));
assert_string_equal(filtered_msg->elements[1].name, "bar");
assert_int_equal(filtered_msg->elements[1].num_values, 1);
assert_int_equal(filtered_msg->elements[1].values[0].length,
- sizeof(value2));
+ strlen(value2));
assert_memory_equal(filtered_msg->elements[1].values[0].data,
- value2, sizeof(value2));
+ value2, strlen(value2));
}
/*
@@ -538,15 +539,15 @@ static void test_filter_attrs_two_dup_attr_matched_star(void **state)
const char *attrs[] = {"*", "foo", NULL};
- uint8_t value1[] = "The value.......end";
- uint8_t value2[] = "The value..MUST.end";
+ char value1[] = "The value.......end";
+ char value2[] = "The value..MUST.end";
struct ldb_val value_1 = {
- .data = value1,
- .length = (sizeof(value1))
+ .data = (uint8_t *)value1,
+ .length = strlen(value1)
};
struct ldb_val value_2 = {
- .data = value2,
- .length = (sizeof(value2))
+ .data = (uint8_t *)value2,
+ .length = strlen(value2)
};
/* foo and bar are the other order to in attrs */
@@ -586,15 +587,15 @@ static void test_filter_attrs_two_dup_attr_matched_star(void **state)
assert_string_equal(filtered_msg->elements[0].name, "bar");
assert_int_equal(filtered_msg->elements[0].num_values, 1);
assert_int_equal(filtered_msg->elements[0].values[0].length,
- sizeof(value1));
+ strlen(value1));
assert_memory_equal(filtered_msg->elements[0].values[0].data,
- value1, sizeof(value1));
+ value1, strlen(value1));
assert_string_equal(filtered_msg->elements[1].name, "bar");
assert_int_equal(filtered_msg->elements[1].num_values, 1);
assert_int_equal(filtered_msg->elements[1].values[0].length,
- sizeof(value2));
+ strlen(value2));
assert_memory_equal(filtered_msg->elements[1].values[0].data,
- value2, sizeof(value2));
+ value2, strlen(value2));
/*
* assert the ldb_filter_attrs does not modify filtered_msg.dn
* in this case
@@ -619,10 +620,10 @@ static void test_filter_attrs_one_attr_matched_star(void **state)
const char *attrs[] = {"*", NULL};
- uint8_t value[] = "The value.......end";
+ char value[] = "The value.......end";
struct ldb_val value_1 = {
- .data = value,
- .length = (sizeof(value))
+ .data = (uint8_t *)value,
+ .length = strlen(value)
};
struct ldb_message_element element_1 = {
.name = "foo",
@@ -676,15 +677,15 @@ static void test_filter_attrs_two_attr_matched_star(void **state)
const char *attrs[] = {"*", NULL};
- uint8_t value1[] = "The value.......end";
- uint8_t value2[] = "The value..MUST.end";
+ char value1[] = "The value.......end";
+ char value2[] = "The value..MUST.end";
struct ldb_val value_1 = {
- .data = value1,
- .length = (sizeof(value1))
+ .data = (uint8_t *)value1,
+ .length = strlen(value1)
};
struct ldb_val value_2 = {
- .data = value2,
- .length = (sizeof(value2))
+ .data = (uint8_t *)value2,
+ .length = strlen(value2)
};
struct ldb_message_element elements[] = {
{
@@ -750,10 +751,10 @@ static void test_filter_attrs_one_attr_matched_star_no_dn(void **state)
const char *attrs[] = {"*", NULL};
- uint8_t value[] = "The value.......end";
+ char value[] = "The value.......end";
struct ldb_val value_1 = {
- .data = value,
- .length = (sizeof(value))
+ .data = (uint8_t *)value,
+ .length = strlen(value)
};
struct ldb_message_element element_1 = {
.name = "foo",
@@ -789,10 +790,10 @@ static void test_filter_attrs_one_attr_matched_star_dn(void **state)
const char *attrs[] = {"*", "distinguishedName", NULL};
- uint8_t value[] = "The value.......end";
+ char value[] = "The value.......end";
struct ldb_val value_1 = {
- .data = value,
- .length = (sizeof(value))
+ .data = (uint8_t *)value,
+ .length = strlen(value)
};
struct ldb_message_element element_1 = {
.name = "foo",
@@ -844,10 +845,10 @@ static void test_filter_attrs_one_attr_matched_dn(void **state)
const char *attrs[] = {"distinguishedName", NULL};
- uint8_t value[] = "The value.......end";
+ char value[] = "The value.......end";
struct ldb_val value_1 = {
- .data = value,
- .length = (sizeof(value))
+ .data = (uint8_t *)value,
+ .length = strlen(value)
};
struct ldb_message_element element_1 = {
.name = "foo",
@@ -894,10 +895,10 @@ static void test_filter_attrs_one_attr_empty_list(void **state)
const char *attrs[] = {NULL};
- uint8_t value[] = "The value.......end";
+ char value[] = "The value.......end";
struct ldb_val value_1 = {
- .data = value,
- .length = (sizeof(value))
+ .data = (uint8_t *)value,
+ .length = strlen(value)
};
struct ldb_message_element element_1 = {
.name = "foo",
--
2.25.1

View File

@ -0,0 +1,48 @@
From 936bfcb6ef804d2224072f3770ca09fe2596ee1f Mon Sep 17 00:00:00 2001
From: Joseph Sutton <josephsutton@catalyst.net.nz>
Date: Wed, 15 Feb 2023 14:08:57 +1300
Subject: [PATCH 08/34] CVE-2023-0614 ldb:tests: Ensure all tests are accounted
for
Add ldb_filter_attrs_test to the list of tests so that it actually gets
run.
Remove a duplicate ldb_msg_test that was accidentally added in commit
5ca90e758ade97fb5e335029c7a1768094e70564.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15270
Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Conflict: NA
Reference: https://attachments.samba.org/attachment.cgi?id=17821
---
lib/ldb/wscript | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/lib/ldb/wscript b/lib/ldb/wscript
index 60bb7cf48b3..c862229822d 100644
--- a/lib/ldb/wscript
+++ b/lib/ldb/wscript
@@ -627,7 +627,6 @@ def test(ctx):
'ldb_msg_test',
'ldb_tdb_mod_op_test',
'ldb_tdb_guid_mod_op_test',
- 'ldb_msg_test',
'ldb_tdb_kv_ops_test',
'ldb_tdb_test',
'ldb_match_test',
@@ -637,7 +636,9 @@ def test(ctx):
# on operations which the TDB backend does not currently
# support
# 'ldb_key_value_sub_txn_tdb_test'
- 'ldb_parse_test']
+ 'ldb_parse_test',
+ 'ldb_filter_attrs_test',
+ ]
# if LIB_LDAP and LIB_LBER defined, then we can test ldb_ldap backend
# behavior regression for bz#14413
--
2.25.1

View File

@ -0,0 +1,104 @@
From 83217ce77381f8faa3cde948e15a36db234d3033 Mon Sep 17 00:00:00 2001
From: Joseph Sutton <josephsutton@catalyst.net.nz>
Date: Fri, 3 Mar 2023 17:23:42 +1300
Subject: [PATCH 09/34] CVE-2023-0614 ldb: Add function to take ownership of an
ldb message
Many places in Samba depend upon various components of an ldb message
being talloc allocated, and hence able to be used as talloc contexts.
The elements and values of an unpacked ldb message point to unowned data
inside the memory-mapped database, and this function ensures that such
messages have talloc ownership of said elements and values.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15270
Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Conflict: NA
Reference: https://attachments.samba.org/attachment.cgi?id=17821
---
lib/ldb/common/ldb_pack.c | 41 ++++++++++++++++++++++++++++++++++++
lib/ldb/include/ldb_module.h | 4 ++++
2 files changed, 45 insertions(+)
diff --git a/lib/ldb/common/ldb_pack.c b/lib/ldb/common/ldb_pack.c
index e7dd364008a..028d96a619a 100644
--- a/lib/ldb/common/ldb_pack.c
+++ b/lib/ldb/common/ldb_pack.c
@@ -690,6 +690,7 @@ static int ldb_unpack_data_flags_v1(struct ldb_context *ldb,
element->values = NULL;
if ((flags & LDB_UNPACK_DATA_FLAG_NO_VALUES_ALLOC) && element->num_values == 1) {
element->values = &ldb_val_single_array[nelem];
+ element->flags |= LDB_FLAG_INTERNAL_SHARED_VALUES;
} else if (element->num_values != 0) {
element->values = talloc_array(message->elements,
struct ldb_val,
@@ -932,6 +933,7 @@ static int ldb_unpack_data_flags_v2(struct ldb_context *ldb,
if ((flags & LDB_UNPACK_DATA_FLAG_NO_VALUES_ALLOC) &&
element->num_values == 1) {
element->values = &ldb_val_single_array[nelem];
+ element->flags |= LDB_FLAG_INTERNAL_SHARED_VALUES;
} else if (element->num_values != 0) {
element->values = talloc_array(message->elements,
struct ldb_val,
@@ -1259,3 +1261,42 @@ failed:
TALLOC_FREE(filtered_msg->elements);
return -1;
}
+
+/* Have an unpacked ldb message take talloc ownership of its elements. */
+int ldb_msg_elements_take_ownership(struct ldb_message *msg)
+{
+ unsigned int i = 0;
+
+ for (i = 0; i < msg->num_elements; i++) {
+ struct ldb_message_element *el = &msg->elements[i];
+ const char *name;
+ unsigned int j;
+
+ name = talloc_strdup(msg->elements,
+ el->name);
+ if (name == NULL) {
+ return -1;
+ }
+ el->name = name;
+
+ if (el->flags & LDB_FLAG_INTERNAL_SHARED_VALUES) {
+ struct ldb_val *values = talloc_memdup(msg->elements, el->values,
+ sizeof(struct ldb_val) * el->num_values);
+ if (values == NULL) {
+ return -1;
+ }
+ el->values = values;
+ el->flags &= ~LDB_FLAG_INTERNAL_SHARED_VALUES;
+ }
+
+ for (j = 0; j < el->num_values; j++) {
+ struct ldb_val val = ldb_val_dup(el->values, &el->values[j]);
+ if (val.data == NULL && el->values[j].length != 0) {
+ return -1;
+ }
+ el->values[j] = val;
+ }
+ }
+
+ return LDB_SUCCESS;
+}
diff --git a/lib/ldb/include/ldb_module.h b/lib/ldb/include/ldb_module.h
index 8481fd3991a..8c7f33496fb 100644
--- a/lib/ldb/include/ldb_module.h
+++ b/lib/ldb/include/ldb_module.h
@@ -542,6 +542,10 @@ int ldb_filter_attrs(struct ldb_context *ldb,
const struct ldb_message *msg,
const char *const *attrs,
struct ldb_message *filtered_msg);
+
+/* Have an unpacked ldb message take talloc ownership of its elements. */
+int ldb_msg_elements_take_ownership(struct ldb_message *msg);
+
/*
* Unpack a ldb message from a linear buffer in ldb_val
*
--
2.25.1

View File

@ -0,0 +1,62 @@
From a9b625bc8ab00b83b55bcd21ba0df48e73e4df29 Mon Sep 17 00:00:00 2001
From: Joseph Sutton <josephsutton@catalyst.net.nz>
Date: Fri, 3 Mar 2023 17:26:04 +1300
Subject: [PATCH 10/34] CVE-2023-0614 ldb: Add function to remove excess
capacity from an ldb message
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15270
Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
[abartlet@samba.org Adapted to conflict from lack of new
ldb_ascii_toupper() in ldb_private.h]
Conflict: NA
Reference: https://attachments.samba.org/attachment.cgi?id=17821
---
lib/ldb/common/ldb_msg.c | 16 ++++++++++++++++
lib/ldb/include/ldb_private.h | 3 +++
2 files changed, 19 insertions(+)
diff --git a/lib/ldb/common/ldb_msg.c b/lib/ldb/common/ldb_msg.c
index cbc7e32b2ba..2ea2cce2e83 100644
--- a/lib/ldb/common/ldb_msg.c
+++ b/lib/ldb/common/ldb_msg.c
@@ -1497,6 +1497,22 @@ void ldb_msg_remove_attr(struct ldb_message *msg, const char *attr)
}
}
+/* Reallocate elements to drop any excess capacity. */
+void ldb_msg_shrink_to_fit(struct ldb_message *msg)
+{
+ if (msg->num_elements > 0) {
+ struct ldb_message_element *elements = talloc_realloc(msg,
+ msg->elements,
+ struct ldb_message_element,
+ msg->num_elements);
+ if (elements != NULL) {
+ msg->elements = elements;
+ }
+ } else {
+ TALLOC_FREE(msg->elements);
+ }
+}
+
/*
return a LDAP formatted GeneralizedTime string
*/
diff --git a/lib/ldb/include/ldb_private.h b/lib/ldb/include/ldb_private.h
index 4deb24691ca..338e71def6d 100644
--- a/lib/ldb/include/ldb_private.h
+++ b/lib/ldb/include/ldb_private.h
@@ -317,4 +317,7 @@ int ldb_match_message(struct ldb_context *ldb,
const struct ldb_parse_tree *tree,
enum ldb_scope scope, bool *matched);
+/* Reallocate elements to drop any excess capacity. */
+void ldb_msg_shrink_to_fit(struct ldb_message *msg);
+
#endif
--
2.25.1

View File

@ -0,0 +1,68 @@
From 8b7374780e3e7b67e51a1b54a09bf48d89fa9f26 Mon Sep 17 00:00:00 2001
From: Joseph Sutton <josephsutton@catalyst.net.nz>
Date: Fri, 3 Mar 2023 17:27:38 +1300
Subject: [PATCH 11/34] CVE-2023-0614 ldb: Add function to add
distinguishedName to message
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15270
Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
[abartlet@samba.org Adapted to conflict from lack of new
ldb_ascii_toupper() in ldb_private.h]
Conflict: NA
Reference: https://attachments.samba.org/attachment.cgi?id=17821
---
lib/ldb/common/ldb_pack.c | 6 +++---
lib/ldb/include/ldb_private.h | 5 +++++
2 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/lib/ldb/common/ldb_pack.c b/lib/ldb/common/ldb_pack.c
index 028d96a619a..b0b0d64a5ba 100644
--- a/lib/ldb/common/ldb_pack.c
+++ b/lib/ldb/common/ldb_pack.c
@@ -1098,7 +1098,7 @@ int ldb_unpack_data(struct ldb_context *ldb,
/*
add the special distinguishedName element
*/
-static int msg_add_distinguished_name(struct ldb_message *msg)
+int ldb_msg_add_distinguished_name(struct ldb_message *msg)
{
const char *dn_attr = "distinguishedName";
char *dn = NULL;
@@ -1158,7 +1158,7 @@ int ldb_filter_attrs(struct ldb_context *ldb,
/* Shortcuts for the simple cases */
} else if (add_dn && i == 1) {
- if (msg_add_distinguished_name(filtered_msg) != 0) {
+ if (ldb_msg_add_distinguished_name(filtered_msg) != 0) {
goto failed;
}
return 0;
@@ -1238,7 +1238,7 @@ int ldb_filter_attrs(struct ldb_context *ldb,
filtered_msg->num_elements = num_elements;
if (add_dn) {
- if (msg_add_distinguished_name(filtered_msg) != 0) {
+ if (ldb_msg_add_distinguished_name(filtered_msg) != 0) {
goto failed;
}
}
diff --git a/lib/ldb/include/ldb_private.h b/lib/ldb/include/ldb_private.h
index 338e71def6d..ca43817d07a 100644
--- a/lib/ldb/include/ldb_private.h
+++ b/lib/ldb/include/ldb_private.h
@@ -320,4 +320,9 @@ int ldb_match_message(struct ldb_context *ldb,
/* Reallocate elements to drop any excess capacity. */
void ldb_msg_shrink_to_fit(struct ldb_message *msg);
+/*
+ add the special distinguishedName element
+*/
+int ldb_msg_add_distinguished_name(struct ldb_message *msg);
+
#endif
--
2.25.1

File diff suppressed because it is too large Load Diff

File diff suppressed because it is too large Load Diff

View File

@ -0,0 +1,293 @@
From 8e84201f508b2893b6ec9383c656cccb08b148ef Mon Sep 17 00:00:00 2001
From: Joseph Sutton <josephsutton@catalyst.net.nz>
Date: Mon, 27 Feb 2023 10:31:52 +1300
Subject: [PATCH 14/34] CVE-2023-0614 ldb: Make use of
ldb_filter_attrs_in_place()
Change all uses of ldb_kv_filter_attrs() to use
ldb_filter_attrs_in_place() instead. This function does less work than
its predecessor, and no longer requires the allocation of a second ldb
message. Some of the work is able to be split out into separate
functions that each accomplish a single task, with a purpose to make the
code clearer.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15270
Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Conflict: NA
Reference: https://attachments.samba.org/attachment.cgi?id=17821
---
lib/ldb/ldb_key_value/ldb_kv.h | 6 +-
lib/ldb/ldb_key_value/ldb_kv_index.c | 27 +++++----
lib/ldb/ldb_key_value/ldb_kv_search.c | 86 ++++++++++++++-------------
source4/torture/ldb/ldb.c | 12 ++--
4 files changed, 66 insertions(+), 65 deletions(-)
diff --git a/lib/ldb/ldb_key_value/ldb_kv.h b/lib/ldb/ldb_key_value/ldb_kv.h
index ac474b04b4c..7d5a40e76e9 100644
--- a/lib/ldb/ldb_key_value/ldb_kv.h
+++ b/lib/ldb/ldb_key_value/ldb_kv.h
@@ -301,10 +301,8 @@ int ldb_kv_search_key(struct ldb_module *module,
const struct ldb_val ldb_key,
struct ldb_message *msg,
unsigned int unpack_flags);
-int ldb_kv_filter_attrs(struct ldb_context *ldb,
- const struct ldb_message *msg,
- const char *const *attrs,
- struct ldb_message *filtered_msg);
+int ldb_kv_filter_attrs_in_place(struct ldb_message *msg,
+ const char *const *attrs);
int ldb_kv_search(struct ldb_kv_context *ctx);
/*
diff --git a/lib/ldb/ldb_key_value/ldb_kv_index.c b/lib/ldb/ldb_key_value/ldb_kv_index.c
index d70e5f619ef..203266ea8c9 100644
--- a/lib/ldb/ldb_key_value/ldb_kv_index.c
+++ b/lib/ldb/ldb_key_value/ldb_kv_index.c
@@ -2264,7 +2264,6 @@ static int ldb_kv_index_filter(struct ldb_kv_private *ldb_kv,
{
struct ldb_context *ldb = ldb_module_get_ctx(ac->module);
struct ldb_message *msg;
- struct ldb_message *filtered_msg;
unsigned int i;
unsigned int num_keys = 0;
uint8_t previous_guid_key[LDB_KV_GUID_KEY_SIZE] = {0};
@@ -2456,27 +2455,31 @@ static int ldb_kv_index_filter(struct ldb_kv_private *ldb_kv,
continue;
}
- filtered_msg = ldb_msg_new(ac);
- if (filtered_msg == NULL) {
- TALLOC_FREE(keys);
- TALLOC_FREE(msg);
+ ret = ldb_msg_add_distinguished_name(msg);
+ if (ret == -1) {
+ talloc_free(msg);
return LDB_ERR_OPERATIONS_ERROR;
}
- filtered_msg->dn = talloc_steal(filtered_msg, msg->dn);
-
/* filter the attributes that the user wants */
- ret = ldb_kv_filter_attrs(ldb, msg, ac->attrs, filtered_msg);
+ ret = ldb_kv_filter_attrs_in_place(msg, ac->attrs);
+ if (ret != LDB_SUCCESS) {
+ talloc_free(keys);
+ talloc_free(msg);
+ return LDB_ERR_OPERATIONS_ERROR;
+ }
- talloc_free(msg);
+ ldb_msg_shrink_to_fit(msg);
- if (ret == -1) {
- TALLOC_FREE(filtered_msg);
+ /* Ensure the message elements are all talloc'd. */
+ ret = ldb_msg_elements_take_ownership(msg);
+ if (ret != LDB_SUCCESS) {
talloc_free(keys);
+ talloc_free(msg);
return LDB_ERR_OPERATIONS_ERROR;
}
- ret = ldb_module_send_entry(ac->req, filtered_msg, NULL);
+ ret = ldb_module_send_entry(ac->req, msg, NULL);
if (ret != LDB_SUCCESS) {
/* Regardless of success or failure, the msg
* is the callbacks responsiblity, and should
diff --git a/lib/ldb/ldb_key_value/ldb_kv_search.c b/lib/ldb/ldb_key_value/ldb_kv_search.c
index 46031b99c16..f3333510eab 100644
--- a/lib/ldb/ldb_key_value/ldb_kv_search.c
+++ b/lib/ldb/ldb_key_value/ldb_kv_search.c
@@ -292,15 +292,13 @@ int ldb_kv_search_dn1(struct ldb_module *module,
/*
* filter the specified list of attributes from msg,
- * adding requested attributes, and perhaps all for *,
- * but not the DN to filtered_msg.
+ * adding requested attributes, and perhaps all for *.
+ * The DN will not be added if it is missing.
*/
-int ldb_kv_filter_attrs(struct ldb_context *ldb,
- const struct ldb_message *msg,
- const char *const *attrs,
- struct ldb_message *filtered_msg)
+int ldb_kv_filter_attrs_in_place(struct ldb_message *msg,
+ const char *const *attrs)
{
- return ldb_filter_attrs(ldb, msg, attrs, filtered_msg);
+ return ldb_filter_attrs_in_place(msg, attrs);
}
/*
@@ -313,7 +311,7 @@ static int search_func(_UNUSED_ struct ldb_kv_private *ldb_kv,
{
struct ldb_context *ldb;
struct ldb_kv_context *ac;
- struct ldb_message *msg, *filtered_msg;
+ struct ldb_message *msg;
struct timeval now;
int ret, timeval_cmp;
bool matched;
@@ -410,25 +408,31 @@ static int search_func(_UNUSED_ struct ldb_kv_private *ldb_kv,
return 0;
}
- filtered_msg = ldb_msg_new(ac);
- if (filtered_msg == NULL) {
- TALLOC_FREE(msg);
+ ret = ldb_msg_add_distinguished_name(msg);
+ if (ret == -1) {
+ talloc_free(msg);
return LDB_ERR_OPERATIONS_ERROR;
}
- filtered_msg->dn = talloc_steal(filtered_msg, msg->dn);
-
/* filter the attributes that the user wants */
- ret = ldb_kv_filter_attrs(ldb, msg, ac->attrs, filtered_msg);
- talloc_free(msg);
+ ret = ldb_kv_filter_attrs_in_place(msg, ac->attrs);
+ if (ret != LDB_SUCCESS) {
+ talloc_free(msg);
+ ac->error = LDB_ERR_OPERATIONS_ERROR;
+ return -1;
+ }
- if (ret == -1) {
- TALLOC_FREE(filtered_msg);
+ ldb_msg_shrink_to_fit(msg);
+
+ /* Ensure the message elements are all talloc'd. */
+ ret = ldb_msg_elements_take_ownership(msg);
+ if (ret != LDB_SUCCESS) {
+ talloc_free(msg);
ac->error = LDB_ERR_OPERATIONS_ERROR;
return -1;
}
- ret = ldb_module_send_entry(ac->req, filtered_msg, NULL);
+ ret = ldb_module_send_entry(ac->req, msg, NULL);
if (ret != LDB_SUCCESS) {
ac->request_terminated = true;
/* the callback failed, abort the operation */
@@ -491,7 +495,7 @@ static int ldb_kv_search_full(struct ldb_kv_context *ctx)
static int ldb_kv_search_and_return_base(struct ldb_kv_private *ldb_kv,
struct ldb_kv_context *ctx)
{
- struct ldb_message *msg, *filtered_msg;
+ struct ldb_message *msg;
struct ldb_context *ldb = ldb_module_get_ctx(ctx->module);
const char *dn_linearized;
const char *msg_dn_linearized;
@@ -549,12 +553,6 @@ static int ldb_kv_search_and_return_base(struct ldb_kv_private *ldb_kv,
dn_linearized = ldb_dn_get_linearized(ctx->base);
msg_dn_linearized = ldb_dn_get_linearized(msg->dn);
- filtered_msg = ldb_msg_new(ctx);
- if (filtered_msg == NULL) {
- talloc_free(msg);
- return LDB_ERR_OPERATIONS_ERROR;
- }
-
if (strcmp(dn_linearized, msg_dn_linearized) == 0) {
/*
* If the DN is exactly the same string, then
@@ -562,36 +560,42 @@ static int ldb_kv_search_and_return_base(struct ldb_kv_private *ldb_kv,
* returned result, as it has already been
* casefolded
*/
- filtered_msg->dn = ldb_dn_copy(filtered_msg, ctx->base);
+ struct ldb_dn *dn = ldb_dn_copy(msg, ctx->base);
+ if (dn != NULL) {
+ msg->dn = dn;
+ }
}
- /*
- * If the ldb_dn_copy() failed, or if we did not choose that
- * optimisation (filtered_msg is zeroed at allocation),
- * steal the one from the unpack
- */
- if (filtered_msg->dn == NULL) {
- filtered_msg->dn = talloc_steal(filtered_msg, msg->dn);
+ ret = ldb_msg_add_distinguished_name(msg);
+ if (ret == -1) {
+ talloc_free(msg);
+ return LDB_ERR_OPERATIONS_ERROR;
}
/*
* filter the attributes that the user wants.
*/
- ret = ldb_kv_filter_attrs(ldb, msg, ctx->attrs, filtered_msg);
- if (ret == -1) {
+ ret = ldb_kv_filter_attrs_in_place(msg, ctx->attrs);
+ if (ret != LDB_SUCCESS) {
+ talloc_free(msg);
+ return LDB_ERR_OPERATIONS_ERROR;
+ }
+
+ ldb_msg_shrink_to_fit(msg);
+
+ /* Ensure the message elements are all talloc'd. */
+ ret = ldb_msg_elements_take_ownership(msg);
+ if (ret != LDB_SUCCESS) {
talloc_free(msg);
- filtered_msg = NULL;
return LDB_ERR_OPERATIONS_ERROR;
}
/*
- * Remove any extended components possibly copied in from
- * msg->dn, we just want the casefold components
+ * Remove any extended components, we just want the casefold components
*/
- ldb_dn_remove_extended_components(filtered_msg->dn);
- talloc_free(msg);
+ ldb_dn_remove_extended_components(msg->dn);
- ret = ldb_module_send_entry(ctx->req, filtered_msg, NULL);
+ ret = ldb_module_send_entry(ctx->req, msg, NULL);
if (ret != LDB_SUCCESS) {
/* Regardless of success or failure, the msg
* is the callbacks responsiblity, and should
diff --git a/source4/torture/ldb/ldb.c b/source4/torture/ldb/ldb.c
index bd0ae3a382a..c170416bec4 100644
--- a/source4/torture/ldb/ldb.c
+++ b/source4/torture/ldb/ldb.c
@@ -1634,7 +1634,6 @@ static bool torture_ldb_unpack_and_filter(struct torture_context *torture,
TALLOC_CTX *mem_ctx = talloc_new(torture);
struct ldb_context *ldb;
struct ldb_val data = *discard_const_p(struct ldb_val, data_p);
- struct ldb_message *unpack_msg = ldb_msg_new(mem_ctx);
struct ldb_message *msg = ldb_msg_new(mem_ctx);
const char *lookup_names[] = {"instanceType", "nonexistent",
"whenChanged", "objectClass",
@@ -1649,18 +1648,15 @@ static bool torture_ldb_unpack_and_filter(struct torture_context *torture,
"Failed to init samba");
torture_assert_int_equal(torture,
- ldb_unpack_data(ldb, &data, unpack_msg),
+ ldb_unpack_data(ldb, &data, msg),
0, "ldb_unpack_data failed");
- torture_assert_int_equal(torture, unpack_msg->num_elements, 13,
+ torture_assert_int_equal(torture, msg->num_elements, 13,
"Got wrong count of elements");
- msg->dn = talloc_steal(msg, unpack_msg->dn);
-
torture_assert_int_equal(torture,
- ldb_filter_attrs(ldb, unpack_msg,
- lookup_names, msg),
- 0, "ldb_kv_filter_attrs failed");
+ ldb_filter_attrs_in_place(msg, lookup_names),
+ 0, "ldb_filter_attrs_in_place failed");
/* Compare data in binary form */
torture_assert_int_equal(torture, msg->num_elements, 6,
--
2.25.1

View File

@ -0,0 +1,143 @@
From 9e07df4e268fd49358382e8d90c2d151efb66569 Mon Sep 17 00:00:00 2001
From: Joseph Sutton <josephsutton@catalyst.net.nz>
Date: Tue, 7 Feb 2023 09:35:24 +1300
Subject: [PATCH 15/34] CVE-2023-0614 s4:dsdb/extended_dn_in: Don't modify a
search tree we don't own
In extended_dn_fix_filter() we had:
req->op.search.tree = ldb_parse_tree_copy_shallow(req, req->op.search.tree);
which overwrote the parse tree on an existing ldb request with a fixed
up tree. This became a problem if a module performed another search with
that same request structure, as extended_dn_in would try to fix up the
already-modified tree for a second time. The fixed-up tree element now
having an extended DN, it would fall foul of the ldb_dn_match_allowed()
check in extended_dn_filter_callback(), and be replaced with an
ALWAYS_FALSE match rule. In practice this meant that <GUID={}> searches
would only work for one search in an ldb request, and fail for
subsequent ones.
Fix this by creating a new request with the modified tree, and leaving
the original request unmodified.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15270
Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Conflict: NA
Reference: https://attachments.samba.org/attachment.cgi?id=17821
---
.../dsdb/samdb/ldb_modules/extended_dn_in.c | 40 +++++++++++++++----
1 file changed, 32 insertions(+), 8 deletions(-)
diff --git a/source4/dsdb/samdb/ldb_modules/extended_dn_in.c b/source4/dsdb/samdb/ldb_modules/extended_dn_in.c
index 2d0513a738b..1dc1e1f2d42 100644
--- a/source4/dsdb/samdb/ldb_modules/extended_dn_in.c
+++ b/source4/dsdb/samdb/ldb_modules/extended_dn_in.c
@@ -48,6 +48,7 @@
struct extended_search_context {
struct ldb_module *module;
struct ldb_request *req;
+ struct ldb_parse_tree *tree;
struct ldb_dn *basedn;
struct ldb_dn *dn;
char *wellknown_object;
@@ -200,7 +201,7 @@ static int extended_base_callback(struct ldb_request *req, struct ldb_reply *are
ldb_module_get_ctx(ac->module), ac->req,
ac->basedn,
ac->req->op.search.scope,
- ac->req->op.search.tree,
+ ac->tree,
ac->req->op.search.attrs,
ac->req->controls,
ac, extended_final_callback,
@@ -515,11 +516,14 @@ static int extended_dn_filter_callback(struct ldb_parse_tree *tree, void *privat
*/
static int extended_dn_fix_filter(struct ldb_module *module,
struct ldb_request *req,
- uint32_t default_dsdb_flags)
+ uint32_t default_dsdb_flags,
+ struct ldb_parse_tree **down_tree)
{
struct extended_dn_filter_ctx *filter_ctx;
int ret;
+ *down_tree = NULL;
+
filter_ctx = talloc_zero(req, struct extended_dn_filter_ctx);
if (filter_ctx == NULL) {
return ldb_module_oom(module);
@@ -550,12 +554,12 @@ static int extended_dn_fix_filter(struct ldb_module *module,
filter_ctx->test_only = false;
filter_ctx->matched = false;
- req->op.search.tree = ldb_parse_tree_copy_shallow(req, req->op.search.tree);
- if (req->op.search.tree == NULL) {
+ *down_tree = ldb_parse_tree_copy_shallow(req, req->op.search.tree);
+ if (*down_tree == NULL) {
return ldb_oom(ldb_module_get_ctx(module));
}
- ret = ldb_parse_tree_walk(req->op.search.tree, extended_dn_filter_callback, filter_ctx);
+ ret = ldb_parse_tree_walk(*down_tree, extended_dn_filter_callback, filter_ctx);
if (ret != LDB_SUCCESS) {
talloc_free(filter_ctx);
return ret;
@@ -572,7 +576,8 @@ static int extended_dn_fix_filter(struct ldb_module *module,
static int extended_dn_in_fix(struct ldb_module *module, struct ldb_request *req, struct ldb_dn *dn)
{
struct extended_search_context *ac;
- struct ldb_request *down_req;
+ struct ldb_request *down_req = NULL;
+ struct ldb_parse_tree *down_tree = NULL;
int ret;
struct ldb_dn *base_dn = NULL;
enum ldb_scope base_dn_scope = LDB_SCOPE_BASE;
@@ -595,7 +600,7 @@ static int extended_dn_in_fix(struct ldb_module *module, struct ldb_request *req
}
if (req->operation == LDB_SEARCH) {
- ret = extended_dn_fix_filter(module, req, dsdb_flags);
+ ret = extended_dn_fix_filter(module, req, dsdb_flags, &down_tree);
if (ret != LDB_SUCCESS) {
return ret;
}
@@ -603,7 +608,25 @@ static int extended_dn_in_fix(struct ldb_module *module, struct ldb_request *req
if (!ldb_dn_has_extended(dn)) {
/* Move along there isn't anything to see here */
- return ldb_next_request(module, req);
+ if (down_tree == NULL) {
+ down_req = req;
+ } else {
+ ret = ldb_build_search_req_ex(&down_req,
+ ldb_module_get_ctx(module), req,
+ req->op.search.base,
+ req->op.search.scope,
+ down_tree,
+ req->op.search.attrs,
+ req->controls,
+ req, dsdb_next_callback,
+ req);
+ if (ret != LDB_SUCCESS) {
+ return ret;
+ }
+ LDB_REQ_SET_LOCATION(down_req);
+ }
+
+ return ldb_next_request(module, down_req);
} else {
/* It looks like we need to map the DN */
const struct ldb_val *sid_val, *guid_val, *wkguid_val;
@@ -690,6 +713,7 @@ static int extended_dn_in_fix(struct ldb_module *module, struct ldb_request *req
ac->module = module;
ac->req = req;
+ ac->tree = (down_tree != NULL) ? down_tree : req->op.search.tree;
ac->dn = dn;
ac->basedn = NULL; /* Filled in if the search finds the DN by SID/GUID etc */
ac->wellknown_object = wellknown_object;
--
2.25.1

View File

@ -0,0 +1,58 @@
From 13e0b2190c802dc876b465a201b8dc9f35f9720c Mon Sep 17 00:00:00 2001
From: Joseph Sutton <josephsutton@catalyst.net.nz>
Date: Tue, 7 Feb 2023 09:48:37 +1300
Subject: [PATCH 16/34] CVE-2023-0614 s4:dsdb:tests: Fix <GUID={}> search in
confidential attributes test
The object returned by schema_format_value() is a bytes object.
Therefore the search expression would resemble:
(lastKnownParent=<GUID=b'00000000-0000-0000-0000-000000000000'>)
which, due to the extra characters, would fail to match anything.
Fix it to be:
(lastKnownParent=<GUID=00000000-0000-0000-0000-000000000000>)
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15270
Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Conflict: NA
Reference: https://attachments.samba.org/attachment.cgi?id=17821
---
source4/dsdb/tests/python/confidential_attr.py | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/source4/dsdb/tests/python/confidential_attr.py b/source4/dsdb/tests/python/confidential_attr.py
index d5c7785485a..1c9c456917a 100755
--- a/source4/dsdb/tests/python/confidential_attr.py
+++ b/source4/dsdb/tests/python/confidential_attr.py
@@ -924,12 +924,12 @@ class ConfidentialAttrTestDirsync(ConfidentialAttrCommon):
self.assert_negative_searches(has_rights_to="all",
samdb=self.ldb_admin)
- def get_guid(self, dn):
+ def get_guid_string(self, dn):
"""Returns an object's GUID (in string format)"""
res = self.ldb_admin.search(base=dn, attrs=["objectGUID"],
scope=SCOPE_BASE)
guid = res[0]['objectGUID'][0]
- return self.ldb_admin.schema_format_value("objectGUID", guid)
+ return self.ldb_admin.schema_format_value("objectGUID", guid).decode('utf-8')
def make_attr_preserve_on_delete(self):
"""Marks the attribute under test as being preserve on delete"""
@@ -978,7 +978,7 @@ class ConfidentialAttrTestDirsync(ConfidentialAttrCommon):
# deleted objects, but only from this particular test run. We can do
# this by matching lastKnownParent against this test case's OU, which
# will match any deleted child objects.
- ou_guid = self.get_guid(self.ou)
+ ou_guid = self.get_guid_string(self.ou)
deleted_filter = "(lastKnownParent=<GUID={0}>)".format(ou_guid)
# the extra-filter will get combined via AND with the search expression
--
2.25.1

View File

@ -0,0 +1,49 @@
From e167524e306241ba14f925f412e3f89828ed8c61 Mon Sep 17 00:00:00 2001
From: Joseph Sutton <josephsutton@catalyst.net.nz>
Date: Thu, 25 Aug 2022 20:15:33 +1200
Subject: [PATCH 17/34] schema_samba4.ldif: Allocate previously added OIDs
DSDB_CONTROL_FORCE_ALLOW_VALIDATED_DNS_HOSTNAME_SPN_WRITE_OID was added
to source4/dsdb/samdb/samdb.h in commit
c2ab1f4696fa3f52918a126d0b37993a07f68bcb.
DSDB_EXTENDED_SCHEMA_LOAD was added in commit
1fd4cdfafaa6a41c824d1b3d76635bf3e446de0f.
Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
(cherry picked from commit 672ec6135f9ae3d7b5439523a4f456c19fb03a88)
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15270
[abartlet@samba.org This required as context for the above bug]
Conflict: NA
Reference: https://attachments.samba.org/attachment.cgi?id=17821
---
source4/setup/schema_samba4.ldif | 2 ++
1 file changed, 2 insertions(+)
diff --git a/source4/setup/schema_samba4.ldif b/source4/setup/schema_samba4.ldif
index a31b67750d4..54f48bde20e 100644
--- a/source4/setup/schema_samba4.ldif
+++ b/source4/setup/schema_samba4.ldif
@@ -231,6 +231,7 @@
#Allocated: DSDB_CONTROL_INVALID_NOT_IMPLEMENTED 1.3.6.1.4.1.7165.4.3.32
#Allocated: DSDB_CONTROL_PASSWORD_ACL_VALIDATION_OID 1.3.6.1.4.1.7165.4.3.33
#Allocated: DSDB_CONTROL_TRANSACTION_IDENTIFIER_OID 1.3.6.1.4.1.7165.4.3.34
+#Allocated: DSDB_CONTROL_FORCE_ALLOW_VALIDATED_DNS_HOSTNAME_SPN_WRITE_OID 1.3.6.1.4.1.7165.4.3.35
# Extended 1.3.6.1.4.1.7165.4.4.x
@@ -243,6 +244,7 @@
#Allocated: DSDB_EXTENDED_SEC_DESC_PROPAGATION_OID 1.3.6.1.4.1.7165.4.4.7
#Allocated: DSDB_EXTENDED_CREATE_OWN_RID_SET 1.3.6.1.4.1.7165.4.4.8
#Allocated: DSDB_EXTENDED_ALLOCATE_RID 1.3.6.1.4.1.7165.4.4.9
+#Allocated: DSDB_EXTENDED_SCHEMA_LOAD 1.3.6.1.4.1.7165.4.4.10
############
--
2.25.1

View File

@ -0,0 +1,34 @@
From 829157cf07ec887a7164600a53bdc688aa4717be Mon Sep 17 00:00:00 2001
From: Joseph Sutton <josephsutton@catalyst.net.nz>
Date: Tue, 7 Feb 2023 09:25:48 +1300
Subject: [PATCH 18/34] CVE-2023-0614 schema_samba4.ldif: Allocate previously
added OID
DSDB_CONTROL_CALCULATED_DEFAULT_SD_OID was added in commit
08187833fee57a8dba6c67546dfca516cd1f9d7a.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15270
Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Conflict: NA
Reference: https://attachments.samba.org/attachment.cgi?id=17821
---
source4/setup/schema_samba4.ldif | 1 +
1 file changed, 1 insertion(+)
diff --git a/source4/setup/schema_samba4.ldif b/source4/setup/schema_samba4.ldif
index 54f48bde20e..79800bfd6df 100644
--- a/source4/setup/schema_samba4.ldif
+++ b/source4/setup/schema_samba4.ldif
@@ -232,6 +232,7 @@
#Allocated: DSDB_CONTROL_PASSWORD_ACL_VALIDATION_OID 1.3.6.1.4.1.7165.4.3.33
#Allocated: DSDB_CONTROL_TRANSACTION_IDENTIFIER_OID 1.3.6.1.4.1.7165.4.3.34
#Allocated: DSDB_CONTROL_FORCE_ALLOW_VALIDATED_DNS_HOSTNAME_SPN_WRITE_OID 1.3.6.1.4.1.7165.4.3.35
+#Allocated: DSDB_CONTROL_CALCULATED_DEFAULT_SD_OID 1.3.6.1.4.1.7165.4.3.36
# Extended 1.3.6.1.4.1.7165.4.4.x
--
2.25.1

View File

@ -0,0 +1,205 @@
From 009026fae8eea85789bd912cbf397423480485da Mon Sep 17 00:00:00 2001
From: Joseph Sutton <josephsutton@catalyst.net.nz>
Date: Fri, 27 Jan 2023 08:32:41 +1300
Subject: [PATCH 19/34] CVE-2023-0614 tests/krb5: Add test for confidential
attributes timing differences
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15270
Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Conflict: NA
Reference: https://attachments.samba.org/attachment.cgi?id=17821
---
selftest/knownfail.d/confidential-attr-timing | 1 +
.../dsdb/tests/python/confidential_attr.py | 162 ++++++++++++++++++
2 files changed, 163 insertions(+)
create mode 100644 selftest/knownfail.d/confidential-attr-timing
diff --git a/selftest/knownfail.d/confidential-attr-timing b/selftest/knownfail.d/confidential-attr-timing
new file mode 100644
index 00000000000..e213cdb16d3
--- /dev/null
+++ b/selftest/knownfail.d/confidential-attr-timing
@@ -0,0 +1 @@
+^samba4.ldap.confidential_attr.python\(ad_dc_slowtests\).__main__.ConfidentialAttrTestDirsync.test_timing_attack\(ad_dc_slowtests\)
diff --git a/source4/dsdb/tests/python/confidential_attr.py b/source4/dsdb/tests/python/confidential_attr.py
index 1c9c456917a..031c9690ba6 100755
--- a/source4/dsdb/tests/python/confidential_attr.py
+++ b/source4/dsdb/tests/python/confidential_attr.py
@@ -25,6 +25,9 @@ sys.path.insert(0, "bin/python")
import samba
import os
+import random
+import statistics
+import time
from samba.tests.subunitrun import SubunitOptions, TestProgram
import samba.getopt as options
from ldb import SCOPE_BASE, SCOPE_SUBTREE
@@ -1022,4 +1025,163 @@ class ConfidentialAttrTestDirsync(ConfidentialAttrCommon):
self.assert_conf_attr_searches(has_rights_to=0)
self.assert_negative_searches(has_rights_to=0, dc_mode=dc_mode)
+ def test_timing_attack(self):
+ # Create the machine account.
+ mach_name = f'conf_timing_{random.randint(0, 0xffff)}'
+ mach_dn = Dn(self.ldb_admin, f'CN={mach_name},{self.ou}')
+ details = {
+ 'dn': mach_dn,
+ 'objectclass': 'computer',
+ 'sAMAccountName': f'{mach_name}$',
+ }
+ self.ldb_admin.add(details)
+
+ # Get the machine account's GUID.
+ res = self.ldb_admin.search(mach_dn,
+ attrs=['objectGUID'],
+ scope=SCOPE_BASE)
+ mach_guid = res[0].get('objectGUID', idx=0)
+
+ # Now we can create an msFVE-RecoveryInformation object that is a child
+ # of the machine account object.
+ recovery_dn = Dn(self.ldb_admin, str(mach_dn))
+ recovery_dn.add_child('CN=recovery_info')
+
+ secret_pw = 'Secret007'
+ not_secret_pw = 'Secret008'
+
+ secret_pw_utf8 = secret_pw.encode('utf-8')
+
+ # The crucial attribute, msFVE-RecoveryPassword, is a confidential
+ # attribute.
+ conf_attr = 'msFVE-RecoveryPassword'
+
+ m = Message(recovery_dn)
+ m['objectClass'] = 'msFVE-RecoveryInformation'
+ m['msFVE-RecoveryGuid'] = mach_guid
+ m[conf_attr] = secret_pw
+ self.ldb_admin.add(m)
+
+ attrs = [conf_attr]
+
+ # Search for the confidential attribute as administrator, ensuring it
+ # is visible.
+ res = self.ldb_admin.search(recovery_dn,
+ attrs=attrs,
+ scope=SCOPE_BASE)
+ self.assertEqual(1, len(res))
+ pw = res[0].get(conf_attr, idx=0)
+ self.assertEqual(secret_pw_utf8, pw)
+
+ # Repeat the search with an expression matching on the confidential
+ # attribute. This should also work.
+ res = self.ldb_admin.search(
+ recovery_dn,
+ attrs=attrs,
+ expression=f'({conf_attr}={secret_pw})',
+ scope=SCOPE_BASE)
+ self.assertEqual(1, len(res))
+ pw = res[0].get(conf_attr, idx=0)
+ self.assertEqual(secret_pw_utf8, pw)
+
+ # Search for the attribute as an unprivileged user. It should not be
+ # visible.
+ user_res = self.ldb_user.search(recovery_dn,
+ attrs=attrs,
+ scope=SCOPE_BASE)
+ pw = user_res[0].get(conf_attr, idx=0)
+ # The attribute should be None.
+ self.assertIsNone(pw)
+
+ # We use LDAP_MATCHING_RULE_TRANSITIVE_EVAL to create a search
+ # expression that takes a long time to execute, by setting off another
+ # search each time it is evaluated. It makes no difference that the
+ # object on which we're searching has no 'member' attribute.
+ dummy_dn = 'cn=user,cn=users,dc=samba,dc=example,dc=com'
+ slow_subexpr = f'(member:1.2.840.113556.1.4.1941:={dummy_dn})'
+ slow_expr = f'(|{slow_subexpr * 100})'
+
+ # The full search expression. It comprises a match on the confidential
+ # attribute joined by an AND to our slow search expression, The AND
+ # operator is short-circuiting, so if our first subexpression fails to
+ # match, we'll bail out of the search early. Otherwise, we'll evaluate
+ # the slow part; as its subexpressions are joined by ORs, and will all
+ # fail to match, every one of them will need to be evaluated. By
+ # measuring how long the search takes, we'll be able to infer whether
+ # the confidential attribute matched or not.
+
+ # This is bad if we are not an administrator, and are able to use this
+ # to determine the values of confidential attributes. Therefore we need
+ # to ensure we can't observe any difference in timing.
+ correct_expr = f'(&({conf_attr}={secret_pw}){slow_expr})'
+ wrong_expr = f'(&({conf_attr}={not_secret_pw}){slow_expr})'
+
+ def standard_uncertainty_bounds(times):
+ mean = statistics.mean(times)
+ stdev = statistics.stdev(times, mean)
+
+ return (mean - stdev, mean + stdev)
+
+ # Perform a number of searches with both correct and incorrect
+ # expressions, and return the uncertainty bounds for each.
+ def time_searches(samdb):
+ warmup_samples = 3
+ samples = 10
+ matching_times = []
+ non_matching_times = []
+
+ for _ in range(warmup_samples):
+ samdb.search(recovery_dn,
+ attrs=attrs,
+ expression=correct_expr,
+ scope=SCOPE_BASE)
+
+ for _ in range(samples):
+ # Measure the time taken for a search, for both a matching and
+ # a non-matching search expression.
+
+ prev = time.time()
+ samdb.search(recovery_dn,
+ attrs=attrs,
+ expression=correct_expr,
+ scope=SCOPE_BASE)
+ now = time.time()
+ matching_times.append(now - prev)
+
+ prev = time.time()
+ samdb.search(recovery_dn,
+ attrs=attrs,
+ expression=wrong_expr,
+ scope=SCOPE_BASE)
+ now = time.time()
+ non_matching_times.append(now - prev)
+
+ matching = standard_uncertainty_bounds(matching_times)
+ non_matching = standard_uncertainty_bounds(non_matching_times)
+ return matching, non_matching
+
+ def assertRangesDistinct(a, b):
+ a0, a1 = a
+ b0, b1 = b
+ self.assertLess(min(a1, b1), max(a0, b0))
+
+ def assertRangesOverlap(a, b):
+ a0, a1 = a
+ b0, b1 = b
+ self.assertGreaterEqual(min(a1, b1), max(a0, b0))
+
+ # For an administrator, the uncertainty bounds for matching and
+ # non-matching searches should be distinct. This shows that the two
+ # cases are distinguishable, and therefore that confidential attributes
+ # are visible.
+ admin_matching, admin_non_matching = time_searches(self.ldb_admin)
+ assertRangesDistinct(admin_matching, admin_non_matching)
+
+ # The user cannot view the confidential attribute, so the uncertainty
+ # bounds for matching and non-matching searches must overlap. The two
+ # cases must be indistinguishable.
+ user_matching, user_non_matching = time_searches(self.ldb_user)
+ assertRangesOverlap(user_matching, user_non_matching)
+
+
TestProgram(module=__name__, opts=subunitopts)
--
2.25.1

View File

@ -0,0 +1,66 @@
From b23fb132e63a6d3d791469614593c43906686ac9 Mon Sep 17 00:00:00 2001
From: Joseph Sutton <josephsutton@catalyst.net.nz>
Date: Fri, 3 Mar 2023 17:31:54 +1300
Subject: [PATCH 20/34] CVE-2023-0614 ldb: Add ldb_parse_tree_get_attr()
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15270
Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Conflict: NA
Reference: https://attachments.samba.org/attachment.cgi?id=17821
---
lib/ldb/common/ldb_parse.c | 25 +++++++++++++++++++++++++
lib/ldb/include/ldb_module.h | 3 +++
2 files changed, 28 insertions(+)
diff --git a/lib/ldb/common/ldb_parse.c b/lib/ldb/common/ldb_parse.c
index f0045ad2093..2d102ff750e 100644
--- a/lib/ldb/common/ldb_parse.c
+++ b/lib/ldb/common/ldb_parse.c
@@ -997,3 +997,28 @@ struct ldb_parse_tree *ldb_parse_tree_copy_shallow(TALLOC_CTX *mem_ctx,
return nt;
}
+
+/* Get the attribute (if any) associated with the top node of a parse tree. */
+const char *ldb_parse_tree_get_attr(const struct ldb_parse_tree *tree)
+{
+ switch (tree->operation) {
+ case LDB_OP_AND:
+ case LDB_OP_OR:
+ case LDB_OP_NOT:
+ return NULL;
+ case LDB_OP_EQUALITY:
+ return tree->u.equality.attr;
+ case LDB_OP_SUBSTRING:
+ return tree->u.substring.attr;
+ case LDB_OP_GREATER:
+ case LDB_OP_LESS:
+ case LDB_OP_APPROX:
+ return tree->u.comparison.attr;
+ case LDB_OP_PRESENT:
+ return tree->u.present.attr;
+ case LDB_OP_EXTENDED:
+ return tree->u.extended.attr;
+ }
+
+ return NULL;
+}
diff --git a/lib/ldb/include/ldb_module.h b/lib/ldb/include/ldb_module.h
index 4ae381ba5be..bd369ed9512 100644
--- a/lib/ldb/include/ldb_module.h
+++ b/lib/ldb/include/ldb_module.h
@@ -490,6 +490,9 @@ int ldb_init_module(const char *version);
*/
bool ldb_dn_replace_components(struct ldb_dn *dn, struct ldb_dn *new_dn);
+/* Get the attribute (if any) associated with the top node of a parse tree. */
+const char *ldb_parse_tree_get_attr(const struct ldb_parse_tree *tree);
+
/*
walk a parse tree, calling the provided callback on each node
*/
--
2.25.1

View File

@ -0,0 +1,102 @@
From 3127ba92bf91fa6f666552ac31cb06ffdc6d7e63 Mon Sep 17 00:00:00 2001
From: Joseph Sutton <josephsutton@catalyst.net.nz>
Date: Mon, 27 Feb 2023 13:40:33 +1300
Subject: [PATCH 21/34] CVE-2023-0614 s4-acl: Split out logic to remove access
checking attributes
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15270
Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Conflict: NA
Reference: https://attachments.samba.org/attachment.cgi?id=17821
---
source4/dsdb/samdb/ldb_modules/acl_read.c | 58 ++++++++++++++---------
1 file changed, 35 insertions(+), 23 deletions(-)
diff --git a/source4/dsdb/samdb/ldb_modules/acl_read.c b/source4/dsdb/samdb/ldb_modules/acl_read.c
index 8814a816797..3980c44345e 100644
--- a/source4/dsdb/samdb/ldb_modules/acl_read.c
+++ b/source4/dsdb/samdb/ldb_modules/acl_read.c
@@ -545,6 +545,39 @@ static int check_search_ops_access(struct aclread_context *ac,
return ret;
}
+/*
+ * Whether this attribute was added to perform access checks and must be
+ * removed.
+ */
+static bool should_remove_attr(const char *attr, const struct aclread_context *ac)
+{
+ if (ac->added_nTSecurityDescriptor &&
+ ldb_attr_cmp("nTSecurityDescriptor", attr) == 0)
+ {
+ return true;
+ }
+
+ if (ac->added_objectSid &&
+ ldb_attr_cmp("objectSid", attr) == 0)
+ {
+ return true;
+ }
+
+ if (ac->added_instanceType &&
+ ldb_attr_cmp("instanceType", attr) == 0)
+ {
+ return true;
+ }
+
+ if (ac->added_objectClass &&
+ ldb_attr_cmp("objectClass", attr) == 0)
+ {
+ return true;
+ }
+
+ return false;
+}
+
static int aclread_callback(struct ldb_request *req, struct ldb_reply *ares)
{
struct ldb_context *ldb;
@@ -619,7 +652,6 @@ static int aclread_callback(struct ldb_request *req, struct ldb_reply *ares)
/* for every element in the message check RP */
for (i=0; i < msg->num_elements; i++) {
const struct dsdb_attribute *attr;
- bool is_sd, is_objectsid, is_instancetype, is_objectclass;
uint32_t access_mask;
attr = dsdb_attribute_by_lDAPDisplayName(ac->schema,
msg->elements[i].name);
@@ -631,28 +663,8 @@ static int aclread_callback(struct ldb_request *req, struct ldb_reply *ares)
ret = LDB_ERR_OPERATIONS_ERROR;
goto fail;
}
- is_sd = ldb_attr_cmp("nTSecurityDescriptor",
- msg->elements[i].name) == 0;
- is_objectsid = ldb_attr_cmp("objectSid",
- msg->elements[i].name) == 0;
- is_instancetype = ldb_attr_cmp("instanceType",
- msg->elements[i].name) == 0;
- is_objectclass = ldb_attr_cmp("objectClass",
- msg->elements[i].name) == 0;
- /* these attributes were added to perform access checks and must be removed */
- if (is_objectsid && ac->added_objectSid) {
- ldb_msg_element_mark_inaccessible(&msg->elements[i]);
- continue;
- }
- if (is_instancetype && ac->added_instanceType) {
- ldb_msg_element_mark_inaccessible(&msg->elements[i]);
- continue;
- }
- if (is_objectclass && ac->added_objectClass) {
- ldb_msg_element_mark_inaccessible(&msg->elements[i]);
- continue;
- }
- if (is_sd && ac->added_nTSecurityDescriptor) {
+ /* Remove attributes added to perform access checks. */
+ if (should_remove_attr(msg->elements[i].name, ac)) {
ldb_msg_element_mark_inaccessible(&msg->elements[i]);
continue;
}
--
2.25.1

View File

@ -0,0 +1,52 @@
From 652fecd7d037992b89ed1a4eb17f9f467c2aadf7 Mon Sep 17 00:00:00 2001
From: Joseph Sutton <josephsutton@catalyst.net.nz>
Date: Mon, 27 Feb 2023 12:19:08 +1300
Subject: [PATCH 22/34] CVE-2023-0614 s4-dsdb: Add samdb_result_dom_sid_buf()
This function parses a SID from an ldb_message, similar to
samdb_result_dom_sid(), but does it without allocating anything.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15270
Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Conflict: NA
Reference: https://attachments.samba.org/attachment.cgi?id=17821
---
source4/dsdb/common/util.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/source4/dsdb/common/util.c b/source4/dsdb/common/util.c
index a30ae662c1e..b556f06cb63 100644
--- a/source4/dsdb/common/util.c
+++ b/source4/dsdb/common/util.c
@@ -365,6 +365,26 @@ struct dom_sid *samdb_result_dom_sid(TALLOC_CTX *mem_ctx, const struct ldb_messa
return sid;
}
+/*
+ pull a dom_sid structure from a objectSid in a result set.
+*/
+int samdb_result_dom_sid_buf(const struct ldb_message *msg,
+ const char *attr,
+ struct dom_sid *sid)
+{
+ ssize_t ret;
+ const struct ldb_val *v = NULL;
+ v = ldb_msg_find_ldb_val(msg, attr);
+ if (v == NULL) {
+ return LDB_ERR_NO_SUCH_ATTRIBUTE;
+ }
+ ret = sid_parse(v->data, v->length, sid);
+ if (ret == -1) {
+ return LDB_ERR_OPERATIONS_ERROR;
+ }
+ return LDB_SUCCESS;
+}
+
/*
pull a guid structure from a objectGUID in a result set.
*/
--
2.25.1

View File

@ -0,0 +1,218 @@
From 3ebfe8666c02ab2de823457c68a922d0b0437cec Mon Sep 17 00:00:00 2001
From: Joseph Sutton <josephsutton@catalyst.net.nz>
Date: Mon, 27 Feb 2023 13:55:36 +1300
Subject: [PATCH 23/34] CVE-2023-0614 s4-acl: Split out function to set up
access checking variables
These variables are often used together, and it is useful to have the
setup code in one place.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15270
Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
[abartlet@samba.org adapted to the use of
acl_check_access_on_attribute as
acl_check_access_on_attribute_implicit_owner is
only in Samba 4.18 and newer]
Conflict: NA
Reference: https://attachments.samba.org/attachment.cgi?id=17821
---
source4/dsdb/samdb/ldb_modules/acl_read.c | 113 +++++++++++++++-------
1 file changed, 80 insertions(+), 33 deletions(-)
diff --git a/source4/dsdb/samdb/ldb_modules/acl_read.c b/source4/dsdb/samdb/ldb_modules/acl_read.c
index 3980c44345e..6659c71c965 100644
--- a/source4/dsdb/samdb/ldb_modules/acl_read.c
+++ b/source4/dsdb/samdb/ldb_modules/acl_read.c
@@ -70,6 +70,13 @@ struct aclread_private {
struct ldb_val sd_cached_blob;
};
+struct access_check_context {
+ struct security_descriptor *sd;
+ struct dom_sid sid_buf;
+ const struct dom_sid *sid;
+ const struct dsdb_class *objectclass;
+};
+
/*
* the object has a parent, so we have to check for visibility
*
@@ -254,7 +261,7 @@ static int aclread_check_object_visible(struct aclread_context *ac,
*/
static int aclread_get_sd_from_ldb_message(struct aclread_context *ac,
- struct ldb_message *acl_res,
+ const struct ldb_message *acl_res,
struct security_descriptor **sd)
{
struct ldb_message_element *sd_element;
@@ -358,7 +365,7 @@ static uint32_t get_attr_access_mask(const struct dsdb_attribute *attr,
struct parse_tree_aclread_ctx {
struct aclread_context *ac;
TALLOC_CTX *mem_ctx;
- struct dom_sid *sid;
+ const struct dom_sid *sid;
struct ldb_dn *dn;
struct security_descriptor *sd;
const struct dsdb_class *objectclass;
@@ -372,7 +379,7 @@ static int check_attr_access_rights(TALLOC_CTX *mem_ctx, const char *attr_name,
struct aclread_context *ac,
struct security_descriptor *sd,
const struct dsdb_class *objectclass,
- struct dom_sid *sid, struct ldb_dn *dn)
+ const struct dom_sid *sid, struct ldb_dn *dn)
{
int ret;
const struct dsdb_attribute *attr = NULL;
@@ -448,6 +455,69 @@ static const char * parse_tree_get_attr(struct ldb_parse_tree *tree)
return attr;
}
+static int setup_access_check_context(struct aclread_context *ac,
+ const struct ldb_message *msg,
+ struct access_check_context *ctx)
+{
+ int ret;
+
+ /*
+ * Fetch the schema so we can check which attributes are
+ * considered confidential.
+ */
+ if (ac->schema == NULL) {
+ struct ldb_context *ldb = ldb_module_get_ctx(ac->module);
+
+ /* Cache the schema for later use. */
+ ac->schema = dsdb_get_schema(ldb, ac);
+
+ if (ac->schema == NULL) {
+ return ldb_error(ldb, LDB_ERR_OPERATIONS_ERROR,
+ "aclread_callback: Error obtaining schema.");
+ }
+ }
+
+ /* Fetch the object's security descriptor. */
+ ret = aclread_get_sd_from_ldb_message(ac, msg, &ctx->sd);
+ if (ret != LDB_SUCCESS) {
+ ldb_debug_set(ldb_module_get_ctx(ac->module), LDB_DEBUG_FATAL,
+ "acl_read: cannot get descriptor of %s: %s\n",
+ ldb_dn_get_linearized(msg->dn), ldb_strerror(ret));
+ return LDB_ERR_OPERATIONS_ERROR;
+ } else if (ctx->sd == NULL) {
+ ldb_debug_set(ldb_module_get_ctx(ac->module), LDB_DEBUG_FATAL,
+ "acl_read: cannot get descriptor of %s (attribute not found)\n",
+ ldb_dn_get_linearized(msg->dn));
+ return LDB_ERR_OPERATIONS_ERROR;
+ }
+ /*
+ * Get the most specific structural object class for the ACL check
+ */
+ ctx->objectclass = dsdb_get_structural_oc_from_msg(ac->schema, msg);
+ if (ctx->objectclass == NULL) {
+ ldb_asprintf_errstring(ldb_module_get_ctx(ac->module),
+ "acl_read: Failed to find a structural class for %s",
+ ldb_dn_get_linearized(msg->dn));
+ return LDB_ERR_OPERATIONS_ERROR;
+ }
+
+ /* Fetch the object's SID. */
+ ret = samdb_result_dom_sid_buf(msg, "objectSid", &ctx->sid_buf);
+ if (ret == LDB_SUCCESS) {
+ ctx->sid = &ctx->sid_buf;
+ } else if (ret == LDB_ERR_NO_SUCH_ATTRIBUTE) {
+ /* This is expected. */
+ ctx->sid = NULL;
+ } else {
+ ldb_asprintf_errstring(ldb_module_get_ctx(ac->module),
+ "acl_read: Failed to parse objectSid as dom_sid for %s",
+ ldb_dn_get_linearized(msg->dn));
+ return ret;
+ }
+
+ return LDB_SUCCESS;
+}
+
/*
* Checks a single attribute in the search parse-tree to make sure the user has
* sufficient rights to view it.
@@ -522,7 +592,7 @@ static int check_search_ops_access(struct aclread_context *ac,
TALLOC_CTX *mem_ctx,
struct security_descriptor *sd,
const struct dsdb_class *objectclass,
- struct dom_sid *sid, struct ldb_dn *dn,
+ const struct dom_sid *sid, struct ldb_dn *dn,
bool *suppress_result)
{
int ret;
@@ -585,10 +655,8 @@ static int aclread_callback(struct ldb_request *req, struct ldb_reply *ares)
struct ldb_message *msg;
int ret;
unsigned int i;
- struct security_descriptor *sd = NULL;
- struct dom_sid *sid = NULL;
+ struct access_check_context acl_ctx;
TALLOC_CTX *tmp_ctx;
- const struct dsdb_class *objectclass;
bool suppress_result = false;
ac = talloc_get_type_abort(req->context, struct aclread_context);
@@ -604,32 +672,11 @@ static int aclread_callback(struct ldb_request *req, struct ldb_reply *ares)
switch (ares->type) {
case LDB_REPLY_ENTRY:
msg = ares->message;
- ret = aclread_get_sd_from_ldb_message(ac, msg, &sd);
+ ret = setup_access_check_context(ac, msg, &acl_ctx);
if (ret != LDB_SUCCESS) {
- ldb_debug_set(ldb, LDB_DEBUG_FATAL,
- "acl_read: cannot get descriptor of %s: %s\n",
- ldb_dn_get_linearized(msg->dn), ldb_strerror(ret));
- ret = LDB_ERR_OPERATIONS_ERROR;
- goto fail;
- } else if (sd == NULL) {
- ldb_debug_set(ldb, LDB_DEBUG_FATAL,
- "acl_read: cannot get descriptor of %s (attribute not found)\n",
- ldb_dn_get_linearized(msg->dn));
- ret = LDB_ERR_OPERATIONS_ERROR;
- goto fail;
- }
- /*
- * Get the most specific structural object class for the ACL check
- */
- objectclass = dsdb_get_structural_oc_from_msg(ac->schema, msg);
- if (objectclass == NULL) {
- ldb_asprintf_errstring(ldb, "acl_read: Failed to find a structural class for %s",
- ldb_dn_get_linearized(msg->dn));
- ret = LDB_ERR_OPERATIONS_ERROR;
- goto fail;
+ return ret;
}
- sid = samdb_result_dom_sid(tmp_ctx, msg, "objectSid");
if (!ldb_dn_is_null(msg->dn)) {
/*
* this is a real object, so we have
@@ -678,8 +725,8 @@ static int aclread_callback(struct ldb_request *req, struct ldb_reply *ares)
ret = acl_check_access_on_attribute(ac->module,
tmp_ctx,
- sd,
- sid,
+ acl_ctx.sd,
+ acl_ctx.sid,
access_mask,
attr,
objectclass);
@@ -733,7 +780,7 @@ static int aclread_callback(struct ldb_request *req, struct ldb_reply *ares)
* check access rights for the search attributes, as well as the
* attribute values actually being returned
*/
- ret = check_search_ops_access(ac, tmp_ctx, sd, objectclass, sid,
+ ret = check_search_ops_access(ac, tmp_ctx, acl_ctx.sd, acl_ctx.objectclass, acl_ctx.sid,
msg->dn, &suppress_result);
if (ret != LDB_SUCCESS) {
ldb_debug_set(ldb, LDB_DEBUG_FATAL,
--
2.25.1

File diff suppressed because it is too large Load Diff

View File

@ -0,0 +1,54 @@
From 976b0c37d82daf1269664a1fce8dfcca5770456b Mon Sep 17 00:00:00 2001
From: Joseph Sutton <josephsutton@catalyst.net.nz>
Date: Mon, 27 Feb 2023 13:31:44 +1300
Subject: [PATCH 25/34] CVE-2023-0614 s4-acl: Avoid calling
dsdb_module_am_system() if we can help it
If the AS_SYSTEM control is present, we know we have system privileges,
and have no need to call dsdb_module_am_system().
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15270
Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Conflict: NA
Reference: https://attachments.samba.org/attachment.cgi?id=17821
---
source4/dsdb/samdb/ldb_modules/acl_read.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/source4/dsdb/samdb/ldb_modules/acl_read.c b/source4/dsdb/samdb/ldb_modules/acl_read.c
index 8ca8607b925..6dcc3c9b36e 100644
--- a/source4/dsdb/samdb/ldb_modules/acl_read.c
+++ b/source4/dsdb/samdb/ldb_modules/acl_read.c
@@ -860,7 +860,7 @@ static int aclread_search(struct ldb_module *module, struct ldb_request *req)
int ret;
struct aclread_context *ac;
struct ldb_request *down_req;
- struct ldb_control *as_system = ldb_request_get_control(req, LDB_CONTROL_AS_SYSTEM_OID);
+ bool am_system;
struct ldb_result *res;
struct aclread_private *p;
bool need_sd = false;
@@ -877,11 +877,16 @@ static int aclread_search(struct ldb_module *module, struct ldb_request *req)
ldb = ldb_module_get_ctx(module);
p = talloc_get_type(ldb_module_get_private(module), struct aclread_private);
+ am_system = ldb_request_get_control(req, LDB_CONTROL_AS_SYSTEM_OID) != NULL;
+ if (!am_system) {
+ am_system = dsdb_module_am_system(module);
+ }
+
/* skip access checks if we are system or system control is supplied
* or this is not LDAP server request */
if (!p || !p->enabled ||
- dsdb_module_am_system(module)
- || as_system || !is_untrusted) {
+ am_system ||
+ !is_untrusted) {
return ldb_next_request(module, req);
}
/* no checks on special dn */
--
2.25.1

View File

@ -0,0 +1,131 @@
From 8cf6c358e27a3926397e0d5fbb562e76926589d9 Mon Sep 17 00:00:00 2001
From: Joseph Sutton <josephsutton@catalyst.net.nz>
Date: Thu, 16 Feb 2023 12:35:34 +1300
Subject: [PATCH 26/34] CVE-2023-0614 ldb: Use binary search to check whether
attribute is secret
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15270
Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Conflict: NA
Reference: https://attachments.samba.org/attachment.cgi?id=17821
---
source4/dsdb/samdb/ldb_modules/acl_read.c | 56 ++++++++++++++---------
1 file changed, 35 insertions(+), 21 deletions(-)
diff --git a/source4/dsdb/samdb/ldb_modules/acl_read.c b/source4/dsdb/samdb/ldb_modules/acl_read.c
index 6dcc3c9b36e..21f72fbe720 100644
--- a/source4/dsdb/samdb/ldb_modules/acl_read.c
+++ b/source4/dsdb/samdb/ldb_modules/acl_read.c
@@ -79,6 +79,7 @@ struct aclread_private {
struct security_descriptor *sd_cached;
struct ldb_val sd_cached_blob;
const char **password_attrs;
+ size_t num_password_attrs;
};
struct access_check_context {
@@ -512,22 +513,18 @@ static int aclread_get_sd_from_ldb_message(struct aclread_context *ac,
/* Check whether the attribute is a password attribute. */
static bool attr_is_secret(const char *attr, const struct aclread_private *private_data)
{
- unsigned i;
+ const char **found = NULL;
if (private_data->password_attrs == NULL) {
return false;
}
- for (i = 0; private_data->password_attrs[i] != NULL; ++i) {
- const char *password_attr = private_data->password_attrs[i];
- if (ldb_attr_cmp(attr, password_attr) != 0) {
- continue;
- }
-
- return true;
- }
-
- return false;
+ BINARY_ARRAY_SEARCH_V(private_data->password_attrs,
+ private_data->num_password_attrs,
+ attr,
+ ldb_attr_cmp,
+ found);
+ return found != NULL;
}
/*
@@ -1128,6 +1125,14 @@ static int acl_redact_msg_for_filter(struct ldb_module *module, struct ldb_reque
return LDB_SUCCESS;
}
+static int ldb_attr_cmp_fn(const void *_a, const void *_b)
+{
+ const char * const *a = _a;
+ const char * const *b = _b;
+
+ return ldb_attr_cmp(*a, *b);
+}
+
static int aclread_init(struct ldb_module *module)
{
struct ldb_context *ldb = ldb_module_get_ctx(module);
@@ -1188,7 +1193,7 @@ static int aclread_init(struct ldb_module *module)
}
p->password_attrs = talloc_array(p, const char *,
password_attributes->num_values +
- ARRAY_SIZE(secret_attrs) + 1);
+ ARRAY_SIZE(secret_attrs));
if (!p->password_attrs) {
talloc_free(mem_ctx);
return ldb_oom(ldb);
@@ -1223,7 +1228,10 @@ static int aclread_init(struct ldb_module *module)
}
n++;
}
- p->password_attrs[n] = NULL;
+ p->num_password_attrs = n;
+
+ /* Sort the password attributes so we can use binary search. */
+ TYPESAFE_QSORT(p->password_attrs, p->num_password_attrs, ldb_attr_cmp_fn);
ret = ldb_register_redact_callback(ldb, acl_redact_msg_for_filter, module);
if (ret != LDB_SUCCESS) {
@@ -1247,19 +1255,25 @@ done:
module,
NULL);
if (!userPassword_support) {
+ const char **found = NULL;
+
/*
* Remove the userPassword attribute, as it is not
* considered secret.
*/
- for (i = 0; p->password_attrs[i] != NULL; ++i) {
- if (ldb_attr_cmp(p->password_attrs[i], "userPassword") == 0) {
- break;
+ BINARY_ARRAY_SEARCH_V(p->password_attrs,
+ p->num_password_attrs,
+ "userPassword",
+ ldb_attr_cmp,
+ found);
+ if (found != NULL) {
+ size_t found_idx = found - p->password_attrs;
+
+ /* Shift following elements backwards by one. */
+ for (i = found_idx; i < p->num_password_attrs - 1; ++i) {
+ p->password_attrs[i] = p->password_attrs[i + 1];
}
- }
-
- /* Shift following elements backwards by one. */
- for (; p->password_attrs[i] != NULL; ++i) {
- p->password_attrs[i] = p->password_attrs[i + 1];
+ --p->num_password_attrs;
}
}
}
--
2.25.1

View File

@ -0,0 +1,147 @@
From 94efa3fc3053a623a7a5c3a4a6428356bc334152 Mon Sep 17 00:00:00 2001
From: Joseph Sutton <josephsutton@catalyst.net.nz>
Date: Tue, 14 Feb 2023 13:17:24 +1300
Subject: [PATCH 27/34] CVE-2023-0614 ldb: Centralise checking for inaccessible
matches
This makes it less likely that we forget to handle a case.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15270
Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Conflict: NA
Reference: https://attachments.samba.org/attachment.cgi?id=17821
---
lib/ldb-samba/ldb_matching_rules.c | 5 ---
lib/ldb/common/ldb_match.c | 56 +++++++++++++++++-------------
2 files changed, 31 insertions(+), 30 deletions(-)
diff --git a/lib/ldb-samba/ldb_matching_rules.c b/lib/ldb-samba/ldb_matching_rules.c
index 0c4c31e49f9..b86594c1823 100644
--- a/lib/ldb-samba/ldb_matching_rules.c
+++ b/lib/ldb-samba/ldb_matching_rules.c
@@ -87,11 +87,6 @@ static int ldb_eval_transitive_filter_helper(TALLOC_CTX *mem_ctx,
return LDB_SUCCESS;
}
- if (ldb_msg_element_is_inaccessible(el)) {
- *matched = false;
- return LDB_SUCCESS;
- }
-
/*
* If the value to match is present in the attribute values of the
* current entry being visited, set matched to true and return OK
diff --git a/lib/ldb/common/ldb_match.c b/lib/ldb/common/ldb_match.c
index 17314f1b9ca..645cb695ef1 100644
--- a/lib/ldb/common/ldb_match.c
+++ b/lib/ldb/common/ldb_match.c
@@ -98,11 +98,6 @@ static int ldb_match_present(struct ldb_context *ldb,
return LDB_SUCCESS;
}
- if (ldb_msg_element_is_inaccessible(el)) {
- *matched = false;
- return LDB_SUCCESS;
- }
-
a = ldb_schema_attribute_by_name(ldb, el->name);
if (!a) {
return LDB_ERR_INVALID_ATTRIBUTE_SYNTAX;
@@ -144,11 +139,6 @@ static int ldb_match_comparison(struct ldb_context *ldb,
return LDB_SUCCESS;
}
- if (ldb_msg_element_is_inaccessible(el)) {
- *matched = false;
- return LDB_SUCCESS;
- }
-
a = ldb_schema_attribute_by_name(ldb, el->name);
if (!a) {
return LDB_ERR_INVALID_ATTRIBUTE_SYNTAX;
@@ -219,11 +209,6 @@ static int ldb_match_equality(struct ldb_context *ldb,
return LDB_SUCCESS;
}
- if (ldb_msg_element_is_inaccessible(el)) {
- *matched = false;
- return LDB_SUCCESS;
- }
-
a = ldb_schema_attribute_by_name(ldb, el->name);
if (a == NULL) {
return LDB_ERR_INVALID_ATTRIBUTE_SYNTAX;
@@ -385,11 +370,6 @@ static int ldb_match_substring(struct ldb_context *ldb,
return LDB_SUCCESS;
}
- if (ldb_msg_element_is_inaccessible(el)) {
- *matched = false;
- return LDB_SUCCESS;
- }
-
for (i = 0; i < el->num_values; i++) {
int ret;
ret = ldb_wildcard_compare(ldb, tree, el->values[i], matched);
@@ -463,11 +443,6 @@ static int ldb_match_bitmask(struct ldb_context *ldb,
return LDB_SUCCESS;
}
- if (ldb_msg_element_is_inaccessible(el)) {
- *matched = false;
- return LDB_SUCCESS;
- }
-
for (i=0;i<el->num_values;i++) {
int ret;
struct ldb_val *v = &el->values[i];
@@ -556,6 +531,26 @@ static int ldb_match_extended(struct ldb_context *ldb,
&tree->u.extended.value, matched);
}
+static bool ldb_must_suppress_match(const struct ldb_message *msg,
+ const struct ldb_parse_tree *tree)
+{
+ const char *attr = NULL;
+ struct ldb_message_element *el = NULL;
+
+ attr = ldb_parse_tree_get_attr(tree);
+ if (attr == NULL) {
+ return false;
+ }
+
+ /* find the message element */
+ el = ldb_msg_find_element(msg, attr);
+ if (el == NULL) {
+ return false;
+ }
+
+ return ldb_msg_element_is_inaccessible(el);
+}
+
/*
Check if a particular message will match the given filter
@@ -580,6 +575,17 @@ int ldb_match_message(struct ldb_context *ldb,
return LDB_SUCCESS;
}
+ /*
+ * Suppress matches on confidential attributes (handled
+ * manually in extended matches as these can do custom things
+ * like read other parts of the DB or other attributes).
+ */
+ if (tree->operation != LDB_OP_EXTENDED) {
+ if (ldb_must_suppress_match(msg, tree)) {
+ return LDB_SUCCESS;
+ }
+ }
+
switch (tree->operation) {
case LDB_OP_AND:
for (i=0;i<tree->u.list.num_elements;i++) {
--
2.25.1

View File

@ -0,0 +1,155 @@
From a0c888bd0ed2ec5b4d84f9df241bebd5d428818c Mon Sep 17 00:00:00 2001
From: Joseph Sutton <josephsutton@catalyst.net.nz>
Date: Fri, 3 Mar 2023 17:35:55 +1300
Subject: [PATCH 28/34] CVE-2023-0614 ldb: Filter on search base before
redacting message
Redaction may be expensive if we end up needing to fetch a security
descriptor to verify rights to an attribute. Checking the search scope
is probably cheaper, so do that first.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15270
Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Conflict: NA
Reference: https://attachments.samba.org/attachment.cgi?id=17821
---
lib/ldb/common/ldb_match.c | 8 +++---
lib/ldb/include/ldb_private.h | 8 ++++++
lib/ldb/ldb_key_value/ldb_kv_index.c | 40 +++++++++++++++------------
lib/ldb/ldb_key_value/ldb_kv_search.c | 14 ++++++++--
4 files changed, 47 insertions(+), 23 deletions(-)
diff --git a/lib/ldb/common/ldb_match.c b/lib/ldb/common/ldb_match.c
index 645cb695ef1..8e27ecbe723 100644
--- a/lib/ldb/common/ldb_match.c
+++ b/lib/ldb/common/ldb_match.c
@@ -38,10 +38,10 @@
/*
check if the scope matches in a search result
*/
-static int ldb_match_scope(struct ldb_context *ldb,
- struct ldb_dn *base,
- struct ldb_dn *dn,
- enum ldb_scope scope)
+int ldb_match_scope(struct ldb_context *ldb,
+ struct ldb_dn *base,
+ struct ldb_dn *dn,
+ enum ldb_scope scope)
{
int ret = 0;
diff --git a/lib/ldb/include/ldb_private.h b/lib/ldb/include/ldb_private.h
index b0a42f6421c..5e29de34f79 100644
--- a/lib/ldb/include/ldb_private.h
+++ b/lib/ldb/include/ldb_private.h
@@ -322,6 +322,14 @@ int ldb_match_message(struct ldb_context *ldb,
const struct ldb_parse_tree *tree,
enum ldb_scope scope, bool *matched);
+/*
+ check if the scope matches in a search result
+*/
+int ldb_match_scope(struct ldb_context *ldb,
+ struct ldb_dn *base,
+ struct ldb_dn *dn,
+ enum ldb_scope scope);
+
/* Reallocate elements to drop any excess capacity. */
void ldb_msg_shrink_to_fit(struct ldb_message *msg);
diff --git a/lib/ldb/ldb_key_value/ldb_kv_index.c b/lib/ldb/ldb_key_value/ldb_kv_index.c
index 163052fecf7..aac0913f431 100644
--- a/lib/ldb/ldb_key_value/ldb_kv_index.c
+++ b/lib/ldb/ldb_key_value/ldb_kv_index.c
@@ -2428,31 +2428,37 @@ static int ldb_kv_index_filter(struct ldb_kv_private *ldb_kv,
return LDB_ERR_OPERATIONS_ERROR;
}
- if (ldb->redact.callback != NULL) {
- ret = ldb->redact.callback(ldb->redact.module, ac->req, msg);
- if (ret != LDB_SUCCESS) {
- talloc_free(msg);
- return ret;
- }
- }
-
/*
* We trust the index for LDB_SCOPE_ONELEVEL
* unless the index key has been truncated.
*
* LDB_SCOPE_BASE is not passed in by our only caller.
*/
- if (ac->scope == LDB_SCOPE_ONELEVEL &&
- ldb_kv->cache->one_level_indexes &&
- scope_one_truncation == KEY_NOT_TRUNCATED) {
- ret = ldb_match_message(ldb, msg, ac->tree,
- ac->scope, &matched);
- } else {
- ret = ldb_match_msg_error(ldb, msg,
- ac->tree, ac->base,
- ac->scope, &matched);
+ if (ac->scope != LDB_SCOPE_ONELEVEL ||
+ !ldb_kv->cache->one_level_indexes ||
+ scope_one_truncation != KEY_NOT_TRUNCATED)
+ {
+ /*
+ * The redaction callback may be expensive to call if it
+ * fetches a security descriptor. Check the DN early and
+ * bail out if it doesn't match the base.
+ */
+ if (!ldb_match_scope(ldb, ac->base, msg->dn, ac->scope)) {
+ talloc_free(msg);
+ continue;
+ }
+ }
+
+ if (ldb->redact.callback != NULL) {
+ ret = ldb->redact.callback(ldb->redact.module, ac->req, msg);
+ if (ret != LDB_SUCCESS) {
+ talloc_free(msg);
+ return ret;
+ }
}
+ ret = ldb_match_message(ldb, msg, ac->tree,
+ ac->scope, &matched);
if (ret != LDB_SUCCESS) {
talloc_free(keys);
talloc_free(msg);
diff --git a/lib/ldb/ldb_key_value/ldb_kv_search.c b/lib/ldb/ldb_key_value/ldb_kv_search.c
index d187ba223e1..27f68caef01 100644
--- a/lib/ldb/ldb_key_value/ldb_kv_search.c
+++ b/lib/ldb/ldb_key_value/ldb_kv_search.c
@@ -395,6 +395,16 @@ static int search_func(_UNUSED_ struct ldb_kv_private *ldb_kv,
}
}
+ /*
+ * The redaction callback may be expensive to call if it fetches a
+ * security descriptor. Check the DN early and bail out if it doesn't
+ * match the base.
+ */
+ if (!ldb_match_scope(ldb, ac->base, msg->dn, ac->scope)) {
+ talloc_free(msg);
+ return 0;
+ }
+
if (ldb->redact.callback != NULL) {
ret = ldb->redact.callback(ldb->redact.module, ac->req, msg);
if (ret != LDB_SUCCESS) {
@@ -404,8 +414,8 @@ static int search_func(_UNUSED_ struct ldb_kv_private *ldb_kv,
}
/* see if it matches the given expression */
- ret = ldb_match_msg_error(ldb, msg,
- ac->tree, ac->base, ac->scope, &matched);
+ ret = ldb_match_message(ldb, msg,
+ ac->tree, ac->scope, &matched);
if (ret != LDB_SUCCESS) {
talloc_free(msg);
ac->error = LDB_ERR_OPERATIONS_ERROR;
--
2.25.1

View File

@ -0,0 +1,108 @@
From 62d100b64a25c740187f687dd058a543d43984ec Mon Sep 17 00:00:00 2001
From: Joseph Sutton <josephsutton@catalyst.net.nz>
Date: Fri, 24 Feb 2023 10:03:25 +1300
Subject: [PATCH 29/34] CVE-2023-0614 s4-dsdb: Treat confidential attributes as
unindexed
In the unlikely case that someone adds a confidential indexed attribute
to the schema, LDAP search expressions on that attribute could disclose
information via timing differences. Let's not use the index for searches
on confidential attributes.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15270
Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Conflict: NA
Reference: https://attachments.samba.org/attachment.cgi?id=17821
---
source4/dsdb/samdb/ldb_modules/extended_dn_in.c | 10 +++++++++-
source4/dsdb/schema/schema_description.c | 7 +++++++
source4/dsdb/schema/schema_init.c | 11 +++++++++--
source4/dsdb/schema/schema_set.c | 9 ++++++++-
4 files changed, 33 insertions(+), 4 deletions(-)
diff --git a/source4/dsdb/samdb/ldb_modules/extended_dn_in.c b/source4/dsdb/samdb/ldb_modules/extended_dn_in.c
index 1dc1e1f2d42..248bb66f039 100644
--- a/source4/dsdb/samdb/ldb_modules/extended_dn_in.c
+++ b/source4/dsdb/samdb/ldb_modules/extended_dn_in.c
@@ -423,7 +423,15 @@ static int extended_dn_filter_callback(struct ldb_parse_tree *tree, void *privat
guid_val = ldb_dn_get_extended_component(dn, "GUID");
sid_val = ldb_dn_get_extended_component(dn, "SID");
- if (!guid_val && !sid_val && (attribute->searchFlags & SEARCH_FLAG_ATTINDEX)) {
+ /*
+ * Is the attribute indexed? By treating confidential attributes
+ * as unindexed, we force searches to go through the unindexed
+ * search path, avoiding observable timing differences.
+ */
+ if (!guid_val && !sid_val &&
+ (attribute->searchFlags & SEARCH_FLAG_ATTINDEX) &&
+ !(attribute->searchFlags & SEARCH_FLAG_CONFIDENTIAL))
+ {
/* if it is indexed, then fixing the string DN will do
no good here, as we will not find the attribute in
the index. So for now fall through to a standard DN
diff --git a/source4/dsdb/schema/schema_description.c b/source4/dsdb/schema/schema_description.c
index 243a02a15f3..5fc70154bf8 100644
--- a/source4/dsdb/schema/schema_description.c
+++ b/source4/dsdb/schema/schema_description.c
@@ -160,6 +160,13 @@ char *schema_attribute_to_extendedInfo(TALLOC_CTX *mem_ctx, const struct dsdb_at
attribute->rangeUpper,
GUID_hexstring(tmp_ctx, &attribute->schemaIDGUID),
GUID_hexstring(tmp_ctx, &attribute->attributeSecurityGUID),
+ /*
+ * We actually ignore the indexed
+ * flag for confidential
+ * attributes, but we'll include
+ * it for the purposes of
+ * description.
+ */
(attribute->searchFlags & SEARCH_FLAG_ATTINDEX),
attribute->systemOnly);
talloc_free(tmp_ctx);
diff --git a/source4/dsdb/schema/schema_init.c b/source4/dsdb/schema/schema_init.c
index a3b00497b6b..c8197b86306 100644
--- a/source4/dsdb/schema/schema_init.c
+++ b/source4/dsdb/schema/schema_init.c
@@ -514,8 +514,15 @@ static int dsdb_schema_setup_ldb_schema_attribute(struct ldb_context *ldb,
if (attr->isSingleValued) {
a->flags |= LDB_ATTR_FLAG_SINGLE_VALUE;
}
-
- if (attr->searchFlags & SEARCH_FLAG_ATTINDEX) {
+
+ /*
+ * Is the attribute indexed? By treating confidential attributes as
+ * unindexed, we force searches to go through the unindexed search path,
+ * avoiding observable timing differences.
+ */
+ if (attr->searchFlags & SEARCH_FLAG_ATTINDEX &&
+ !(attr->searchFlags & SEARCH_FLAG_CONFIDENTIAL))
+ {
a->flags |= LDB_ATTR_FLAG_INDEXED;
}
diff --git a/source4/dsdb/schema/schema_set.c b/source4/dsdb/schema/schema_set.c
index 45faa0912ec..03cf2405595 100644
--- a/source4/dsdb/schema/schema_set.c
+++ b/source4/dsdb/schema/schema_set.c
@@ -221,7 +221,14 @@ int dsdb_schema_set_indices_and_attributes(struct ldb_context *ldb,
break;
}
- if (attr->searchFlags & SEARCH_FLAG_ATTINDEX) {
+ /*
+ * Is the attribute indexed? By treating confidential attributes
+ * as unindexed, we force searches to go through the unindexed
+ * search path, avoiding observable timing differences.
+ */
+ if (attr->searchFlags & SEARCH_FLAG_ATTINDEX &&
+ !(attr->searchFlags & SEARCH_FLAG_CONFIDENTIAL))
+ {
/*
* When preparing to downgrade Samba, we need to write
* out an LDB without the new key word ORDERED_INTEGER.
--
2.25.1

View File

@ -0,0 +1,49 @@
From d857b406e44254b37aa91d0c8d226e417ce123ce Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet@samba.org>
Date: Thu, 2 Mar 2023 16:31:17 +1300
Subject: [PATCH 30/34] CVE-2023-0614 dsdb: Add DSDB_MARK_REQ_UNTRUSTED
This will allow our dsdb helper search functions to mark the new
request as untrusted, forcing read ACL evaluation (per current behaviour).
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15270
Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Conflict: NA
Reference: https://attachments.samba.org/attachment.cgi?id=17821
---
source4/dsdb/common/util.c | 4 ++++
source4/dsdb/common/util.h | 1 +
2 files changed, 5 insertions(+)
diff --git a/source4/dsdb/common/util.c b/source4/dsdb/common/util.c
index b556f06cb63..39b29cd2a0c 100644
--- a/source4/dsdb/common/util.c
+++ b/source4/dsdb/common/util.c
@@ -4878,6 +4878,10 @@ int dsdb_request_add_controls(struct ldb_request *req, uint32_t dsdb_flags)
}
}
+ if (dsdb_flags & DSDB_MARK_REQ_UNTRUSTED) {
+ ldb_req_mark_untrusted(req);
+ }
+
return LDB_SUCCESS;
}
diff --git a/source4/dsdb/common/util.h b/source4/dsdb/common/util.h
index e1854644d53..5bb96d60b3c 100644
--- a/source4/dsdb/common/util.h
+++ b/source4/dsdb/common/util.h
@@ -43,6 +43,7 @@
#define DSDB_MODIFY_PARTIAL_REPLICA 0x04000
#define DSDB_PASSWORD_BYPASS_LAST_SET 0x08000
#define DSDB_REPLMD_VANISH_LINKS 0x10000
+#define DSDB_MARK_REQ_UNTRUSTED 0x20000
bool is_attr_in_list(const char * const * attrs, const char *attr);
--
2.25.1

View File

@ -0,0 +1,43 @@
From 0151a2a4c4cea64b0d931aa0e227b187abf28280 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet@samba.org>
Date: Fri, 3 Mar 2023 16:49:00 +1300
Subject: [PATCH 31/34] CVE-2023-0614 dsdb: Add pre-cleanup and
self.addCleanup() of OU created in match_rules tests
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15270
Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Conflict: NA
Reference: https://attachments.samba.org/attachment.cgi?id=17821
---
lib/ldb-samba/tests/match_rules.py | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/lib/ldb-samba/tests/match_rules.py b/lib/ldb-samba/tests/match_rules.py
index abf485c9eab..2af1dd6a070 100755
--- a/lib/ldb-samba/tests/match_rules.py
+++ b/lib/ldb-samba/tests/match_rules.py
@@ -31,11 +31,19 @@ class MatchRulesTests(samba.tests.TestCase):
self.ou_groups = "OU=groups,%s" % self.ou
self.ou_computers = "OU=computers,%s" % self.ou
+ try:
+ self.ldb.delete(self.ou, ["tree_delete:1"])
+ except LdbError as e:
+ pass
+
# Add a organizational unit to create objects
self.ldb.add({
"dn": self.ou,
"objectclass": "organizationalUnit"})
+ self.addCleanup(self.ldb.delete, self.ou, controls=['tree_delete:0'])
+
+
# Add the following OU hierarchy and set otherWellKnownObjects,
# which has BinaryDN syntax:
#
--
2.25.1

View File

@ -0,0 +1,319 @@
From 91ecb27e706ac4500fb0d5242cf412173f8a5e7a Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet@samba.org>
Date: Thu, 2 Mar 2023 16:51:25 +1300
Subject: [PATCH 32/34] CVE-2023-0614 lib/ldb-samba: Add test for
SAMBA_LDAP_MATCH_RULE_TRANSITIVE_EVAL / LDAP_MATCHING_RULE_IN_CHAIN with and
ACL hidden attributes
The chain for transitive evaluation does consider ACLs, avoiding the disclosure of
confidential information.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15270
Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Conflict: NA
Reference: https://attachments.samba.org/attachment.cgi?id=17821
---
lib/ldb-samba/tests/match_rules.py | 127 ++++++++++++----------
lib/ldb-samba/tests/match_rules_remote.py | 104 ++++++++++++++++++
source4/selftest/tests.py | 1 +
3 files changed, 175 insertions(+), 57 deletions(-)
create mode 100755 lib/ldb-samba/tests/match_rules_remote.py
diff --git a/lib/ldb-samba/tests/match_rules.py b/lib/ldb-samba/tests/match_rules.py
index 2af1dd6a070..2fe6c3e2264 100755
--- a/lib/ldb-samba/tests/match_rules.py
+++ b/lib/ldb-samba/tests/match_rules.py
@@ -20,13 +20,18 @@ from ldb import SCOPE_BASE, SCOPE_SUBTREE, SCOPE_ONELEVEL
# Windows appear to preserve casing of the RDN and uppercase the other keys.
-class MatchRulesTests(samba.tests.TestCase):
+class MatchRulesTestsBase(samba.tests.TestCase):
def setUp(self):
- super(MatchRulesTests, self).setUp()
- self.lp = lp
- self.ldb = SamDB(host, credentials=creds, session_info=system_session(lp), lp=lp)
+ super().setUp()
+ self.lp = self.sambaopts.get_loadparm()
+ self.creds = self.credopts.get_credentials(self.lp)
+
+ self.ldb = SamDB(self.host, credentials=self.creds,
+ session_info=system_session(self.lp),
+ lp=self.lp)
self.base_dn = self.ldb.domain_dn()
- self.ou = "OU=matchrulestest,%s" % self.base_dn
+ self.ou_rdn = "OU=matchrulestest"
+ self.ou = self.ou_rdn + "," + self.base_dn
self.ou_users = "OU=users,%s" % self.ou
self.ou_groups = "OU=groups,%s" % self.ou
self.ou_computers = "OU=computers,%s" % self.ou
@@ -212,6 +217,39 @@ class MatchRulesTests(samba.tests.TestCase):
FLAG_MOD_ADD, "member")
self.ldb.modify(m)
+ # Add a couple of ms-Exch-Configuration-Container to test forward-link
+ # attributes without backward link (addressBookRoots2)
+ # e1
+ # |--> e2
+ # | |--> c1
+ self.ldb.add({
+ "dn": "cn=e1,%s" % self.ou,
+ "objectclass": "msExchConfigurationContainer"})
+ self.ldb.add({
+ "dn": "cn=e2,%s" % self.ou,
+ "objectclass": "msExchConfigurationContainer"})
+
+ m = Message()
+ m.dn = Dn(self.ldb, "cn=e2,%s" % self.ou)
+ m["e1"] = MessageElement("cn=c1,%s" % self.ou_computers,
+ FLAG_MOD_ADD, "addressBookRoots2")
+ self.ldb.modify(m)
+
+ m = Message()
+ m.dn = Dn(self.ldb, "cn=e1,%s" % self.ou)
+ m["e1"] = MessageElement("cn=e2,%s" % self.ou,
+ FLAG_MOD_ADD, "addressBookRoots2")
+ self.ldb.modify(m)
+
+
+
+class MatchRulesTests(MatchRulesTestsBase):
+ def setUp(self):
+ self.sambaopts = sambaopts
+ self.credopts = credopts
+ self.host = host
+ super().setUp()
+
# The msDS-RevealedUsers is owned by system and cannot be modified
# directly. Set the schemaUpgradeInProgress flag as workaround
# and create this hierarchy:
@@ -251,33 +289,6 @@ class MatchRulesTests(samba.tests.TestCase):
m["e1"] = MessageElement("0", FLAG_MOD_REPLACE, "schemaUpgradeInProgress")
self.ldb.modify(m)
- # Add a couple of ms-Exch-Configuration-Container to test forward-link
- # attributes without backward link (addressBookRoots2)
- # e1
- # |--> e2
- # | |--> c1
- self.ldb.add({
- "dn": "cn=e1,%s" % self.ou,
- "objectclass": "msExchConfigurationContainer"})
- self.ldb.add({
- "dn": "cn=e2,%s" % self.ou,
- "objectclass": "msExchConfigurationContainer"})
-
- m = Message()
- m.dn = Dn(self.ldb, "cn=e2,%s" % self.ou)
- m["e1"] = MessageElement("cn=c1,%s" % self.ou_computers,
- FLAG_MOD_ADD, "addressBookRoots2")
- self.ldb.modify(m)
-
- m = Message()
- m.dn = Dn(self.ldb, "cn=e1,%s" % self.ou)
- m["e1"] = MessageElement("cn=e2,%s" % self.ou,
- FLAG_MOD_ADD, "addressBookRoots2")
- self.ldb.modify(m)
-
- def tearDown(self):
- super(MatchRulesTests, self).tearDown()
- self.ldb.delete(self.ou, controls=['tree_delete:0'])
def test_u1_member_of_g4(self):
# Search without transitive match must return 0 results
@@ -953,8 +964,12 @@ class MatchRulesTests(samba.tests.TestCase):
class MatchRuleConditionTests(samba.tests.TestCase):
def setUp(self):
super(MatchRuleConditionTests, self).setUp()
- self.lp = lp
- self.ldb = SamDB(host, credentials=creds, session_info=system_session(lp), lp=lp)
+ self.lp = sambaopts.get_loadparm()
+ self.creds = credopts.get_credentials(self.lp)
+
+ self.ldb = SamDB(host, credentials=self.creds,
+ session_info=system_session(self.lp),
+ lp=self.lp)
self.base_dn = self.ldb.domain_dn()
self.ou = "OU=matchruleconditiontests,%s" % self.base_dn
self.ou_users = "OU=users,%s" % self.ou
@@ -1753,32 +1768,30 @@ class MatchRuleConditionTests(samba.tests.TestCase):
self.ou_groups, self.ou_computers))
self.assertEqual(len(res1), 0)
+if __name__ == "__main__":
-parser = optparse.OptionParser("match_rules.py [options] <host>")
-sambaopts = options.SambaOptions(parser)
-parser.add_option_group(sambaopts)
-parser.add_option_group(options.VersionOptions(parser))
-
-# use command line creds if available
-credopts = options.CredentialsOptions(parser)
-parser.add_option_group(credopts)
-opts, args = parser.parse_args()
-subunitopts = SubunitOptions(parser)
-parser.add_option_group(subunitopts)
+ parser = optparse.OptionParser("match_rules.py [options] <host>")
+ sambaopts = options.SambaOptions(parser)
+ parser.add_option_group(sambaopts)
+ parser.add_option_group(options.VersionOptions(parser))
-if len(args) < 1:
- parser.print_usage()
- sys.exit(1)
+ # use command line creds if available
+ credopts = options.CredentialsOptions(parser)
+ parser.add_option_group(credopts)
+ opts, args = parser.parse_args()
+ subunitopts = SubunitOptions(parser)
+ parser.add_option_group(subunitopts)
-host = args[0]
+ if len(args) < 1:
+ parser.print_usage()
+ sys.exit(1)
-lp = sambaopts.get_loadparm()
-creds = credopts.get_credentials(lp)
+ host = args[0]
-if "://" not in host:
- if os.path.isfile(host):
- host = "tdb://%s" % host
- else:
- host = "ldap://%s" % host
+ if "://" not in host:
+ if os.path.isfile(host):
+ host = "tdb://%s" % host
+ else:
+ host = "ldap://%s" % host
-TestProgram(module=__name__, opts=subunitopts)
+ TestProgram(module=__name__, opts=subunitopts)
diff --git a/lib/ldb-samba/tests/match_rules_remote.py b/lib/ldb-samba/tests/match_rules_remote.py
new file mode 100755
index 00000000000..122231f2a60
--- /dev/null
+++ b/lib/ldb-samba/tests/match_rules_remote.py
@@ -0,0 +1,104 @@
+#!/usr/bin/env python3
+
+import optparse
+import sys
+import os
+import samba
+import samba.getopt as options
+
+from samba.tests.subunitrun import SubunitOptions, TestProgram
+
+from samba.samdb import SamDB
+from samba.auth import system_session
+from samba import sd_utils
+from samba.ndr import ndr_unpack
+from ldb import Message, MessageElement, Dn, LdbError
+from ldb import FLAG_MOD_ADD, FLAG_MOD_REPLACE, FLAG_MOD_DELETE
+from ldb import SCOPE_BASE, SCOPE_SUBTREE, SCOPE_ONELEVEL
+
+from match_rules import MatchRulesTestsBase
+
+
+class MatchRulesTestsUser(MatchRulesTestsBase):
+ def setUp(self):
+ self.sambaopts = sambaopts
+ self.credopts = credopts
+ self.host = host
+ super().setUp()
+ self.sd_utils = sd_utils.SDUtils(self.ldb)
+
+ self.user_pass = "samba123@"
+ self.match_test_user = "matchtestuser"
+ self.ldb.newuser(self.match_test_user,
+ self.user_pass,
+ userou=self.ou_rdn)
+ user_creds = self.insta_creds(template=self.creds,
+ username=self.match_test_user,
+ userpass=self.user_pass)
+ self.user_ldb = SamDB(host, credentials=user_creds, lp=self.lp)
+ token_res = self.user_ldb.search(scope=SCOPE_BASE,
+ base="",
+ attrs=["tokenGroups"])
+ self.user_sid = ndr_unpack(samba.dcerpc.security.dom_sid,
+ token_res[0]["tokenGroups"][0])
+
+ self.member_attr_guid = "bf9679c0-0de6-11d0-a285-00aa003049e2"
+
+ def test_with_denied_link(self):
+
+ # add an ACE that denies the user Read Property (RP) access to
+ # the member attr (which is similar to making the attribute
+ # confidential)
+ ace = "(OD;;RP;{0};;{1})".format(self.member_attr_guid,
+ self.user_sid)
+ g2_dn = Dn(self.ldb, "CN=g2,%s" % self.ou_groups)
+
+ # add the ACE that denies access to the attr under test
+ self.sd_utils.dacl_add_ace(g2_dn, ace)
+
+ # Search without transitive match must return 0 results
+ res1 = self.ldb.search("cn=g4,%s" % self.ou_groups,
+ scope=SCOPE_BASE,
+ expression="member=cn=u1,%s" % self.ou_users)
+ self.assertEqual(len(res1), 0)
+
+ # Search with transitive match must return 1 results
+ res1 = self.ldb.search("cn=g4,%s" % self.ou_groups,
+ scope=SCOPE_BASE,
+ expression="member:1.2.840.113556.1.4.1941:=cn=u1,%s" % self.ou_users)
+ self.assertEqual(len(res1), 1)
+ self.assertEqual(str(res1[0].dn).lower(), ("CN=g4,%s" % self.ou_groups).lower())
+
+ # Search as a user match must return 0 results as the intermediate link can't be seen
+ res1 = self.user_ldb.search("cn=g4,%s" % self.ou_groups,
+ scope=SCOPE_BASE,
+ expression="member:1.2.840.113556.1.4.1941:=cn=u1,%s" % self.ou_users)
+ self.assertEqual(len(res1), 0)
+
+
+
+parser = optparse.OptionParser("match_rules_remote.py [options] <host>")
+sambaopts = options.SambaOptions(parser)
+parser.add_option_group(sambaopts)
+parser.add_option_group(options.VersionOptions(parser))
+
+# use command line creds if available
+credopts = options.CredentialsOptions(parser)
+parser.add_option_group(credopts)
+opts, args = parser.parse_args()
+subunitopts = SubunitOptions(parser)
+parser.add_option_group(subunitopts)
+
+if len(args) < 1:
+ parser.print_usage()
+ sys.exit(1)
+
+host = args[0]
+
+if "://" not in host:
+ if os.path.isfile(host):
+ host = "tdb://%s" % host
+ else:
+ host = "ldap://%s" % host
+
+TestProgram(module=__name__, opts=subunitopts)
diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py
index c6bf668aa9c..a09945f40da 100755
--- a/source4/selftest/tests.py
+++ b/source4/selftest/tests.py
@@ -1322,6 +1322,7 @@ for env in ['ad_dc_default:local', 'schema_dc:local']:
plantestsuite_loadlist("samba4.urgent_replication.python(ad_dc_ntvfs)", "ad_dc_ntvfs:local", [python, os.path.join(DSDB_PYTEST_DIR, "urgent_replication.py"), '$PREFIX_ABS/ad_dc_ntvfs/private/sam.ldb', '$LOADLIST', '$LISTOPT'])
plantestsuite_loadlist("samba4.ldap.dirsync.python(ad_dc_ntvfs)", "ad_dc_ntvfs", [python, os.path.join(DSDB_PYTEST_DIR, "dirsync.py"), '$SERVER', '-U"$USERNAME%$PASSWORD"', '--workgroup=$DOMAIN', '$LOADLIST', '$LISTOPT'])
plantestsuite_loadlist("samba4.ldap.match_rules.python", "ad_dc_ntvfs", [python, os.path.join(srcdir(), "lib/ldb-samba/tests/match_rules.py"), '$PREFIX_ABS/ad_dc_ntvfs/private/sam.ldb', '-U"$USERNAME%$PASSWORD"', '--workgroup=$DOMAIN', '$LOADLIST', '$LISTOPT'])
+plantestsuite_loadlist("samba4.ldap.match_rules.python", "ad_dc_ntvfs", [python, os.path.join(srcdir(), "lib/ldb-samba/tests/match_rules_remote.py"), '$SERVER', '-U"$USERNAME%$PASSWORD"', '--workgroup=$DOMAIN', '$LOADLIST', '$LISTOPT'])
plantestsuite("samba4.ldap.index.python", "none", [python, os.path.join(srcdir(), "lib/ldb-samba/tests/index.py")])
plantestsuite_loadlist("samba4.ldap.notification.python(ad_dc_ntvfs)", "ad_dc_ntvfs", [python, os.path.join(DSDB_PYTEST_DIR, "notification.py"), '$SERVER', '-U"$USERNAME%$PASSWORD"', '--workgroup=$DOMAIN', '$LOADLIST', '$LISTOPT'])
plantestsuite_loadlist("samba4.ldap.sites.python(ad_dc_default)", "ad_dc_default", [python, os.path.join(DSDB_PYTEST_DIR, "sites.py"), '$SERVER', '-U"$USERNAME%$PASSWORD"', '--workgroup=$DOMAIN', '$LOADLIST', '$LISTOPT'])
--
2.25.1

View File

@ -0,0 +1,39 @@
From bb092fc576868e30edf78136894472f95c4b039d Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet@samba.org>
Date: Thu, 2 Mar 2023 17:24:15 +1300
Subject: [PATCH 33/34] CVE-2023-0614 lib/ldb-samba Ensure ACLs are evaluated
on SAMBA_LDAP_MATCH_RULE_TRANSITIVE_EVAL / LDAP_MATCHING_RULE_IN_CHAIN
Setting the LDB_HANDLE_FLAG_UNTRUSTED tells the acl_read module to operate on this request.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15270
Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Conflict: NA
Reference: https://attachments.samba.org/attachment.cgi?id=17821
---
lib/ldb-samba/ldb_matching_rules.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/lib/ldb-samba/ldb_matching_rules.c b/lib/ldb-samba/ldb_matching_rules.c
index b86594c1823..59d1385f4e3 100644
--- a/lib/ldb-samba/ldb_matching_rules.c
+++ b/lib/ldb-samba/ldb_matching_rules.c
@@ -67,7 +67,12 @@ static int ldb_eval_transitive_filter_helper(TALLOC_CTX *mem_ctx,
* Note also that we don't have the original request
* here, so we can not apply controls or timeouts here.
*/
- ret = dsdb_search_dn(ldb, tmp_ctx, &res, to_visit->dn, attrs, 0);
+ ret = dsdb_search_dn(ldb,
+ tmp_ctx,
+ &res,
+ to_visit->dn,
+ attrs,
+ DSDB_MARK_REQ_UNTRUSTED);
if (ret != LDB_SUCCESS) {
talloc_free(tmp_ctx);
return ret;
--
2.25.1

View File

@ -0,0 +1,108 @@
From faa7babcd8db9ef14398848d0c398578d1a79d85 Mon Sep 17 00:00:00 2001
From: Rob van der Linde <rob@catalyst.net.nz>
Date: Mon, 27 Feb 2023 14:06:23 +1300
Subject: [PATCH] CVE-2023-0922 set default ldap client sasl wrapping to seal
This avoids sending new or reset passwords in the clear
(integrity protected only) from samba-tool in particular.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15315
Signed-off-by: Rob van der Linde <rob@catalyst.net.nz>
Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Conflict: NA
Reference: https://attachments.samba.org/attachment.cgi?id=17831
---
.../ldap/clientldapsaslwrapping.xml | 27 +++++++++----------
lib/param/loadparm.c | 2 +-
python/samba/tests/auth_log.py | 2 +-
source3/param/loadparm.c | 2 +-
4 files changed, 16 insertions(+), 17 deletions(-)
diff --git a/docs-xml/smbdotconf/ldap/clientldapsaslwrapping.xml b/docs-xml/smbdotconf/ldap/clientldapsaslwrapping.xml
index 3152f0682dd..21bd2090057 100644
--- a/docs-xml/smbdotconf/ldap/clientldapsaslwrapping.xml
+++ b/docs-xml/smbdotconf/ldap/clientldapsaslwrapping.xml
@@ -18,25 +18,24 @@
</para>
<para>
- This option is needed in the case of Domain Controllers enforcing
- the usage of signed LDAP connections (e.g. Windows 2000 SP3 or higher).
- LDAP sign and seal can be controlled with the registry key
- "<literal>HKLM\System\CurrentControlSet\Services\</literal>
- <literal>NTDS\Parameters\LDAPServerIntegrity</literal>"
- on the Windows server side.
- </para>
+ This option is needed firstly to secure the privacy of
+ administrative connections from <command>samba-tool</command>,
+ including in particular new or reset passwords for users. For
+ this reason the default is <emphasis>seal</emphasis>.</para>
- <para>
- Depending on the used KRB5 library (MIT and older Heimdal versions)
- it is possible that the message "integrity only" is not supported.
- In this case, <emphasis>sign</emphasis> is just an alias for
- <emphasis>seal</emphasis>.
+ <para>Additionally, <command>winbindd</command> and the
+ <command>net</command> tool can use LDAP to communicate with
+ Domain Controllers, so this option also controls the level of
+ privacy for those connections. All supported AD DC versions
+ will enforce the usage of at least signed LDAP connections by
+ default, so a value of at least <emphasis>sign</emphasis> is
+ required in practice.
</para>
<para>
- The default value is <emphasis>sign</emphasis>. That implies synchronizing the time
+ The default value is <emphasis>seal</emphasis>. That implies synchronizing the time
with the KDC in the case of using <emphasis>Kerberos</emphasis>.
</para>
</description>
-<value type="default">sign</value>
+<value type="default">seal</value>
</samba:parameter>
diff --git a/lib/param/loadparm.c b/lib/param/loadparm.c
index 6ab7fa89db7..16cb0d47f31 100644
--- a/lib/param/loadparm.c
+++ b/lib/param/loadparm.c
@@ -2990,7 +2990,7 @@ struct loadparm_context *loadparm_init(TALLOC_CTX *mem_ctx)
lpcfg_do_global_parameter(lp_ctx, "ldap debug threshold", "10");
- lpcfg_do_global_parameter(lp_ctx, "client ldap sasl wrapping", "sign");
+ lpcfg_do_global_parameter(lp_ctx, "client ldap sasl wrapping", "seal");
lpcfg_do_global_parameter(lp_ctx, "mdns name", "netbios");
diff --git a/python/samba/tests/auth_log.py b/python/samba/tests/auth_log.py
index d166b93d90a..8f9f487f82a 100644
--- a/python/samba/tests/auth_log.py
+++ b/python/samba/tests/auth_log.py
@@ -470,7 +470,7 @@ class AuthLogTests(samba.tests.auth_log_base.AuthLogTestBase):
def isLastExpectedMessage(msg):
return (msg["type"] == "Authorization" and
msg["Authorization"]["serviceDescription"] == "LDAP" and
- msg["Authorization"]["transportProtection"] == "SIGN" and
+ msg["Authorization"]["transportProtection"] == "SEAL" and
msg["Authorization"]["authType"] == "krb5")
self.samdb = SamDB(url="ldap://%s" % os.environ["SERVER"],
diff --git a/source3/param/loadparm.c b/source3/param/loadparm.c
index 05a5ae20abe..12718ced9e7 100644
--- a/source3/param/loadparm.c
+++ b/source3/param/loadparm.c
@@ -756,7 +756,7 @@ static void init_globals(struct loadparm_context *lp_ctx, bool reinit_globals)
Globals.ldap_debug_level = 0;
Globals.ldap_debug_threshold = 10;
- Globals.client_ldap_sasl_wrapping = ADS_AUTH_SASL_SIGN;
+ Globals.client_ldap_sasl_wrapping = ADS_AUTH_SASL_SEAL;
Globals.ldap_server_require_strong_auth =
LDAP_SERVER_REQUIRE_STRONG_AUTH_YES;
--
2.25.1

View File

@ -5,7 +5,7 @@
%global talloc_version 2.3.4
%global tdb_version 1.4.7
%global tevent_version 0.13.0
%global ldb_version 2.6.1
%global ldb_version 2.6.1-2
%undefine _strict_symbol_defs_build
@ -48,7 +48,7 @@
Name: samba
Version: 4.17.5
Release: 3
Release: 4
Summary: A suite for Linux to interoperate with Windows
License: GPLv3+ and LGPLv3+
@ -66,6 +66,48 @@ Source8: usershares.conf.vendor
Source201: README.downgrade
Patch0001: backport-0001-CVE-2023-0614.patch
Patch0002: backport-0002-CVE-2023-0614.patch
Patch0003: backport-0003-CVE-2023-0614.patch
Patch0004: backport-0004-CVE-2023-0614.patch
Patch0005: backport-0005-CVE-2023-0614.patch
Patch0006: backport-0006-CVE-2023-0614.patch
Patch0007: backport-0007-CVE-2023-0614.patch
Patch0008: backport-0008-CVE-2023-0614.patch
Patch0009: backport-0009-CVE-2023-0614.patch
Patch0010: backport-0010-CVE-2023-0614.patch
Patch0011: backport-0011-CVE-2023-0614.patch
Patch0012: backport-0012-CVE-2023-0614.patch
Patch0013: backport-0013-CVE-2023-0614.patch
Patch0014: backport-0014-CVE-2023-0614.patch
Patch0015: backport-0015-CVE-2023-0614.patch
Patch0016: backport-0016-CVE-2023-0614.patch
Patch0017: backport-0017-CVE-2023-0614.patch
Patch0018: backport-0018-CVE-2023-0614.patch
Patch0019: backport-0019-CVE-2023-0614.patch
Patch0020: backport-0020-CVE-2023-0614.patch
Patch0021: backport-0021-CVE-2023-0614.patch
Patch0022: backport-0022-CVE-2023-0614.patch
Patch0023: backport-0023-CVE-2023-0614.patch
Patch0024: backport-0024-CVE-2023-0614.patch
Patch0025: backport-0025-CVE-2023-0614.patch
Patch0026: backport-0026-CVE-2023-0614.patch
Patch0027: backport-0027-CVE-2023-0614.patch
Patch0028: backport-0028-CVE-2023-0614.patch
Patch0029: backport-0029-CVE-2023-0614.patch
Patch0030: backport-0030-CVE-2023-0614.patch
Patch0031: backport-0031-CVE-2023-0614.patch
Patch0032: backport-0032-CVE-2023-0614.patch
Patch0033: backport-0033-CVE-2023-0614.patch
Patch0034: backport-0034-CVE-2023-0614.patch
Patch0035: backport-0035-CVE-2023-0614.patch
Patch0036: backport-0001-CVE-2023-0225.patch
Patch0037: backport-0002-CVE-2023-0225.patch
Patch0038: backport-0003-CVE-2023-0225.patch
Patch0039: backport-0004-CVE-2023-0225.patch
Patch0040: backport-CVE-2023-0922.patch
BuildRequires: avahi-devel bison dbus-devel docbook-style-xsl e2fsprogs-devel flex gawk gnupg2 gnutls-devel >= 3.4.7 gpgme-devel
BuildRequires: jansson-devel krb5-devel >= %{required_mit_krb5} libacl-devel libaio-devel libarchive-devel libattr-devel
BuildRequires: libcap-devel libicu-devel libcmocka-devel libtirpc-devel libuuid-devel libxslt lmdb ncurses-devel openldap-devel
@ -3481,6 +3523,12 @@ fi
%endif
%changelog
* Sat Apr 01 2023 xinghe <xinghe2@h-partners.com> - 4.17.5-4
- Type:cves
- ID:CVE-2023-0225 CVE-2023-0614 CVE-2023-0922
- SUG:NA
- DESC:fix CVE-2023-0225 CVE-2023-0614 CVE-2023-0922
* Tue Mar 7 2023 doupengda <doupengda@loongson.cn> - 4.17.5-3
- Type:bugfix
- ID:NA