Skip to content

gdbusauthmechanismsha1: Use the same timeouts as libdbus

Simon McVittie requested to merge wip/2164-dbus-sha1-timeout into master

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 (closed)
Thanks: Philip Withnall
Thanks: Thiago Macieira
Signed-off-by: Simon McVittie smcv@collabora.com


This supersedes !1581 (closed), applying my review feedback from that MR (I didn't really want to force-push non-trivial changes into @pwithnall's fork of GLib). I've represented it as a new commit from me rather than as a modified version of Philip's commit on !1581 (closed), crediting Philip in the commit message - I hope that's OK?

/cc @thiagomacieira @pwithnall

Edited by Simon McVittie

Merge request reports

Loading