Deduplicate matching Xauthority records

src/x-authority.c

@@ -290,17 +290,24 @@ x_authority_write (XAuthority *auth, XAuthWriteMode mode, const gchar *filename,
             address_matches = i == priv->address_length;
         }
 
+        gboolean record_matches = priv->family == a_priv->family &&
+                                  address_matches &&
+                                  strcmp (priv->number, a_priv->number) == 0;
+
         /* If this record matches, then update or delete it */
-        if (!matched &&
-            priv->family == a_priv->family &&
-            address_matches &&
-            strcmp (priv->number, a_priv->number) == 0)
+        if (record_matches)
         {
-            matched = TRUE;
             if (mode == XAUTH_WRITE_MODE_REMOVE)
+            {
+                matched = TRUE;
+                continue;
+            }
+
+            if (matched)
                 continue;
-            else
-                x_authority_set_authorization_data (a, priv->authorization_data, priv->authorization_data_length);
+
+            matched = TRUE;
+            x_authority_set_authorization_data (a, priv->authorization_data, priv->authorization_data_length);
         }
 
         records = g_list_append (records, g_steal_pointer (&a));

tests/Makefile.am

@@ -72,6 +72,7 @@ TESTS = \
 	test-session-stderr-backup \
 	test-xauthority \
 	test-corrupt-xauthority \
+	test-stale-xauthority \
 	test-system-xauthority \
 	test-sessions-gobject \
 	test-user-renamed \
@@ -395,6 +396,7 @@ EXTRA_DIST = \
 	scripts/console-kit.conf \
 	scripts/console-kit-no-xdg-runtime.conf \
 	scripts/corrupt-xauthority.conf \
+	scripts/stale-xauthority.conf \
 	scripts/crash-authenticate.conf \
 	scripts/cred-error.conf \
 	scripts/cred-expired.conf \

tests/scripts/stale-xauthority.conf

@@ -0,0 +1,40 @@
+#
+# Check stale X authority records are replaced on login
+#
+
+[Seat:*]
+autologin-user=stale-xauth
+user-session=default
+
+#?*START-DAEMON
+#?RUNNER DAEMON-START
+
+# X server starts
+#?XSERVER-0 START VT=7 SEAT=seat0
+
+# Daemon connects when X server is ready
+#?*XSERVER-0 INDICATE-READY
+#?XSERVER-0 INDICATE-READY
+#?XSERVER-0 ACCEPT-CONNECT
+
+# Session starts
+#?SESSION-X-0 START XDG_SEAT=seat0 XDG_VTNR=7 XDG_GREETER_DATA_DIR=.*/stale-xauth XDG_SESSION_TYPE=x11 XDG_SESSION_DESKTOP=default USER=stale-xauth
+#?LOGIN1 ACTIVATE-SESSION SESSION=c0
+#?XSERVER-0 ACCEPT-CONNECT
+#?SESSION-X-0 CONNECT-XSERVER
+
+# Check where the X authority is
+#?*SESSION-X-0 READ-ENV NAME=XAUTHORITY
+#?SESSION-X-0 READ-ENV NAME=XAUTHORITY VALUE=.*/home/stale-xauth/.Xauthority
+
+# Check duplicate cookies for the current display are replaced, while other entries remain
+#?*SESSION-X-0 CHECK-X-AUTHORITY
+#?SESSION-X-0 CHECK-X-AUTHORITY MODE=rw-------
+#?*SESSION-X-0 CHECK-X-AUTHORITY-RECORDS
+#?SESSION-X-0 CHECK-X-AUTHORITY-RECORDS RECORDS=2
+
+# Cleanup
+#?*STOP-DAEMON
+#?SESSION-X-0 TERMINATE SIGNAL=15
+#?XSERVER-0 TERMINATE SIGNAL=15
+#?RUNNER DAEMON-EXIT STATUS=0

tests/src/test-runner.c

@@ -2515,6 +2515,61 @@ cp (GFile *src, GFile *dst)
         g_error ("Failed to copy %s to %s: %s", g_file_peek_path (s), g_file_peek_path (d), error->message);
 }
 
+static void
+append_card16 (GString *data, guint16 value)
+{
+    g_string_append_c (data, value >> 8);
+    g_string_append_c (data, value & 0xFF);
+}
+
+static void
+append_xauthority_data (GString *data, const guint8 *value, guint16 value_length)
+{
+    append_card16 (data, value_length);
+    g_string_append_len (data, (const gchar *) value, value_length);
+}
+
+static void
+append_xauthority_string (GString *data, const gchar *value)
+{
+    append_xauthority_data (data, (const guint8 *) value, strlen (value));
+}
+
+static void
+append_xauthority_record (GString *data, guint16 family, const gchar *address, const gchar *number, const guint8 *cookie)
+{
+    append_card16 (data, family);
+    append_xauthority_string (data, address);
+    append_xauthority_string (data, number);
+    append_xauthority_string (data, "MIT-MAGIC-COOKIE-1");
+    append_xauthority_data (data, cookie, 16);
+}
+
+static void
+write_stale_xauthority (const gchar *path)
+{
+    const guint8 stale_cookie[16] = {
+        0x00, 0x01, 0x02, 0x03,
+        0x04, 0x05, 0x06, 0x07,
+        0x08, 0x09, 0x0A, 0x0B,
+        0x0C, 0x0D, 0x0E, 0x0F
+    };
+    const guint8 other_cookie[16] = {
+        0xF0, 0xF1, 0xF2, 0xF3,
+        0xF4, 0xF5, 0xF6, 0xF7,
+        0xF8, 0xF9, 0xFA, 0xFB,
+        0xFC, 0xFD, 0xFE, 0xFF
+    };
+
+    g_autoptr(GString) data = g_string_new ("");
+    append_xauthority_record (data, 256, "lightdm-test", "0", stale_cookie);
+    append_xauthority_record (data, 256, "lightdm-test", "0", stale_cookie);
+    append_xauthority_record (data, 256, "stale-host", "9", other_cookie);
+
+    g_file_set_contents (path, data->str, data->len, NULL);
+    chmod (path, S_IRUSR | S_IWUSR);
+}
+
 int
 main (int argc, char **argv)
 {
@@ -2774,6 +2829,8 @@ main (int argc, char **argv)
         {"prop-user",        "",          "TEST",               1033},
         /* This account has the home directory changed by PAM during authentication */
         {"change-home-dir",    "",       "Change Home Dir User", 1034},
+        /* This account has an existing stale X authority */
+        {"stale-xauth",      "password",  "Stale Xauthority",   1035},
         {NULL,               NULL,        NULL,                    0}
     };
     g_autoptr(GString) passwd_data = g_string_new ("");
@@ -2828,6 +2885,13 @@ main (int argc, char **argv)
             chmod (path, S_IRUSR | S_IWUSR);
         }
 
+        /* Write stale X authority file */
+        if (strcmp (users[i].user_name, "stale-xauth") == 0)
+        {
+            g_autofree gchar *path = g_build_filename (home_dir, users[i].user_name, ".Xauthority", NULL);
+            write_stale_xauthority (path);
+        }
+
         /* Add passwd file entry */
         g_string_append_printf (passwd_data, "%s:%s:%d:%d:%s:%s/home/%s:/bin/sh\n", users[i].user_name, users[i].password, users[i].uid, users[i].uid, users[i].real_name, temp_dir, users[i].user_name);
 

tests/src/test-session.c

@@ -27,6 +27,59 @@ static xcb_connection_t *connection;
 
 static LightDMGreeter *greeter = NULL;
 
+static gboolean
+read_card16 (const guint8 *data, gsize data_length, gsize *offset, guint16 *value)
+{
+    if (*offset + 2 > data_length)
+        return FALSE;
+
+    *value = data[*offset] << 8 | data[*offset + 1];
+    *offset += 2;
+
+    return TRUE;
+}
+
+static gboolean
+skip_data (gsize data_length, gsize *offset, guint16 length)
+{
+    if (*offset + length > data_length)
+        return FALSE;
+
+    *offset += length;
+
+    return TRUE;
+}
+
+static gint
+count_xauthority_records (const gchar *filename)
+{
+    g_autofree guint8 *data = NULL;
+    gsize data_length = 0;
+    if (!g_file_get_contents (filename, (gchar **) &data, &data_length, NULL))
+        return -1;
+
+    gint records = 0;
+    gsize offset = 0;
+    while (offset < data_length)
+    {
+        guint16 length = 0;
+        if (!read_card16 (data, data_length, &offset, &length) ||
+            !read_card16 (data, data_length, &offset, &length) ||
+            !skip_data (data_length, &offset, length) ||
+            !read_card16 (data, data_length, &offset, &length) ||
+            !skip_data (data_length, &offset, length) ||
+            !read_card16 (data, data_length, &offset, &length) ||
+            !skip_data (data_length, &offset, length) ||
+            !read_card16 (data, data_length, &offset, &length) ||
+            !skip_data (data_length, &offset, length))
+            return -1;
+
+        records++;
+    }
+
+    return records;
+}
+
 static gboolean
 sigint_cb (gpointer user_data)
 {
@@ -193,6 +246,15 @@ request_cb (const gchar *name, GHashTable *params)
         status_notify ("%s CHECK-X-AUTHORITY MODE=%s", session_id, mode_string->str);
     }
 
+    else if (strcmp (name, "CHECK-X-AUTHORITY-RECORDS") == 0)
+    {
+        g_autofree gchar *xauthority = g_strdup (g_getenv ("XAUTHORITY"));
+        if (!xauthority)
+            xauthority = g_build_filename (g_get_home_dir (), ".Xauthority", NULL);
+
+        status_notify ("%s CHECK-X-AUTHORITY-RECORDS RECORDS=%d", session_id, count_xauthority_records (xauthority));
+    }
+
     else if (strcmp (name, "WRITE-SHARED-DATA") == 0)
     {
         const gchar *data = g_hash_table_lookup (params, "DATA");

tests/test-stale-xauthority

@@ -0,0 +1,2 @@
+#!/bin/sh
+./src/dbus-env ./src/test-runner stale-xauthority test-gobject-greeter