GDBus: Add nonce-tcp: test case
authorDavid Zeuthen <davidz@redhat.com>
Fri, 16 Jul 2010 17:19:48 +0000 (13:19 -0400)
committerDavid Zeuthen <davidz@redhat.com>
Fri, 16 Jul 2010 17:22:45 +0000 (13:22 -0400)
Also fix a couple of TODO items in gdbusaddress.c

Signed-off-by: David Zeuthen <davidz@redhat.com>
gio/gdbusaddress.c
gio/tests/gdbus-peer.c

index 4b815e6..afc8ec6 100644 (file)
@@ -25,6 +25,8 @@
 #include <stdlib.h>
 #include <string.h>
 #include <sys/wait.h>
+#include <stdio.h>
+#include <errno.h>
 
 #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,
index 284f3d6..af85fcd 100644 (file)
@@ -29,6 +29,9 @@
 #include <sys/stat.h>
 #include <fcntl.h>
 
+/* for g_unlink() */
+#include <glib/gstdio.h>
+
 #include <gio/gunixsocketaddress.h>
 #include <gio/gunixfdlist.h>
 
@@ -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();