gdbusauthmechanismsha1: Use the same timeouts as libdbuswip/2164-dbus-sha1-timeout

For interoperability with libdbus, we want to use compatible timeouts.
In particular, this fixes a spurious failure of the `gdbus-server-auth`
test caused by libdbus and gdbus choosing to expire the key (cookie) at
different times, as diagnosed by Thiago Macieira. Previously, the libdbus
client would decline to use keys older than 7 minutes, but the GDBus
server would not generate a new key until the old key was 10 minutes old.

For completeness, also adjust the other arbitrary timeouts in the
DBUS_COOKIE_SHA1 mechanism to be the same as in libdbus. To make it
easier to align with libdbus, create internal macros with the same names
and values used in dbus-keyring.c.

* maximum time a key can be in the future due to clock skew between
  systems sharing a home directory
  - spec says "a reasonable time in the future"
  - was 1 day
  - now 5 minutes
  - MAX_TIME_TRAVEL_SECONDS

* time to generate a new key if the newest is older
  - spec says "If no recent keys remain, the server may generate a new
    key", but that isn't practical, because in reality we need a grace
    period during which an old key will not be used for new authentication
    attempts but old authentication attempts can continue (in practice both
    libdbus and GDBus implemented this logic)
  - was 10 minutes
  - now 5 minutes
  - NEW_KEY_TIMEOUT_SECONDS

* time to discard old keys
  - spec says "the timeout can be fairly short"
  - was 15 minutes
  - now 7 minutes
  - EXPIRE_KEYS_TIMEOUT_SECONDS

* time allowed for a client using an old key to authenticate, before
  that key gets deleted
  - was at least 5 minutes
  - now at least 2 minutes
  - at least (EXPIRE_KEYS_TIMEOUT_SECONDS - NEW_KEY_TIMEOUT_SECONDS)

Based on a merge request by Philip Withnall.

Fixes: #2164
Thanks: Philip Withnall
Thanks: Thiago Macieira
Signed-off-by: Simon McVittie <smcv@collabora.com>

1 files changed, 40 insertions, 13 deletions

diff --git a/gio/gdbusauthmechanismsha1.c b/gio/gdbusauthmechanismsha1.c
index 416d8cc32..baa4e59d9 100644
--- a/gio/gdbusauthmechanismsha1.c
+++ b/gio/gdbusauthmechanismsha1.c
@@ -43,6 +43,37 @@
 
 #include "glibintl.h"
 
+/*
+ * Arbitrary timeouts for keys in the keyring.
+ * For interoperability, these match the reference implementation, libdbus.
+ * To make them easier to compare, their names also match libdbus
+ * (see dbus/dbus-keyring.c).
+ */
+
+/*
+ * Maximum age of a key before we create a new key to use in challenges:
+ * 5 minutes.
+ */
+#define NEW_KEY_TIMEOUT_SECONDS (60*5)
+
+/*
+ * Time before we drop a key from the keyring: 7 minutes.
+ * Authentication will succeed if it takes less than
+ * EXPIRE_KEYS_TIMEOUT_SECONDS - NEW_KEY_TIMEOUT_SECONDS (2 minutes)
+ * to complete.
+ * The spec says "delete any cookies that are old (the timeout can be
+ * fairly short)".
+ */
+#define EXPIRE_KEYS_TIMEOUT_SECONDS (NEW_KEY_TIMEOUT_SECONDS + (60*2))
+
+/*
+ * Maximum amount of time a key can be in the future due to clock skew
+ * with a shared home directory: 5 minutes.
+ * The spec says "a reasonable time in the future".
+ */
+#define MAX_TIME_TRAVEL_SECONDS (60*5)
+
+
 struct _GDBusAuthMechanismSha1Private
 {
   gboolean is_client;
@@ -750,12 +781,8 @@ keyring_generate_entry (const gchar  *cookie_context,
             {
               /* Oddball case: entry is more recent than our current wall-clock time..
                * This is OK, it means that another server on another machine but with
-               * same $HOME wrote the entry.
-               *
-               * So discard the entry if it's more than 1 day in the future ("reasonable
-               * time in the future").
-               */
-              if (line_when - now > 24*60*60)
+               * same $HOME wrote the entry. */
+              if (line_when - now > MAX_TIME_TRAVEL_SECONDS)
                 {
                   keep_entry = FALSE;
                   _log ("Deleted SHA1 cookie from %" G_GUINT64_FORMAT " seconds in the future", line_when - now);
@@ -763,8 +790,8 @@ keyring_generate_entry (const gchar  *cookie_context,
             }
           else
             {
-              /* Discard entry if it's older than 15 minutes ("can be fairly short") */
-              if (now - line_when > 15*60)
+              /* Discard entry if it's too old. */
+              if (now - line_when > EXPIRE_KEYS_TIMEOUT_SECONDS)
                 {
                   keep_entry = FALSE;
                 }
@@ -782,13 +809,13 @@ keyring_generate_entry (const gchar  *cookie_context,
                                       line_when,
                                       tokens[2]);
               max_line_id = MAX (line_id, max_line_id);
-              /* Only reuse entry if not older than 10 minutes.
+              /* Only reuse entry if not older than 5 minutes.
                *
-               * (We need a bit of grace time compared to 15 minutes above.. otherwise
-               * there's a race where we reuse the 14min59.9 secs old entry and a
-               * split-second later another server purges the now 15 minute old entry.)
+               * (We need a bit of grace time compared to 7 minutes above.. otherwise
+               * there's a race where we reuse the 6min59.9 secs old entry and a
+               * split-second later another server purges the now 7 minute old entry.)
                */
-              if (now - line_when < 10 * 60)
+              if (now - line_when < NEW_KEY_TIMEOUT_SECONDS)
                 {
                   if (!have_id)
                     {