Commit 8ad179a8 authored by Alban Crequy's avatar Alban Crequy Committed by Simon McVittie

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=80919Reviewed-by: 's avatarSimon McVittie <simon.mcvittie@collabora.co.uk>
parent 54d26df5
......@@ -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;
......@@ -758,6 +760,8 @@ bus_context_new (const DBusString *config_file,
goto failed;
}
context->watches_enabled = TRUE;
context->registry = bus_registry_new (context);
if (context->registry == NULL)
{
......@@ -1658,3 +1662,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);
}
}
......@@ -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 */
......@@ -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);
......@@ -2348,7 +2342,6 @@ bus_transaction_add_cancel_hook (BusTransaction *transaction,
return TRUE;
}
#ifdef DBUS_ENABLE_STATS
int
bus_connections_get_n_active (BusConnections *connections)
{
......@@ -2361,6 +2354,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)
{
......
......@@ -139,9 +139,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);
......
......@@ -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,
......
......@@ -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 */
......
......@@ -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.
*
......
......@@ -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);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment