From 04c45e1418d9ed95cfca17e62b4e61981582f007 Mon Sep 17 00:00:00 2001 From: David Zeuthen Date: Wed, 11 Apr 2012 23:05:33 -0400 Subject: [PATCH] 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 as required by the D-Bus spec. 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. The fix is straightforward - just make libdbus-1 follow the spec and use number of seconds since the epoch as required by the spec. First, we change _dbus_get_current_time() back so it always returns time since the Epoch. Then we 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 Reviewed-by: Simon McVittie Bug: https://bugs.freedesktop.org/show_bug.cgi?id=48580 --- bus/connection.c | 10 +++++----- bus/expirelist.c | 24 ++++++++++++------------ dbus/dbus-connection.c | 4 ++-- dbus/dbus-internals.c | 3 +++ dbus/dbus-mainloop.c | 8 ++++---- dbus/dbus-sysdeps-unix.c | 25 +++++++++++++++++++++++-- dbus/dbus-sysdeps-win.c | 19 +++++++++++++++++-- dbus/dbus-sysdeps.h | 3 +++ test/break-loader.c | 2 +- test/name-test/test-pending-call-dispatch.c | 4 ++-- test/name-test/test-pending-call-timeout.c | 4 ++-- 11 files changed, 74 insertions(+), 32 deletions(-) diff --git a/bus/connection.c b/bus/connection.c index 97e5f64..d69758c 100644 --- a/bus/connection.c +++ b/bus/connection.c @@ -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, diff --git a/bus/expirelist.c b/bus/expirelist.c index 3c87c11..1a12ea4 100644 --- a/bus/expirelist.c +++ b/bus/expirelist.c @@ -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); diff --git a/dbus/dbus-connection.c b/dbus/dbus-connection.c index ac0b2c0..ee33b6c 100644 --- a/dbus/dbus-connection.c +++ b/dbus/dbus-connection.c @@ -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; diff --git a/dbus/dbus-internals.c b/dbus/dbus-internals.c index a5db146..89a2b90 100644 --- a/dbus/dbus-internals.c +++ b/dbus/dbus-internals.c @@ -660,6 +660,9 @@ _dbus_generate_uuid (DBusGUID *uuid) { long now; + /* don't use monotonic time because the UUID may be saved to disk, e.g. + * it may persist across reboots + */ _dbus_get_current_time (&now, NULL); uuid->as_uint32s[DBUS_UUID_LENGTH_WORDS - 1] = DBUS_UINT32_TO_BE (now); diff --git a/dbus/dbus-mainloop.c b/dbus/dbus-mainloop.c index 8a30f7f..cef676a 100644 --- a/dbus/dbus-mainloop.c +++ b/dbus/dbus-mainloop.c @@ -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); diff --git a/dbus/dbus-sysdeps-unix.c b/dbus/dbus-sysdeps-unix.c index 9fddca7..7a94d10 100644 --- a/dbus/dbus-sysdeps-unix.c +++ b/dbus/dbus-sysdeps-unix.c @@ -2601,8 +2601,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) { #ifdef HAVE_MONOTONIC_CLOCK struct timespec ts; @@ -2625,6 +2625,27 @@ _dbus_get_current_time (long *tv_sec, } /** + * 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_current_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 e30e92f..ee1f2cc 100644 --- a/dbus/dbus-sysdeps-win.c +++ b/dbus/dbus-sysdeps-win.c @@ -1872,10 +1872,11 @@ _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 + * @param tv_usec return location for number of microseconds (thousandths) */ void _dbus_get_current_time (long *tv_sec, @@ -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 (thousandths) + */ +void +_dbus_get_monotonic_time (long *tv_sec, + long *tv_usec) +{ + /* no implementation yet, fall back to wall-clock time */ + _dbus_get_current_time (tv_sec, tv_usec); +} /** * signal (SIGPIPE, SIG_IGN); diff --git a/dbus/dbus-sysdeps.h b/dbus/dbus-sysdeps.h index f8438f4..52477d0 100644 --- a/dbus/dbus-sysdeps.h +++ b/dbus/dbus-sysdeps.h @@ -311,6 +311,9 @@ 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); + /** * directory interface */ diff --git a/test/break-loader.c b/test/break-loader.c index 542f36f..e62b8c2 100644 --- 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 57582d4..c8b5a46 100644 --- 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 381113b..d051fab 100644 --- 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; -- 1.7.10