166 lines
5.4 KiB
Diff
166 lines
5.4 KiB
Diff
From 1d5b155619bc532c46932965b215bd73a920e56f Mon Sep 17 00:00:00 2001
|
|
From: Andrew Bartlett <abartlet@samba.org>
|
|
Date: Mon, 27 Sep 2021 16:47:46 +1300
|
|
Subject: [PATCH] CVE-2021-3670 ldb: Confirm the request has not yet timed out
|
|
in ldb filter processing
|
|
|
|
The LDB filter processing is where the time is spent in the LDB stack
|
|
but the timeout event will not get run while this is ongoing, so we
|
|
must confirm we have not yet timed out manually.
|
|
|
|
RN: Ensure that the LDB request has not timed out during filter processing
|
|
as the LDAP server MaxQueryDuration is otherwise not honoured.
|
|
|
|
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14694
|
|
|
|
Signed-off-by: Andrew Bartlett <abartlet@samba.org>
|
|
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
|
|
|
|
---
|
|
ldb_key_value/ldb_kv.c | 2 ++
|
|
ldb_key_value/ldb_kv.h | 10 +++++++++
|
|
ldb_key_value/ldb_kv_index.c | 41 +++++++++++++++++++++++++++++++++++
|
|
ldb_key_value/ldb_kv_search.c | 33 +++++++++++++++++++++++++++-
|
|
4 files changed, 85 insertions(+), 1 deletion(-)
|
|
|
|
diff --git a/ldb_key_value/ldb_kv.c b/ldb_key_value/ldb_kv.c
|
|
index 4e7b8a1..8e36dc6 100644
|
|
--- a/ldb_key_value/ldb_kv.c
|
|
+++ b/ldb_key_value/ldb_kv.c
|
|
@@ -2082,6 +2082,8 @@ static int ldb_kv_handle_request(struct ldb_module *module,
|
|
}
|
|
}
|
|
|
|
+ ac->timeout_timeval = tv;
|
|
+
|
|
/* set a spy so that we do not try to use the request context
|
|
* if it is freed before ltdb_callback fires */
|
|
ac->spy = talloc(req, struct ldb_kv_req_spy);
|
|
diff --git a/ldb_key_value/ldb_kv.h b/ldb_key_value/ldb_kv.h
|
|
index f9dffae..ac474b0 100644
|
|
--- a/ldb_key_value/ldb_kv.h
|
|
+++ b/ldb_key_value/ldb_kv.h
|
|
@@ -152,6 +152,16 @@ struct ldb_kv_context {
|
|
struct ldb_module *module;
|
|
struct ldb_request *req;
|
|
|
|
+ /*
|
|
+ * Required as we might not get to the event loop before the
|
|
+ * timeout, so we need some old-style cooperative multitasking
|
|
+ * here.
|
|
+ */
|
|
+ struct timeval timeout_timeval;
|
|
+
|
|
+ /* Used to throttle calls to gettimeofday() */
|
|
+ size_t timeout_counter;
|
|
+
|
|
bool request_terminated;
|
|
struct ldb_kv_req_spy *spy;
|
|
|
|
diff --git a/ldb_key_value/ldb_kv_index.c b/ldb_key_value/ldb_kv_index.c
|
|
index 8e756c1..aca542c 100644
|
|
--- a/ldb_key_value/ldb_kv_index.c
|
|
+++ b/ldb_key_value/ldb_kv_index.c
|
|
@@ -2324,6 +2324,47 @@ static int ldb_kv_index_filter(struct ldb_kv_private *ldb_kv,
|
|
for (i = 0; i < num_keys; i++) {
|
|
int ret;
|
|
bool matched;
|
|
+
|
|
+ /*
|
|
+ * Check the time every 64 records, to reduce calls to
|
|
+ * gettimeofday(). This is a compromise, not all
|
|
+ * calls to ldb_match_message() will take the same
|
|
+ * time, most will run quickly but by luck it might be
|
|
+ * possible to have 64 records that are slow, doing a
|
|
+ * recursive search via LDAP_MATCHING_RULE_IN_CHAIN.
|
|
+ *
|
|
+ * Thankfully this is after index processing so only
|
|
+ * on the subset that matches some index (but still
|
|
+ * possibly a big one like objectclass=user)
|
|
+ */
|
|
+ if (i % 64 == 0) {
|
|
+ struct timeval now = tevent_timeval_current();
|
|
+ int timeval_cmp = tevent_timeval_compare(&ac->timeout_timeval,
|
|
+ &now);
|
|
+
|
|
+ /*
|
|
+ * The search has taken too long. This is the
|
|
+ * most likely place for our time to expire,
|
|
+ * as we are checking the records after the
|
|
+ * index set intersection. This is now the
|
|
+ * slow process of checking if the records
|
|
+ * actually match.
|
|
+ *
|
|
+ * The tevent based timeout is not likely to
|
|
+ * be hit, sadly, as we don't run an event
|
|
+ * loop.
|
|
+ *
|
|
+ * While we are indexed and most of the work
|
|
+ * should have been done already, the
|
|
+ * ldb_match_* calls can be quite expensive if
|
|
+ * the caller uses LDAP_MATCHING_RULE_IN_CHAIN
|
|
+ */
|
|
+ if (timeval_cmp <= 0) {
|
|
+ talloc_free(keys);
|
|
+ return LDB_ERR_TIME_LIMIT_EXCEEDED;
|
|
+ }
|
|
+ }
|
|
+
|
|
msg = ldb_msg_new(ac);
|
|
if (!msg) {
|
|
talloc_free(keys);
|
|
diff --git a/ldb_key_value/ldb_kv_search.c b/ldb_key_value/ldb_kv_search.c
|
|
index f2d9619..9636828 100644
|
|
--- a/ldb_key_value/ldb_kv_search.c
|
|
+++ b/ldb_key_value/ldb_kv_search.c
|
|
@@ -314,7 +314,8 @@ 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;
|
|
- int ret;
|
|
+ struct timeval now;
|
|
+ int ret, timeval_cmp;
|
|
bool matched;
|
|
|
|
ac = talloc_get_type(state, struct ldb_kv_context);
|
|
@@ -341,6 +342,36 @@ static int search_func(_UNUSED_ struct ldb_kv_private *ldb_kv,
|
|
return 0;
|
|
}
|
|
|
|
+ /*
|
|
+ * Check the time every 64 records, to reduce calls to
|
|
+ * gettimeofday(). This is a compromise, not all calls to
|
|
+ * ldb_match_message() will take the same time, most will fail
|
|
+ * quickly but by luck it might be possible to have 64 records
|
|
+ * that are slow, doing a recursive search via
|
|
+ * LDAP_MATCHING_RULE_IN_CHAIN.
|
|
+ */
|
|
+ if (ac->timeout_counter++ % 64 == 0) {
|
|
+ now = tevent_timeval_current();
|
|
+ timeval_cmp = tevent_timeval_compare(&ac->timeout_timeval,
|
|
+ &now);
|
|
+
|
|
+ /*
|
|
+ * The search has taken too long. This is the most
|
|
+ * likely place for our time to expire, as we are in
|
|
+ * an un-indexed search and we return the data from
|
|
+ * within this loop. The tevent based timeout is not
|
|
+ * likely to be hit, sadly.
|
|
+ *
|
|
+ * ldb_match_msg_error() can be quite expensive if a
|
|
+ * LDAP_MATCHING_RULE_IN_CHAIN extended match was
|
|
+ * specified.
|
|
+ */
|
|
+ if (timeval_cmp <= 0) {
|
|
+ ac->error = LDB_ERR_TIME_LIMIT_EXCEEDED;
|
|
+ return -1;
|
|
+ }
|
|
+ }
|
|
+
|
|
msg = ldb_msg_new(ac);
|
|
if (!msg) {
|
|
ac->error = LDB_ERR_OPERATIONS_ERROR;
|
|
--
|
|
2.43.0
|
|
|