]> git.ipfire.org Git - thirdparty/dbus.git/commitdiff
Avoid using monotonic time in the DBUS_COOKIE_SHA1 authentication method
authorDavid Zeuthen <davidz@redhat.com>
Thu, 12 Apr 2012 03:05:33 +0000 (23:05 -0400)
committerDavid Zeuthen <davidz@redhat.com>
Thu, 12 Apr 2012 14:53:50 +0000 (10:53 -0400)
When libdbus-1 moved to using monotonic time support for the
DBUS_COOKIE_SHA1 authentication was broken, in particular
interoperability with non-libdbus-1 implementations such as GDBus.

The problem is that if monotonic clocks are available in the OS,
_dbus_get_current_time() will not return the number of seconds since
the Epoch so using it for DBUS_COOKIE_SHA1 will violate the D-Bus
specification. If both peers are using libdbus-1 it's not a problem
since both ends will use the wrong time and thus agree. However, if
the other end is another implementation and following the spec it will
not work.

First, we change _dbus_get_current_time() back so it always returns
time since the Epoch and we then rename it _dbus_get_real_time() to
make this clear. We then introduce _dbus_get_monotonic_time() and
carefully make all current users of _dbus_get_current_time() use it,
if applicable. During this audit, one of the callers,
_dbus_generate_uuid(), was currently using monotonic time but it was
decided to make it use real time instead.

Signed-off-by: David Zeuthen <davidz@redhat.com>
Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=48580

13 files changed:
bus/connection.c
bus/expirelist.c
dbus/dbus-connection.c
dbus/dbus-internals.c
dbus/dbus-keyring.c
dbus/dbus-mainloop.c
dbus/dbus-sysdeps-unix.c
dbus/dbus-sysdeps-win.c
dbus/dbus-sysdeps.c
dbus/dbus-sysdeps.h
test/break-loader.c
test/name-test/test-pending-call-dispatch.c
test/name-test/test-pending-call-timeout.c

index 97e5f64b0d22d939b8ec287bdc18dca70a756b84..d69758c97c1aa0e4c287a965f94f93a024e4c5eb 100644 (file)
@@ -606,8 +606,8 @@ bus_connections_setup_connection (BusConnections *connections,
   d->connections = connections;
   d->connection = connection;
   
-  _dbus_get_current_time (&d->connection_tv_sec,
-                          &d->connection_tv_usec);
+  _dbus_get_monotonic_time (&d->connection_tv_sec,
+                            &d->connection_tv_usec);
   
   _dbus_assert (connection_data_slot >= 0);
   
@@ -776,7 +776,7 @@ bus_connections_expire_incomplete (BusConnections *connections)
       DBusList *link;
       int auth_timeout;
       
-      _dbus_get_current_time (&tv_sec, &tv_usec);
+      _dbus_get_monotonic_time (&tv_sec, &tv_usec);
       auth_timeout = bus_context_get_auth_timeout (connections->context);
   
       link = _dbus_list_get_first_link (&connections->incomplete);
@@ -1772,8 +1772,8 @@ bus_connections_expect_reply (BusConnections  *connections,
   cprd->pending = pending;
   cprd->connections = connections;
   
-  _dbus_get_current_time (&pending->expire_item.added_tv_sec,
-                          &pending->expire_item.added_tv_usec);
+  _dbus_get_monotonic_time (&pending->expire_item.added_tv_sec,
+                            &pending->expire_item.added_tv_usec);
 
   _dbus_verbose ("Added pending reply %p, replier %p receiver %p serial %u\n",
                  pending,
index 3c87c1198aeb8f8dd5aeab2c83842a7917725a05..1a12ea456af2e00fdd95460c2a60a4876f7c8ad9 100644 (file)
@@ -123,9 +123,9 @@ bus_expire_list_recheck_immediately (BusExpireList *list)
 }
 
 static int
-do_expiration_with_current_time (BusExpireList *list,
-                                 long           tv_sec,
-                                 long           tv_usec)
+do_expiration_with_monotonic_time (BusExpireList *list,
+                                   long           tv_sec,
+                                   long           tv_usec)
 {
   DBusList *link;
   int next_interval, min_wait_time, items_to_expire;
@@ -194,9 +194,9 @@ bus_expirelist_expire (BusExpireList *list)
     {
       long tv_sec, tv_usec;
 
-      _dbus_get_current_time (&tv_sec, &tv_usec);
+      _dbus_get_monotonic_time (&tv_sec, &tv_usec);
 
-      next_interval = do_expiration_with_current_time (list, tv_sec, tv_usec);
+      next_interval = do_expiration_with_monotonic_time (list, tv_sec, tv_usec);
     }
 
   bus_expire_timeout_set_interval (list->timeout, next_interval);
@@ -340,7 +340,7 @@ bus_expire_list_test (const DBusString *test_data_dir)
                               test_expire_func, NULL);
   _dbus_assert (list != NULL);
 
-  _dbus_get_current_time (&tv_sec, &tv_usec);
+  _dbus_get_monotonic_time (&tv_sec, &tv_usec);
 
   tv_sec_not_expired = tv_sec;
   tv_usec_not_expired = tv_usec;
@@ -367,22 +367,22 @@ bus_expire_list_test (const DBusString *test_data_dir)
     _dbus_assert_not_reached ("out of memory");
 
   next_interval =
-    do_expiration_with_current_time (list, tv_sec_not_expired,
-                                     tv_usec_not_expired);
+    do_expiration_with_monotonic_time (list, tv_sec_not_expired,
+                                       tv_usec_not_expired);
   _dbus_assert (item->expire_count == 0);
   _dbus_verbose ("next_interval = %d\n", next_interval);
   _dbus_assert (next_interval == 1);
   
   next_interval =
-    do_expiration_with_current_time (list, tv_sec_expired,
-                                     tv_usec_expired);
+    do_expiration_with_monotonic_time (list, tv_sec_expired,
+                                       tv_usec_expired);
   _dbus_assert (item->expire_count == 1);
   _dbus_verbose ("next_interval = %d\n", next_interval);
   _dbus_assert (next_interval == -1);
 
   next_interval =
-    do_expiration_with_current_time (list, tv_sec_past,
-                                     tv_usec_past);
+    do_expiration_with_monotonic_time (list, tv_sec_past,
+                                       tv_usec_past);
   _dbus_assert (item->expire_count == 1);
   _dbus_verbose ("next_interval = %d\n", next_interval);
   _dbus_assert (next_interval == 1000 + EXPIRE_AFTER);
index ac0b2c0e7313ab67bbab305c2e77417236673229..ee33b6ccc735c4d50c9966319e118c377804c58d 100644 (file)
@@ -2388,7 +2388,7 @@ _dbus_connection_block_pending_call (DBusPendingCall *pending)
    * below
    */
   timeout = _dbus_pending_call_get_timeout_unlocked (pending);
-  _dbus_get_current_time (&start_tv_sec, &start_tv_usec);
+  _dbus_get_monotonic_time (&start_tv_sec, &start_tv_usec);
   if (timeout)
     {
       timeout_milliseconds = dbus_timeout_get_interval (timeout);
@@ -2445,7 +2445,7 @@ _dbus_connection_block_pending_call (DBusPendingCall *pending)
         return;
     }
   
-  _dbus_get_current_time (&tv_sec, &tv_usec);
+  _dbus_get_monotonic_time (&tv_sec, &tv_usec);
   elapsed_milliseconds = (tv_sec - start_tv_sec) * 1000 +
          (tv_usec - start_tv_usec) / 1000;
   
index a5db146045a46b73b0083706f2017245b08c7b04..0e5d807e78a9166f6919f0073da33d17f5d34ccd 100644 (file)
@@ -660,7 +660,10 @@ _dbus_generate_uuid (DBusGUID *uuid)
 {
   long now;
 
-  _dbus_get_current_time (&now, NULL);
+  /* don't use monotonic time because the UUID may be saved to disk, e.g.
+   * it may persist across reboots
+   */
+  _dbus_get_real_time (&now, NULL);
 
   uuid->as_uint32s[DBUS_UUID_LENGTH_WORDS - 1] = DBUS_UINT32_TO_BE (now);
   
index 35e8d74e06316a7fed998521bda7b13c7ffe9ff8..23b9df5a0afa396490100a5dc13c89ba2756cde8 100644 (file)
@@ -353,7 +353,7 @@ add_new_key (DBusKey  **keys_p,
       goto out;
     }
 
-  _dbus_get_current_time (&timestamp, NULL);
+  _dbus_get_real_time (&timestamp, NULL);
       
   keys[n_keys-1].id = id;
   keys[n_keys-1].creation_time = timestamp;
@@ -428,7 +428,7 @@ _dbus_keyring_reload (DBusKeyring *keyring,
   retval = FALSE;
   have_lock = FALSE;
 
-  _dbus_get_current_time (&now, NULL);
+  _dbus_get_real_time (&now, NULL);
   
   if (add_new)
     {
@@ -908,7 +908,7 @@ find_recent_key (DBusKeyring *keyring)
   int i;
   long tv_sec, tv_usec;
 
-  _dbus_get_current_time (&tv_sec, &tv_usec);
+  _dbus_get_real_time (&tv_sec, &tv_usec);
   
   i = 0;
   while (i < keyring->n_keys)
index 8a30f7fe0159100020718073194883292e35b025..cef676a3a9723710a8921eac30fe382485630ae9 100644 (file)
@@ -91,8 +91,8 @@ timeout_callback_new (DBusTimeout         *timeout)
     return NULL;
 
   cb->timeout = timeout;
-  _dbus_get_current_time (&cb->last_tv_sec,
-                          &cb->last_tv_usec);
+  _dbus_get_monotonic_time (&cb->last_tv_sec,
+                            &cb->last_tv_usec);
   return cb;
 }
 
@@ -619,7 +619,7 @@ _dbus_loop_iterate (DBusLoop     *loop,
       unsigned long tv_sec;
       unsigned long tv_usec;
       
-      _dbus_get_current_time (&tv_sec, &tv_usec);
+      _dbus_get_monotonic_time (&tv_sec, &tv_usec);
 
       link = _dbus_list_get_first_link (&loop->timeouts);
       while (link != NULL)
@@ -728,7 +728,7 @@ _dbus_loop_iterate (DBusLoop     *loop,
       unsigned long tv_sec;
       unsigned long tv_usec;
 
-      _dbus_get_current_time (&tv_sec, &tv_usec);
+      _dbus_get_monotonic_time (&tv_sec, &tv_usec);
 
       /* It'd be nice to avoid this O(n) thingy here */
       link = _dbus_list_get_first_link (&loop->timeouts);
index 9fddca73da333c3d0c927b3fcd8daceece7402a1..cef8bd310aaea7a1a998ef7c4ea702367060bcd9 100644 (file)
@@ -2598,11 +2598,11 @@ _dbus_poll (DBusPollFD *fds,
  * available, to avoid problems when the system time changes.
  *
  * @param tv_sec return location for number of seconds
- * @param tv_usec return location for number of microseconds (thousandths)
+ * @param tv_usec return location for number of microseconds
  */
 void
-_dbus_get_current_time (long *tv_sec,
-                        long *tv_usec)
+_dbus_get_monotonic_time (long *tv_sec,
+                          long *tv_usec)
 {
 #ifdef HAVE_MONOTONIC_CLOCK
   struct timespec ts;
@@ -2624,6 +2624,27 @@ _dbus_get_current_time (long *tv_sec,
 #endif
 }
 
+/**
+ * Get current time, as in gettimeofday(). Never uses the monotonic
+ * clock.
+ *
+ * @param tv_sec return location for number of seconds
+ * @param tv_usec return location for number of microseconds
+ */
+void
+_dbus_get_real_time (long *tv_sec,
+                     long *tv_usec)
+{
+  struct timeval t;
+
+  gettimeofday (&t, NULL);
+
+  if (tv_sec)
+    *tv_sec = t.tv_sec;
+  if (tv_usec)
+    *tv_usec = t.tv_usec;
+}
+
 /**
  * Creates a directory; succeeds if the directory
  * is created or already existed.
index e30e92fe7d57ce668947e50a4d4ed705ae9f2df1..397520af7d9cd10fdf1320666150a6f6666bec97 100644 (file)
@@ -1872,14 +1872,15 @@ _dbus_sleep_milliseconds (int milliseconds)
 
 
 /**
- * Get current time, as in gettimeofday().
+ * Get current time, as in gettimeofday(). Never uses the monotonic
+ * clock.
  *
  * @param tv_sec return location for number of seconds
  * @param tv_usec return location for number of microseconds
  */
 void
-_dbus_get_current_time (long *tv_sec,
-                        long *tv_usec)
+_dbus_get_real_time (long *tv_sec,
+                     long *tv_usec)
 {
   FILETIME ft;
   dbus_uint64_t time64;
@@ -1901,6 +1902,20 @@ _dbus_get_current_time (long *tv_sec,
     *tv_usec = time64 % 1000000;
 }
 
+/**
+ * Get current time, as in gettimeofday(). Use the monotonic clock if
+ * available, to avoid problems when the system time changes.
+ *
+ * @param tv_sec return location for number of seconds
+ * @param tv_usec return location for number of microseconds
+ */
+void
+_dbus_get_monotonic_time (long *tv_sec,
+                          long *tv_usec)
+{
+  /* no implementation yet, fall back to wall-clock time */
+  _dbus_get_real_time (tv_sec, tv_usec);
+}
 
 /**
  * signal (SIGPIPE, SIG_IGN);
index 1a93cea79ad01e003bdb718a9f017414aeffc06e..861bfec9e2990557d136a9af021e9f7ec28d82d2 100644 (file)
@@ -508,7 +508,7 @@ _dbus_generate_pseudorandom_bytes_buffer (char *buffer,
   _dbus_verbose ("Falling back to pseudorandom for %d bytes\n",
                  n_bytes);
   
-  _dbus_get_current_time (NULL, &tv_usec);
+  _dbus_get_real_time (NULL, &tv_usec);
   srand (tv_usec);
   
   i = 0;
index f8438f4350d40d942cc9c769aab31f6790caa360..c03e2f54b46f4ec30d83c67dc8e18488c48d86d7 100644 (file)
@@ -308,8 +308,11 @@ int _dbus_poll (DBusPollFD *fds,
 
 void _dbus_sleep_milliseconds (int milliseconds);
 
-void _dbus_get_current_time (long *tv_sec,
-                             long *tv_usec);
+void _dbus_get_real_time (long *tv_sec,
+                          long *tv_usec);
+
+void _dbus_get_monotonic_time (long *tv_sec,
+                               long *tv_usec);
 
 /**
  * directory interface
index 542f36ffd12d8a85a38fa89aab2e9646a6827435..e62b8c20788ede926a122e45bb8b353fa3e4d61c 100644 (file)
@@ -667,7 +667,7 @@ get_random_seed (void)
 
     fprintf (stderr, "could not open/read /dev/urandom, using current time for seed\n");
 
-    _dbus_get_current_time (NULL, &tv_usec);
+    _dbus_get_monotonic_time (NULL, &tv_usec);
 
     seed = tv_usec;
   }
index 57582d4905c847e49e2d4df70a45aef2baa4f4d9..c8b5a46783df2ab0e234e8ad905269a632234a22 100644 (file)
@@ -98,9 +98,9 @@ main (int argc, char *argv[])
     {
       long delta;
       
-      _dbus_get_current_time (&start_tv_sec, &start_tv_usec);
+      _dbus_get_monotonic_time (&start_tv_sec, &start_tv_usec);
       _run_iteration (conn);
-      _dbus_get_current_time (&end_tv_sec, &end_tv_usec);
+      _dbus_get_monotonic_time (&end_tv_sec, &end_tv_usec);
 
       /* we just care about seconds */
       delta = end_tv_sec - start_tv_sec;
index 381113bd23bb182fd944c059e917f5e1ac67206a..d051faba959a01cd25e0de86c3f8654cf10bc00c 100644 (file)
@@ -82,9 +82,9 @@ main (int argc, char *argv[])
     {
       long delta;
       
-      _dbus_get_current_time (&start_tv_sec, &start_tv_usec);
+      _dbus_get_monotonic_time (&start_tv_sec, &start_tv_usec);
       _run_iteration (conn);
-      _dbus_get_current_time (&end_tv_sec, &end_tv_usec);
+      _dbus_get_monotonic_time (&end_tv_sec, &end_tv_usec);
 
       /* we just care about seconds */
       delta = end_tv_sec - start_tv_sec;