232 lines
7.4 KiB
Diff
232 lines
7.4 KiB
Diff
From f87af853e2db99269acea572dcc109bc6f797aa9 Mon Sep 17 00:00:00 2001
|
|
From: Joseph Sutton <josephsutton@catalyst.net.nz>
|
|
Date: Fri, 3 Mar 2023 17:34:29 +1300
|
|
Subject: [PATCH 24/34] CVE-2023-0614 ldb: Prevent disclosure of confidential
|
|
attributes
|
|
|
|
Add a hook, acl_redact_msg_for_filter(), in the aclread module, that
|
|
marks inaccessible any message elements used by an LDAP search filter
|
|
that the user has no right to access. Make the various ldb_match_*()
|
|
functions check whether message elements are accessible, and refuse to
|
|
match any that are not. Remaining message elements, not mentioned in the
|
|
search filter, are checked in aclread_callback(), and any inaccessible
|
|
elements are removed at this point.
|
|
|
|
Certain attributes, namely objectClass, distinguishedName, name, and
|
|
objectGUID, are always present, and hence the presence of said
|
|
attributes is always allowed to be checked in a search filter. This
|
|
corresponds with the behaviour of Windows.
|
|
|
|
Further, we unconditionally allow the attributes isDeleted and
|
|
isRecycled in a check for presence or equality. Windows is not known to
|
|
make this special exception, but it seems mostly harmless, and should
|
|
mitigate the performance impact on searches made by the show_deleted
|
|
module.
|
|
|
|
As a result of all these changes, our behaviour regarding confidential
|
|
attributes happens to match Windows more closely. For the test in
|
|
confidential_attr.py, we can now model our attribute handling with
|
|
DC_MODE_RETURN_ALL, which corresponds to the behaviour exhibited by
|
|
Windows.
|
|
|
|
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15270
|
|
|
|
Pair-Programmed-With: Andrew Bartlett <abartlet@samba.org>
|
|
|
|
Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
|
|
Signed-off-by: Andrew Bartlett <abartlet@samba.org>
|
|
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
|
|
|
|
[abartlet@samba.org adapted due to Samba 4.17 and lower
|
|
not having the patches for CVE-2020-25720]
|
|
|
|
Conflict: NA
|
|
Reference: https://attachments.samba.org/attachment.cgi?id=17821
|
|
---
|
|
common/ldb_match.c | 37 +
|
|
include/ldb_module.h | 11 +
|
|
include/ldb_private.h | 5 +
|
|
ldb_key_value/ldb_kv_index.c | 8 +
|
|
ldb_key_value/ldb_kv_search.c | 15 +
|
|
12 files changed, 672 insertions(+), 455 deletions(-)
|
|
delete mode 100644 selftest/knownfail.d/confidential-attr-timing
|
|
|
|
diff --git a/common/ldb_match.c b/common/ldb_match.c
|
|
index 2f4d41f3441..17314f1b9ca 100644
|
|
--- a/common/ldb_match.c
|
|
+++ b/common/ldb_match.c
|
|
@@ -98,6 +98,11 @@ 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;
|
|
@@ -139,6 +144,11 @@ 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;
|
|
@@ -209,6 +219,11 @@ 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;
|
|
@@ -370,6 +385,11 @@ 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);
|
|
@@ -443,6 +463,11 @@ 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];
|
|
@@ -741,3 +766,15 @@ int ldb_register_extended_match_rule(struct ldb_context *ldb,
|
|
return LDB_SUCCESS;
|
|
}
|
|
|
|
+int ldb_register_redact_callback(struct ldb_context *ldb,
|
|
+ ldb_redact_fn redact_fn,
|
|
+ struct ldb_module *module)
|
|
+{
|
|
+ if (ldb->redact.callback != NULL) {
|
|
+ return LDB_ERR_ENTRY_ALREADY_EXISTS;
|
|
+ }
|
|
+
|
|
+ ldb->redact.callback = redact_fn;
|
|
+ ldb->redact.module = module;
|
|
+ return LDB_SUCCESS;
|
|
+}
|
|
diff --git a/include/ldb_module.h b/include/ldb_module.h
|
|
index bd369ed9512..0f14b1ad346 100644
|
|
--- a/include/ldb_module.h
|
|
+++ b/include/ldb_module.h
|
|
@@ -102,6 +102,12 @@ struct ldb_module;
|
|
*/
|
|
#define LDB_FLAG_INTERNAL_SHARED_VALUES 0x200
|
|
|
|
+/*
|
|
+ * this attribute has been access checked. We know the user has the right to
|
|
+ * view it. Used internally in Samba aclread module.
|
|
+ */
|
|
+#define LDB_FLAG_INTERNAL_ACCESS_CHECKED 0x400
|
|
+
|
|
/* an extended match rule that always fails to match */
|
|
#define SAMBA_LDAP_MATCH_ALWAYS_FALSE "1.3.6.1.4.1.7165.4.5.1"
|
|
|
|
@@ -520,6 +526,11 @@ 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);
|
|
|
|
+typedef int (*ldb_redact_fn)(struct ldb_module *, struct ldb_request *, struct ldb_message *);
|
|
+int ldb_register_redact_callback(struct ldb_context *ldb,
|
|
+ ldb_redact_fn redact_fn,
|
|
+ struct ldb_module *module);
|
|
+
|
|
/*
|
|
* these pack/unpack functions are exposed in the library for use by
|
|
* ldb tools like ldbdump and for use in tests,
|
|
diff --git a/include/ldb_private.h b/include/ldb_private.h
|
|
index ca43817d07a..b0a42f6421c 100644
|
|
--- a/include/ldb_private.h
|
|
+++ b/include/ldb_private.h
|
|
@@ -119,6 +119,11 @@ struct ldb_context {
|
|
struct ldb_extended_match_entry *prev, *next;
|
|
} *extended_match_rules;
|
|
|
|
+ struct {
|
|
+ struct ldb_module *module;
|
|
+ ldb_redact_fn callback;
|
|
+ } redact;
|
|
+
|
|
/* custom utf8 functions */
|
|
struct ldb_utf8_fns utf8_fns;
|
|
|
|
diff --git a/ldb_key_value/ldb_kv_index.c b/ldb_key_value/ldb_kv_index.c
|
|
index 203266ea8c9..163052fecf7 100644
|
|
--- a/ldb_key_value/ldb_kv_index.c
|
|
+++ b/ldb_key_value/ldb_kv_index.c
|
|
@@ -2428,6 +2428,14 @@ 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.
|
|
diff --git a/ldb_key_value/ldb_kv_search.c b/ldb_key_value/ldb_kv_search.c
|
|
index f3333510eab..d187ba223e1 100644
|
|
--- a/ldb_key_value/ldb_kv_search.c
|
|
+++ b/ldb_key_value/ldb_kv_search.c
|
|
@@ -395,6 +395,14 @@ static int search_func(_UNUSED_ struct ldb_kv_private *ldb_kv,
|
|
}
|
|
}
|
|
|
|
+ if (ldb->redact.callback != NULL) {
|
|
+ ret = ldb->redact.callback(ldb->redact.module, ac->req, msg);
|
|
+ if (ret != LDB_SUCCESS) {
|
|
+ talloc_free(msg);
|
|
+ return ret;
|
|
+ }
|
|
+ }
|
|
+
|
|
/* see if it matches the given expression */
|
|
ret = ldb_match_msg_error(ldb, msg,
|
|
ac->tree, ac->base, ac->scope, &matched);
|
|
@@ -530,6 +538,13 @@ static int ldb_kv_search_and_return_base(struct ldb_kv_private *ldb_kv,
|
|
return ret;
|
|
}
|
|
|
|
+ if (ldb->redact.callback != NULL) {
|
|
+ ret = ldb->redact.callback(ldb->redact.module, ctx->req, msg);
|
|
+ if (ret != LDB_SUCCESS) {
|
|
+ talloc_free(msg);
|
|
+ return ret;
|
|
+ }
|
|
+ }
|
|
|
|
/*
|
|
* We use this, not ldb_match_msg_error() as we know
|
|
--
|