diff --git a/bus/activation.c b/bus/activation.c index f2981e18..ffdc6fc5 100644 --- a/bus/activation.c +++ b/bus/activation.c @@ -1838,7 +1838,7 @@ bus_activation_activate_service (BusActivation *activation, if (auto_activation && entry != NULL && - !bus_context_check_security_policy (activation->context, + BUS_RESULT_TRUE != bus_context_check_security_policy (activation->context, transaction, connection, /* sender */ NULL, /* addressed recipient */ @@ -2137,7 +2137,7 @@ bus_activation_activate_service (BusActivation *activation, bus_connection_get_loginfo (connection)); /* Wonderful, systemd is connected, let's just send the msg */ res = bus_dispatch_matches (activation_transaction, NULL, - systemd, message, NULL, error); + systemd, message, NULL, error); if (res == BUS_RESULT_TRUE) retval = TRUE; diff --git a/bus/config-parser.c b/bus/config-parser.c index b54b0e4d..b5f1dd11 100644 --- a/bus/config-parser.c +++ b/bus/config-parser.c @@ -1615,7 +1615,7 @@ append_rule_from_element (BusConfigParser *parser, error)) return FALSE; - rule = bus_policy_rule_new (BUS_POLICY_RULE_SEND, access); + rule = bus_policy_rule_new (BUS_POLICY_RULE_SEND, access); if (rule == NULL) goto nomem; @@ -1720,7 +1720,7 @@ append_rule_from_element (BusConfigParser *parser, error)) return FALSE; - rule = bus_policy_rule_new (BUS_POLICY_RULE_RECEIVE, access); + rule = bus_policy_rule_new (BUS_POLICY_RULE_RECEIVE, access); if (rule == NULL) goto nomem; diff --git a/bus/connection.c b/bus/connection.c index f9e563ba..ee933847 100644 --- a/bus/connection.c +++ b/bus/connection.c @@ -2406,10 +2406,8 @@ bus_transaction_send_from_driver (BusTransaction *transaction, */ res = bus_context_check_security_policy (bus_transaction_get_context (transaction), transaction, - NULL, connection, connection, - message, NULL, &error, - &deferred_message); - + NULL, connection, connection, message, NULL, + &error, &deferred_message); if (res == BUS_RESULT_FALSE) { if (!bus_transaction_capture_error_reply (transaction, connection, diff --git a/bus/dispatch.c b/bus/dispatch.c index 625add56..4b84c217 100644 --- a/bus/dispatch.c +++ b/bus/dispatch.c @@ -215,9 +215,11 @@ bus_dispatch_matches (BusTransaction *transaction, result = BUS_RESULT_LATER; if (result == BUS_RESULT_LATER) - result = bus_context_check_security_policy(context, transaction, - sender, addressed_recipient, addressed_recipient, message, NULL, error, - &deferred_message); + result = bus_context_check_security_policy (context, transaction, + sender, addressed_recipient, + addressed_recipient, + message, NULL, error, + &deferred_message); if (result == BUS_RESULT_FALSE) return BUS_RESULT_FALSE; @@ -496,9 +498,8 @@ bus_dispatch (DBusConnection *connection, } res = bus_context_check_security_policy (context, transaction, - connection, NULL, NULL, message, - NULL, &error, - &deferred_message); + connection, NULL, NULL, message, NULL, + &error, &deferred_message); if (res == BUS_RESULT_FALSE) { _dbus_verbose ("Security policy rejected message\n"); diff --git a/bus/driver.c b/bus/driver.c index 5ee60cb6..aaeb3b24 100644 --- a/bus/driver.c +++ b/bus/driver.c @@ -2286,7 +2286,7 @@ out: return ret; } -static dbus_bool_t +static BusResult bus_driver_handle_get_machine_id (DBusConnection *connection, BusTransaction *transaction, DBusMessage *message, @@ -2301,7 +2301,7 @@ bus_driver_handle_get_machine_id (DBusConnection *connection, if (!_dbus_string_init (&uuid)) { BUS_SET_OOM (error); - return FALSE; + return BUS_RESULT_FALSE; } if (!_dbus_get_local_machine_uuid_encoded (&uuid, error)) @@ -2326,7 +2326,7 @@ bus_driver_handle_get_machine_id (DBusConnection *connection, _dbus_string_free (&uuid); dbus_message_unref (reply); - return TRUE; + return BUS_RESULT_TRUE; oom: _DBUS_ASSERT_ERROR_IS_CLEAR (error); @@ -2340,29 +2340,30 @@ fail: dbus_message_unref (reply); _dbus_string_free (&uuid); - return FALSE; + return BUS_RESULT_FALSE; } -static dbus_bool_t +static BusResult bus_driver_handle_ping (DBusConnection *connection, BusTransaction *transaction, DBusMessage *message, DBusError *error) { - return bus_driver_send_ack_reply (connection, transaction, message, error); + return bus_driver_send_ack_reply (connection, transaction, message, error) == TRUE + ? BUS_RESULT_TRUE : BUS_RESULT_FALSE; } -static dbus_bool_t bus_driver_handle_get (DBusConnection *connection, +static BusResult bus_driver_handle_get (DBusConnection *connection, BusTransaction *transaction, DBusMessage *message, DBusError *error); -static dbus_bool_t bus_driver_handle_get_all (DBusConnection *connection, +static BusResult bus_driver_handle_get_all (DBusConnection *connection, BusTransaction *transaction, DBusMessage *message, DBusError *error); -static dbus_bool_t bus_driver_handle_set (DBusConnection *connection, +static BusResult bus_driver_handle_set (DBusConnection *connection, BusTransaction *transaction, DBusMessage *message, DBusError *error); @@ -2833,6 +2834,38 @@ bus_driver_handle_introspect (DBusConnection *connection, return BUS_RESULT_FALSE; } +/* + * Set @error and return FALSE if the message is not directed to the + * dbus-daemon by its canonical object path. This is hardening against + * system services with poorly-written security policy files, which + * might allow sending dangerously broad equivalence classes of messages + * such as "anything with this assumed-to-be-safe object path". + * + * dbus-daemon is unusual in that it normally ignores the object path + * of incoming messages; we need to keep that behaviour for the "read" + * read-only method calls like GetConnectionUnixUser for backwards + * compatibility, but it seems safer to be more restrictive for things + * intended to be root-only or privileged-developers-only. + * + * It is possible that there are other system services with the same + * quirk as dbus-daemon. + */ +dbus_bool_t +bus_driver_check_message_is_for_us (DBusMessage *message, + DBusError *error) +{ + if (!dbus_message_has_path (message, DBUS_PATH_DBUS)) + { + dbus_set_error (error, DBUS_ERROR_ACCESS_DENIED, + "Method '%s' is only available at the canonical object path '%s'", + dbus_message_get_member (message), DBUS_PATH_DBUS); + + return FALSE; + } + + return TRUE; +} + BusResult bus_driver_handle_message (DBusConnection *connection, BusTransaction *transaction, @@ -3112,7 +3145,7 @@ interface_handler_find_property (const InterfaceHandler *ih, return NULL; } -static dbus_bool_t +static BusResult bus_driver_handle_get (DBusConnection *connection, BusTransaction *transaction, DBusMessage *message, @@ -3133,18 +3166,18 @@ bus_driver_handle_get (DBusConnection *connection, DBUS_TYPE_STRING, &iface, DBUS_TYPE_STRING, &prop, DBUS_TYPE_INVALID)) - return FALSE; + return BUS_RESULT_FALSE; /* We only implement Properties on /org/freedesktop/DBus so far. */ ih = bus_driver_find_interface (iface, TRUE, error); if (ih == NULL) - return FALSE; + return BUS_RESULT_FALSE; handler = interface_handler_find_property (ih, prop, error); if (handler == NULL) - return FALSE; + return BUS_RESULT_FALSE; context = bus_transaction_get_context (transaction); @@ -3172,17 +3205,17 @@ bus_driver_handle_get (DBusConnection *connection, goto oom; dbus_message_unref (reply); - return TRUE; + return BUS_RESULT_TRUE; oom: if (reply != NULL) dbus_message_unref (reply); BUS_SET_OOM (error); - return FALSE; + return BUS_RESULT_FALSE; } -static dbus_bool_t +static BusResult bus_driver_handle_get_all (DBusConnection *connection, BusTransaction *transaction, DBusMessage *message, @@ -3201,13 +3234,13 @@ bus_driver_handle_get_all (DBusConnection *connection, if (!dbus_message_get_args (message, error, DBUS_TYPE_STRING, &iface, DBUS_TYPE_INVALID)) - return FALSE; + return BUS_RESULT_FALSE; /* We only implement Properties on /org/freedesktop/DBus so far. */ ih = bus_driver_find_interface (iface, TRUE, error); if (ih == NULL) - return FALSE; + return BUS_RESULT_FALSE; context = bus_transaction_get_context (transaction); @@ -3242,7 +3275,7 @@ bus_driver_handle_get_all (DBusConnection *connection, goto oom; dbus_message_unref (reply); - return TRUE; + return BUS_RESULT_TRUE; oom_abandon_message: _dbus_asv_abandon (&reply_iter, &array_iter); @@ -3252,10 +3285,10 @@ oom: dbus_message_unref (reply); BUS_SET_OOM (error); - return FALSE; + return BUS_RESULT_FALSE; } -static dbus_bool_t +static BusResult bus_driver_handle_set (DBusConnection *connection, BusTransaction *transaction, DBusMessage *message, @@ -3284,15 +3317,15 @@ bus_driver_handle_set (DBusConnection *connection, ih = bus_driver_find_interface (iface, TRUE, error); if (ih == NULL) - return FALSE; + return BUS_RESULT_FALSE; handler = interface_handler_find_property (ih, prop, error); if (handler == NULL) - return FALSE; + return BUS_RESULT_FALSE; /* We don't implement any properties that can be set yet. */ dbus_set_error (error, DBUS_ERROR_PROPERTY_READ_ONLY, "Property '%s.%s' cannot be set", iface, prop); - return FALSE; + return BUS_RESULT_FALSE; } diff --git a/bus/policy.h b/bus/policy.h index 39d7cc54..28ce8f2f 100644 --- a/bus/policy.h +++ b/bus/policy.h @@ -161,26 +161,26 @@ dbus_bool_t bus_policy_merge (BusPolicy *policy, BusClientPolicy* bus_client_policy_new (void); BusClientPolicy* bus_client_policy_ref (BusClientPolicy *policy); void bus_client_policy_unref (BusClientPolicy *policy); -BusResult bus_client_policy_check_can_send (DBusConnection *sender, - BusClientPolicy *policy, - BusRegistry *registry, - dbus_bool_t requested_reply, - DBusConnection *addressed_recipient, - DBusConnection *receiver, - DBusMessage *message, - dbus_int32_t *toggles, - dbus_bool_t *log, - const char **privilege_param, +BusResult bus_client_policy_check_can_send (DBusConnection *sender, + BusClientPolicy *policy, + BusRegistry *registry, + dbus_bool_t requested_reply, + DBusConnection *addressed_recipient, + DBusConnection *receiver, + DBusMessage *message, + dbus_int32_t *toggles, + dbus_bool_t *log, + const char **privilege_param, BusDeferredMessage **deferred_message); -BusResult bus_client_policy_check_can_receive (BusClientPolicy *policy, - BusRegistry *registry, - dbus_bool_t requested_reply, - DBusConnection *sender, - DBusConnection *addressed_recipient, - DBusConnection *proposed_recipient, - DBusMessage *message, - dbus_int32_t *toggles, - const char **privilege_param, +BusResult bus_client_policy_check_can_receive (BusClientPolicy *policy, + BusRegistry *registry, + dbus_bool_t requested_reply, + DBusConnection *sender, + DBusConnection *addressed_recipient, + DBusConnection *proposed_recipient, + DBusMessage *message, + dbus_int32_t *toggles, + const char **privilege_param, BusDeferredMessage **deferred_message); BusResult bus_client_policy_check_can_own (BusClientPolicy *policy, const DBusString *service_name, diff --git a/bus/stats.c b/bus/stats.c index 4ba72d6e..c25be98e 100644 --- a/bus/stats.c +++ b/bus/stats.c @@ -51,6 +51,9 @@ bus_stats_handle_get_stats (DBusConnection *connection, _DBUS_ASSERT_ERROR_IS_CLEAR (error); + if (!bus_driver_check_message_is_for_us (message, error)) + return BUS_RESULT_FALSE; + context = bus_transaction_get_context (transaction); connections = bus_context_get_connections (context);