[glib/wip/2164-dbus-sha1-timeout] gdbusauthmechanismsha1: Use the same timeouts as libdbus

[Simon McVittie <smcv src gnome org>]

commit 92183fb70383fea247c7d76e31df4670e9615766
Author: Simon McVittie <smcv collabora com>
Date:   Mon Sep 7 11:29:06 2020 +0100

    gdbusauthmechanismsha1: Use the same timeouts as libdbus
    
    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>

 gio/gdbusauthmechanismsha1.c | 53 +++++++++++++++++++++++++++++++++-----------
 1 file 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)
                     {