From 870351439769a35344ead435f2490ed933dd8ff4 Mon Sep 17 00:00:00 2001 From: David Zeuthen Date: Thu, 12 Apr 2012 11:16:48 -0400 Subject: 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 Reviewed-by: Simon McVittie Bug: https://bugs.freedesktop.org/show_bug.cgi?id=48580 --- bus/connection.c | 10 +++++----- bus/expirelist.c | 4 ++-- dbus/dbus-connection.c | 4 ++-- dbus/dbus-internals.c | 2 +- dbus/dbus-keyring.c | 6 +++--- dbus/dbus-mainloop.c | 8 ++++---- dbus/dbus-sysdeps-unix.c | 25 +++++++++++++++++++++++-- dbus/dbus-sysdeps-win.c | 18 ++++++++++++++++-- dbus/dbus-sysdeps.c | 2 +- dbus/dbus-sysdeps.h | 7 +++++-- test/break-loader.c | 2 +- test/name-test/test-pending-call-dispatch.c | 4 ++-- test/name-test/test-pending-call-timeout.c | 4 ++-- 13 files changed, 67 insertions(+), 29 deletions(-) diff --git a/bus/connection.c b/bus/connection.c index 8e7d222a..727d6a8f 100644 --- 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 946a615c..5d45b9a5 100644 --- 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 8b8310dd..4f2eb6b8 100644 --- 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 95a491f9..8c1dd222 100644 --- 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 35e8d74e..23b9df5a 100644 --- 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 (×tamp, NULL); + _dbus_get_real_time (×tamp, 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 43159a70..1ff8425c 100644 --- 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 1a8f6fc2..aec03e83 100644 --- 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 aea704d2..b22a3718 100644 --- 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 89062764..1b220313 100644 --- 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 d4883c1b..a784db27 100644 --- 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 7bfa7227..1e222569 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 57582d49..c8b5a467 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 381113bd..d051faba 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; -- cgit v1.2.3