Skip to content

Deduplicate matching Xauthority records #439

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
zhangpan2001 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Deduplicate matching Xauthority records #439

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 16 additions & 12 deletions src/x-authority.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ x_authority_write (XAuthority *auth, XAuthWriteMode mode, const gchar *filename,
g_warning ("Error reading existing Xauthority: %s", read_error->message);
}
GList *records = NULL;
gboolean matched = FALSE;
gboolean matching_record_found = FALSE;
while (input_offset != input_length)
{
g_autoptr(XAuthority) a = g_object_new (X_AUTHORITY_TYPE, NULL);
Expand Down Expand Up @@ -290,24 +290,28 @@ x_authority_write (XAuthority *auth, XAuthWriteMode mode, const gchar *filename,
address_matches = i == priv->address_length;
}

/* 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)
gboolean record_matches = priv->family == a_priv->family &&
address_matches &&
strcmp (priv->number, a_priv->number) == 0 &&
strcmp (priv->authorization_name, a_priv->authorization_name) == 0;

/* Remove all matching records, or update and keep only the first one */
if (record_matches)
Comment thread
zhangpan2001 marked this conversation as resolved.
{
matched = TRUE;
if (mode == XAUTH_WRITE_MODE_REMOVE)
gboolean duplicate_record = matching_record_found;
matching_record_found = TRUE;

if (mode == XAUTH_WRITE_MODE_REMOVE || duplicate_record)
continue;
else
x_authority_set_authorization_data (a, priv->authorization_data, priv->authorization_data_length);

x_authority_set_authorization_data (a, priv->authorization_data, priv->authorization_data_length);
}

records = g_list_append (records, g_steal_pointer (&a));
}

/* If didn't exist, then add a new one */
if (!matched)
/* If setting or replacing a missing record, then add a new one */
if (mode != XAUTH_WRITE_MODE_REMOVE && !matching_record_found)
records = g_list_append (records, g_object_ref (auth));

/* Write records back */
Expand Down
2 changes: 2 additions & 0 deletions tests/Makefile.am
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Expand Down Expand Up @@ -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 \
Expand Down
40 changes: 40 additions & 0 deletions tests/scripts/stale-xauthority.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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 MATCHING=1 STALE=0 UNRELATED=1

# Cleanup
#?*STOP-DAEMON
#?SESSION-X-0 TERMINATE SIGNAL=15
#?XSERVER-0 TERMINATE SIGNAL=15
#?RUNNER DAEMON-EXIT STATUS=0
15 changes: 15 additions & 0 deletions tests/src/Makefile.am
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ noinst_PROGRAMS = dbus-env \
vnc-client \
X \
Xvnc
check_PROGRAMS = test-x-authority-write
TESTS = $(check_PROGRAMS)
dist_noinst_SCRIPTS = lightdm-session \
test-python-greeter
noinst_LTLIBRARIES = libsystem.la
Expand Down Expand Up @@ -61,6 +63,19 @@ test_runner_LDADD = \
$(GIO_LIBS) \
$(GIO_UNIX_LIBS)

test_x_authority_write_SOURCES = \
test-x-authority-write.c \
$(top_srcdir)/src/x-authority.c \
$(top_srcdir)/src/x-authority.h
test_x_authority_write_CFLAGS = \
-I$(top_srcdir)/src \
$(WARN_CFLAGS) \
$(GOBJECT_CFLAGS) \
$(GLIB_CFLAGS)
test_x_authority_write_LDADD = \
$(GOBJECT_LIBS) \
$(GLIB_LIBS)

X_SOURCES = X.c x-authority.c x-authority.h x-common.c x-common.h x-server.c x-server.h xdmcp-client.c xdmcp-client.h status.c status.h
X_CFLAGS = \
$(WARN_CFLAGS) \
Expand Down
64 changes: 64 additions & 0 deletions tests/src/test-runner.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2520,6 +2520,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)
{
Expand Down Expand Up @@ -2779,6 +2834,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 ("");
Expand Down Expand Up @@ -2833,6 +2890,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);

Expand Down
133 changes: 133 additions & 0 deletions tests/src/test-session.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,123 @@ 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
read_xauthority_data (const guint8 *data, gsize data_length, gsize *offset, const guint8 **value, guint16 *value_length)
{
if (!read_card16 (data, data_length, offset, value_length) ||
*offset + *value_length > data_length)
return FALSE;

*value = data + *offset;
*offset += *value_length;

return TRUE;
}

typedef struct
{
guint16 family;
const guint8 *address;
guint16 address_length;
const guint8 *number;
guint16 number_length;
const guint8 *authorization_name;
guint16 authorization_name_length;
const guint8 *authorization_data;
guint16 authorization_data_length;
} XAuthorityRecordView;

static gboolean
read_xauthority_record (const guint8 *data, gsize data_length, gsize *offset, XAuthorityRecordView *record)
{
return read_card16 (data, data_length, offset, &record->family) &&
read_xauthority_data (data, data_length, offset, &record->address, &record->address_length) &&
read_xauthority_data (data, data_length, offset, &record->number, &record->number_length) &&
read_xauthority_data (data, data_length, offset, &record->authorization_name, &record->authorization_name_length) &&
read_xauthority_data (data, data_length, offset, &record->authorization_data, &record->authorization_data_length);
}

static gboolean
xauthority_data_matches (const guint8 *value, guint16 value_length, const guint8 *expected, gsize expected_length)
{
return value_length == expected_length && memcmp (value, expected, expected_length) == 0;
}

static gboolean
xauthority_string_matches (const guint8 *value, guint16 value_length, const gchar *expected)
{
return xauthority_data_matches (value, value_length, (const guint8 *) expected, strlen (expected));
}

static gboolean
summarize_stale_xauthority (const gchar *filename, gint *record_count, gint *matching_count, gint *stale_count, gint *unrelated_count)
{
const guint8 stale_cookie[16] = {
0x00, 0x01, 0x02, 0x03,
0x04, 0x05, 0x06, 0x07,
0x08, 0x09, 0x0A, 0x0B,
0x0C, 0x0D, 0x0E, 0x0F
};
const guint8 unrelated_cookie[16] = {
0xF0, 0xF1, 0xF2, 0xF3,
0xF4, 0xF5, 0xF6, 0xF7,
0xF8, 0xF9, 0xFA, 0xFB,
0xFC, 0xFD, 0xFE, 0xFF
};
g_autofree guint8 *data = NULL;
gsize data_length = 0;
if (!g_file_get_contents (filename, (gchar **) &data, &data_length, NULL))
return FALSE;

*record_count = 0;
*matching_count = 0;
*stale_count = 0;
*unrelated_count = 0;

gsize offset = 0;
while (offset < data_length)
{
XAuthorityRecordView record;
if (!read_xauthority_record (data, data_length, &offset, &record))
return FALSE;

(*record_count)++;

gboolean matching_record = record.family == 256 &&
xauthority_string_matches (record.address, record.address_length, "lightdm-test") &&
xauthority_string_matches (record.number, record.number_length, "0") &&
xauthority_string_matches (record.authorization_name, record.authorization_name_length, "MIT-MAGIC-COOKIE-1");
if (matching_record)
{
(*matching_count)++;
if (xauthority_data_matches (record.authorization_data, record.authorization_data_length, stale_cookie, sizeof (stale_cookie)))
(*stale_count)++;
}

gboolean unrelated_record = record.family == 256 &&
xauthority_string_matches (record.address, record.address_length, "stale-host") &&
xauthority_string_matches (record.number, record.number_length, "9") &&
xauthority_string_matches (record.authorization_name, record.authorization_name_length, "MIT-MAGIC-COOKIE-1") &&
xauthority_data_matches (record.authorization_data, record.authorization_data_length, unrelated_cookie, sizeof (unrelated_cookie));
if (unrelated_record)
(*unrelated_count)++;
}

return TRUE;
}

static gboolean
sigint_cb (gpointer user_data)
{
Expand Down Expand Up @@ -193,6 +310,22 @@ 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);

gint record_count;
gint matching_count;
gint stale_count;
gint unrelated_count;
if (summarize_stale_xauthority (xauthority, &record_count, &matching_count, &stale_count, &unrelated_count))
status_notify ("%s CHECK-X-AUTHORITY-RECORDS RECORDS=%d MATCHING=%d STALE=%d UNRELATED=%d", session_id, record_count, matching_count, stale_count, unrelated_count);
else
status_notify ("%s CHECK-X-AUTHORITY-RECORDS ERROR=INVALID", session_id);
}

else if (strcmp (name, "WRITE-SHARED-DATA") == 0)
{
const gchar *data = g_hash_table_lookup (params, "DATA");
Expand Down
Loading