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
This commit is contained in:
Havoc Pennington 2003-10-14 05:16:56 +00:00
parent bfb5de511c
commit 64f5ae1a79
11 changed files with 527 additions and 19 deletions

View file

@ -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:

View file

@ -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;
}

View file

@ -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,

View file

@ -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,

View file

@ -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;

View file

@ -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);

View file

@ -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

View file

@ -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))

View file

@ -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;
}

View file

@ -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

View file

@ -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