From ff5283ab92c668453fd2f28c1715a1e0e9b949f5 Mon Sep 17 00:00:00 2001 From: Havoc Pennington Date: Fri, 27 Dec 2002 00:44:41 +0000 Subject: 2002-12-26 Havoc Pennington * dbus/dbus-marshal.h (DBUS_COMPILER_BYTE_ORDER): #ifdef WORDS_BIGENDIAN then compiler byte order is DBUS_BIG_ENDIAN, doh * dbus/dbus-marshal.c: Add macros to do int swapping in-place and avoid swap_bytes() overhead (ignoring possible assembly stuff for now). Main point is because I wanted unpack_uint32 to implement _dbus_verbose_bytes (_dbus_verbose_bytes): new function * dbus/dbus-string.c (_dbus_string_validate_ascii): new function * dbus/dbus-message.c (_dbus_message_loader_get_is_corrupted): add mechanism to handle a corrupt message stream (_dbus_message_loader_new): fix preallocation to only prealloc, not prelengthen * dbus/dbus-string.c (_dbus_string_skip_blank): fix this function (_dbus_string_test): enhance tests for copy/move and fix the functions * dbus/dbus-transport-unix.c: Hold references in more places to avoid reentrancy problems * dbus/dbus-transport.c: ditto * dbus/dbus-connection.c (dbus_connection_dispatch_message): don't leak reference count in no-message case * test/watch.c (do_mainloop): handle adding/removing watches during iteration over the watches. Also, ref the connection/server stored on a watch, so we don't try to mangle a destroyed one. * dbus/dbus-transport-unix.c (do_authentication): perform authentication * dbus/dbus-auth.c (get_state): add a state AUTHENTICATED_WITH_UNUSED_BYTES and return it if required (_dbus_auth_get_unused_bytes): append the unused bytes to the passed in string, rather than prepend * dbus/dbus-transport.c (_dbus_transport_init_base): create the auth conversation DBusAuth * dbus/dbus-transport-unix.c (_dbus_transport_new_for_fd) (_dbus_transport_new_for_domain_socket): when creating a transport, pass in whether it's a client-side or server-side transport so we know which DBusAuth to create --- ChangeLog | 66 +++++++ dbus/dbus-auth.c | 140 +++++++++++--- dbus/dbus-auth.h | 7 +- dbus/dbus-connection.c | 47 ++++- dbus/dbus-internals.h | 5 + dbus/dbus-marshal.c | 147 ++++++++++++-- dbus/dbus-marshal.h | 9 +- dbus/dbus-message-internal.h | 2 + dbus/dbus-message.c | 40 +++- dbus/dbus-server-unix.c | 2 +- dbus/dbus-string.c | 80 +++++++- dbus/dbus-string.h | 4 + dbus/dbus-transport-protected.h | 12 +- dbus/dbus-transport-unix.c | 419 +++++++++++++++++++++++++++++++++------- dbus/dbus-transport-unix.h | 4 +- dbus/dbus-transport.c | 99 +++++++++- dbus/dbus-transport.h | 34 ++-- dbus/dbus-watch.c | 2 +- doc/dbus-sasl-profile.txt | 2 +- test/watch.c | 81 +++++++- 20 files changed, 1031 insertions(+), 171 deletions(-) diff --git a/ChangeLog b/ChangeLog index 110de95e..a43ef421 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,69 @@ +2002-12-26 Havoc Pennington + + * dbus/dbus-marshal.h (DBUS_COMPILER_BYTE_ORDER): #ifdef + WORDS_BIGENDIAN then compiler byte order is DBUS_BIG_ENDIAN, + doh + + * dbus/dbus-marshal.c: Add macros to do int swapping in-place and + avoid swap_bytes() overhead (ignoring possible assembly stuff for + now). Main point is because I wanted unpack_uint32 to implement + _dbus_verbose_bytes + (_dbus_verbose_bytes): new function + + * dbus/dbus-string.c (_dbus_string_validate_ascii): new function + + * dbus/dbus-message.c (_dbus_message_loader_get_is_corrupted): add + mechanism to handle a corrupt message stream + (_dbus_message_loader_new): fix preallocation to only prealloc, + not prelengthen + + * dbus/dbus-string.c (_dbus_string_skip_blank): fix this function + (_dbus_string_test): enhance tests for copy/move and fix the + functions + + * dbus/dbus-transport-unix.c: Hold references in more places to + avoid reentrancy problems + + * dbus/dbus-transport.c: ditto + + * dbus/dbus-connection.c (dbus_connection_dispatch_message): don't + leak reference count in no-message case + + * test/watch.c (do_mainloop): handle adding/removing watches + during iteration over the watches. Also, ref the connection/server + stored on a watch, so we don't try to mangle a destroyed one. + + * dbus/dbus-transport-unix.c (do_authentication): perform + authentication + + * dbus/dbus-auth.c (get_state): add a state + AUTHENTICATED_WITH_UNUSED_BYTES and return it if required + (_dbus_auth_get_unused_bytes): append the unused bytes + to the passed in string, rather than prepend + + * dbus/dbus-transport.c (_dbus_transport_init_base): create + the auth conversation DBusAuth + + * dbus/dbus-transport-unix.c (_dbus_transport_new_for_fd) + (_dbus_transport_new_for_domain_socket): when creating a + transport, pass in whether it's a client-side or server-side + transport so we know which DBusAuth to create + +2002-12-03 Havoc Pennington + + * dbus/dbus-transport-unix.c (unix_finalize): finalize base + _after_ finalizing the derived members + (unix_connection_set): unref watch if we fail to add it + + * dbus/dbus-connection.c (dbus_connection_unref): delete the + transport first, so that the connection owned by the + transport will be valid as the transport finalizes. + + * dbus/dbus-transport-unix.c (unix_finalize): free the write_watch + if necessary, and remove watches from the connection. + + * dbus/dbus-watch.c (_dbus_watch_list_free): improve a comment + 2002-12-26 Anders Carlsson * dbus/dbus-marshal.c: (_dbus_marshal_string), diff --git a/dbus/dbus-auth.c b/dbus/dbus-auth.c index b2fca832..566abaed 100644 --- a/dbus/dbus-auth.c +++ b/dbus/dbus-auth.c @@ -255,7 +255,12 @@ get_state (DBusAuth *auth) if (auth->need_disconnect) return DBUS_AUTH_STATE_NEED_DISCONNECT; else if (auth->authenticated) - return DBUS_AUTH_STATE_AUTHENTICATED; + { + if (_dbus_string_get_length (&auth->incoming) > 0) + return DBUS_AUTH_STATE_AUTHENTICATED_WITH_UNUSED_BYTES; + else + return DBUS_AUTH_STATE_AUTHENTICATED; + } else if (auth->needed_memory) return DBUS_AUTH_STATE_WAITING_FOR_MEMORY; else if (_dbus_string_get_length (&auth->outgoing) > 0) @@ -272,7 +277,10 @@ shutdown_mech (DBusAuth *auth) auth->authenticated = FALSE; if (auth->mech != NULL) - { + { + _dbus_verbose ("Shutting down mechanism %s\n", + auth->mech->mechanism); + if (DBUS_AUTH_IS_CLIENT (auth)) (* auth->mech->client_shutdown_func) (auth); else @@ -483,6 +491,9 @@ process_auth (DBusAuth *auth, auth->mech = find_mech (&mech); if (auth->mech != NULL) { + _dbus_verbose ("Trying mechanism %s\n", + auth->mech->mechanism); + if (!(* auth->mech->server_data_func) (auth, &decoded_response)) goto failed; @@ -578,6 +589,7 @@ process_error_server (DBusAuth *auth, return TRUE; } +/* return FALSE if no memory, TRUE if all OK */ static dbus_bool_t get_word (const DBusString *str, int *start, @@ -585,9 +597,10 @@ get_word (const DBusString *str, { int i; + _dbus_string_skip_blank (str, *start, start); _dbus_string_find_blank (str, *start, &i); - - if (i != *start) + + if (i > *start) { if (!_dbus_string_copy_len (str, *start, i, word, 0)) return FALSE; @@ -616,7 +629,7 @@ process_mechanisms (DBusAuth *auth, { DBusString m; const DBusAuthMechanismHandler *mech; - + if (!_dbus_string_init (&m, _DBUS_INT_MAX)) goto nomem; @@ -635,11 +648,24 @@ process_mechanisms (DBusAuth *auth, * preference. Of course when the server is us, * it lists things in that order anyhow. */ + + _dbus_verbose ("Adding mechanism %s to list we will try\n", + mech->mechanism); if (!_dbus_list_append (& DBUS_AUTH_CLIENT (auth)->mechs_to_try, (void*) mech)) goto nomem; } + else + { + const char *s; + + _dbus_string_get_const_data (&m, &s); + _dbus_verbose ("Server offered mechanism \"%s\" that we don't know how to use\n", + s); + } + + _dbus_string_free (&m); } auth->already_got_mechanisms = TRUE; @@ -706,7 +732,11 @@ process_rejected (DBusAuth *auth, return FALSE; } + auth->mech = mech; _dbus_list_pop_first (& DBUS_AUTH_CLIENT (auth)->mechs_to_try); + + _dbus_verbose ("Trying mechanism %s\n", + auth->mech->mechanism); } else { @@ -815,25 +845,53 @@ process_command (DBusAuth *auth) auth->needed_memory = TRUE; return FALSE; } + + if (eol > _DBUS_ONE_MEGABYTE) + { + /* This is a giant line, someone is trying to hose us. */ + if (!_dbus_string_append (&auth->outgoing, "ERROR \"Command too long\"\r\n")) + goto out; + else + goto next_command; + } if (!_dbus_string_copy_len (&auth->incoming, 0, eol, &command, 0)) goto out; + + if (!_dbus_string_validate_ascii (&command, 0, + _dbus_string_get_length (&command))) + { + _dbus_verbose ("Command contained non-ASCII chars or embedded nul\n"); + if (!_dbus_string_append (&auth->outgoing, "ERROR \"Command contained non-ASCII\"\r\n")) + goto out; + else + goto next_command; + } + + { + const char *q; + _dbus_string_get_const_data (&command, &q); + _dbus_verbose ("got command \"%s\"\n", q); + } _dbus_string_find_blank (&command, 0, &i); _dbus_string_skip_blank (&command, i, &j); - if (i != j) - _dbus_string_delete (&command, i, j); + if (j > i) + _dbus_string_delete (&command, i, j - i); if (!_dbus_string_move (&command, i, &args, 0)) goto out; - + i = 0; while (auth->handlers[i].command != NULL) { if (_dbus_string_equal_c_str (&command, auth->handlers[i].command)) { + _dbus_verbose ("Processing auth command %s\n", + auth->handlers[i].command); + if (!(* auth->handlers[i].func) (auth, &command, &args)) goto out; @@ -847,6 +905,8 @@ process_command (DBusAuth *auth) if (!process_unknown (auth, &command, &args)) goto out; } + + next_command: /* We've succeeded in processing the whole command so drop it out * of the incoming buffer and return TRUE to try another command. @@ -1002,6 +1062,15 @@ _dbus_auth_do_work (DBusAuth *auth) _dbus_verbose ("Disconnecting due to excessive data buffered in auth phase\n"); break; } + + if (auth->mech == NULL && + auth->already_got_mechanisms && + DBUS_AUTH_CLIENT (auth)->mechs_to_try == NULL) + { + auth->need_disconnect = TRUE; + _dbus_verbose ("Disconnecting because we are out of mechanisms to try using\n"); + break; + } } while (process_command (auth)); @@ -1009,40 +1078,54 @@ _dbus_auth_do_work (DBusAuth *auth) } /** - * Gets bytes that need to be sent to the - * peer we're conversing with. + * Gets bytes that need to be sent to the peer we're conversing with. + * After writing some bytes, _dbus_auth_bytes_sent() must be called + * to notify the auth object that they were written. * * @param auth the auth conversation - * @param str initialized string object to be filled in with bytes to send - * @returns #FALSE if not enough memory to fill in the bytes + * @param str return location for a ref to the buffer to send + * @returns #FALSE if nothing to send */ dbus_bool_t -_dbus_auth_get_bytes_to_send (DBusAuth *auth, - DBusString *str) +_dbus_auth_get_bytes_to_send (DBusAuth *auth, + const DBusString **str) { _dbus_assert (auth != NULL); _dbus_assert (str != NULL); + + *str = NULL; if (DBUS_AUTH_IN_END_STATE (auth)) return FALSE; - auth->needed_memory = FALSE; - - _dbus_string_set_length (str, 0); + if (_dbus_string_get_length (&auth->outgoing) == 0) + return FALSE; - if (!_dbus_string_move (&auth->outgoing, - 0, str, 0)) - { - auth->needed_memory = TRUE; - return FALSE; - } + *str = &auth->outgoing; - if (auth->authenticated_pending_output) - auth->authenticated = TRUE; - return TRUE; } +/** + * Notifies the auth conversation object that + * the given number of bytes of the outgoing buffer + * have been written out. + * + * @param auth the auth conversation + * @param bytes_sent number of bytes written out + */ +void +_dbus_auth_bytes_sent (DBusAuth *auth, + int bytes_sent) +{ + _dbus_string_delete (&auth->outgoing, + 0, bytes_sent); + + if (auth->authenticated_pending_output && + _dbus_string_get_length (&auth->outgoing) == 0) + auth->authenticated = TRUE; +} + /** * Stores bytes received from the peer we're conversing with. * @@ -1082,7 +1165,7 @@ _dbus_auth_bytes_received (DBusAuth *auth, * succeeded. * * @param auth the auth conversation - * @param str string to place the unused bytes in + * @param str string to append the unused bytes to * @returns #FALSE if not enough memory to return the bytes */ dbus_bool_t @@ -1093,7 +1176,8 @@ _dbus_auth_get_unused_bytes (DBusAuth *auth, return FALSE; if (!_dbus_string_move (&auth->incoming, - 0, str, 0)) + 0, str, + _dbus_string_get_length (str))) return FALSE; return TRUE; diff --git a/dbus/dbus-auth.h b/dbus/dbus-auth.h index 1d0baa94..824426af 100644 --- a/dbus/dbus-auth.h +++ b/dbus/dbus-auth.h @@ -37,6 +37,7 @@ typedef enum DBUS_AUTH_STATE_WAITING_FOR_MEMORY, DBUS_AUTH_STATE_HAVE_BYTES_TO_SEND, DBUS_AUTH_STATE_NEED_DISCONNECT, + DBUS_AUTH_STATE_AUTHENTICATED_WITH_UNUSED_BYTES, DBUS_AUTH_STATE_AUTHENTICATED } DBusAuthState; @@ -45,8 +46,10 @@ DBusAuth* _dbus_auth_client_new (void); void _dbus_auth_ref (DBusAuth *auth); void _dbus_auth_unref (DBusAuth *auth); DBusAuthState _dbus_auth_do_work (DBusAuth *auth); -dbus_bool_t _dbus_auth_get_bytes_to_send (DBusAuth *auth, - DBusString *str); +dbus_bool_t _dbus_auth_get_bytes_to_send (DBusAuth *auth, + const DBusString **str); +void _dbus_auth_bytes_sent (DBusAuth *auth, + int bytes_sent); dbus_bool_t _dbus_auth_bytes_received (DBusAuth *auth, const DBusString *str); dbus_bool_t _dbus_auth_get_unused_bytes (DBusAuth *auth, diff --git a/dbus/dbus-connection.c b/dbus/dbus-connection.c index 4a40f0b0..e5fa48f0 100644 --- a/dbus/dbus-connection.c +++ b/dbus/dbus-connection.c @@ -94,13 +94,18 @@ dbus_bool_t _dbus_connection_queue_received_message (DBusConnection *connection, DBusMessage *message) { + _dbus_assert (_dbus_transport_get_is_authenticated (connection->transport)); + if (!_dbus_list_append (&connection->incoming_messages, message)) return FALSE; - + dbus_message_ref (message); connection->n_incoming += 1; + _dbus_verbose ("Incoming message %p added to queue, %d incoming\n", + message, connection->n_incoming); + return TRUE; } @@ -140,11 +145,17 @@ void _dbus_connection_message_sent (DBusConnection *connection, DBusMessage *message) { + _dbus_assert (_dbus_transport_get_is_authenticated (connection->transport)); _dbus_assert (message == _dbus_list_get_last (&connection->outgoing_messages)); + _dbus_list_pop_last (&connection->outgoing_messages); dbus_message_unref (message); connection->n_outgoing -= 1; + + _dbus_verbose ("Message %p removed from outgoing queue, %d left to send\n", + message, connection->n_outgoing); + if (connection->n_outgoing == 0) _dbus_transport_messages_pending (connection->transport, connection->n_outgoing); @@ -164,10 +175,11 @@ dbus_bool_t _dbus_connection_add_watch (DBusConnection *connection, DBusWatch *watch) { - return _dbus_watch_list_add_watch (connection->watches, - watch); - - return TRUE; + if (connection->watches) /* null during finalize */ + return _dbus_watch_list_add_watch (connection->watches, + watch); + else + return FALSE; } /** @@ -182,8 +194,9 @@ void _dbus_connection_remove_watch (DBusConnection *connection, DBusWatch *watch) { - _dbus_watch_list_remove_watch (connection->watches, - watch); + if (connection->watches) /* null during finalize */ + _dbus_watch_list_remove_watch (connection->watches, + watch); } static void @@ -442,7 +455,8 @@ dbus_connection_unref (DBusConnection *connection) NULL, NULL, NULL); _dbus_watch_list_free (connection->watches); - + connection->watches = NULL; + _dbus_hash_iter_init (connection->handler_table, &iter); while (_dbus_hash_iter_next (&iter)) { @@ -545,6 +559,9 @@ dbus_connection_send_message (DBusConnection *connection, dbus_message_ref (message); connection->n_outgoing += 1; + _dbus_verbose ("Message %p added to outgoing queue, %d pending to send\n", + message, connection->n_outgoing); + _dbus_message_lock (message); if (connection->n_outgoing == 1) @@ -668,8 +685,15 @@ dbus_connection_pop_message (DBusConnection *connection) { if (connection->n_incoming > 0) { + DBusMessage *message; + + message = _dbus_list_pop_first (&connection->incoming_messages); connection->n_incoming -= 1; - return _dbus_list_pop_first (&connection->incoming_messages); + + _dbus_verbose ("Incoming message %p removed from queue, %d incoming\n", + message, connection->n_incoming); + + return message; } else return NULL; @@ -700,7 +724,10 @@ dbus_connection_dispatch_message (DBusConnection *connection) message = dbus_connection_pop_message (connection); if (message == NULL) - return FALSE; + { + dbus_connection_unref (connection); + return FALSE; + } filter_serial = connection->filters_serial; handler_serial = connection->handlers_serial; diff --git a/dbus/dbus-internals.h b/dbus/dbus-internals.h index fd54a5f1..2b7772f3 100644 --- a/dbus/dbus-internals.h +++ b/dbus/dbus-internals.h @@ -74,6 +74,8 @@ char* _dbus_strdup (const char *str); #define _DBUS_INT_MIN (-_DBUS_INT_MAX - 1) #define _DBUS_INT_MAX 2147483647 #define _DBUS_MAX_SUN_PATH_LENGTH 99 +#define _DBUS_ONE_KILOBYTE 1024 +#define _DBUS_ONE_MEGABYTE 1024 * _DBUS_ONE_KILOBYTE typedef void (* DBusForeachFunction) (void *element, void *data); @@ -81,6 +83,9 @@ typedef void (* DBusForeachFunction) (void *element, dbus_bool_t _dbus_set_fd_nonblocking (int fd, DBusResultCode *result); +void _dbus_verbose_bytes (const unsigned char *data, + int len); + DBUS_END_DECLS; #endif /* DBUS_INTERNALS_H */ diff --git a/dbus/dbus-marshal.c b/dbus/dbus-marshal.c index bbcda141..9fbfb4e1 100644 --- a/dbus/dbus-marshal.c +++ b/dbus/dbus-marshal.c @@ -26,9 +26,37 @@ #include +#define DBUS_UINT32_SWAP_LE_BE_CONSTANT(val) ((dbus_uint32_t) ( \ + (((dbus_uint32_t) (val) & (dbus_uint32_t) 0x000000ffU) << 24) | \ + (((dbus_uint32_t) (val) & (dbus_uint32_t) 0x0000ff00U) << 8) | \ + (((dbus_uint32_t) (val) & (dbus_uint32_t) 0x00ff0000U) >> 8) | \ + (((dbus_uint32_t) (val) & (dbus_uint32_t) 0xff000000U) >> 24))) + +#define DBUS_UINT32_SWAP_LE_BE(val) (DBUS_UINT32_SWAP_LE_BE_CONSTANT (val)) + +#ifdef WORDS_BIGENDIAN +#define DBUS_INT32_TO_BE(val) ((dbus_int32_t) (val)) +#define DBUS_UINT32_TO_BE(val) ((dbus_uint32_t) (val)) +#define DBUS_INT32_TO_LE(val) ((dbus_int32_t) DBUS_UINT32_SWAP_LE_BE (val)) +#define DBUS_UINT32_TO_LE(val) (DBUS_UINT32_SWAP_LE_BE (val)) +#else +#define DBUS_INT32_TO_LE(val) ((dbus_int32_t) (val)) +#define DBUS_UINT32_TO_LE(val) ((dbus_uint32_t) (val)) +#define DBUS_INT32_TO_BE(val) ((dbus_int32_t) DBUS_UINT32_SWAP_LE_BE (val)) +#define DBUS_UINT32_TO_BE(val) (DBUS_UINT32_SWAP_LE_BE (val)) +#endif + +/* The transformation is symmetric, so the FROM just maps to the TO. */ +#define DBUS_INT32_FROM_LE(val) (DBUS_INT32_TO_LE (val)) +#define DBUS_UINT32_FROM_LE(val) (DBUS_UINT32_TO_LE (val)) +#define DBUS_INT32_FROM_BE(val) (DBUS_INT32_TO_BE (val)) +#define DBUS_UINT32_FROM_BE(val) (DBUS_UINT32_TO_BE (val)) + + /* This alignment thing is from ORBit2 */ /* Align a value upward to a boundary, expressed as a number of bytes. - E.g. align to an 8-byte boundary with argument of 8. */ + * E.g. align to an 8-byte boundary with argument of 8. + */ /* * (this + boundary - 1) @@ -61,6 +89,41 @@ swap_bytes (unsigned char *data, } } +static dbus_uint32_t +unpack_uint32 (int byte_order, + const unsigned char *data) +{ + _dbus_assert (DBUS_ALIGN_ADDRESS (data, 4) == data); + + if (byte_order == DBUS_LITTLE_ENDIAN) + return DBUS_UINT32_FROM_LE (*(dbus_uint32_t*)data); + else + return DBUS_UINT32_FROM_BE (*(dbus_uint32_t*)data); +} + +static dbus_int32_t +unpack_int32 (int byte_order, + const unsigned char *data) +{ + _dbus_assert (DBUS_ALIGN_ADDRESS (data, 4) == data); + + if (byte_order == DBUS_LITTLE_ENDIAN) + return DBUS_INT32_FROM_LE (*(dbus_int32_t*)data); + else + return DBUS_INT32_FROM_BE (*(dbus_int32_t*)data); +} + +/** + * @defgroup DBusMarshal marshaling and unmarshaling + * @ingroup DBusInternals + * @brief functions to marshal/unmarshal data from the wire + * + * Types and functions related to converting primitive data types from + * wire format to native machine format, and vice versa. + * + * @{ + */ + dbus_bool_t _dbus_marshal_double (DBusString *str, int byte_order, @@ -161,20 +224,14 @@ _dbus_demarshal_int32 (DBusString *str, int pos, int *new_pos) { - dbus_int32_t retval; const char *buffer; _dbus_string_get_const_data_len (str, &buffer, pos, sizeof (dbus_int32_t)); - retval = *(dbus_int32_t *)buffer; - - if (byte_order != DBUS_COMPILER_BYTE_ORDER) - swap_bytes ((unsigned char *)&retval, sizeof (dbus_int32_t)); - if (new_pos) *new_pos = pos + sizeof (dbus_int32_t); - - return retval; + + return unpack_int32 (byte_order, buffer); } dbus_uint32_t @@ -183,20 +240,14 @@ _dbus_demarshal_uint32 (DBusString *str, int pos, int *new_pos) { - dbus_uint32_t retval; const char *buffer; _dbus_string_get_const_data_len (str, &buffer, pos, sizeof (dbus_uint32_t)); - retval = *(dbus_uint32_t *)buffer; - - if (byte_order != DBUS_COMPILER_BYTE_ORDER) - swap_bytes ((unsigned char *)&retval, sizeof (dbus_uint32_t)); - if (new_pos) *new_pos = pos + sizeof (dbus_uint32_t); - - return retval; + + return unpack_uint32 (byte_order, buffer); } char * @@ -229,6 +280,68 @@ _dbus_demarshal_string (DBusString *str, return retval; } +/** + * If in verbose mode, print a block of binary data. + * + * @param data the data + * @param len the length of the data + */ +void +_dbus_verbose_bytes (const unsigned char *data, + int len) +{ + int i; + const unsigned char *aligned; + + /* Print blanks on first row if appropriate */ + aligned = DBUS_ALIGN_ADDRESS (data, 4); + if (aligned > data) + aligned -= 4; + _dbus_assert (aligned <= data); + + if (aligned != data) + { + _dbus_verbose ("%5d\t%p: ", - (data - aligned), aligned); + while (aligned != data) + { + _dbus_verbose (" "); + ++aligned; + } + } + + /* now print the bytes */ + i = 0; + while (i < len) + { + if (DBUS_ALIGN_ADDRESS (&data[i], 4) == &data[i]) + { + _dbus_verbose ("%5d\t%p: ", + i, &data[i]); + } + + if (data[i] >= 32 && + data[i] <= 126) + _dbus_verbose (" '%c' ", data[i]); + else + _dbus_verbose ("0x%s%x ", + data[i] <= 0xf ? "0" : "", data[i]); + + ++i; + + if (DBUS_ALIGN_ADDRESS (&data[i], 4) == &data[i]) + { + if (i > 3) + _dbus_verbose ("big: %d little: %d", + unpack_uint32 (DBUS_BIG_ENDIAN, &data[i-4]), + unpack_uint32 (DBUS_LITTLE_ENDIAN, &data[i-4])); + + _dbus_verbose ("\n"); + } + } + + _dbus_verbose ("\n"); +} + /** @} */ #ifdef DBUS_BUILD_TESTS diff --git a/dbus/dbus-marshal.h b/dbus/dbus-marshal.h index 92d45c64..e6663047 100644 --- a/dbus/dbus-marshal.h +++ b/dbus/dbus-marshal.h @@ -24,14 +24,19 @@ #ifndef DBUS_MARSHAL_H #define DBUS_MARSHAL_H +#include #include #include #include +#ifndef PACKAGE +#error "config.h not included here" +#endif + #ifdef WORDS_BIGENDIAN -#define DBUS_COMPILER_BYTE_ORDER DBUS_LITTLE_ENDIAN -#else #define DBUS_COMPILER_BYTE_ORDER DBUS_BIG_ENDIAN +#else +#define DBUS_COMPILER_BYTE_ORDER DBUS_LITTLE_ENDIAN #endif dbus_bool_t _dbus_marshal_double (DBusString *str, diff --git a/dbus/dbus-message-internal.h b/dbus/dbus-message-internal.h index dc36a244..9706a284 100644 --- a/dbus/dbus-message-internal.h +++ b/dbus/dbus-message-internal.h @@ -39,6 +39,7 @@ void _dbus_message_lock (DBusMessage *message); DBusMessageLoader* _dbus_message_loader_new (void); void _dbus_message_loader_ref (DBusMessageLoader *loader); void _dbus_message_loader_unref (DBusMessageLoader *loader); + void _dbus_message_loader_get_buffer (DBusMessageLoader *loader, DBusString **buffer); void _dbus_message_loader_return_buffer (DBusMessageLoader *loader, @@ -47,6 +48,7 @@ void _dbus_message_loader_return_buffer (DBusMessageLoader DBusMessage* _dbus_message_loader_pop_message (DBusMessageLoader *loader); +dbus_bool_t _dbus_message_loader_get_is_corrupted (DBusMessageLoader *loader); DBUS_END_DECLS; diff --git a/dbus/dbus-message.c b/dbus/dbus-message.c index 62ac5eaf..aed943f7 100644 --- a/dbus/dbus-message.c +++ b/dbus/dbus-message.c @@ -246,6 +246,8 @@ struct DBusMessageLoader DBusList *messages; /**< Complete messages. */ unsigned int buffer_outstanding : 1; /**< Someone is using the buffer to read */ + + unsigned int corrupted : 1; /**< We got broken data, and are no longer working */ }; /** @@ -284,7 +286,8 @@ _dbus_message_loader_new (void) } /* preallocate the buffer for speed, ignore failure */ - (void) _dbus_string_set_length (&loader->data, INITIAL_LOADER_DATA_LEN); + _dbus_string_set_length (&loader->data, INITIAL_LOADER_DATA_LEN); + _dbus_string_set_length (&loader->data, 0); return loader; } @@ -376,9 +379,28 @@ _dbus_message_loader_return_buffer (DBusMessageLoader *loader, loader->buffer_outstanding = FALSE; + if (loader->corrupted) + return; + while (_dbus_string_get_length (&loader->data) >= 7) { DBusMessage *message; + const char *d; + + _dbus_string_get_const_data (&loader->data, &d); + if (d[0] != 'H' || + d[1] != '\0' || + d[2] != 'B' || + d[3] != 'o' || + d[4] != 'd' || + d[5] != 'y' || + d[6] != '\0') + { + _dbus_verbose_bytes (d, + _dbus_string_get_length (&loader->data)); + loader->corrupted = TRUE; + return; + } message = dbus_message_new (); if (message == NULL) @@ -407,4 +429,20 @@ _dbus_message_loader_pop_message (DBusMessageLoader *loader) return _dbus_list_pop_first (&loader->messages); } + +/** + * Checks whether the loader is confused due to bad data. + * If messages are received that are invalid, the + * loader gets confused and gives up permanently. + * This state is called "corrupted." + * + * @param loader the loader + * @returns #TRUE if the loader is hosed. + */ +dbus_bool_t +_dbus_message_loader_get_is_corrupted (DBusMessageLoader *loader) +{ + return loader->corrupted; +} + /** @} */ diff --git a/dbus/dbus-server-unix.c b/dbus/dbus-server-unix.c index c58923b9..66494bd4 100644 --- a/dbus/dbus-server-unix.c +++ b/dbus/dbus-server-unix.c @@ -77,7 +77,7 @@ handle_new_client_fd (DBusServer *server, if (!_dbus_set_fd_nonblocking (client_fd, NULL)) return; - transport = _dbus_transport_new_for_fd (client_fd); + transport = _dbus_transport_new_for_fd (client_fd, TRUE); if (transport == NULL) { close (client_fd); diff --git a/dbus/dbus-string.c b/dbus/dbus-string.c index ef015c68..c4a51d5b 100644 --- a/dbus/dbus-string.c +++ b/dbus/dbus-string.c @@ -723,7 +723,7 @@ open_gap (int len, memmove (dest->str + insert_at + len, dest->str + insert_at, - dest->len - len); + dest->len - len - insert_at); return TRUE; } @@ -1139,13 +1139,16 @@ _dbus_string_skip_blank (const DBusString *str, i = start; while (i < real->len) { - if (real->str[i] != ' ' || - real->str[i] != '\t') + if (!(real->str[i] == ' ' || + real->str[i] == '\t')) break; ++i; } + _dbus_assert (i == real->len || !(real->str[i] == ' ' || + real->str[i] == '\t')); + if (end) *end = i; } @@ -1586,6 +1589,43 @@ _dbus_string_base64_decode (const DBusString *source, return TRUE; } +/** + * Checks that the given range of the string + * is valid ASCII. If the given range is not contained + * in the string, returns #FALSE. + * + * @param str the string + * @param start first byte index to check + * @param len number of bytes to check + * @returns #TRUE if the byte range exists and is all valid ASCII + */ +dbus_bool_t +_dbus_string_validate_ascii (const DBusString *str, + int start, + int len) +{ + const unsigned char *s; + const unsigned char *end; + DBUS_CONST_STRING_PREAMBLE (str); + _dbus_assert (start >= 0); + _dbus_assert (len >= 0); + + if ((start + len) > real->len) + return FALSE; + + s = real->str + start; + end = s + len; + while (s != end) + { + if (*s == '\0' || + ((*s & ~0x7f) != 0)) + return FALSE; + + ++s; + } + + return TRUE; +} /** @} */ @@ -1780,6 +1820,24 @@ _dbus_string_test (void) _dbus_assert (_dbus_string_get_length (&str) == 0); _dbus_assert (_dbus_string_get_length (&other) == i); + if (!_dbus_string_append (&str, "Hello World")) + _dbus_assert_not_reached ("could not append to string"); + + if (!_dbus_string_move (&str, 0, &other, _dbus_string_get_length (&other))) + _dbus_assert_not_reached ("could not move"); + + _dbus_assert (_dbus_string_get_length (&str) == 0); + _dbus_assert (_dbus_string_get_length (&other) == i * 2); + + if (!_dbus_string_append (&str, "Hello World")) + _dbus_assert_not_reached ("could not append to string"); + + if (!_dbus_string_move (&str, 0, &other, _dbus_string_get_length (&other) / 2)) + _dbus_assert_not_reached ("could not move"); + + _dbus_assert (_dbus_string_get_length (&str) == 0); + _dbus_assert (_dbus_string_get_length (&other) == i * 3); + _dbus_string_free (&other); /* Check copy */ @@ -1797,6 +1855,22 @@ _dbus_string_test (void) _dbus_assert (_dbus_string_get_length (&str) == i); _dbus_assert (_dbus_string_get_length (&other) == i); + + if (!_dbus_string_copy (&str, 0, &other, _dbus_string_get_length (&other))) + _dbus_assert_not_reached ("could not copy"); + + _dbus_assert (_dbus_string_get_length (&str) == i); + _dbus_assert (_dbus_string_get_length (&other) == i * 2); + _dbus_assert (_dbus_string_equal_c_str (&other, + "Hello WorldHello World")); + + if (!_dbus_string_copy (&str, 0, &other, _dbus_string_get_length (&other) / 2)) + _dbus_assert_not_reached ("could not copy"); + + _dbus_assert (_dbus_string_get_length (&str) == i); + _dbus_assert (_dbus_string_get_length (&other) == i * 3); + _dbus_assert (_dbus_string_equal_c_str (&other, + "Hello WorldHello WorldHello World")); _dbus_string_free (&str); _dbus_string_free (&other); diff --git a/dbus/dbus-string.h b/dbus/dbus-string.h index 0d495ab6..471ac0c2 100644 --- a/dbus/dbus-string.h +++ b/dbus/dbus-string.h @@ -159,6 +159,10 @@ dbus_bool_t _dbus_string_base64_decode (const DBusString *source, DBusString *dest, int insert_at); +dbus_bool_t _dbus_string_validate_ascii (const DBusString *str, + int start, + int len); + DBUS_END_DECLS; #endif /* DBUS_STRING_H */ diff --git a/dbus/dbus-transport-protected.h b/dbus/dbus-transport-protected.h index e6c696f0..5105165a 100644 --- a/dbus/dbus-transport-protected.h +++ b/dbus/dbus-transport-protected.h @@ -27,6 +27,7 @@ #include #include #include +#include DBUS_BEGIN_DECLS; @@ -73,12 +74,17 @@ struct DBusTransport DBusConnection *connection; /**< Connection owning this transport. */ DBusMessageLoader *loader; /**< Message-loading buffer. */ - - unsigned int disconnected : 1; /**< TRUE if we are disconnected. */ + + DBusAuth *auth; /**< Authentication conversation */ + + 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 messages_need_sending : 1; /**< #TRUE if we need to write messages out */ }; dbus_bool_t _dbus_transport_init_base (DBusTransport *transport, - const DBusTransportVTable *vtable); + const DBusTransportVTable *vtable, + dbus_bool_t server); void _dbus_transport_finalize_base (DBusTransport *transport); diff --git a/dbus/dbus-transport-unix.c b/dbus/dbus-transport-unix.c index e603fc65..c1487a0f 100644 --- a/dbus/dbus-transport-unix.c +++ b/dbus/dbus-transport-unix.c @@ -64,27 +64,309 @@ struct DBusTransportUnix }; static void -unix_finalize (DBusTransport *transport) +free_watches (DBusTransport *transport) { DBusTransportUnix *unix_transport = (DBusTransportUnix*) transport; - _dbus_transport_finalize_base (transport); - if (unix_transport->watch) { + if (transport->connection) + _dbus_connection_remove_watch (transport->connection, + unix_transport->watch); _dbus_watch_invalidate (unix_transport->watch); _dbus_watch_unref (unix_transport->watch); + unix_transport->watch = NULL; } + + if (unix_transport->write_watch) + { + if (transport->connection) + _dbus_connection_remove_watch (transport->connection, + unix_transport->write_watch); + _dbus_watch_invalidate (unix_transport->write_watch); + _dbus_watch_unref (unix_transport->write_watch); + unix_transport->write_watch = NULL; + } +} + +static void +unix_finalize (DBusTransport *transport) +{ + DBusTransportUnix *unix_transport = (DBusTransportUnix*) transport; + + free_watches (transport); + + _dbus_transport_finalize_base (transport); + + _dbus_assert (unix_transport->watch == NULL); + _dbus_assert (unix_transport->write_watch == NULL); dbus_free (transport); } +static void +check_write_watch (DBusTransport *transport) +{ + DBusTransportUnix *unix_transport = (DBusTransportUnix*) transport; + dbus_bool_t need_write_watch; + + if (transport->connection == NULL) + return; + + _dbus_transport_ref (transport); + + if (_dbus_transport_get_is_authenticated (transport)) + need_write_watch = transport->messages_need_sending; + else + need_write_watch = _dbus_auth_do_work (transport->auth) == DBUS_AUTH_STATE_HAVE_BYTES_TO_SEND; + + if (transport->disconnected) + need_write_watch = FALSE; + + if (need_write_watch && + unix_transport->write_watch == NULL) + { + unix_transport->write_watch = + _dbus_watch_new (unix_transport->fd, + DBUS_WATCH_WRITABLE); + + /* we can maybe add it some other time, just silently bomb */ + if (unix_transport->write_watch == NULL) + return; + + if (!_dbus_connection_add_watch (transport->connection, + unix_transport->write_watch)) + { + _dbus_watch_invalidate (unix_transport->write_watch); + _dbus_watch_unref (unix_transport->write_watch); + unix_transport->write_watch = NULL; + } + } + else if (!need_write_watch && + unix_transport->write_watch != NULL) + { + _dbus_connection_remove_watch (transport->connection, + unix_transport->write_watch); + _dbus_watch_invalidate (unix_transport->write_watch); + _dbus_watch_unref (unix_transport->write_watch); + unix_transport->write_watch = NULL; + } + + _dbus_transport_unref (transport); +} + static void do_io_error (DBusTransport *transport) { + _dbus_transport_ref (transport); _dbus_transport_disconnect (transport); _dbus_connection_transport_error (transport->connection, DBUS_RESULT_DISCONNECTED); + _dbus_transport_unref (transport); +} + +static void +queue_messages (DBusTransport *transport) +{ + DBusMessage *message; + + /* Queue any messages */ + while ((message = _dbus_message_loader_pop_message (transport->loader))) + { + _dbus_verbose ("queueing received message %p\n", message); + + _dbus_connection_queue_received_message (transport->connection, + message); + dbus_message_unref (message); + } + + if (_dbus_message_loader_get_is_corrupted (transport->loader)) + { + _dbus_verbose ("Corrupted message stream, disconnecting\n"); + do_io_error (transport); + } +} + +/* return value is whether we successfully read any new data. */ +static dbus_bool_t +read_data_into_auth (DBusTransport *transport) +{ + DBusTransportUnix *unix_transport = (DBusTransportUnix*) transport; + DBusString buffer; + int bytes_read; + + if (!_dbus_string_init (&buffer, _DBUS_INT_MAX)) + { + /* just disconnect if we don't have memory + * to do an authentication + */ + _dbus_verbose ("No memory for authentication\n"); + do_io_error (transport); + return FALSE; + } + + bytes_read = _dbus_read (unix_transport->fd, + &buffer, unix_transport->max_bytes_read_per_iteration); + + if (bytes_read > 0) + { + _dbus_verbose (" read %d bytes in auth phase\n", bytes_read); + + if (_dbus_auth_bytes_received (transport->auth, + &buffer)) + { + _dbus_string_free (&buffer); + return TRUE; /* We did read some data! woo! */ + } + else + { + /* just disconnect if we don't have memory to do an + * authentication, don't fool with trying to save the buffer + * and who knows what. + */ + _dbus_verbose ("No memory for authentication\n"); + do_io_error (transport); + } + } + else if (bytes_read < 0) + { + /* EINTR already handled for us */ + + if (errno == EAGAIN || + errno == EWOULDBLOCK) + ; /* do nothing, just return FALSE below */ + else + { + _dbus_verbose ("Error reading from remote app: %s\n", + _dbus_strerror (errno)); + do_io_error (transport); + } + } + else if (bytes_read == 0) + { + _dbus_verbose ("Disconnected from remote app\n"); + do_io_error (transport); + } + + _dbus_string_free (&buffer); + return FALSE; +} + +/* Return value is whether we successfully wrote any bytes */ +static dbus_bool_t +write_data_from_auth (DBusTransport *transport) +{ + DBusTransportUnix *unix_transport = (DBusTransportUnix*) transport; + int bytes_written; + const DBusString *buffer; + + if (!_dbus_auth_get_bytes_to_send (transport->auth, + &buffer)) + return FALSE; + + bytes_written = _dbus_write (unix_transport->fd, + buffer, + 0, _dbus_string_get_length (buffer)); + + if (bytes_written > 0) + { + _dbus_auth_bytes_sent (transport->auth, bytes_written); + return TRUE; + } + else if (bytes_written < 0) + { + /* EINTR already handled for us */ + + if (errno == EAGAIN || + errno == EWOULDBLOCK) + ; + else + { + _dbus_verbose ("Error writing to remote app: %s\n", + _dbus_strerror (errno)); + do_io_error (transport); + } + } + + return FALSE; +} + +static void +do_authentication (DBusTransport *transport, + dbus_bool_t do_reading, + dbus_bool_t do_writing) +{ + _dbus_transport_ref (transport); + + while (!_dbus_transport_get_is_authenticated (transport) && + _dbus_transport_get_is_connected (transport)) + { + switch (_dbus_auth_do_work (transport->auth)) + { + case DBUS_AUTH_STATE_WAITING_FOR_INPUT: + _dbus_verbose (" auth state: waiting for input\n"); + if (!do_reading || !read_data_into_auth (transport)) + goto out; + break; + + case DBUS_AUTH_STATE_WAITING_FOR_MEMORY: + /* Screw it, just disconnect */ + _dbus_verbose (" auth state: waiting for memory\n"); + do_io_error (transport); + break; + + case DBUS_AUTH_STATE_HAVE_BYTES_TO_SEND: + _dbus_verbose (" auth state: bytes to send\n"); + if (!do_writing || !write_data_from_auth (transport)) + goto out; + break; + + case DBUS_AUTH_STATE_NEED_DISCONNECT: + _dbus_verbose (" auth state: need to disconnect\n"); + do_io_error (transport); + break; + + case DBUS_AUTH_STATE_AUTHENTICATED_WITH_UNUSED_BYTES: + { + DBusString *buffer; + int orig_len; + + _dbus_verbose (" auth state: auth with unused bytes\n"); + + _dbus_message_loader_get_buffer (transport->loader, + &buffer); + + orig_len = _dbus_string_get_length (buffer); + + if (!_dbus_auth_get_unused_bytes (transport->auth, + buffer)) + { + _dbus_verbose ("Not enough memory to transfer unused bytes from auth conversation\n"); + do_io_error (transport); + } + + _dbus_verbose (" %d unused bytes sent to message loader\n", + _dbus_string_get_length (buffer) - + orig_len); + + _dbus_message_loader_return_buffer (transport->loader, + buffer, + _dbus_string_get_length (buffer) - + orig_len); + + queue_messages (transport); + } + break; + + case DBUS_AUTH_STATE_AUTHENTICATED: + _dbus_verbose (" auth state: authenticated\n"); + break; + } + } + + out: + check_write_watch (transport); + _dbus_transport_unref (transport); } static void @@ -92,10 +374,18 @@ do_writing (DBusTransport *transport) { int total; DBusTransportUnix *unix_transport = (DBusTransportUnix*) transport; + + /* No messages without authentication! */ + if (!_dbus_transport_get_is_authenticated (transport)) + return; + + if (transport->disconnected) + return; total = 0; - while (_dbus_connection_have_messages_to_send (transport->connection)) + while (!transport->disconnected && + _dbus_connection_have_messages_to_send (transport->connection)) { int bytes_written; DBusMessage *message; @@ -149,7 +439,7 @@ do_writing (DBusTransport *transport) goto out; else { - _dbus_verbose ("Error writing to message bus: %s\n", + _dbus_verbose ("Error writing to remote app: %s\n", _dbus_strerror (errno)); do_io_error (transport); goto out; @@ -183,9 +473,12 @@ do_reading (DBusTransport *transport) { DBusTransportUnix *unix_transport = (DBusTransportUnix*) transport; DBusString *buffer; - int buffer_len; int bytes_read; int total; + + /* No messages without authentication! */ + if (!_dbus_transport_get_is_authenticated (transport)) + return; total = 0; @@ -198,10 +491,11 @@ do_reading (DBusTransport *transport) goto out; } + if (transport->disconnected) + goto out; + _dbus_message_loader_get_buffer (transport->loader, &buffer); - - buffer_len = _dbus_string_get_length (buffer); bytes_read = _dbus_read (unix_transport->fd, buffer, unix_transport->max_bytes_read_per_iteration); @@ -219,7 +513,7 @@ do_reading (DBusTransport *transport) goto out; else { - _dbus_verbose ("Error reading from message bus: %s\n", + _dbus_verbose ("Error reading from remote app: %s\n", _dbus_strerror (errno)); do_io_error (transport); goto out; @@ -227,27 +521,17 @@ do_reading (DBusTransport *transport) } else if (bytes_read == 0) { - _dbus_verbose ("Disconnected from message bus\n"); + _dbus_verbose ("Disconnected from remote app\n"); do_io_error (transport); goto out; } else { - DBusMessage *message; - _dbus_verbose (" read %d bytes\n", bytes_read); total += bytes_read; - /* Queue any messages */ - while ((message = _dbus_message_loader_pop_message (transport->loader))) - { - _dbus_verbose ("queueing received message %p\n", message); - - _dbus_connection_queue_received_message (transport->connection, - message); - dbus_message_unref (message); - } + queue_messages (transport); /* Try reading more data until we get EAGAIN and return, or * exceed max bytes per iteration. If in blocking mode of @@ -269,7 +553,7 @@ unix_handle_watch (DBusTransport *transport, _dbus_assert (watch == unix_transport->watch || watch == unix_transport->write_watch); - + if (flags & (DBUS_WATCH_HANGUP | DBUS_WATCH_ERROR)) { _dbus_transport_disconnect (transport); @@ -280,25 +564,26 @@ unix_handle_watch (DBusTransport *transport, if (watch == unix_transport->watch && (flags & DBUS_WATCH_READABLE)) - do_reading (transport); + { + _dbus_verbose ("handling read watch\n"); + do_authentication (transport, TRUE, FALSE); + do_reading (transport); + } else if (watch == unix_transport->write_watch && (flags & DBUS_WATCH_WRITABLE)) - do_writing (transport); + { + _dbus_verbose ("handling write watch\n"); + do_authentication (transport, FALSE, TRUE); + do_writing (transport); + } } static void unix_disconnect (DBusTransport *transport) { DBusTransportUnix *unix_transport = (DBusTransportUnix*) transport; - - if (unix_transport->watch) - { - _dbus_connection_remove_watch (transport->connection, - unix_transport->watch); - _dbus_watch_invalidate (unix_transport->watch); - _dbus_watch_unref (unix_transport->watch); - unix_transport->watch = NULL; - } + + free_watches (transport); close (unix_transport->fd); unix_transport->fd = -1; @@ -325,46 +610,20 @@ unix_connection_set (DBusTransport *transport) watch)) { _dbus_transport_disconnect (transport); + _dbus_watch_unref (watch); return; } unix_transport->watch = watch; + + check_write_watch (transport); } static void unix_messages_pending (DBusTransport *transport, int messages_pending) { - DBusTransportUnix *unix_transport = (DBusTransportUnix*) transport; - - if (messages_pending > 0 && - unix_transport->write_watch == NULL) - { - unix_transport->write_watch = - _dbus_watch_new (unix_transport->fd, - DBUS_WATCH_WRITABLE); - - /* we can maybe add it some other time, just silently bomb */ - if (unix_transport->write_watch == NULL) - return; - - if (!_dbus_connection_add_watch (transport->connection, - unix_transport->write_watch)) - { - _dbus_watch_invalidate (unix_transport->write_watch); - _dbus_watch_unref (unix_transport->write_watch); - unix_transport->write_watch = NULL; - } - } - else if (messages_pending == 0 && - unix_transport->write_watch != NULL) - { - _dbus_connection_remove_watch (transport->connection, - unix_transport->write_watch); - _dbus_watch_invalidate (unix_transport->write_watch); - _dbus_watch_unref (unix_transport->write_watch); - unix_transport->write_watch = NULL; - } + check_write_watch (transport); } static void @@ -377,6 +636,11 @@ unix_do_iteration (DBusTransport *transport, fd_set write_set; dbus_bool_t do_select; + /* "again" has to be up here because on EINTR the fd sets become + * undefined + */ + again: + do_select = FALSE; FD_ZERO (&read_set); @@ -398,8 +662,6 @@ unix_do_iteration (DBusTransport *transport, fd_set err_set; struct timeval timeout; dbus_bool_t use_timeout; - - again: FD_ZERO (&err_set); FD_SET (unix_transport->fd, &err_set); @@ -434,9 +696,16 @@ unix_do_iteration (DBusTransport *transport, do_io_error (transport); else { - if (FD_ISSET (unix_transport->fd, &read_set)) + dbus_bool_t need_read = FD_ISSET (unix_transport->fd, &read_set); + dbus_bool_t need_write = FD_ISSET (unix_transport->fd, &write_set); + + _dbus_verbose ("in iteration, need_read=%d need_write=%d\n", + need_read, need_write); + do_authentication (transport, need_read, need_write); + + if (need_read) do_reading (transport); - if (FD_ISSET (unix_transport->fd, &write_set)) + if (need_write) do_writing (transport); } } @@ -466,10 +735,12 @@ static DBusTransportVTable unix_vtable = { * boil down to a full duplex file descriptor. * * @param fd the file descriptor. + * @param server #TRUE if this transport is on the server side of a connection * @returns the new transport, or #NULL if no memory. */ DBusTransport* -_dbus_transport_new_for_fd (int fd) +_dbus_transport_new_for_fd (int fd, + dbus_bool_t server) { DBusTransportUnix *unix_transport; @@ -478,7 +749,8 @@ _dbus_transport_new_for_fd (int fd) return NULL; if (!_dbus_transport_init_base (&unix_transport->base, - &unix_vtable)) + &unix_vtable, + server)) { dbus_free (unix_transport); return NULL; @@ -490,6 +762,8 @@ _dbus_transport_new_for_fd (int fd) /* These values should probably be tunable or something. */ unix_transport->max_bytes_read_per_iteration = 2048; unix_transport->max_bytes_written_per_iteration = 2048; + + check_write_watch ((DBusTransport*) unix_transport); return (DBusTransport*) unix_transport; } @@ -499,11 +773,13 @@ _dbus_transport_new_for_fd (int fd) * path. * * @param path the path to the domain socket. + * @param server #TRUE if this transport is on the server side of a connection * @param result location to store reason for failure. * @returns a new transport, or #NULL on failure. */ DBusTransport* _dbus_transport_new_for_domain_socket (const char *path, + dbus_bool_t server, DBusResultCode *result) { int fd; @@ -512,8 +788,11 @@ _dbus_transport_new_for_domain_socket (const char *path, fd = _dbus_connect_unix_socket (path, result); if (fd < 0) return NULL; + + _dbus_verbose ("Successfully connected to unix socket %s\n", + path); - transport = _dbus_transport_new_for_fd (fd); + transport = _dbus_transport_new_for_fd (fd, server); if (transport == NULL) { dbus_set_result (result, DBUS_RESULT_NO_MEMORY); diff --git a/dbus/dbus-transport-unix.h b/dbus/dbus-transport-unix.h index c483f05e..0c7d04fa 100644 --- a/dbus/dbus-transport-unix.h +++ b/dbus/dbus-transport-unix.h @@ -27,8 +27,10 @@ DBUS_BEGIN_DECLS; -DBusTransport* _dbus_transport_new_for_fd (int fd); +DBusTransport* _dbus_transport_new_for_fd (int fd, + dbus_bool_t server); DBusTransport* _dbus_transport_new_for_domain_socket (const char *path, + dbus_bool_t server, DBusResultCode *result); diff --git a/dbus/dbus-transport.c b/dbus/dbus-transport.c index e212cf43..085b0224 100644 --- a/dbus/dbus-transport.c +++ b/dbus/dbus-transport.c @@ -25,6 +25,7 @@ #include "dbus-transport-unix.h" #include "dbus-connection-internal.h" #include "dbus-watch.h" +#include "dbus-auth.h" /** * @defgroup DBusTransport DBusTransport object @@ -47,27 +48,63 @@ * or encryption schemes. */ +/** + * Refs a transport and associated connection for reentrancy. + * + * @todo this macro reflects a design mistake, which is that the + * transport has a pointer to its connection. Ownership should move in + * only one direction; the connection should push/pull from the + * transport, rather than vice versa. Then the connection would take + * care of referencing itself when needed. + */ +#define DBUS_TRANSPORT_HOLD_REF(t) \ + _dbus_transport_ref (t); if ((t)->connection) dbus_connection_ref ((t)->connection) + +/** + * Inverse of DBUS_TRANSPORT_HOLD_REF(). + */ +#define DBUS_TRANSPORT_RELEASE_REF(t) \ + if ((t)->connection) dbus_connection_unref ((t)->connection); _dbus_transport_unref (t) + + /** * Initializes the base class members of DBusTransport. * Chained up to by subclasses in their constructor. * * @param transport the transport being created. * @param vtable the subclass vtable. + * @param server #TRUE if this transport is on the server side of a connection * @returns #TRUE on success. */ dbus_bool_t _dbus_transport_init_base (DBusTransport *transport, - const DBusTransportVTable *vtable) + const DBusTransportVTable *vtable, + dbus_bool_t server) { DBusMessageLoader *loader; - + DBusAuth *auth; + loader = _dbus_message_loader_new (); if (loader == NULL) return FALSE; + + if (server) + auth = _dbus_auth_server_new (); + else + auth = _dbus_auth_client_new (); + if (auth == NULL) + { + _dbus_message_loader_unref (loader); + return FALSE; + } transport->refcount = 1; transport->vtable = vtable; transport->loader = loader; + transport->auth = auth; + transport->authenticated = FALSE; + transport->messages_need_sending = FALSE; + transport->disconnected = FALSE; return TRUE; } @@ -85,10 +122,12 @@ _dbus_transport_finalize_base (DBusTransport *transport) _dbus_transport_disconnect (transport); _dbus_message_loader_unref (transport->loader); + _dbus_auth_unref (transport->auth); } /** - * Opens a new transport for the given address. + * Opens a new transport for the given address. (This opens a + * client-side-of-the-connection transport.) * * @todo right now the address is just a Unix domain socket path. * @@ -108,7 +147,9 @@ _dbus_transport_open (const char *address, */ /* Pretend it's just a unix domain socket name for now */ - transport = _dbus_transport_new_for_domain_socket (address, result); + transport = _dbus_transport_new_for_domain_socket (address, + FALSE, + result); return transport; } @@ -161,10 +202,12 @@ _dbus_transport_disconnect (DBusTransport *transport) if (transport->disconnected) return; - + + DBUS_TRANSPORT_HOLD_REF (transport); (* transport->vtable->disconnect) (transport); transport->disconnected = TRUE; + DBUS_TRANSPORT_RELEASE_REF (transport); } /** @@ -173,6 +216,7 @@ _dbus_transport_disconnect (DBusTransport *transport) * or because the server drops its end of the connection. * * @param transport the transport. + * @returns whether we're connected */ dbus_bool_t _dbus_transport_get_is_connected (DBusTransport *transport) @@ -180,6 +224,27 @@ _dbus_transport_get_is_connected (DBusTransport *transport) return !transport->disconnected; } +/** + * Returns #TRUE if we have been authenticated. Will return #TRUE + * even if the transport is disconnected. + * + * @param transport the transport + * @returns whether we're authenticated + */ +dbus_bool_t +_dbus_transport_get_is_authenticated (DBusTransport *transport) +{ + if (transport->authenticated) + return TRUE; + else + { + transport->authenticated = + _dbus_auth_do_work (transport->auth) == DBUS_AUTH_STATE_AUTHENTICATED; + + return transport->authenticated; + } +} + /** * Handles a watch by reading data, writing data, or disconnecting * the transport, as appropriate for the given condition. @@ -202,9 +267,19 @@ _dbus_transport_handle_watch (DBusTransport *transport, return; } - _dbus_watch_sanitize_condition (watch, &condition); + if (dbus_watch_get_fd (watch) < 0) + { + _dbus_warn ("Tried to handle an invalidated watch; this watch should have been removed\n"); + return; + } + _dbus_watch_sanitize_condition (watch, &condition); + + DBUS_TRANSPORT_HOLD_REF (transport); + _dbus_watch_ref (watch); (* transport->vtable->handle_watch) (transport, watch, condition); + _dbus_watch_unref (watch); + DBUS_TRANSPORT_RELEASE_REF (transport); } /** @@ -224,7 +299,9 @@ _dbus_transport_set_connection (DBusTransport *transport, transport->connection = connection; + DBUS_TRANSPORT_HOLD_REF (transport); (* transport->vtable->connection_set) (transport); + DBUS_TRANSPORT_RELEASE_REF (transport); } /** @@ -248,9 +325,13 @@ _dbus_transport_messages_pending (DBusTransport *transport, DBUS_RESULT_DISCONNECTED); return; } - + + transport->messages_need_sending = queue_length > 0; + + DBUS_TRANSPORT_HOLD_REF (transport); (* transport->vtable->messages_pending) (transport, queue_length); + DBUS_TRANSPORT_RELEASE_REF (transport); } /** @@ -281,9 +362,11 @@ _dbus_transport_do_iteration (DBusTransport *transport, DBUS_RESULT_DISCONNECTED); return; } - + + DBUS_TRANSPORT_HOLD_REF (transport); (* transport->vtable->do_iteration) (transport, flags, timeout_milliseconds); + DBUS_TRANSPORT_RELEASE_REF (transport); } /** @} */ diff --git a/dbus/dbus-transport.h b/dbus/dbus-transport.h index 84c1f8da..47062b74 100644 --- a/dbus/dbus-transport.h +++ b/dbus/dbus-transport.h @@ -30,22 +30,24 @@ DBUS_BEGIN_DECLS; typedef struct DBusTransport DBusTransport; -DBusTransport* _dbus_transport_open (const char *address, - DBusResultCode *result); -void _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_handle_watch (DBusTransport *transport, - DBusWatch *watch, - unsigned int condition); -void _dbus_transport_set_connection (DBusTransport *transport, - DBusConnection *connection); -void _dbus_transport_messages_pending (DBusTransport *transport, - int queue_length); -void _dbus_transport_do_iteration (DBusTransport *transport, - unsigned int flags, - int timeout_milliseconds); +DBusTransport* _dbus_transport_open (const char *address, + DBusResultCode *result); +void _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); +dbus_bool_t _dbus_transport_get_is_authenticated (DBusTransport *transport); +void _dbus_transport_handle_watch (DBusTransport *transport, + DBusWatch *watch, + unsigned int condition); +void _dbus_transport_set_connection (DBusTransport *transport, + DBusConnection *connection); +void _dbus_transport_messages_pending (DBusTransport *transport, + int queue_length); +void _dbus_transport_do_iteration (DBusTransport *transport, + unsigned int flags, + int timeout_milliseconds); + DBUS_END_DECLS; diff --git a/dbus/dbus-watch.c b/dbus/dbus-watch.c index 69184f17..78001612 100644 --- a/dbus/dbus-watch.c +++ b/dbus/dbus-watch.c @@ -190,7 +190,7 @@ _dbus_watch_list_new (void) void _dbus_watch_list_free (DBusWatchList *watch_list) { - /* free watch_data as a side effect */ + /* free watch_data and removes watches as a side effect */ _dbus_watch_list_set_functions (watch_list, NULL, NULL, NULL, NULL); diff --git a/doc/dbus-sasl-profile.txt b/doc/dbus-sasl-profile.txt index f71ceb07..44c756ad 100644 --- a/doc/dbus-sasl-profile.txt +++ b/doc/dbus-sasl-profile.txt @@ -26,7 +26,7 @@ The protocol is a line-based protocol, where each line ends with \r\n. Each line begins with an all-caps ASCII command name containing only the character range [A-Z], a space, then any arguments for the command, then the \r\n ending the line. The protocol is -case-sensitive. +case-sensitive. All bytes must be in the ASCII character set. Commands from the client to the server are as follows: diff --git a/test/watch.c b/test/watch.c index 161ae3a9..3b525d37 100644 --- a/test/watch.c +++ b/test/watch.c @@ -15,6 +15,7 @@ #undef MAX #define MAX(a, b) (((a) > (b)) ? (a) : (b)) +static int watch_list_serial = 0; static DBusList *watches = NULL; static dbus_bool_t exited = FALSE; static DBusList *connections = NULL; @@ -31,6 +32,19 @@ typedef struct void *data; } WatchData; +static void +free_watch_data (void *data) +{ + WatchData *wd = data; + + if (wd->type == WATCH_CONNECTION) + dbus_connection_unref (wd->data); + else if (wd->type == WATCH_SERVER) + dbus_server_unref (wd->data); + + dbus_free (wd); +} + static void add_connection_watch (DBusWatch *watch, DBusConnection *connection) @@ -40,17 +54,36 @@ add_connection_watch (DBusWatch *watch, wd = dbus_new0 (WatchData, 1); wd->type = WATCH_CONNECTION; wd->data = connection; + + dbus_connection_ref (connection); _dbus_list_append (&watches, watch); - dbus_watch_set_data (watch, wd, dbus_free); -} + dbus_watch_set_data (watch, wd, free_watch_data); + + watch_list_serial += 1; + +#if 0 + printf ("Added connection %swatch for fd %d\n", + dbus_watch_get_flags (watch) & DBUS_WATCH_WRITABLE ? "write " : "", + dbus_watch_get_fd (watch)); +#endif + } static void remove_connection_watch (DBusWatch *watch, DBusConnection *connection) { - _dbus_list_remove (&watches, watch); + if (!_dbus_list_remove (&watches, watch)) + _dbus_assert_not_reached ("removed nonexistent watch"); + dbus_watch_set_data (watch, NULL, NULL); + + watch_list_serial += 1; + +#if 0 + printf ("Removed connection watch for fd %d\n", + dbus_watch_get_fd (watch)); +#endif } static void @@ -62,18 +95,37 @@ add_server_watch (DBusWatch *watch, wd = dbus_new0 (WatchData, 1); wd->type = WATCH_SERVER; wd->data = server; + + dbus_server_ref (server); _dbus_list_append (&watches, watch); - dbus_watch_set_data (watch, wd, dbus_free); + dbus_watch_set_data (watch, wd, free_watch_data); + + watch_list_serial += 1; + +#if 0 + printf ("Added server %swatch for fd %d\n", + dbus_watch_get_flags (watch) & DBUS_WATCH_WRITABLE ? "write " : "", + dbus_watch_get_fd (watch)); +#endif } static void remove_server_watch (DBusWatch *watch, DBusServer *server) { - _dbus_list_remove (&watches, watch); + if (!_dbus_list_remove (&watches, watch)) + _dbus_assert_not_reached ("removed nonexistent server watch"); + dbus_watch_set_data (watch, NULL, NULL); + + watch_list_serial += 1; + +#if 0 + printf ("Removed server watch for fd %d\n", + dbus_watch_get_fd (watch)); +#endif } static int count = 0; @@ -122,7 +174,6 @@ do_mainloop (void) /* Of course with any real app you'd use GMainLoop or * QSocketNotifier and not have to see all this crap. */ - while (!exited && watches != NULL) { fd_set read_set; @@ -130,7 +181,8 @@ do_mainloop (void) fd_set err_set; int max_fd; DBusList *link; - + int initial_watch_serial; + check_messages (); FD_ZERO (&read_set); @@ -167,6 +219,7 @@ do_mainloop (void) select (max_fd + 1, &read_set, &write_set, &err_set, NULL); + initial_watch_serial = watch_list_serial; link = _dbus_list_get_first_link (&watches); while (link != NULL) { @@ -175,6 +228,20 @@ do_mainloop (void) DBusWatch *watch; unsigned int flags; unsigned int condition; + + if (initial_watch_serial != watch_list_serial) + { + /* Watches were added/removed, + * hosing our list; break out of here + */ + /* A more elegant solution might be to ref + * all watches, then check which have fd >= 0 + * as we iterate over them, since removed + * watches have their fd invalidated. + */ + printf ("Aborting watch iteration due to serial increment\n"); + break; + } watch = link->data; -- cgit