fdo-mirrors/dbus
1
0
Fork 0
mirror of https://gitlab.freedesktop.org/dbus/dbus.git synced 2026-04-28 08:50:41 +02:00
Code Issues Projects Releases Packages Wiki Activity Actions

Fix confusion between "is it authenticated?" and "try to authenticate"

Browse source 
Historically, _dbus_transport_get_is_authenticated() has had the
side-effect of trying to advance the authentication state machine (if
there's enough buffered input to do so). This seems an inappropriate
activity for what looks like a simple getter.

Split it into _dbus_transport_try_to_authenticate (which does what it
always used to do) and _dbus_transport_peek_is_authenticated (which
is the simple getter version).

To minimize the difference in behaviour for the stable branch of D-Bus,
I've only used _dbus_transport_peek_is_authenticated where it was used
in an assertion, which should clearly not have side effects (and I've
checked that the asserting function cannot be called until both
authentication and authorization have completed). Replacing most of the
calls to get_is_authenticated with try_to_authenticate is a possible
piece of future work.

Based on patches from Cosimo Alfarano, who noticed this
assertion-with-side-effects.

Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
https://bugs.freedesktop.org/show_bug.cgi?id=39720
Reviewed-by: Cosimo Alfarano <cosimo.alfarano@collabora.com>
This commit is contained in:
Simon McVittie 2013-08-23 00:43:02 +02:00 • committed by Ralf Habacker
parent 1809c7ad2a
commit 36bb2125d1
5 changed files with 52 additions and 28 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

26
dbus/dbus-connection.c
View file

@ -490,9 +490,9 @@ _dbus_connection_queue_received_message_link (DBusConnection *connection,
DBusPendingCall *pending;
dbus_uint32_t reply_serial;
DBusMessage *message;
_dbus_assert (_dbus_transport_get_is_authenticated (connection->transport));
_dbus_assert (_dbus_transport_peek_is_authenticated (connection->transport));
_dbus_list_append_link (&connection->incoming_messages,
link);
message = link->data;
@ -2977,9 +2977,9 @@ dbus_connection_get_is_authenticated (DBusConnection *connection)
dbus_bool_t res;
_dbus_return_val_if_fail (connection != NULL, FALSE);
CONNECTION_LOCK (connection);
res = _dbus_transport_get_is_authenticated (connection->transport);
res = _dbus_transport_try_to_authenticate (connection->transport);
CONNECTION_UNLOCK (connection);
return res;
@ -5174,10 +5174,10 @@ dbus_connection_get_unix_user (DBusConnection *connection,
_dbus_return_val_if_fail (connection != NULL, FALSE);
_dbus_return_val_if_fail (uid != NULL, FALSE);
CONNECTION_LOCK (connection);
if (!_dbus_transport_get_is_authenticated (connection->transport))
if (!_dbus_transport_try_to_authenticate (connection->transport))
result = FALSE;
else
result = _dbus_transport_get_unix_user (connection->transport,
@ -5210,10 +5210,10 @@ dbus_connection_get_unix_process_id (DBusConnection *connection,
_dbus_return_val_if_fail (connection != NULL, FALSE);
_dbus_return_val_if_fail (pid != NULL, FALSE);
CONNECTION_LOCK (connection);
if (!_dbus_transport_get_is_authenticated (connection->transport))
if (!_dbus_transport_try_to_authenticate (connection->transport))
result = FALSE;
else
result = _dbus_transport_get_unix_process_id (connection->transport,
@ -5245,10 +5245,10 @@ dbus_connection_get_adt_audit_session_data (DBusConnection *connection,
_dbus_return_val_if_fail (connection != NULL, FALSE);
_dbus_return_val_if_fail (data != NULL, FALSE);
_dbus_return_val_if_fail (data_size != NULL, FALSE);
CONNECTION_LOCK (connection);
if (!_dbus_transport_get_is_authenticated (connection->transport))
if (!_dbus_transport_try_to_authenticate (connection->transport))
result = FALSE;
else
result = _dbus_transport_get_adt_audit_session_data (connection->transport,
@ -5341,10 +5341,10 @@ dbus_connection_get_windows_user (DBusConnection *connection,
_dbus_return_val_if_fail (connection != NULL, FALSE);
_dbus_return_val_if_fail (windows_sid_p != NULL, FALSE);
CONNECTION_LOCK (connection);
if (!_dbus_transport_get_is_authenticated (connection->transport))
if (!_dbus_transport_try_to_authenticate (connection->transport))
result = FALSE;
else
result = _dbus_transport_get_windows_user (connection->transport,

2
dbus/dbus-transport-protected.h
View file

@ -111,7 +111,7 @@ struct DBusTransport
DBusFreeFunction free_windows_user_data; /**< Function to free windows_user_data */
unsigned int disconnected : 1; /**< #TRUE if we are disconnected. */
unsigned int authenticated : 1; /**< Cache of auth state; use _dbus_transport_get_is_authenticated() to query value */
unsigned int authenticated : 1; /**< Cache of auth state; use _dbus_transport_peek_is_authenticated() to query value */
unsigned int send_credentials_pending : 1; /**< #TRUE if we need to send credentials */
unsigned int receive_credentials_pending : 1; /**< #TRUE if we need to receive credentials */
unsigned int is_server : 1; /**< #TRUE if on the server side */

16
dbus/dbus-transport-socket.c
View file

@ -135,7 +135,7 @@ check_write_watch (DBusTransport *transport)
_dbus_transport_ref (transport);
if (_dbus_transport_get_is_authenticated (transport))
if (_dbus_transport_try_to_authenticate (transport))
needed = _dbus_connection_has_messages_to_send_unlocked (transport->connection);
else
{
@ -190,7 +190,7 @@ check_read_watch (DBusTransport *transport)
_dbus_transport_ref (transport);
if (_dbus_transport_get_is_authenticated (transport))
if (_dbus_transport_try_to_authenticate (transport))
need_read_watch =
(_dbus_counter_get_size_value (transport->live_messages) < transport->max_live_messages_size) &&
(_dbus_counter_get_unix_fd_value (transport->live_messages) < transport->max_live_messages_unix_fds);
@ -404,7 +404,7 @@ do_authentication (DBusTransport *transport,
oom = FALSE;
orig_auth_state = _dbus_transport_get_is_authenticated (transport);
orig_auth_state = _dbus_transport_try_to_authenticate (transport);
/* This is essential to avoid the check_write_watch() at the end,
* we don't want to add a write watch in do_iteration before
@ -419,7 +419,7 @@ do_authentication (DBusTransport *transport,
_dbus_transport_ref (transport);
while (!_dbus_transport_get_is_authenticated (transport) &&
while (!_dbus_transport_try_to_authenticate (transport) &&
_dbus_transport_get_is_connected (transport))
{
if (!exchange_credentials (transport, do_reading, do_writing))
@ -477,7 +477,7 @@ do_authentication (DBusTransport *transport,
out:
if (auth_completed)
*auth_completed = (orig_auth_state != _dbus_transport_get_is_authenticated (transport));
*auth_completed = (orig_auth_state != _dbus_transport_try_to_authenticate (transport));
check_read_watch (transport);
check_write_watch (transport);
@ -498,7 +498,7 @@ do_writing (DBusTransport *transport)
dbus_bool_t oom;
/* No messages without authentication! */
if (!_dbus_transport_get_is_authenticated (transport))
if (!_dbus_transport_try_to_authenticate (transport))
{
_dbus_verbose ("Not authenticated, not writing anything\n");
return TRUE;
@ -703,7 +703,7 @@ do_reading (DBusTransport *transport)
_dbus_verbose ("fd = %d\n",socket_transport->fd);
/* No messages without authentication! */
if (!_dbus_transport_get_is_authenticated (transport))
if (!_dbus_transport_try_to_authenticate (transport))
return TRUE;
oom = FALSE;
@ -1055,7 +1055,7 @@ socket_do_iteration (DBusTransport *transport,
poll_fd.fd = socket_transport->fd;
poll_fd.events = 0;
if (_dbus_transport_get_is_authenticated (transport))
if (_dbus_transport_try_to_authenticate (transport))
{
/* This is kind of a hack; if we have stuff to write, then try
* to avoid the poll. This is probably about a 5% speedup on an

33
dbus/dbus-transport.c
View file

@ -686,10 +686,33 @@ auth_via_default_rules (DBusTransport *transport)
return allow;
}
/**
* Returns #TRUE if we have been authenticated. It will return #TRUE even if
* the transport is now disconnected, but was ever authenticated before
* disconnecting.
*
* This replaces the older _dbus_transport_get_is_authenticated() which
* had side-effects.
*
* @param transport the transport
* @returns whether we're authenticated
*/
dbus_bool_t
_dbus_transport_peek_is_authenticated (DBusTransport *transport)
{
return transport->authenticated;
}
/**
* Returns #TRUE if we have been authenticated. Will return #TRUE
* even if the transport is disconnected.
* Returns #TRUE if we have been authenticated. It will return #TRUE even if
* the transport is now disconnected, but was ever authenticated before
* disconnecting.
*
* If we have not finished authenticating, but we have enough buffered input
* to finish the job, then this function will do so before it returns.
*
* This used to be called _dbus_transport_get_is_authenticated(), but that
* name seems inappropriate for a function with side-effects.
*
* @todo we drop connection->mutex when calling the unix_user_function,
* and windows_user_function, which may not be safe really.
@ -698,7 +721,7 @@ auth_via_default_rules (DBusTransport *transport)
* @returns whether we're authenticated
*/
dbus_bool_t
_dbus_transport_get_is_authenticated (DBusTransport *transport)
_dbus_transport_try_to_authenticate (DBusTransport *transport)
{
if (transport->authenticated)
return TRUE;
@ -1085,12 +1108,12 @@ _dbus_transport_get_dispatch_status (DBusTransport *transport)
_dbus_counter_get_unix_fd_value (transport->live_messages) >= transport->max_live_messages_unix_fds)
return DBUS_DISPATCH_COMPLETE; /* complete for now */
if (!_dbus_transport_get_is_authenticated (transport))
if (!_dbus_transport_try_to_authenticate (transport))
{
if (_dbus_auth_do_work (transport->auth) ==
DBUS_AUTH_STATE_WAITING_FOR_MEMORY)
return DBUS_DISPATCH_NEED_MEMORY;
else if (!_dbus_transport_get_is_authenticated (transport))
else if (!_dbus_transport_try_to_authenticate (transport))
return DBUS_DISPATCH_COMPLETE;
}

3
dbus/dbus-transport.h
View file

@ -38,7 +38,8 @@ DBusTransport* _dbus_transport_ref (DBusTransport
void _dbus_transport_unref (DBusTransport *transport);
void _dbus_transport_disconnect (DBusTransport *transport);
dbus_bool_t _dbus_transport_get_is_connected (DBusTransport *transport);
dbus_bool_t _dbus_transport_get_is_authenticated (DBusTransport *transport);
dbus_bool_t _dbus_transport_peek_is_authenticated (DBusTransport *transport);
dbus_bool_t _dbus_transport_try_to_authenticate (DBusTransport *transport);
dbus_bool_t _dbus_transport_get_is_anonymous (DBusTransport *transport);
dbus_bool_t _dbus_transport_can_pass_unix_fd (DBusTransport *transport);