1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
281
282
283
284
285
286
287
288
289
290
291
292
293
294
295
296
297
298
299
300
|
From 89219baab0bf6ff05142518110f45c8159be8092 Mon Sep 17 00:00:00 2001
From: Alban Crequy <alban.crequy@collabora.co.uk>
Date: Fri, 4 Jul 2014 15:05:51 +0100
Subject: [PATCH 04/10] Stop listening on DBusServer sockets when reaching
max_incomplete_connections
This addresses the parts of CVE-2014-3639 not already addressed by
reducing the default authentication timeout.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=80851
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=80919
Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
(cherry picked from commit 8ad179a8dad789fc6a5402780044bc0ec3d41115)
---
bus/bus.c | 37 +++++++++++++++++++++++++++++++++++++
bus/bus.h | 1 +
bus/connection.c | 42 ++++++++++++++++++------------------------
bus/connection.h | 3 ++-
dbus/dbus-server-protected.h | 5 ++---
dbus/dbus-server.c | 19 +++++--------------
dbus/dbus-watch.c | 21 +++++++++++++++++++++
dbus/dbus-watch.h | 2 ++
8 files changed, 88 insertions(+), 42 deletions(-)
diff --git a/bus/bus.c b/bus/bus.c
index e80e708..7ffe772 100644
--- a/bus/bus.c
+++ b/bus/bus.c
@@ -39,6 +39,7 @@
#include <dbus/dbus-hash.h>
#include <dbus/dbus-credentials.h>
#include <dbus/dbus-internals.h>
+#include <dbus/dbus-server-protected.h>
#ifdef DBUS_CYGWIN
#include <signal.h>
@@ -68,6 +69,7 @@ struct BusContext
unsigned int keep_umask : 1;
unsigned int allow_anonymous : 1;
unsigned int systemd_activation : 1;
+ dbus_bool_t watches_enabled;
};
static dbus_int32_t server_data_slot = -1;
@@ -746,6 +748,8 @@ bus_context_new (const DBusString *config_file,
goto failed;
}
+ context->watches_enabled = TRUE;
+
context->registry = bus_registry_new (context);
if (context->registry == NULL)
{
@@ -1646,3 +1650,36 @@ bus_context_check_security_policy (BusContext *context,
_dbus_verbose ("security policy allowing message\n");
return TRUE;
}
+
+void
+bus_context_check_all_watches (BusContext *context)
+{
+ DBusList *link;
+ dbus_bool_t enabled = TRUE;
+
+ if (bus_connections_get_n_incomplete (context->connections) >=
+ bus_context_get_max_incomplete_connections (context))
+ {
+ enabled = FALSE;
+ }
+
+ if (context->watches_enabled == enabled)
+ return;
+
+ context->watches_enabled = enabled;
+
+ for (link = _dbus_list_get_first_link (&context->servers);
+ link != NULL;
+ link = _dbus_list_get_next_link (&context->servers, link))
+ {
+ /* A BusContext might contains several DBusServer (if there are
+ * several <listen> configuration items) and a DBusServer might
+ * contain several DBusWatch in its DBusWatchList (if getaddrinfo
+ * returns several addresses on a dual IPv4-IPv6 stack or if
+ * systemd passes several fds).
+ * We want to enable/disable them all.
+ */
+ DBusServer *server = link->data;
+ _dbus_server_toggle_all_watches (server, enabled);
+ }
+}
diff --git a/bus/bus.h b/bus/bus.h
index 3597884..400c9d0 100644
--- a/bus/bus.h
+++ b/bus/bus.h
@@ -125,5 +125,6 @@ dbus_bool_t bus_context_check_security_policy (BusContext
DBusConnection *proposed_recipient,
DBusMessage *message,
DBusError *error);
+void bus_context_check_all_watches (BusContext *context);
#endif /* BUS_BUS_H */
diff --git a/bus/connection.c b/bus/connection.c
index d69758c..b3d87a7 100644
--- a/bus/connection.c
+++ b/bus/connection.c
@@ -293,6 +293,10 @@ bus_connection_disconnected (DBusConnection *connection)
_dbus_list_remove_link (&d->connections->incomplete, d->link_in_connection_list);
d->link_in_connection_list = NULL;
d->connections->n_incomplete -= 1;
+
+ /* If we have dropped below the max. number of incomplete
+ * connections, start accept()ing again */
+ bus_context_check_all_watches (d->connections->context);
}
_dbus_assert (d->connections->n_incomplete >= 0);
@@ -688,31 +692,17 @@ bus_connections_setup_connection (BusConnections *connections,
dbus_connection_ref (connection);
- /* Note that we might disconnect ourselves here, but it only takes
- * effect on return to the main loop. We call this to free up
- * expired connections if possible, and to queue the timeout for our
- * own expiration.
- */
bus_connections_expire_incomplete (connections);
- /* And we might also disconnect ourselves here, but again it
- * only takes effect on return to main loop.
- */
- if (connections->n_incomplete >
- bus_context_get_max_incomplete_connections (connections->context))
- {
- _dbus_verbose ("Number of incomplete connections exceeds max, dropping oldest one\n");
-
- _dbus_assert (connections->incomplete != NULL);
- /* Disconnect the oldest unauthenticated connection. FIXME
- * would it be more secure to drop a *random* connection? This
- * algorithm seems to mean that if someone can create new
- * connections quickly enough, they can keep anyone else from
- * completing authentication. But random may or may not really
- * help with that, a more elaborate solution might be required.
- */
- dbus_connection_close (connections->incomplete->data);
- }
+ /* The listening socket is removed from the main loop,
+ * i.e. does not accept(), while n_incomplete is at its
+ * maximum value; so we shouldn't get here in that case */
+ _dbus_assert (connections->n_incomplete <=
+ bus_context_get_max_incomplete_connections (connections->context));
+
+ /* If we have the maximum number of incomplete connections,
+ * stop accept()ing any more, to avert a DoS. See fd.o #80919 */
+ bus_context_check_all_watches (d->connections->context);
retval = TRUE;
@@ -1419,6 +1409,10 @@ bus_connection_complete (DBusConnection *connection,
_dbus_assert (d->connections->n_incomplete >= 0);
_dbus_assert (d->connections->n_completed > 0);
+ /* If we have dropped below the max. number of incomplete
+ * connections, start accept()ing again */
+ bus_context_check_all_watches (d->connections->context);
+
/* See if we can remove the timeout */
bus_connections_expire_incomplete (d->connections);
@@ -2316,7 +2310,6 @@ bus_transaction_add_cancel_hook (BusTransaction *transaction,
return TRUE;
}
-#ifdef DBUS_ENABLE_STATS
int
bus_connections_get_n_active (BusConnections *connections)
{
@@ -2329,6 +2322,7 @@ bus_connections_get_n_incomplete (BusConnections *connections)
return connections->n_incomplete;
}
+#ifdef DBUS_ENABLE_STATS
int
bus_connections_get_total_match_rules (BusConnections *connections)
{
diff --git a/bus/connection.h b/bus/connection.h
index c936021..06846e7 100644
--- a/bus/connection.h
+++ b/bus/connection.h
@@ -138,9 +138,10 @@ dbus_bool_t bus_transaction_add_cancel_hook (BusTransaction *
void *data,
DBusFreeFunction free_data_function);
-/* called by stats.c, only present if DBUS_ENABLE_STATS */
int bus_connections_get_n_active (BusConnections *connections);
int bus_connections_get_n_incomplete (BusConnections *connections);
+
+/* called by stats.c, only present if DBUS_ENABLE_STATS */
int bus_connections_get_total_match_rules (BusConnections *connections);
int bus_connections_get_peak_match_rules (BusConnections *connections);
int bus_connections_get_peak_match_rules_per_conn (BusConnections *connections);
diff --git a/dbus/dbus-server-protected.h b/dbus/dbus-server-protected.h
index dd5234b..e6dbd1e 100644
--- a/dbus/dbus-server-protected.h
+++ b/dbus/dbus-server-protected.h
@@ -99,9 +99,8 @@ dbus_bool_t _dbus_server_add_watch (DBusServer *server,
DBusWatch *watch);
void _dbus_server_remove_watch (DBusServer *server,
DBusWatch *watch);
-void _dbus_server_toggle_watch (DBusServer *server,
- DBusWatch *watch,
- dbus_bool_t enabled);
+void _dbus_server_toggle_all_watches (DBusServer *server,
+ dbus_bool_t enabled);
dbus_bool_t _dbus_server_add_timeout (DBusServer *server,
DBusTimeout *timeout);
void _dbus_server_remove_timeout (DBusServer *server,
diff --git a/dbus/dbus-server.c b/dbus/dbus-server.c
index b62c2b4..0b8ef00 100644
--- a/dbus/dbus-server.c
+++ b/dbus/dbus-server.c
@@ -312,26 +312,17 @@ _dbus_server_remove_watch (DBusServer *server,
}
/**
- * Toggles a watch and notifies app via server's
- * DBusWatchToggledFunction if available. It's an error to call this
- * function on a watch that was not previously added.
+ * Toggles all watch and notifies app via server's
+ * DBusWatchToggledFunction if available.
*
* @param server the server.
- * @param watch the watch to toggle.
* @param enabled whether to enable or disable
*/
void
-_dbus_server_toggle_watch (DBusServer *server,
- DBusWatch *watch,
- dbus_bool_t enabled)
+_dbus_server_toggle_all_watches (DBusServer *server,
+ dbus_bool_t enabled)
{
- _dbus_assert (watch != NULL);
-
- HAVE_LOCK_CHECK (server);
- protected_change_watch (server, watch,
- NULL, NULL,
- _dbus_watch_list_toggle_watch,
- enabled);
+ _dbus_watch_list_toggle_all_watches (server->watches, enabled);
}
/** Function to be called in protected_change_timeout() with refcount held */
diff --git a/dbus/dbus-watch.c b/dbus/dbus-watch.c
index b9f4ac2..ece5cb5 100644
--- a/dbus/dbus-watch.c
+++ b/dbus/dbus-watch.c
@@ -454,6 +454,27 @@ _dbus_watch_list_toggle_watch (DBusWatchList *watch_list,
}
/**
+ * Sets all watches to the given enabled state, invoking the
+ * application's DBusWatchToggledFunction if appropriate.
+ *
+ * @param watch_list the watch list.
+ * @param enabled #TRUE to enable
+ */
+void
+_dbus_watch_list_toggle_all_watches (DBusWatchList *watch_list,
+ dbus_bool_t enabled)
+{
+ DBusList *link;
+
+ for (link = _dbus_list_get_first_link (&watch_list->watches);
+ link != NULL;
+ link = _dbus_list_get_next_link (&watch_list->watches, link))
+ {
+ _dbus_watch_list_toggle_watch (watch_list, link->data, enabled);
+ }
+}
+
+/**
* Sets the handler for the watch.
*
* @todo this function only exists because of the weird
diff --git a/dbus/dbus-watch.h b/dbus/dbus-watch.h
index c583214..321740e 100644
--- a/dbus/dbus-watch.h
+++ b/dbus/dbus-watch.h
@@ -76,6 +76,8 @@ void _dbus_watch_list_remove_watch (DBusWatchList *watch_li
void _dbus_watch_list_toggle_watch (DBusWatchList *watch_list,
DBusWatch *watch,
dbus_bool_t enabled);
+void _dbus_watch_list_toggle_all_watches (DBusWatchList *watch_list,
+ dbus_bool_t enabled);
dbus_bool_t _dbus_watch_get_enabled (DBusWatch *watch);
dbus_bool_t _dbus_watch_get_oom_last_time (DBusWatch *watch);
--
2.1.0
|
