From: David Zeuthen Date: Fri, 16 Jul 2010 17:19:48 +0000 (-0400) Subject: GDBus: Add nonce-tcp: test case X-Git-Tag: 2.25.12~49 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=f30aef9a105a2cb44bac1bbe3b8bb93387a41d9a;p=platform%2Fupstream%2Fglib.git GDBus: Add nonce-tcp: test case Also fix a couple of TODO items in gdbusaddress.c Signed-off-by: David Zeuthen --- diff --git a/gio/gdbusaddress.c b/gio/gdbusaddress.c index 4b815e6..afc8ec6 100644 --- a/gio/gdbusaddress.c +++ b/gio/gdbusaddress.c @@ -25,6 +25,8 @@ #include #include #include +#include +#include #include "gioerror.h" #include "gdbusutils.h" @@ -148,19 +150,16 @@ is_valid_unix (const gchar *address_entry, { if (tmpdir != NULL || abstract != NULL) goto meaningless; - /* TODO: validate path */ } else if (tmpdir != NULL) { if (path != NULL || abstract != NULL) goto meaningless; - /* TODO: validate tmpdir */ } else if (abstract != NULL) { if (path != NULL || tmpdir != NULL) goto meaningless; - /* TODO: validate abstract */ } else { @@ -460,9 +459,21 @@ _g_dbus_address_parse_entry (const gchar *address_entry, goto out; } - /* TODO: actually validate that no illegal characters are present before and after then '=' sign */ key = g_uri_unescape_segment (kv_pair, s, NULL); value = g_uri_unescape_segment (s + 1, kv_pair + strlen (kv_pair), NULL); + if (key == NULL || value == NULL) + { + g_set_error (error, + G_IO_ERROR, + G_IO_ERROR_INVALID_ARGUMENT, + _("Error unescaping key or value in Key/Value pair %d, `%s', in address element `%s'"), + n, + kv_pair, + address_entry); + g_free (key); + g_free (value); + goto out; + } g_hash_table_insert (key_value_pairs, key, value); } @@ -603,7 +614,7 @@ g_dbus_address_connect (const gchar *address_entry, } } - /* TODO: deal with family */ + /* TODO: deal with family key/value-pair */ connectable = g_network_address_new (host, port); } else if (g_strcmp0 (address_entry, "autolaunch:") == 0) @@ -651,50 +662,67 @@ g_dbus_address_connect (const gchar *address_entry, if (nonce_file != NULL) { - gchar *nonce_contents; - gsize nonce_length; + gchar nonce_contents[16 + 1]; + size_t num_bytes_read; + FILE *f; - /* TODO: too dangerous to read the entire file? (think denial-of-service etc.) */ - if (!g_file_get_contents (nonce_file, - &nonce_contents, - &nonce_length, - error)) + /* be careful to read only 16 bytes - we also check that the file is only 16 bytes long */ + f = fopen (nonce_file, "rb"); + if (f == NULL) { - g_prefix_error (error, _("Error reading nonce file `%s':"), nonce_file); + g_set_error (error, + G_IO_ERROR, + G_IO_ERROR_INVALID_ARGUMENT, + _("Error opening nonce file `%s': %s"), + nonce_file, + g_strerror (errno)); g_object_unref (ret); ret = NULL; goto out; } - - if (nonce_length != 16) + num_bytes_read = fread (nonce_contents, + sizeof (gchar), + 16 + 1, + f); + if (num_bytes_read != 16) { - /* G_GSIZE_FORMAT doesn't work with gettext, so we use %lu */ - g_set_error (error, - G_IO_ERROR, - G_IO_ERROR_INVALID_ARGUMENT, - _("The nonce-file `%s' was %lu bytes. Expected 16 bytes."), - nonce_file, - nonce_length); - g_free (nonce_contents); + if (num_bytes_read == 0) + { + g_set_error (error, + G_IO_ERROR, + G_IO_ERROR_INVALID_ARGUMENT, + _("Error reading from nonce file `%s': %s"), + nonce_file, + g_strerror (errno)); + } + else + { + g_set_error (error, + G_IO_ERROR, + G_IO_ERROR_INVALID_ARGUMENT, + _("Error reading from nonce file `%s', expected 16 bytes, got %d"), + nonce_file, + (gint) num_bytes_read); + } g_object_unref (ret); ret = NULL; + fclose (f); goto out; } + fclose (f); if (!g_output_stream_write_all (g_io_stream_get_output_stream (ret), nonce_contents, - nonce_length, + 16, NULL, cancellable, error)) { - g_prefix_error (error, _("Error write contents of nonce file `%s' to stream:"), nonce_file); + g_prefix_error (error, _("Error writing contents of nonce file `%s' to stream:"), nonce_file); g_object_unref (ret); ret = NULL; - g_free (nonce_contents); goto out; } - g_free (nonce_contents); } } @@ -732,7 +760,6 @@ g_dbus_address_try_connect_one (const gchar *address_entry, if (ret == NULL) goto out; - /* TODO: validate that guid is of correct format */ guid = g_hash_table_lookup (key_value_pairs, "guid"); if (guid != NULL && out_guid != NULL) *out_guid = g_strdup (guid); @@ -1103,7 +1130,6 @@ get_session_address_dbus_launch (GError **error) /* ---------------------------------------------------------------------------------------------------- */ -/* TODO: implement for UNIX, Win32 and OS X */ static gchar * get_session_address_platform_specific (GError **error) { @@ -1112,6 +1138,7 @@ get_session_address_platform_specific (GError **error) /* need to handle OS X in a different way since `dbus-launch --autolaunch' probably won't work there */ ret = get_session_address_dbus_launch (error); #else + /* TODO: implement for UNIX, Win32 and OS X */ ret = NULL; g_set_error (error, G_IO_ERROR, diff --git a/gio/tests/gdbus-peer.c b/gio/tests/gdbus-peer.c index 284f3d6..af85fcd 100644 --- a/gio/tests/gdbus-peer.c +++ b/gio/tests/gdbus-peer.c @@ -29,6 +29,9 @@ #include #include +/* for g_unlink() */ +#include + #include #include @@ -980,6 +983,205 @@ delayed_message_processing (void) /* ---------------------------------------------------------------------------------------------------- */ +static gboolean +nonce_tcp_on_authorize_authenticated_peer (GDBusAuthObserver *observer, + GIOStream *stream, + GCredentials *credentials, + gpointer user_data) +{ + PeerData *data = user_data; + gboolean authorized; + + data->num_connection_attempts++; + + authorized = TRUE; + if (!data->accept_connection) + { + authorized = FALSE; + g_main_loop_quit (loop); + } + + return authorized; +} + +/* Runs in thread we created GDBusServer in (since we didn't pass G_DBUS_SERVER_FLAGS_RUN_IN_THREAD) */ +static void +nonce_tcp_on_new_connection (GDBusServer *server, + GDBusConnection *connection, + gpointer user_data) +{ + PeerData *data = user_data; + + g_ptr_array_add (data->current_connections, g_object_ref (connection)); + + g_main_loop_quit (loop); +} + +static gpointer +nonce_tcp_service_thread_func (gpointer user_data) +{ + PeerData *data = user_data; + GMainContext *service_context; + GDBusAuthObserver *observer; + GError *error; + + service_context = g_main_context_new (); + g_main_context_push_thread_default (service_context); + + error = NULL; + observer = g_dbus_auth_observer_new (); + server = g_dbus_server_new_sync ("nonce-tcp:", + G_DBUS_SERVER_FLAGS_NONE, + test_guid, + observer, + NULL, /* cancellable */ + &error); + g_assert_no_error (error); + + g_signal_connect (server, + "new-connection", + G_CALLBACK (nonce_tcp_on_new_connection), + data); + g_signal_connect (observer, + "authorize-authenticated-peer", + G_CALLBACK (nonce_tcp_on_authorize_authenticated_peer), + data); + g_object_unref (observer); + + g_dbus_server_start (server); + + service_loop = g_main_loop_new (service_context, FALSE); + g_main_loop_run (service_loop); + + g_main_context_pop_thread_default (service_context); + + g_main_loop_unref (service_loop); + g_main_context_unref (service_context); + + /* test code specifically unrefs the server - see below */ + g_assert (server == NULL); + + return NULL; +} + +static void +test_nonce_tcp (void) +{ + PeerData data; + GError *error; + GThread *service_thread; + GDBusConnection *c; + gchar *s; + gchar *nonce_file; + gboolean res; + const gchar *address; + + memset (&data, '\0', sizeof (PeerData)); + data.current_connections = g_ptr_array_new_with_free_func (g_object_unref); + + error = NULL; + server = NULL; + service_loop = NULL; + service_thread = g_thread_create (nonce_tcp_service_thread_func, + &data, + TRUE, + &error); + while (service_loop == NULL) + g_thread_yield (); + g_assert (server != NULL); + + + /* bring up a connection and accept it */ + data.accept_connection = TRUE; + error = NULL; + c = g_dbus_connection_new_for_address_sync (g_dbus_server_get_client_address (server), + G_DBUS_CONNECTION_FLAGS_AUTHENTICATION_CLIENT, + NULL, /* GDBusAuthObserver */ + NULL, /* cancellable */ + &error); + g_assert_no_error (error); + g_assert (c != NULL); + while (data.current_connections->len < 1) + g_main_loop_run (loop); + g_assert_cmpint (data.current_connections->len, ==, 1); + g_assert_cmpint (data.num_connection_attempts, ==, 1); + g_assert (g_dbus_connection_get_unique_name (c) == NULL); + g_assert_cmpstr (g_dbus_connection_get_guid (c), ==, test_guid); + g_object_unref (c); + + /* now, try to subvert the nonce file (this assumes noncefile is the last key/value pair) + */ + + address = g_dbus_server_get_client_address (server); + + s = strstr (address, "noncefile="); + g_assert (s != NULL); + s += sizeof "noncefile=" - 1; + nonce_file = g_strdup (s); + + /* First try invalid data in the nonce file - this will actually + * make the client send this and the server will reject it. The way + * it works is that if the nonce doesn't match, the server will + * simply close the connection. So, from the client point of view, + * we can see a variety of errors. + */ + error = NULL; + res = g_file_set_contents (nonce_file, + "0123456789012345", + -1, + &error); + g_assert_no_error (error); + g_assert (res); + c = g_dbus_connection_new_for_address_sync (address, + G_DBUS_CONNECTION_FLAGS_AUTHENTICATION_CLIENT, + NULL, /* GDBusAuthObserver */ + NULL, /* cancellable */ + &error); + _g_assert_error_domain (error, G_IO_ERROR); + g_assert (c == NULL); + + /* Then try with a nonce-file of incorrect length - this will make + * the client complain - we won't even try connecting to the server + * for this + */ + error = NULL; + res = g_file_set_contents (nonce_file, + "0123456789012345_", + -1, + &error); + g_assert_no_error (error); + g_assert (res); + c = g_dbus_connection_new_for_address_sync (address, + G_DBUS_CONNECTION_FLAGS_AUTHENTICATION_CLIENT, + NULL, /* GDBusAuthObserver */ + NULL, /* cancellable */ + &error); + g_assert_error (error, G_IO_ERROR, G_IO_ERROR_INVALID_ARGUMENT); + g_assert (c == NULL); + + /* Finally try with no nonce-file at all */ + g_assert_cmpint (g_unlink (nonce_file), ==, 0); + error = NULL; + c = g_dbus_connection_new_for_address_sync (address, + G_DBUS_CONNECTION_FLAGS_AUTHENTICATION_CLIENT, + NULL, /* GDBusAuthObserver */ + NULL, /* cancellable */ + &error); + g_assert_error (error, G_IO_ERROR, G_IO_ERROR_INVALID_ARGUMENT); + g_assert (c == NULL); + + g_free (nonce_file); + + g_dbus_server_stop (server); + g_object_unref (server); + server = NULL; + + g_main_loop_quit (service_loop); + g_thread_join (service_thread); +} + +/* ---------------------------------------------------------------------------------------------------- */ + int main (int argc, char *argv[]) @@ -1002,6 +1204,7 @@ main (int argc, g_test_add_func ("/gdbus/peer-to-peer", test_peer); g_test_add_func ("/gdbus/delayed-message-processing", delayed_message_processing); + g_test_add_func ("/gdbus/nonce-tcp", test_nonce_tcp); ret = g_test_run();