From fe117f074137c38828076c14989e4c0ebbcb41aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edgar=20Fu=C3=9F?= Date: Wed, 5 Jun 2019 17:42:18 +0200 Subject: Work around a race with password-less logins and double clicks There is a race in the greeter (greeter as in lightdm-gtk-greeter) easily triggered by double-clicking on the login button (or clicking it with a bouncing mouse button) with a password-less login. The problem arises when the daemon has already sent SERVER_MESSAGE_END_AUTHENTICATION to the greeter, but before it digests it, it handles more X11 events (e.g. the second click), sending the daemon a GREETER_MESSAGE_CONTINUE_AUTHENTICATION (before the GREETER_MESSAGE_START_SESSION) which will confuse handle_continue_authentication(), making it call session_respond_error(), which then sends a PAM_CONV_ERR to the session child, which will in turn get confused because it's expecting the (length of the) session error file name. As I don't really know how to avoid the race in the greeter (you would need to teach it prioritizing messages from the master over messages from X11), work around it by setting a flag in GreeterPrivate if SERVER_MESSAGE_END_AUTHENTICATION has been sent and ignore GREETER_MESSAGE_CONTINUE_AUTHENTICATION if that flag is set. The change was verified on 1.18.3 and then adopted to HEAD. --- src/greeter.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/greeter.c b/src/greeter.c index 50d0defd..75fe9ff6 100644 --- a/src/greeter.c +++ b/src/greeter.c @@ -75,6 +75,14 @@ typedef struct /* TRUE if logging into guest session */ gboolean guest_account_authenticated; + /* Avoid a race where we already sent SERVER_MESSAGE_END_AUTHENTICATION to the greeter, + * but before it digests it, it handles more X11 events, sending us a GREETER_MESSAGE_CONTINUE_AUTHENTICATION + * (before the GREETER_MESSAGE_START_SESSION) which will confuse handle_continue_authentication(), + * calling session_respond_error(), which then sends a PAM_CONV_ERR to the session child, + * which will confuse that because it's expecting the (length of the) session error file name. + */ + gboolean have_sent_end_authentication; + /* Communication channels to communicate with */ int to_greeter_input; int from_greeter_output; @@ -410,11 +418,13 @@ send_end_authentication (Greeter *greeter, guint32 sequence_number, const gchar { guint8 message[MAX_MESSAGE_LENGTH]; gsize offset = 0; + GreeterPrivate *priv = greeter_get_instance_private (greeter); write_header (message, MAX_MESSAGE_LENGTH, SERVER_MESSAGE_END_AUTHENTICATION, int_length () + string_length (username) + int_length (), &offset); write_int (message, MAX_MESSAGE_LENGTH, sequence_number, &offset); write_string (message, MAX_MESSAGE_LENGTH, username, &offset); write_int (message, MAX_MESSAGE_LENGTH, result, &offset); write_message (greeter, message, offset); + priv->have_sent_end_authentication = TRUE; } void @@ -489,6 +499,7 @@ reset_session (Greeter *greeter) } priv->guest_account_authenticated = FALSE; + priv->have_sent_end_authentication = FALSE; } static void @@ -648,6 +659,12 @@ handle_continue_authentication (Greeter *greeter, gchar **secrets) size_t messages_length = session_get_messages_length (priv->authentication_session); const struct pam_message *messages = session_get_messages (priv->authentication_session); + /* We may have already sent SERVER_MESSAGE_END_AUTHENTICATION, but the greeter may not have digested it yet. */ + if (priv->have_sent_end_authentication) { + g_debug ("Ignoring continue authentication"); + return; + } + /* Check correct number of responses */ int n_prompts = 0; for (int i = 0; i < messages_length; i++) -- cgit v1.2.1