Avoid using monotonic time in the DBUS_COOKIE_SHA1 authentication method

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

diff --git a/bus/connection.c b/bus/connection.c
index 8e7d222a71abaa79a603c51eb764464fa5108dd2..727d6a8f4db879eb65fe3463504a1d52865ebaf3 100644 (file)
--- a/bus/connection.c
+++ b/bus/connection.c
@@ -623,8 +623,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);
   
@@ -793,7 +793,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);
@@ -1763,8 +1763,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,

diff --git a/bus/expirelist.c b/bus/expirelist.c
index 946a615c40bd07a29c510bdf318d5deb42cf94ec..5d45b9a58cc6291771b4873eedd10167a880c0aa 100644 (file)
--- a/bus/expirelist.c
+++ b/bus/expirelist.c
@@ -205,7 +205,7 @@ 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);
     }
@@ -351,7 +351,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;

diff --git a/dbus/dbus-connection.c b/dbus/dbus-connection.c
index 8b8310ddc39cab8c2f29f55c31f845d9bc58b725..4f2eb6b8ddd55faf05efb6f2dbc9b3f44e079ea2 100644 (file)
--- a/dbus/dbus-connection.c
+++ b/dbus/dbus-connection.c
@@ -2377,7 +2377,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);
@@ -2434,7 +2434,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;
   

diff --git a/dbus/dbus-internals.c b/dbus/dbus-internals.c
index 95a491f99582abfce797c44bf78e26d4b56cce5a..8c1dd2227d39b77469ed277cc73f164c60658fe6 100644 (file)
--- a/dbus/dbus-internals.c
+++ b/dbus/dbus-internals.c
@@ -595,7 +595,7 @@ _dbus_generate_uuid (DBusGUID *uuid)
 {
   long now;
 
-  _dbus_get_current_time (&now, NULL);
+  _dbus_get_real_time (&now, NULL);
 
   uuid->as_uint32s[DBUS_UUID_LENGTH_WORDS - 1] = DBUS_UINT32_TO_BE (now);
   

diff --git a/dbus/dbus-keyring.c b/dbus/dbus-keyring.c
index 35e8d74e06316a7fed998521bda7b13c7ffe9ff8..23b9df5a0afa396490100a5dc13c89ba2756cde8 100644 (file)
--- a/dbus/dbus-keyring.c
+++ b/dbus/dbus-keyring.c
@@ -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)

diff --git a/dbus/dbus-mainloop.c b/dbus/dbus-mainloop.c
index 43159a70ba174efdce4418b48d29ccf02780f262..1ff8425c4756fb5f5a262f413d5dc9496646d798 100644 (file)
--- a/dbus/dbus-mainloop.c
+++ b/dbus/dbus-mainloop.c
@@ -136,8 +136,8 @@ timeout_callback_new (DBusTimeout         *timeout,
 
   cb->timeout = timeout;
   cb->function = function;
-  _dbus_get_current_time (&cb->last_tv_sec,
-                          &cb->last_tv_usec);
+  _dbus_get_monotonic_time (&cb->last_tv_sec,
+                            &cb->last_tv_usec);
   cb->callback.refcount = 1;    
   cb->callback.type = CALLBACK_TIMEOUT;
   cb->callback.data = data;
@@ -653,7 +653,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->callbacks);
       while (link != NULL)
@@ -723,7 +723,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->callbacks);

diff --git a/dbus/dbus-sysdeps-unix.c b/dbus/dbus-sysdeps-unix.c
index 1a8f6fc2512591e189c3d3564367235da2df7aef..aec03e83023debdab46d890823f005f1a58184d7 100644 (file)
--- a/dbus/dbus-sysdeps-unix.c
+++ b/dbus/dbus-sysdeps-unix.c
@@ -2529,8 +2529,8 @@ _dbus_poll (DBusPollFD *fds,
  * @param tv_usec return location for number of microseconds (thousandths)
  */
 void
-_dbus_get_current_time (long *tv_sec,
-                        long *tv_usec)
+_dbus_get_monotonic_time (long *tv_sec,
+                          long *tv_usec)
 {
   struct timeval t;
 
@@ -2552,6 +2552,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 (thousandths)
+ */
+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.

diff --git a/dbus/dbus-sysdeps-win.c b/dbus/dbus-sysdeps-win.c
index aea704d2370510c283ea2edfdb3c568014f5fa2c..b22a37185ce8b28d8f0ca18a31e7956aef7328e9 100644 (file)
--- a/dbus/dbus-sysdeps-win.c
+++ b/dbus/dbus-sysdeps-win.c
@@ -1888,8 +1888,8 @@ _dbus_sleep_milliseconds (int milliseconds)
  * @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;
@@ -1911,6 +1911,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);

diff --git a/dbus/dbus-sysdeps.c b/dbus/dbus-sysdeps.c
index 89062764bd1290cbd305927f0fa7ade86285ed13..1b2203132067e23aa9cbe9bdf716aaeedfa0ad1e 100644 (file)
--- a/dbus/dbus-sysdeps.c
+++ b/dbus/dbus-sysdeps.c
@@ -805,7 +805,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_monotonic_time (NULL, &tv_usec);
   srand (tv_usec);
   
   i = 0;

diff --git a/dbus/dbus-sysdeps.h b/dbus/dbus-sysdeps.h
index d4883c1b6055e714849b66db7be9429531e78415..a784db27e4862c86f54c33f339c60d01dc34a5c4 100644 (file)
--- a/dbus/dbus-sysdeps.h
+++ b/dbus/dbus-sysdeps.h
@@ -310,8 +310,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_monotonic_time (long *tv_sec,
+                               long *tv_usec);
+
+void _dbus_get_real_time (long *tv_sec,
+                          long *tv_usec);
 
 /**
  * directory interface

diff --git a/test/break-loader.c b/test/break-loader.c
index 7bfa72273663722896b0dcf3d69f07430cde7162..1e222569ae0789a70c016d3bcd7feb60e18f4da6 100644 (file)
--- a/test/break-loader.c
+++ b/test/break-loader.c
@@ -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;
   }

diff --git a/test/name-test/test-pending-call-dispatch.c b/test/name-test/test-pending-call-dispatch.c
index 57582d4905c847e49e2d4df70a45aef2baa4f4d9..c8b5a46783df2ab0e234e8ad905269a632234a22 100644 (file)
--- a/test/name-test/test-pending-call-dispatch.c
+++ b/test/name-test/test-pending-call-dispatch.c
@@ -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;

diff --git a/test/name-test/test-pending-call-timeout.c b/test/name-test/test-pending-call-timeout.c
index 381113bd23bb182fd944c059e917f5e1ac67206a..d051faba959a01cd25e0de86c3f8654cf10bc00c 100644 (file)
--- a/test/name-test/test-pending-call-timeout.c
+++ b/test/name-test/test-pending-call-timeout.c
@@ -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;