From 72cc39a2062e820412b980524e18181c62870433 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Thu, 16 Oct 2014 15:55:19 +0100 Subject: [PATCH 2/8] DBusTransport: record abnormal disconnections Normal disconnection is DBUS_ERROR_DISCONNECTED. --- dbus/dbus-connection.c | 3 +- dbus/dbus-transport-protected.h | 1 + dbus/dbus-transport-socket.c | 50 ++++++++++++++++------- dbus/dbus-transport.c | 90 ++++++++++++++++++++++++++++++++++------- dbus/dbus-transport.h | 9 ++++- 5 files changed, 122 insertions(+), 31 deletions(-) diff --git a/dbus/dbus-connection.c b/dbus/dbus-connection.c index b574207..4036819 100644 --- a/dbus/dbus-connection.c +++ b/dbus/dbus-connection.c @@ -2866,7 +2866,8 @@ _dbus_connection_close_possibly_shared_and_unlock (DBusConnection *connection) */ _dbus_connection_ref_unlocked (connection); - _dbus_transport_disconnect (connection->transport); + _dbus_transport_disconnect (connection->transport, DBUS_ERROR_DISCONNECTED, + NULL); /* This has the side effect of queuing the disconnect message link * (unless we don't have enough memory, possibly, so don't assert it). diff --git a/dbus/dbus-transport-protected.h b/dbus/dbus-transport-protected.h index 396f0ff..5f5868c 100644 --- a/dbus/dbus-transport-protected.h +++ b/dbus/dbus-transport-protected.h @@ -110,6 +110,7 @@ struct DBusTransport DBusFreeFunction free_windows_user_data; /**< Function to free windows_user_data */ + DBusError disconnect_error; /**< Set if we are disconnected. */ unsigned int disconnected : 1; /**< #TRUE if we are disconnected. */ 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 */ diff --git a/dbus/dbus-transport-socket.c b/dbus/dbus-transport-socket.c index c1e4701..2f8cc20 100644 --- a/dbus/dbus-transport-socket.c +++ b/dbus/dbus-transport-socket.c @@ -232,10 +232,17 @@ check_read_watch (DBusTransport *transport) } static void -do_io_error (DBusTransport *transport) +do_io_error (DBusTransport *transport, + const char *error, + const char *message, + ...) { + va_list args; + _dbus_transport_ref (transport); - _dbus_transport_disconnect (transport); + va_start (args, message); + _dbus_transport_disconnect_va (transport, error, message, args); + va_end (args); _dbus_transport_unref (transport); } @@ -279,7 +286,9 @@ read_data_into_auth (DBusTransport *transport, { _dbus_verbose ("Error reading from remote app: %s\n", _dbus_strerror (saved_errno)); - do_io_error (transport); + do_io_error (transport, _dbus_error_from_errno (saved_errno), + "Error reading from remote app: %s", + _dbus_strerror (saved_errno)); } return FALSE; @@ -289,7 +298,7 @@ read_data_into_auth (DBusTransport *transport, _dbus_assert (bytes_read == 0); _dbus_verbose ("Disconnected from remote app\n"); - do_io_error (transport); + do_io_error (transport, DBUS_ERROR_DISCONNECTED, NULL); return FALSE; } @@ -328,7 +337,9 @@ write_data_from_auth (DBusTransport *transport) { _dbus_verbose ("Error writing to remote app: %s\n", _dbus_strerror (saved_errno)); - do_io_error (transport); + do_io_error (transport, _dbus_error_from_errno (saved_errno), + "Error writing to remote app: %s", + _dbus_strerror (saved_errno)); } } @@ -357,8 +368,9 @@ exchange_credentials (DBusTransport *transport, else { _dbus_verbose ("Failed to write credentials: %s\n", error.message); + do_io_error (transport, error.name, + "Error writing credentials: %s", error.message); dbus_error_free (&error); - do_io_error (transport); } } @@ -380,8 +392,9 @@ exchange_credentials (DBusTransport *transport, else { _dbus_verbose ("Failed to read credentials %s\n", error.message); + do_io_error (transport, error.name, + "Error reading credentials: %s", error.message); dbus_error_free (&error); - do_io_error (transport); } } @@ -423,7 +436,7 @@ do_authentication (DBusTransport *transport, _dbus_transport_ref (transport); while (!_dbus_transport_try_to_authenticate (transport) && - _dbus_transport_get_is_connected (transport)) + _dbus_transport_get_is_connected (transport, NULL)) { if (!exchange_credentials (transport, do_reading, do_writing)) { @@ -468,7 +481,7 @@ do_authentication (DBusTransport *transport, case DBUS_AUTH_STATE_NEED_DISCONNECT: _dbus_verbose (" %s auth state: need to disconnect\n", TRANSPORT_SIDE (transport)); - do_io_error (transport); + do_io_error (transport, DBUS_ERROR_AUTH_FAILED, NULL); break; case DBUS_AUTH_STATE_AUTHENTICATED: @@ -696,7 +709,9 @@ do_writing (DBusTransport *transport) { _dbus_verbose ("Error writing to remote app: %s\n", _dbus_strerror (saved_errno)); - do_io_error (transport); + do_io_error (transport, _dbus_error_from_errno (saved_errno), + "Error writing to remote app: %s", + _dbus_strerror (saved_errno)); goto out; } } @@ -870,14 +885,16 @@ do_reading (DBusTransport *transport) { _dbus_verbose ("Error reading from remote app: %s\n", _dbus_strerror (saved_errno)); - do_io_error (transport); + do_io_error (transport, _dbus_error_from_errno (saved_errno), + "Error reading from remote app: %s", + _dbus_strerror (saved_errno)); goto out; } } else if (bytes_read == 0) { _dbus_verbose ("Disconnected from remote app\n"); - do_io_error (transport); + do_io_error (transport, DBUS_ERROR_DISCONNECTED, NULL); goto out; } else @@ -944,7 +961,8 @@ socket_handle_watch (DBusTransport *transport, if (!(flags & DBUS_WATCH_READABLE) && unix_error_with_read_to_come (transport, watch, flags)) { _dbus_verbose ("Hang up or error on watch\n"); - _dbus_transport_disconnect (transport); + /* FIXME: should this be treated as abnormal disconnection? */ + _dbus_transport_disconnect (transport, DBUS_ERROR_DISCONNECTED, NULL); return TRUE; } @@ -1183,7 +1201,11 @@ socket_do_iteration (DBusTransport *transport, */ if (poll_fd.revents & _DBUS_POLLERR) - do_io_error (transport); + { + do_io_error (transport, _dbus_error_from_errno (saved_errno), + "Error polling stream: %s", + _dbus_strerror (saved_errno)); + } else { dbus_bool_t need_read = (poll_fd.revents & _DBUS_POLLIN) > 0; diff --git a/dbus/dbus-transport.c b/dbus/dbus-transport.c index f63e0ce..5aed2fa 100644 --- a/dbus/dbus-transport.c +++ b/dbus/dbus-transport.c @@ -167,6 +167,7 @@ _dbus_transport_init_base (DBusTransport *transport, transport->live_messages = counter; transport->authenticated = FALSE; transport->disconnected = FALSE; + dbus_error_init (&transport->disconnect_error); transport->is_server = (server_guid != NULL); transport->send_credentials_pending = !transport->is_server; transport->receive_credentials_pending = transport->is_server; @@ -216,7 +217,7 @@ void _dbus_transport_finalize_base (DBusTransport *transport) { if (!transport->disconnected) - _dbus_transport_disconnect (transport); + _dbus_transport_disconnect (transport, DBUS_ERROR_DISCONNECTED, NULL); if (transport->free_unix_user_data != NULL) (* transport->free_unix_user_data) (transport->unix_user_data); @@ -233,6 +234,8 @@ _dbus_transport_finalize_base (DBusTransport *transport) dbus_free (transport->expected_guid); if (transport->credentials) _dbus_credentials_unref (transport->credentials); + + dbus_error_free (&transport->disconnect_error); } @@ -499,18 +502,36 @@ _dbus_transport_unref (DBusTransport *transport) * */ void -_dbus_transport_disconnect (DBusTransport *transport) +_dbus_transport_disconnect (DBusTransport *transport, + const char *error, + const char *message, + ...) +{ + va_list args; + + va_start (args, message); + _dbus_transport_disconnect_va (transport, error, message, args); + va_end (args); +} + +void +_dbus_transport_disconnect_va (DBusTransport *transport, + const char *error, + const char *message, + va_list args) { _dbus_verbose ("start\n"); + _dbus_assert (error != NULL); _dbus_assert (transport->vtable->disconnect != NULL); if (transport->disconnected) return; (* transport->vtable->disconnect) (transport); - + transport->disconnected = TRUE; + _dbus_set_error_va (&transport->disconnect_error, error, message, args); _dbus_verbose ("end\n"); } @@ -524,9 +545,18 @@ _dbus_transport_disconnect (DBusTransport *transport) * @returns whether we're connected */ dbus_bool_t -_dbus_transport_get_is_connected (DBusTransport *transport) +_dbus_transport_get_is_connected (DBusTransport *transport, + DBusError *error) { - return !transport->disconnected; + if (transport->disconnected) + { + _dbus_assert (dbus_error_is_set (&transport->disconnect_error)); + dbus_set_error (error, transport->disconnect_error.name, + "%s", transport->disconnect_error.message); + return FALSE; + } + + return TRUE; } static dbus_bool_t @@ -568,7 +598,9 @@ auth_via_unix_user_function (DBusTransport *transport) _dbus_verbose ("Client UID "DBUS_UID_FORMAT " was rejected, disconnecting\n", _dbus_credentials_get_unix_uid (auth_identity)); - _dbus_transport_disconnect (transport); + _dbus_transport_disconnect (transport, DBUS_ERROR_ACCESS_DENIED, + "Client UID " DBUS_UID_FORMAT " was rejected", + _dbus_credentials_get_unix_uid (auth_identity)); } return allow; @@ -618,12 +650,20 @@ auth_via_windows_user_function (DBusTransport *transport) { _dbus_verbose ("Client SID '%s' was rejected, disconnecting\n", _dbus_credentials_get_windows_sid (auth_identity)); - _dbus_transport_disconnect (transport); + _dbus_transport_disconnect (transport, DBUS_ERROR_ACCESS_DENIED, + "Client SID '%s' was rejected", + _dbus_credentials_get_windows_sid (auth_identity)); } return allow; } +static inline const char * +unnullify (const char *s) +{ + return s == NULL ? "" : s; +} + static dbus_bool_t auth_via_default_rules (DBusTransport *transport) { @@ -666,18 +706,30 @@ auth_via_default_rules (DBusTransport *transport) else { if (_dbus_credentials_include(our_identity,DBUS_CREDENTIAL_WINDOWS_SID)) + { _dbus_verbose ("Client authorized as SID '%s'" " but our SID is '%s', disconnecting\n", - (_dbus_credentials_get_windows_sid(auth_identity) ? - _dbus_credentials_get_windows_sid(auth_identity) : ""), - (_dbus_credentials_get_windows_sid(our_identity) ? - _dbus_credentials_get_windows_sid(our_identity) : "")); + unnullify (_dbus_credentials_get_windows_sid(auth_identity)), + unnullify (_dbus_credentials_get_windows_sid(our_identity))); + _dbus_transport_disconnect (transport, DBUS_ERROR_AUTH_FAILED, + "Client authorized as SID '%s'" + " but our SID is '%s'", + unnullify (_dbus_credentials_get_windows_sid(auth_identity)), + unnullify (_dbus_credentials_get_windows_sid(our_identity))); + } else + { _dbus_verbose ("Client authorized as UID "DBUS_UID_FORMAT " but our UID is "DBUS_UID_FORMAT", disconnecting\n", _dbus_credentials_get_unix_uid(auth_identity), _dbus_credentials_get_unix_uid(our_identity)); - _dbus_transport_disconnect (transport); + _dbus_transport_disconnect (transport, DBUS_ERROR_AUTH_FAILED, + "Client authorized as UID " DBUS_UID_FORMAT + " but our UID is " DBUS_UID_FORMAT, + _dbus_credentials_get_unix_uid (auth_identity), + _dbus_credentials_get_unix_uid (our_identity)); + } + allow = FALSE; } @@ -765,7 +817,10 @@ _dbus_transport_try_to_authenticate (DBusTransport *transport) { _dbus_verbose ("Client expected GUID '%s' and we got '%s' from the server\n", transport->expected_guid, server_guid); - _dbus_transport_disconnect (transport); + _dbus_transport_disconnect (transport, DBUS_ERROR_AUTH_FAILED, + "Expected GUID '%s' does not match server GUID '%s'", + transport->expected_guid, + server_guid); _dbus_connection_unref_unlocked (transport->connection); return FALSE; } @@ -1140,6 +1195,7 @@ dbus_bool_t _dbus_transport_queue_messages (DBusTransport *transport) { DBusDispatchStatus status; + DBusValidity validity; #if 0 _dbus_verbose ("_dbus_transport_queue_messages()\n"); @@ -1180,10 +1236,14 @@ _dbus_transport_queue_messages (DBusTransport *transport) } } - if (_dbus_message_loader_get_is_corrupted (transport->loader)) + validity = _dbus_message_loader_get_corruption_reason (transport->loader); + + if (validity != DBUS_VALID) { _dbus_verbose ("Corrupted message stream, disconnecting\n"); - _dbus_transport_disconnect (transport); + _dbus_transport_disconnect (transport, DBUS_ERROR_INVALID_ARGS, + "Disconnecting sender of corrupted message (%s)", + _dbus_validity_to_error_message (validity)); } return status != DBUS_DISPATCH_NEED_MEMORY; diff --git a/dbus/dbus-transport.h b/dbus/dbus-transport.h index 39c74c4..c65a17a 100644 --- a/dbus/dbus-transport.h +++ b/dbus/dbus-transport.h @@ -36,8 +36,15 @@ DBusTransport* _dbus_transport_open (DBusAddressEntry DBusError *error); DBusTransport* _dbus_transport_ref (DBusTransport *transport); void _dbus_transport_unref (DBusTransport *transport); -void _dbus_transport_disconnect (DBusTransport *transport); dbus_bool_t _dbus_transport_get_is_connected (DBusTransport *transport); +void _dbus_transport_disconnect (DBusTransport *transport, + const char *error, + const char *format, + ...); +void _dbus_transport_disconnect_va (DBusTransport *transport, + const char *error, + const char *format, + va_list args); 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); -- 2.1.1