Commit e8d396ef authored by Havoc Pennington's avatar Havoc Pennington

2003-04-27 Havoc Pennington <hp@pobox.com>

	Unbreak my code...

	* dbus/dbus-transport.c (_dbus_transport_get_dispatch_status):
	report correct status if we finish processing authentication
	inside this function.

	* bus/activation.c (try_send_activation_failure): use
	bus_transaction_send_error_reply

	* bus/connection.c (bus_connection_get_groups): return an error
	explaining the problem

	* bus/bus.c (bus_context_check_security_policy): implement
	restriction here that inactive connections can only send the
	hello message. Also, allow bus driver to send anything to
	any recipient.

	* bus/connection.c (bus_connection_complete): create the
	BusClientPolicy here instead of on-demand.
	(bus_connection_get_policy): don't return an error

	* dbus/dbus-message.c (dbus_message_new_error_reply): allow NULL
	sender field in message being replied to

	* bus/bus.c (bus_context_check_security_policy): fix silly typo
	causing it to return FALSE always

	* bus/policy.c (bus_client_policy_check_can_send): fix bug where
	we checked sender rather than destination
parent b3bd48ed
2003-04-27 Havoc Pennington <hp@pobox.com>
Unbreak my code...
* dbus/dbus-transport.c (_dbus_transport_get_dispatch_status):
report correct status if we finish processing authentication
inside this function.
* bus/activation.c (try_send_activation_failure): use
bus_transaction_send_error_reply
* bus/connection.c (bus_connection_get_groups): return an error
explaining the problem
* bus/bus.c (bus_context_check_security_policy): implement
restriction here that inactive connections can only send the
hello message. Also, allow bus driver to send anything to
any recipient.
* bus/connection.c (bus_connection_complete): create the
BusClientPolicy here instead of on-demand.
(bus_connection_get_policy): don't return an error
* dbus/dbus-message.c (dbus_message_new_error_reply): allow NULL
sender field in message being replied to
* bus/bus.c (bus_context_check_security_policy): fix silly typo
causing it to return FALSE always
* bus/policy.c (bus_client_policy_check_can_send): fix bug where
we checked sender rather than destination
2003-04-25 Havoc Pennington <hp@redhat.com>
test suite is slightly hosed at the moment, will fix soon
......
......@@ -641,7 +641,6 @@ try_send_activation_failure (BusPendingActivation *pending_activation,
const DBusError *how)
{
BusActivation *activation;
DBusMessage *message;
DBusList *link;
BusTransaction *transaction;
......@@ -659,21 +658,13 @@ try_send_activation_failure (BusPendingActivation *pending_activation,
if (dbus_connection_get_is_connected (entry->connection))
{
message = dbus_message_new_error_reply (entry->activation_message,
how->name,
how->message);
if (!message)
if (!bus_transaction_send_error_reply (transaction,
entry->connection,
how,
entry->activation_message))
goto error;
if (!bus_transaction_send_from_driver (transaction, entry->connection, message))
{
dbus_message_unref (message);
goto error;
}
dbus_message_unref (message);
}
link = next;
}
......
......@@ -789,9 +789,12 @@ bus_context_allow_user (BusContext *context,
BusClientPolicy*
bus_context_create_client_policy (BusContext *context,
DBusConnection *connection)
DBusConnection *connection,
DBusError *error)
{
return bus_policy_create_client_policy (context->policy, connection);
_DBUS_ASSERT_ERROR_IS_CLEAR (error);
return bus_policy_create_client_policy (context->policy, connection,
error);
}
int
......@@ -848,36 +851,75 @@ bus_context_check_security_policy (BusContext *context,
BusClientPolicy *recipient_policy;
/* NULL sender/receiver means the bus driver */
if (sender != NULL)
{
_dbus_assert (dbus_connection_get_is_authenticated (sender));
sender_policy = bus_connection_get_policy (sender, error);
if (sender_policy == NULL)
if (bus_connection_is_active (sender))
{
_DBUS_ASSERT_ERROR_IS_SET (error);
return FALSE;
sender_policy = bus_connection_get_policy (sender);
_dbus_assert (sender_policy != NULL);
}
else
{
/* Policy for inactive connections is that they can only send
* the hello message to the bus driver
*/
if (recipient == NULL &&
dbus_message_has_name (message, DBUS_MESSAGE_HELLO))
{
_dbus_verbose ("security check allowing %s message\n",
DBUS_MESSAGE_HELLO);
return TRUE;
}
else
{
_dbus_verbose ("security check disallowing non-%s message\n",
DBUS_MESSAGE_HELLO);
dbus_set_error (error, DBUS_ERROR_ACCESS_DENIED,
"Client tried to send a message other than %s without being registered",
DBUS_MESSAGE_HELLO);
return FALSE;
}
}
return FALSE;
}
else
sender_policy = NULL;
_dbus_assert ((sender != NULL && sender_policy != NULL) ||
(sender == NULL && sender_policy == NULL));
if (recipient != NULL)
{
_dbus_assert (dbus_connection_get_is_authenticated (recipient));
recipient_policy = bus_connection_get_policy (recipient, error);
if (recipient_policy == NULL)
/* only the bus driver can send to an inactive recipient (as it
* owns no services, so other apps can't address it). Inactive
* recipients can receive any message.
*/
if (bus_connection_is_active (recipient))
{
_DBUS_ASSERT_ERROR_IS_SET (error);
return FALSE;
recipient_policy = bus_connection_get_policy (recipient);
_dbus_assert (recipient_policy != NULL);
}
else if (sender == NULL)
{
_dbus_verbose ("security check using NULL recipient policy for message from bus\n");
recipient_policy = NULL;
}
else
{
_dbus_assert_not_reached ("a message was somehow sent to an inactive recipient from a source other than the message bus\n");
recipient_policy = NULL;
}
return FALSE;
}
else
recipient_policy = NULL;
if (sender &&
_dbus_assert ((recipient != NULL && recipient_policy != NULL) ||
(recipient != NULL && sender == NULL && recipient_policy == NULL) ||
(recipient == NULL && recipient_policy == NULL));
if (sender_policy &&
!bus_client_policy_check_can_send (sender_policy,
context->registry, recipient,
message))
......@@ -893,7 +935,7 @@ bus_context_check_security_policy (BusContext *context,
return FALSE;
}
if (recipient &&
if (recipient_policy &&
!bus_client_policy_check_can_receive (recipient_policy,
context->registry, sender,
message))
......
......@@ -72,7 +72,9 @@ DBusUserDatabase* bus_context_get_user_database (BusContext
dbus_bool_t bus_context_allow_user (BusContext *context,
unsigned long uid);
BusClientPolicy* bus_context_create_client_policy (BusContext *context,
DBusConnection *connection);
DBusConnection *connection,
DBusError *error);
int bus_context_get_activation_timeout (BusContext *context);
int bus_context_get_auth_timeout (BusContext *context);
int bus_context_get_max_completed_connections (BusContext *context);
......
......@@ -295,17 +295,17 @@ bus_config_parser_new (const DBusString *basedir)
parser->limits.max_incoming_bytes = _DBUS_ONE_MEGABYTE * 63;
parser->limits.max_outgoing_bytes = _DBUS_ONE_MEGABYTE * 63;
parser->limits.max_message_size = _DBUS_ONE_MEGABYTE * 32;
#ifdef DBUS_BUILD_TESTS
parser->limits.activation_timeout = 6000; /* 6 seconds */
#else
parser->limits.activation_timeout = 15000; /* 15 seconds */
#endif
/* Making this long means the user has to wait longer for an error
* message if something screws up, but making it too short means
* they might see a false failure.
*/
parser->limits.activation_timeout = 25000; /* 25 seconds */
/* Making this long risks making a DOS attack easier, but too short
* and legitimate auth will fail. If interactive auth (ask user for
* password) is allowed, then potentially it has to be quite long.
*/
*/
parser->limits.auth_timeout = 30000; /* 30 seconds */
parser->limits.max_incomplete_connections = 32;
......
......@@ -748,7 +748,8 @@ expire_incomplete_timeout (void *data)
dbus_bool_t
bus_connection_get_groups (DBusConnection *connection,
unsigned long **groups,
int *n_groups)
int *n_groups,
DBusError *error)
{
BusConnectionData *d;
unsigned long uid;
......@@ -767,8 +768,9 @@ bus_connection_get_groups (DBusConnection *connection,
{
if (!_dbus_user_database_get_groups (user_database,
uid, groups, n_groups,
NULL))
error))
{
_DBUS_ASSERT_ERROR_IS_SET (error);
_dbus_verbose ("Did not get any groups for UID %lu\n",
uid);
return FALSE;
......@@ -792,7 +794,8 @@ bus_connection_is_in_group (DBusConnection *connection,
unsigned long *group_ids;
int n_group_ids;
if (!bus_connection_get_groups (connection, &group_ids, &n_group_ids))
if (!bus_connection_get_groups (connection, &group_ids, &n_group_ids,
NULL))
return FALSE;
i = 0;
......@@ -811,47 +814,14 @@ bus_connection_is_in_group (DBusConnection *connection,
}
BusClientPolicy*
bus_connection_get_policy (DBusConnection *connection,
DBusError *error)
bus_connection_get_policy (DBusConnection *connection)
{
BusConnectionData *d;
d = BUS_CONNECTION_DATA (connection);
_dbus_assert (d != NULL);
if (!dbus_connection_get_is_authenticated (connection))
{
_dbus_verbose ("Tried to get policy for unauthenticated connection!\n");
dbus_set_error (error, DBUS_ERROR_ACCESS_DENIED,
"Connection is not yet authenticated; the pre-authentication "
"implicit security policy is to deny everything");
return NULL;
}
/* We do lazy creation of the policy because
* it can only be done post-authentication.
*/
if (d->policy == NULL)
{
d->policy =
bus_context_create_client_policy (d->connections->context,
connection);
/* we may have a NULL policy on OOM or error getting list of
* groups for a user. In the latter case we don't handle it so
* well currently, as it will just keep failing over and over.
*/
}
if (d->policy == NULL)
{
dbus_set_error (error, DBUS_ERROR_ACCESS_DENIED,
"There was an error creating the security policy for connection \"%s\"; "
"all operations will fail for now.",
d->name ? d->name : "(inactive)");
return NULL;
}
_dbus_assert (d->policy != NULL);
return d->policy;
}
......@@ -1142,8 +1112,9 @@ bus_connection_get_n_services_owned (DBusConnection *connection)
}
dbus_bool_t
bus_connection_set_name (DBusConnection *connection,
const DBusString *name)
bus_connection_complete (DBusConnection *connection,
const DBusString *name,
DBusError *error)
{
BusConnectionData *d;
unsigned long uid;
......@@ -1151,19 +1122,43 @@ bus_connection_set_name (DBusConnection *connection,
d = BUS_CONNECTION_DATA (connection);
_dbus_assert (d != NULL);
_dbus_assert (d->name == NULL);
_dbus_assert (d->policy == NULL);
if (!_dbus_string_copy_data (name, &d->name))
return FALSE;
{
BUS_SET_OOM (error);
return FALSE;
}
_dbus_assert (d->name != NULL);
_dbus_verbose ("Name %s assigned to %p\n", d->name, connection);
d->policy = bus_context_create_client_policy (d->connections->context,
connection,
error);
/* we may have a NULL policy on OOM or error getting list of
* groups for a user. In the latter case we don't handle it so
* well currently, as it will just keep failing over and over.
*/
if (d->policy == NULL)
{
_dbus_verbose ("Failed to create security policy for connection %p\n",
connection);
_DBUS_ASSERT_ERROR_IS_SET (error);
dbus_free (d->name);
d->name = NULL;
return FALSE;
}
if (dbus_connection_get_unix_user (connection, &uid))
{
if (!adjust_connections_for_uid (d->connections,
uid, 1))
{
BUS_SET_OOM (error);
dbus_free (d->name);
d->name = NULL;
return FALSE;
......@@ -1586,7 +1581,10 @@ bus_transaction_send_error_reply (BusTransaction *transaction,
_dbus_assert (error != NULL);
_DBUS_ASSERT_ERROR_IS_SET (error);
_dbus_verbose ("Sending error reply %s \"%s\"\n",
error->name, error->message);
reply = dbus_message_new_error_reply (in_reply_to,
error->name,
error->message);
......
......@@ -56,11 +56,13 @@ void bus_connections_expire_incomplete (BusConnections
dbus_bool_t bus_connection_is_active (DBusConnection *connection);
const char *bus_connection_get_name (DBusConnection *connection);
dbus_bool_t bus_connection_preallocate_oom_error (DBusConnection *connection);
void bus_connection_send_oom_error (DBusConnection *connection,
DBusMessage *in_reply_to);
/* called by services.c */
dbus_bool_t bus_connection_add_owned_service (DBusConnection *connection,
BusService *service);
......@@ -71,9 +73,9 @@ void bus_connection_add_owned_service_link (DBusConnection *connection,
int bus_connection_get_n_services_owned (DBusConnection *connection);
/* called by driver.c */
dbus_bool_t bus_connection_set_name (DBusConnection *connection,
const DBusString *name);
const char *bus_connection_get_name (DBusConnection *connection);
dbus_bool_t bus_connection_complete (DBusConnection *connection,
const DBusString *name,
DBusError *error);
/* called by dispatch.c when the connection is dropped */
void bus_connection_disconnected (DBusConnection *connection);
......@@ -82,9 +84,9 @@ dbus_bool_t bus_connection_is_in_group (DBusConnection *connection,
unsigned long gid);
dbus_bool_t bus_connection_get_groups (DBusConnection *connection,
unsigned long **groups,
int *n_groups);
BusClientPolicy* bus_connection_get_policy (DBusConnection *connection,
int *n_groups,
DBusError *error);
BusClientPolicy* bus_connection_get_policy (DBusConnection *connection);
/* transaction API so we can send or not send a block of messages as a whole */
......
......@@ -183,10 +183,14 @@ bus_dispatch (DBusConnection *connection,
{
if (!bus_context_check_security_policy (context,
connection, NULL, message, &error))
goto out;
{
_dbus_verbose ("Security policy rejected message\n");
goto out;
}
_dbus_verbose ("Giving message to %s\n", DBUS_SERVICE_DBUS);
if (!bus_driver_handle_message (connection, transaction, message, &error))
goto out;
goto out;
}
else if (!bus_connection_is_active (connection)) /* clients must talk to bus driver first */
{
......@@ -249,6 +253,7 @@ bus_dispatch (DBusConnection *connection,
/* If we disconnected it, we won't bother to send it any error
* messages.
*/
_dbus_verbose ("Not sending error to connection we disconnected\n");
}
else if (dbus_error_has_name (&error, DBUS_ERROR_NO_MEMORY))
{
......
......@@ -298,9 +298,9 @@ bus_driver_handle_hello (DBusConnection *connection,
goto out_0;
}
if (!bus_connection_set_name (connection, &unique_name))
if (!bus_connection_complete (connection, &unique_name, error))
{
BUS_SET_OOM (error);
_DBUS_ASSERT_ERROR_IS_SET (error);
goto out_0;
}
......@@ -627,15 +627,8 @@ bus_driver_handle_message (DBusConnection *connection,
name = dbus_message_get_name (message);
sender = dbus_message_get_sender (message);
if (sender == NULL && (strcmp (name, DBUS_MESSAGE_HELLO) != 0))
{
dbus_set_error (error, DBUS_ERROR_ACCESS_DENIED,
"Client tried to send a message other than %s without being registered",
DBUS_MESSAGE_HELLO);
dbus_connection_disconnect (connection);
return FALSE;
}
/* security checks should have kept this from getting here */
_dbus_assert (sender != NULL || strcmp (name, DBUS_MESSAGE_HELLO) == 0);
if (dbus_message_get_reply_serial (message) == 0)
{
......
......@@ -24,6 +24,7 @@
#include "policy.h"
#include "services.h"
#include "test.h"
#include "utils.h"
#include <dbus/dbus-list.h>
#include <dbus/dbus-hash.h>
#include <dbus/dbus-internals.h>
......@@ -233,20 +234,22 @@ add_list_to_client (DBusList **list,
BusClientPolicy*
bus_policy_create_client_policy (BusPolicy *policy,
DBusConnection *connection)
DBusConnection *connection,
DBusError *error)
{
BusClientPolicy *client;
unsigned long uid;
_dbus_assert (dbus_connection_get_is_authenticated (connection));
_DBUS_ASSERT_ERROR_IS_CLEAR (error);
client = bus_client_policy_new ();
if (client == NULL)
return NULL;
goto nomem;
if (!add_list_to_client (&policy->default_rules,
client))
goto failed;
goto nomem;
/* we avoid the overhead of looking up user's groups
* if we don't have any group rules anyway
......@@ -257,7 +260,7 @@ bus_policy_create_client_policy (BusPolicy *policy,
int n_groups;
int i;
if (!bus_connection_get_groups (connection, &groups, &n_groups))
if (!bus_connection_get_groups (connection, &groups, &n_groups, error))
goto failed;
i = 0;
......@@ -273,7 +276,7 @@ bus_policy_create_client_policy (BusPolicy *policy,
if (!add_list_to_client (list, client))
{
dbus_free (groups);
goto failed;
goto nomem;
}
}
......@@ -284,7 +287,11 @@ bus_policy_create_client_policy (BusPolicy *policy,
}
if (!dbus_connection_get_unix_user (connection, &uid))
goto failed;
{
dbus_set_error (error, DBUS_ERROR_FAILED,
"No user ID known for connection, cannot determine security policy\n");
goto failed;
}
if (_dbus_hash_table_get_n_entries (policy->rules_by_uid) > 0)
{
......@@ -296,20 +303,24 @@ bus_policy_create_client_policy (BusPolicy *policy,
if (list != NULL)
{
if (!add_list_to_client (list, client))
goto failed;
goto nomem;
}
}
if (!add_list_to_client (&policy->mandatory_rules,
client))
goto failed;
goto nomem;
bus_client_policy_optimize (client);
return client;
nomem:
BUS_SET_OOM (error);
failed:
bus_client_policy_unref (client);
_DBUS_ASSERT_ERROR_IS_SET (error);
if (client)
bus_client_policy_unref (client);
return NULL;
}
......@@ -675,6 +686,8 @@ bus_client_policy_check_can_send (BusClientPolicy *policy,
* in the config file, i.e. last rule that applies wins
*/
_dbus_verbose (" (policy) checking send rules\n");
allowed = FALSE;
link = _dbus_list_get_first_link (&policy->rules);
while (link != NULL)
......@@ -689,13 +702,19 @@ bus_client_policy_check_can_send (BusClientPolicy *policy,
*/
if (rule->type != BUS_POLICY_RULE_SEND)
continue;
{
_dbus_verbose (" (policy) skipping non-send rule\n");
continue;
}
if (rule->d.send.message_name != NULL)
{
if (!dbus_message_has_name (message,
rule->d.send.message_name))
continue;
{
_dbus_verbose (" (policy) skipping rule for different message name\n");
continue;
}
}
if (rule->d.send.destination != NULL)
......@@ -707,9 +726,13 @@ bus_client_policy_check_can_send (BusClientPolicy *policy,
*/
if (receiver == NULL)
{
if (!dbus_message_has_sender (message,
rule->d.send.destination))
continue;
if (!dbus_message_has_destination (message,
rule->d.send.destination))
{
_dbus_verbose (" (policy) skipping rule because message dest is not %s\n",
rule->d.send.destination);
continue;
}
}
else
{
......@@ -720,15 +743,26 @@ bus_client_policy_check_can_send (BusClientPolicy *policy,
service = bus_registry_lookup (registry, &str);
if (service == NULL)
continue;
{
_dbus_verbose (" (policy) skipping rule because dest %s doesn't exist\n",
rule->d.send.destination);
continue;
}
if (!bus_service_has_owner (service, receiver))
continue;
{
_dbus_verbose (" (policy) skipping rule because dest %s isn't owned by receiver\n",
rule->d.send.destination);
continue;
}
}
}
/* Use this rule */
allowed = rule->allow;
_dbus_verbose (" (policy) used rule, allow now = %d\n",
allowed);
}
return allowed;
......@@ -747,6 +781,8 @@ bus_client_policy_check_can_receive (BusClientPolicy *policy,
* in the config file, i.e. last rule that applies wins
*/
_dbus_verbose (" (policy) checking receive rules\n");
allowed = FALSE;
link = _dbus_list_get_first_link (&policy->rules);
while (link != NULL)
......@@ -761,13 +797,19 @@ bus_client_policy_check_can_receive (BusClientPolicy *policy,
*/
if (rule->type != BUS_POLICY_RULE_RECEIVE)
continue;
{
_dbus_verbose (" (policy) skipping non-receive rule\n");
continue;
}
if (rule->d.receive.message_name != NULL)
{
if (!dbus_message_has_name (message,
rule->d.receive.message_name))
continue;
{
_dbus_verbose (" (policy) skipping rule for different message name\n");
continue;
}
}
if (rule->d.receive.origin != NULL)
......@@ -781,7 +823,11 @@ bus_client_policy_check_can_receive (BusClientPolicy *policy,
{
if (!dbus_message_has_sender (message,
rule->d.receive.origin))
continue;
{
_dbus_verbose (" (policy) skipping rule because message sender is not %s\n",
rule->d.receive.origin);
continue;
}
}
else
{
......@@ -793,15 +839,26 @@ bus_client_policy_check_can_receive (BusClientPolicy *policy,
service = bus_registry_lookup (registry, &str);
if (service == NULL)
continue;
{
_dbus_verbose (" (policy) skipping rule because origin %s doesn't exist\n",
rule->d.receive.origin);
continue;
}
if (!bus_service_has_owner (service, sender))
continue;
{
_dbus_verbose (" (policy) skipping rule because origin %s isn't owned by sender\n",
rule->d.receive.origin);
continue;
}
}
}
/* Use this rule */
allowed = rule->allow;
_dbus_verbose (" (policy) used rule, allow now = %d\n",
allowed);
}
return allowed;
......
......@@ -96,7 +96,8 @@ BusPolicy* bus_policy_new (void);
void bus_policy_ref (BusPolicy *policy);
void bus_policy_unref (BusPolicy *policy);
BusClientPolicy* bus_policy_create_client_policy (BusPolicy *policy,
DBusConnection *connection);
DBusConnection *connection,
DBusError *error);
dbus_bool_t bus_policy_allow_user (BusPolicy *policy,
DBusUserDatabase *user_database,
unsigned long uid);
......
......@@ -286,12 +286,8 @@ bus_registry_acquire_service (BusRegistry *registry,
goto out;
}
policy = bus_connection_get_policy (connection, error);
if (policy == NULL)
{
_DBUS_ASSERT_ERROR_IS_SET (error);
goto out;
}
policy = bus_connection_get_policy (connection);
_dbus_assert (policy != NULL);
if (!bus_client_policy_check_can_own (policy, connection,
service_name))
......
......@@ -304,7 +304,11 @@ bus_test_run_clients_loop (dbus_bool_t block_once)
_dbus_loop_dispatch (client_loop);
/* Do one blocking wait, since we're expecting data */
_dbus_loop_iterate (client_loop, block_once);
if (block_once)
{
_dbus_verbose ("---> blocking on \"client side\"\n");
_dbus_loop_iterate (client_loop, TRUE);
}
/* Then mop everything up */