diff options
| -rw-r--r-- | ChangeLog | 14 | ||||
| -rw-r--r-- | bus/bus.c | 82 | ||||
| -rw-r--r-- | bus/bus.h | 7 | ||||
| -rw-r--r-- | bus/config-parser.c | 15 | ||||
| -rw-r--r-- | bus/connection.c | 375 | ||||
| -rw-r--r-- | bus/connection.h | 13 | ||||
| -rw-r--r-- | bus/dbus-daemon-1.1.in | 5 | ||||
| -rw-r--r-- | bus/dispatch.c | 6 | ||||
| -rw-r--r-- | bus/expirelist.c | 12 | ||||
| -rw-r--r-- | bus/expirelist.h | 2 | ||||
| -rw-r--r-- | doc/TODO | 15 |
11 files changed, 527 insertions, 19 deletions
@@ -1,3 +1,17 @@ +2003-10-14 Havoc Pennington <hp@redhat.com> + + * bus/connection.c: implement pending reply tracking using + BusExpireList + + * bus/bus.c (bus_context_check_security_policy): verify that a + reply is pending in order to allow a reply to be sent. Deny + messages of unknown type. + + * bus/dbus-daemon-1.1.in: update to mention new resource limits + + * bus/bus.c (bus_context_get_max_replies_per_connection): new + (bus_context_get_reply_timeout): new + 2003-10-13 Seth Nickell <seth@gnome.org> * python/Makefile.am: @@ -872,6 +872,18 @@ bus_context_get_max_match_rules_per_connection (BusContext *context) return context->limits.max_match_rules_per_connection; } +int +bus_context_get_max_replies_per_connection (BusContext *context) +{ + return context->limits.max_replies_per_connection; +} + +int +bus_context_get_reply_timeout (BusContext *context) +{ + return context->limits.reply_timeout; +} + /* * addressed_recipient is the recipient specified in the message. * @@ -887,6 +899,7 @@ bus_context_get_max_match_rules_per_connection (BusContext *context) */ dbus_bool_t bus_context_check_security_policy (BusContext *context, + BusTransaction *transaction, DBusConnection *sender, DBusConnection *addressed_recipient, DBusConnection *proposed_recipient, @@ -895,10 +908,16 @@ bus_context_check_security_policy (BusContext *context, { BusClientPolicy *sender_policy; BusClientPolicy *recipient_policy; - - _dbus_assert (dbus_message_get_destination (message) == NULL || /* Signal */ - (addressed_recipient != NULL || - strcmp (dbus_message_get_destination (message), DBUS_SERVICE_ORG_FREEDESKTOP_DBUS) == 0)); /* Destination specified or is the bus driver */ + int type; + + type = dbus_message_get_type (message); + + /* dispatch.c was supposed to ensure these invariants */ + _dbus_assert (dbus_message_get_destination (message) != NULL || + type == DBUS_MESSAGE_TYPE_SIGNAL); + _dbus_assert (type == DBUS_MESSAGE_TYPE_SIGNAL || + addressed_recipient != NULL || + strcmp (dbus_message_get_destination (message), DBUS_SERVICE_ORG_FREEDESKTOP_DBUS) == 0); if (sender != NULL) { @@ -906,6 +925,44 @@ bus_context_check_security_policy (BusContext *context, { sender_policy = bus_connection_get_policy (sender); _dbus_assert (sender_policy != NULL); + + switch (type) + { + case DBUS_MESSAGE_TYPE_METHOD_CALL: + case DBUS_MESSAGE_TYPE_SIGNAL: + + /* Continue below to check security policy */ + break; + + case DBUS_MESSAGE_TYPE_METHOD_RETURN: + case DBUS_MESSAGE_TYPE_ERROR: + /* These are only allowed if the reply is listed + * as pending, or the connection is eavesdropping. + * The idea is to prohibit confusing/fake replies. + * FIXME In principle a client that's asked to eavesdrop + * specifically should probably get bogus replies + * even to itself, but here we prohibit that. + */ + + if (proposed_recipient != NULL /* not to the bus driver */ && + addressed_recipient == proposed_recipient /* not eavesdropping */ && + !bus_connections_check_reply (bus_connection_get_connections (sender), + transaction, + sender, addressed_recipient, message, + error)) + return FALSE; + + /* Continue below to check security policy, since reply was expected */ + break; + + default: + _dbus_verbose ("security check disallowing message of unknown type\n"); + + dbus_set_error (error, DBUS_ERROR_ACCESS_DENIED, + "Message bus will not accept messages of unknown type\n"); + + return FALSE; + } } else { @@ -1029,6 +1086,23 @@ bus_context_check_security_policy (BusContext *context, return FALSE; } + if (type == DBUS_MESSAGE_TYPE_METHOD_CALL) + { + /* Record that we will allow a reply here in the future (don't + * bother if the recipient is the bus). Only the addressed recipient + * may reply. + */ + if (sender && addressed_recipient && + !bus_connections_expect_reply (bus_connection_get_connections (sender), + transaction, + sender, addressed_recipient, + message, error)) + { + _dbus_verbose ("Failed to record reply expectation or problem with the message expecting a reply\n"); + return FALSE; + } + } + _dbus_verbose ("security policy allowing message\n"); return TRUE; } @@ -45,7 +45,7 @@ typedef struct BusMatchRule BusMatchRule; typedef struct { - long max_incoming_bytes; /**< How many incoming messages for a single connection */ + long max_incoming_bytes; /**< How many incoming message bytes for a single connection */ long max_outgoing_bytes; /**< How many outgoing bytes can be queued for a single connection */ long max_message_size; /**< Max size of a single message in bytes */ int activation_timeout; /**< How long to wait for an activation to time out */ @@ -56,6 +56,8 @@ typedef struct int max_pending_activations; /**< Max number of pending activations for the entire bus */ int max_services_per_connection; /**< Max number of owned services for a single connection */ int max_match_rules_per_connection; /**< Max number of match rules for a single connection */ + int max_replies_per_connection; /**< Max number of replies that can be pending for each connection */ + int reply_timeout; /**< How long to wait before timing out a reply */ } BusLimits; BusContext* bus_context_new (const DBusString *config_file, @@ -87,7 +89,10 @@ int bus_context_get_max_connections_per_user (BusContext int bus_context_get_max_pending_activations (BusContext *context); int bus_context_get_max_services_per_connection (BusContext *context); int bus_context_get_max_match_rules_per_connection (BusContext *context); +int bus_context_get_max_replies_per_connection (BusContext *context); +int bus_context_get_reply_timeout (BusContext *context); dbus_bool_t bus_context_check_security_policy (BusContext *context, + BusTransaction *transaction, DBusConnection *sender, DBusConnection *addressed_recipient, DBusConnection *proposed_recipient, diff --git a/bus/config-parser.c b/bus/config-parser.c index 3ff3ec50..b3652591 100644 --- a/bus/config-parser.c +++ b/bus/config-parser.c @@ -336,6 +336,9 @@ bus_config_parser_new (const DBusString *basedir, parser->limits.max_services_per_connection = 256; parser->limits.max_match_rules_per_connection = 128; + + parser->limits.reply_timeout = 5 * 60 * 1000; /* 5 minutes */ + parser->limits.max_replies_per_connection = 32; parser->refcount = 1; @@ -1397,6 +1400,12 @@ set_limit (BusConfigParser *parser, must_be_int = TRUE; parser->limits.auth_timeout = value; } + else if (strcmp (name, "reply_timeout") == 0) + { + must_be_positive = TRUE; + must_be_int = TRUE; + parser->limits.reply_timeout = value; + } else if (strcmp (name, "max_completed_connections") == 0) { must_be_positive = TRUE; @@ -1427,6 +1436,12 @@ set_limit (BusConfigParser *parser, must_be_int = TRUE; parser->limits.max_services_per_connection = value; } + else if (strcmp (name, "max_replies_per_connection") == 0) + { + must_be_positive = TRUE; + must_be_int = TRUE; + parser->limits.max_replies_per_connection = value; + } else { dbus_set_error (error, DBUS_ERROR_FAILED, diff --git a/bus/connection.c b/bus/connection.c index 6a4b55d6..2ae1d7d2 100644 --- a/bus/connection.c +++ b/bus/connection.c @@ -55,6 +55,7 @@ struct BusConnections DBusHashTable *completed_by_user; /**< Number of completed connections for each UID */ DBusTimeout *expire_timeout; /**< Timeout for expiring incomplete connections. */ int stamp; /**< Incrementing number */ + BusExpireList *pending_replies; /**< List of pending replies */ }; static dbus_int32_t connection_data_slot = -1; @@ -79,6 +80,13 @@ typedef struct int stamp; /**< connections->stamp last time we were traversed */ } BusConnectionData; +static void bus_pending_reply_expired (BusExpireList *list, + DBusList *link, + void *data); + +static void bus_connection_drop_pending_replies (BusConnections *connections, + DBusConnection *connection); + static dbus_bool_t expire_incomplete_timeout (void *data); #define BUS_CONNECTION_DATA(connection) (dbus_connection_get_data ((connection), connection_data_slot)) @@ -268,12 +276,14 @@ bus_connection_disconnected (DBusConnection *connection) _dbus_assert (d->connections->n_incomplete >= 0); _dbus_assert (d->connections->n_completed >= 0); } + + bus_connection_drop_pending_replies (d->connections, connection); /* frees "d" as side effect */ dbus_connection_set_data (connection, connection_data_slot, NULL, NULL); - + dbus_connection_unref (connection); } @@ -430,16 +440,25 @@ bus_connections_new (BusContext *context) _dbus_timeout_set_enabled (connections->expire_timeout, FALSE); + connections->pending_replies = bus_expire_list_new (bus_context_get_loop (context), + bus_context_get_reply_timeout (context), + bus_pending_reply_expired, + connections); + if (connections->pending_replies == NULL) + goto failed_4; + if (!_dbus_loop_add_timeout (bus_context_get_loop (context), connections->expire_timeout, call_timeout_callback, NULL, NULL)) - goto failed_4; + goto failed_5; connections->refcount = 1; connections->context = context; return connections; + failed_5: + bus_expire_list_free (connections->pending_replies); failed_4: _dbus_timeout_unref (connections->expire_timeout); failed_3: @@ -491,11 +510,14 @@ bus_connections_unref (BusConnections *connections) dbus_connection_ref (connection); dbus_connection_disconnect (connection); bus_connection_disconnected (connection); - dbus_connection_unref (connection); + dbus_connection_unref (connection); } _dbus_assert (connections->n_completed == 0); + _dbus_assert (connections->pending_replies->n_items == 0); + bus_expire_list_free (connections->pending_replies); + _dbus_loop_remove_timeout (bus_context_get_loop (connections->context), connections->expire_timeout, call_timeout_callback, NULL); @@ -1318,6 +1340,352 @@ bus_connections_check_limits (BusConnections *connections, return TRUE; } +static dbus_bool_t +bus_pending_reply_send_no_reply (BusConnections *connections, + BusTransaction *transaction, + BusPendingReply *pending) +{ + DBusMessage *message; + DBusMessageIter iter; + dbus_bool_t retval; + + retval = FALSE; + + message = dbus_message_new (DBUS_MESSAGE_TYPE_ERROR); + if (message == NULL) + return FALSE; + + dbus_message_set_no_reply (message, TRUE); + + if (!dbus_message_set_reply_serial (message, + pending->reply_serial)) + goto out; + + if (!dbus_message_set_error_name (message, + DBUS_ERROR_NO_REPLY)) + goto out; + + dbus_message_append_iter_init (message, &iter); + if (!dbus_message_iter_append_string (&iter, "Message did not receive a reply (timeout by message bus)")) + goto out; + + if (!bus_transaction_send_from_driver (transaction, pending->will_get_reply, + message)) + goto out; + + retval = TRUE; + + out: + dbus_message_unref (message); + return retval; +} + +static void +bus_pending_reply_expired (BusExpireList *list, + DBusList *link, + void *data) +{ + BusPendingReply *pending = link->data; + BusConnections *connections = data; + BusTransaction *transaction; + + /* No reply is forthcoming. So nuke it if we can. If not, + * leave it in the list to try expiring again later when we + * get more memory. + */ + transaction = bus_transaction_new (connections->context); + if (transaction == NULL) + return; + + if (bus_pending_reply_send_no_reply (connections, + transaction, + pending)) + { + _dbus_list_remove_link (&connections->pending_replies->items, + link); + dbus_free (pending); + bus_transaction_cancel_and_free (transaction); + } + + bus_transaction_execute_and_free (transaction); +} + +static void +bus_connection_drop_pending_replies (BusConnections *connections, + DBusConnection *connection) +{ + /* The DBusConnection is almost 100% finalized here, so you can't + * do anything with it except check for pointer equality + */ + DBusList *link; + + link = _dbus_list_get_first_link (&connections->pending_replies->items); + while (link != NULL) + { + DBusList *next; + BusPendingReply *pending; + + next = _dbus_list_get_next_link (&connections->pending_replies->items, + link); + pending = link->data; + + if (pending->will_get_reply == connection) + { + /* We don't need to track this pending reply anymore */ + + _dbus_list_remove_link (&connections->pending_replies->items, + link); + dbus_free (pending); + } + else if (pending->will_send_reply == connection) + { + /* The reply isn't going to be sent, so set things + * up so it will be expired right away + */ + + pending->will_send_reply = NULL; + pending->expire_item.added_tv_sec = 0; + pending->expire_item.added_tv_usec = 0; + + bus_expire_timeout_set_interval (connections->pending_replies->timeout, + 0); + } + + link = next; + } +} + + +typedef struct +{ + BusPendingReply *pending; + BusConnections *connections; +} CancelPendingReplyData; + +static void +cancel_pending_reply (void *data) +{ + CancelPendingReplyData *d = data; + + if (!_dbus_list_remove (&d->connections->pending_replies->items, + d->pending)) + _dbus_assert_not_reached ("pending reply did not exist to be cancelled"); + + dbus_free (d->pending); /* since it's been cancelled */ +} + +static void +cancel_pending_reply_data_free (void *data) +{ + CancelPendingReplyData *d = data; + + /* d->pending should be either freed or still + * in the list of pending replies (owned by someone + * else) + */ + + dbus_free (d); +} + +/* + * Record that a reply is allowed; return TRUE on success. + */ +dbus_bool_t +bus_connections_expect_reply (BusConnections *connections, + BusTransaction *transaction, + DBusConnection *will_get_reply, + DBusConnection *will_send_reply, + DBusMessage *reply_to_this, + DBusError *error) +{ + BusPendingReply *pending; + dbus_uint32_t reply_serial; + DBusList *link; + CancelPendingReplyData *cprd; + + _dbus_assert (will_get_reply != NULL); + _dbus_assert (will_send_reply != NULL); + _dbus_assert (reply_to_this != NULL); + + if (dbus_message_get_no_reply (reply_to_this)) + return TRUE; /* we won't allow a reply, since client doesn't care for one. */ + + reply_serial = dbus_message_get_reply_serial (reply_to_this); + + link = _dbus_list_get_first_link (&connections->pending_replies->items); + while (link != NULL) + { + pending = link->data; + + if (pending->reply_serial == reply_serial && + pending->will_get_reply == will_get_reply && + pending->will_send_reply == will_send_reply) + { + dbus_set_error (error, DBUS_ERROR_ACCESS_DENIED, + "Message has the same reply serial as a currently-outstanding existing method call"); + return FALSE; + } + + link = _dbus_list_get_next_link (&connections->pending_replies->items, + link); + } + + pending = dbus_new0 (BusPendingReply, 1); + if (pending == NULL) + { + BUS_SET_OOM (error); + return FALSE; + } + + cprd = dbus_new0 (CancelPendingReplyData, 1); + if (cprd == NULL) + { + BUS_SET_OOM (error); + dbus_free (pending); + return FALSE; + } + + if (!_dbus_list_prepend (&connections->pending_replies->items, + pending)) + { + BUS_SET_OOM (error); + dbus_free (cprd); + dbus_free (pending); + return FALSE; + } + + if (!bus_transaction_add_cancel_hook (transaction, + cancel_pending_reply, + cprd, + cancel_pending_reply_data_free)) + { + BUS_SET_OOM (error); + _dbus_list_remove (&connections->pending_replies->items, pending); + dbus_free (cprd); + dbus_free (pending); + return FALSE; + } + + cprd->pending = pending; + cprd->connections = connections; + + _dbus_get_current_time (&pending->expire_item.added_tv_sec, + &pending->expire_item.added_tv_usec); + + pending->will_get_reply = will_get_reply; + pending->will_send_reply = will_send_reply; + pending->reply_serial = reply_serial; + + return TRUE; +} + +typedef struct +{ + DBusList *link; + BusConnections *connections; +} CheckPendingReplyData; + +static void +cancel_check_pending_reply (void *data) +{ + CheckPendingReplyData *d = data; + + _dbus_list_prepend_link (&d->connections->pending_replies->items, + d->link); + d->link = NULL; +} + +static void +check_pending_reply_data_free (void *data) +{ + CheckPendingReplyData *d = data; + + if (d->link != NULL) + { + BusPendingReply *pending = d->link->data; + + dbus_free (pending); + _dbus_list_free_link (d->link); + } + + dbus_free (d); +} + +/* + * Check whether a reply is allowed, remove BusPendingReply + * if so, return TRUE if so. + */ +dbus_bool_t +bus_connections_check_reply (BusConnections *connections, + BusTransaction *transaction, + DBusConnection *sending_reply, + DBusConnection *receiving_reply, + DBusMessage *reply, + DBusError *error) +{ + CheckPendingReplyData *cprd; + DBusList *link; + dbus_uint32_t reply_serial; + + _dbus_assert (sending_reply != NULL); + _dbus_assert (receiving_reply != NULL); + + reply_serial = dbus_message_get_reply_serial (reply); + + link = _dbus_list_get_first_link (&connections->pending_replies->items); + while (link != NULL) + { + BusPendingReply *pending = link->data; + + if (pending->reply_serial == reply_serial && + pending->will_get_reply == receiving_reply && + pending->will_send_reply == sending_reply) + { + _dbus_verbose ("Found pending reply\n"); + break; + } + + link = _dbus_list_get_next_link (&connections->pending_replies->items, + link); + } + + if (link == NULL) + { + _dbus_verbose ("No pending reply expected, disallowing this reply\n"); + + dbus_set_error (error, DBUS_ERROR_ACCESS_DENIED, + "%s message sent with reply serial %u, but no such reply was requested (or it has timed out already)\n", + dbus_message_get_type (reply) == DBUS_MESSAGE_TYPE_METHOD_RETURN ? + "method return" : "error", + reply_serial); + return FALSE; + } + + cprd = dbus_new0 (CheckPendingReplyData, 1); + if (cprd == NULL) + { + BUS_SET_OOM (error); + return FALSE; + } + + if (!bus_transaction_add_cancel_hook (transaction, + cancel_check_pending_reply, + cprd, + check_pending_reply_data_free)) + { + BUS_SET_OOM (error); + dbus_free (cprd); + return FALSE; + } + + cprd->link = link; + cprd->connections = connections; + + _dbus_list_remove_link (&connections->pending_replies->items, + link); + + return TRUE; +} /* * Transactions @@ -1443,6 +1811,7 @@ bus_transaction_send_from_driver (BusTransaction *transaction, * eat it; the driver doesn't care about getting a reply. */ if (!bus_context_check_security_policy (bus_transaction_get_context (transaction), + transaction, NULL, connection, connection, message, NULL)) return TRUE; diff --git a/bus/connection.h b/bus/connection.h index d9fd727e..00a7ce49 100644 --- a/bus/connection.h +++ b/bus/connection.h @@ -55,6 +55,19 @@ dbus_bool_t bus_connections_check_limits (BusConnections DBusError *error); void bus_connections_expire_incomplete (BusConnections *connections); +dbus_bool_t bus_connections_expect_reply (BusConnections *connections, + BusTransaction *transaction, + DBusConnection *will_get_reply, + DBusConnection *will_send_reply, + DBusMessage *reply_to_this, + DBusError *error); +dbus_bool_t bus_connections_check_reply (BusConnections *connections, + BusTransaction *transaction, + DBusConnection *sending_reply, + DBusConnection *receiving_reply, + DBusMessage *reply, + DBusError *error); + dbus_bool_t bus_connection_mark_stamp (DBusConnection *connection); dbus_bool_t bus_connection_is_active (DBusConnection *connection); diff --git a/bus/dbus-daemon-1.1.in b/bus/dbus-daemon-1.1.in index b272a62e..06bbbd13 100644 --- a/bus/dbus-daemon-1.1.in +++ b/bus/dbus-daemon-1.1.in @@ -280,6 +280,11 @@ Available limit names are: progress at the same time "max_services_per_connection": max number of services a single connection can own + "max_replies_per_connection" : max number of pending method + replies per connection + (number of calls-in-progress) + "reply_timeout" : milliseconds (thousandths) + until a method call times out .fi .PP diff --git a/bus/dispatch.c b/bus/dispatch.c index e238cb4c..6c5eadf1 100644 --- a/bus/dispatch.c +++ b/bus/dispatch.c @@ -42,7 +42,7 @@ send_one_message (DBusConnection *connection, BusTransaction *transaction, DBusError *error) { - if (!bus_context_check_security_policy (context, + if (!bus_context_check_security_policy (context, transaction, sender, addressed_recipient, connection, @@ -229,7 +229,7 @@ bus_dispatch (DBusConnection *connection, if (service_name && strcmp (service_name, DBUS_SERVICE_ORG_FREEDESKTOP_DBUS) == 0) /* to bus driver */ { - if (!bus_context_check_security_policy (context, + if (!bus_context_check_security_policy (context, transaction, connection, NULL, NULL, message, &error)) { _dbus_verbose ("Security policy rejected message\n"); @@ -272,7 +272,7 @@ bus_dispatch (DBusConnection *connection, addressed_recipient = bus_service_get_primary_owner (service); _dbus_assert (addressed_recipient != NULL); - if (!bus_context_check_security_policy (context, + if (!bus_context_check_security_policy (context, transaction, connection, addressed_recipient, addressed_recipient, message, &error)) diff --git a/bus/expirelist.c b/bus/expirelist.c index f732ebd1..a1ce226d 100644 --- a/bus/expirelist.c +++ b/bus/expirelist.c @@ -141,7 +141,13 @@ do_expiration_with_current_time (BusExpireList *list, if (elapsed >= (double) list->expire_after) { _dbus_verbose ("Expiring an item %p\n", item); - (* list->expire_func) (list, item, list->data); + + /* If the expire function fails, we just end up expiring + * this item next time we walk through the list. Which is in + * indeterminate time since we don't know what next_interval + * will be. + */ + (* list->expire_func) (list, link, list->data); } else { @@ -201,12 +207,12 @@ typedef struct static void test_expire_func (BusExpireList *list, - BusExpireItem *item, + DBusList *link, void *data) { TestExpireItem *t; - t = (TestExpireItem*) item; + t = (TestExpireItem*) link->data; t->expire_count += 1; } diff --git a/bus/expirelist.h b/bus/expirelist.h index 2e7752fc..c843a446 100644 --- a/bus/expirelist.h +++ b/bus/expirelist.h @@ -32,7 +32,7 @@ typedef struct BusExpireList BusExpireList; typedef struct BusExpireItem BusExpireItem; typedef void (* BusExpireFunc) (BusExpireList *list, - BusExpireItem *item, + DBusList *link, void *data); struct BusExpireList @@ -31,9 +31,6 @@ some basic spec'ing out of the GLib/Qt level stubs/skels stuff will be needed to understand the right approach. - - there are various bits of code to manage ref/unref of data slots, that should - be merged into a generic facility - - assorted _-prefixed symbols in libdbus aren't actually used by libdbus, only by the message bus. These bloat up the library size. Not sure how to fix, really. @@ -73,7 +70,10 @@ async calls. - the invalid messages in the test suite are all useless because - they are invalid for the wrong reasons due to protocol changes + they are invalid for the wrong reasons due to protocol changes. + (Consider extending test suite to validate that they are + invalid for right reason, e.g. an "INVALID_ERROR Foo" line + in the message files) - I don't want to introduce DBusObject, but refcounting and object data could still be factored out into an internal "base class" @@ -111,3 +111,10 @@ - the GLib bindings varargs take DBUS_TYPE_WHATEVER and return stuff allocated with dbus_malloc(); should this be made more "G" at some expense in code duplication? + + - need to define bus behavior if you send a message to + yourself; is it an error, or allowed? If allowed, + we need to have a test for it in the test suite. + + - the max_replies_per_connection resource limit isn't implemented + |