Re: [gdm-list] [PATCH] some pam related improvements
- From: Ludwig Nussel <ludwig nussel suse de>
- To: gdm-list gnome org
- Subject: Re: [gdm-list] [PATCH] some pam related improvements
- Date: Mon, 5 Feb 2007 16:40:51 +0100
Brian Cameron wrote:
> Can you resend the patch updated based on our discussion so far? I'd
> like to get this upstream but would like to see a patch that takes
> into account our discussion so far.
I've split up the patch into smaller chunks.
cu
Ludwig
--
(o_ Ludwig Nussel
//\ SUSE LINUX Products GmbH, Development
V_/_ http://www.suse.de/
- don't make the start again button insensitive as the prompt for
the username is not necessarily the first.
- don't tell backend that a user was selected in the face browser if
the backend itself caused that to avoid aborting an ongoing
authentication.
- allow the message boxes to expand
- don't preset message with "please enter your username" as the
first prompt might be a different one.
Index: gdm2/gui/gdmlogin.c
===================================================================
--- gdm2.orig/gui/gdmlogin.c
+++ gdm2/gui/gdmlogin.c
@@ -1558,7 +1558,8 @@ process_operation (guchar op_code,
gdm_config_get_bool (GDM_KEY_SOUND_ON_LOGIN));
gtk_label_set_text_with_mnemonic (GTK_LABEL (label), _("_Username:"));
greeter_probably_login_prompt = TRUE;
- gtk_widget_set_sensitive (start_again_button, FALSE);
+ /* prompt for username is not necessarily the first */
+ gtk_widget_set_sensitive (start_again_button, TRUE);
} else {
gtk_widget_set_sensitive (start_again_button, TRUE);
if (tmp != NULL)
@@ -1584,17 +1585,18 @@ process_operation (guchar op_code,
login_window_resize (FALSE /* force */);
+#if 0 /* sends interruption to backend which would cause it to redo auth */
if (greeter_probably_login_prompt == TRUE && selected_user != NULL)
face_browser_select_user (selected_user);
+#endif
break;
case GDM_NOECHO:
tmp = ve_locale_to_utf8 (args);
+ gtk_widget_set_sensitive (start_again_button, TRUE);
if (tmp != NULL && strcmp (tmp, _("Password:")) == 0) {
- gtk_widget_set_sensitive (start_again_button, TRUE);
gtk_label_set_text_with_mnemonic (GTK_LABEL (label), _("_Password:"));
} else {
- gtk_widget_set_sensitive (start_again_button, FALSE);
if (tmp != NULL)
gtk_label_set_text (GTK_LABEL (label), tmp);
}
@@ -2686,7 +2688,9 @@ gdm_login_gui_init (void)
/* Put in error box here */
err_box = gtk_label_new (NULL);
gtk_widget_set_name (err_box, "Error box");
+#if 0 /* widget doesn't expand anymore after this call */
gtk_widget_set_size_request (err_box, -1, 40);
+#endif
g_signal_connect (G_OBJECT (err_box), "destroy",
G_CALLBACK (gtk_widget_destroyed),
&err_box);
@@ -2756,14 +2760,16 @@ gdm_login_gui_init (void)
(GtkAttachOptions) (GTK_EXPAND | GTK_FILL),
(GtkAttachOptions) (GTK_FILL), 0, 10);
- msg = gtk_label_new (_("Please enter your username"));
+ msg = gtk_label_new (NULL);
gtk_widget_set_name (msg, "Message");
gtk_label_set_line_wrap (GTK_LABEL (msg), TRUE);
gtk_label_set_justify (GTK_LABEL (msg), GTK_JUSTIFY_LEFT);
gtk_table_attach (GTK_TABLE (stack), msg, 0, 1, 6, 7,
(GtkAttachOptions) (GTK_EXPAND | GTK_FILL),
(GtkAttachOptions) (GTK_FILL), 0, 10);
+#if 0 /* widget doesn't expand anymore after this call */
gtk_widget_set_size_request (msg, -1, 30);
+#endif
gtk_widget_ref (msg);
g_object_set_data_full (G_OBJECT (login), "msg", msg,
- display lastlog information after succesful authentication
Index: gdm2/daemon/verify-pam.c
===================================================================
--- gdm2.orig/daemon/verify-pam.c
+++ gdm2/daemon/verify-pam.c
@@ -517,7 +517,6 @@
openlog ("gdm", LOG_PID, LOG_DAEMON);
for (replies = 0; replies < num_msg; replies++) {
- gboolean islogin = FALSE;
const char *m = (*msg)[replies].msg;
m = perhaps_translate_message (m);
@@ -542,7 +541,6 @@
s = gdm_slave_greeter_ctl (GDM_PROMPT, m);
/* this will clear the message */
gdm_slave_greeter_ctl_no_ret (GDM_MSG, "");
- islogin = TRUE;
}
} else {
s = gdm_slave_greeter_ctl (GDM_PROMPT, m);
@@ -557,29 +555,6 @@
return PAM_CONV_ERR;
}
- if (islogin) {
- /* note that we should NOT rely on this for anything
- that is REALLY needed. The reason this is here
- is for the face browser to work correctly. The
- best way would really be to ask all questions at
- once, but that would 1) need some protocol rethinking,
- 2) the gdmgreeter program is really not equipped
- for this, would need theme format change */
- /* Second problem is that pam will not actually set
- PAM_USER itself after it asks for it. This is
- pretty evil, evil, evil, evil, but somewhat necessary
- then. */
- /* FIXME: this is a HACK HACK HACK */
- g_free (tmp_PAM_USER);
- tmp_PAM_USER = g_strdup (s);
-
- if (gdm_get_value_bool (GDM_KEY_DISPLAY_LAST_LOGIN)) {
- char *info = gdm_get_last_info (s);
- gdm_slave_greeter_ctl_no_ret (GDM_ERRBOX, info);
- g_free (info);
- }
- }
-
reply[replies].resp_retcode = PAM_SUCCESS;
reply[replies].resp = strdup (ve_sure_string (s));
g_free (s);
@@ -1025,6 +1000,12 @@
goto pamerr;
}
+ if (gdm_get_value_bool (GDM_KEY_DISPLAY_LAST_LOGIN)) {
+ char *info = gdm_get_last_info (login);
+ gdm_slave_greeter_ctl_no_ret (GDM_MSG, info);
+ g_free (info);
+ }
+
/* Check if the user's account is healthy. */
pamerr = pam_acct_mgmt (pamh, null_tok);
switch (pamerr) {
- cancel all conversations in a pam_authenticate run if a user was
selected in the face browser.
Index: gdm2/daemon/verify-pam.c
===================================================================
--- gdm2.orig/daemon/verify-pam.c
+++ gdm2/daemon/verify-pam.c
@@ -489,7 +489,9 @@ gdm_verify_pam_conv (int num_msg, struct
/* Should never happen unless PAM is on crack and keeps asking questions
after we told it to go away. So tell it to go away again and
maybe it will listen */
- if ( ! gdm_slave_action_pending ())
+ /* well, it actually happens if there are multiple pam modules
+ * with conversations */
+ if ( ! gdm_slave_action_pending () || selected_user)
return PAM_CONV_ERR;
reply = malloc (sizeof (struct pam_response) * num_msg);
- don't try to be smart about the username prompt. If pam asks for a
username then also ask the user about it. The GUI could be made to
preset the text entry with the username instead.
Index: gdm2/daemon/verify-pam.c
===================================================================
--- gdm2.orig/daemon/verify-pam.c
+++ gdm2/daemon/verify-pam.c
@@ -527,13 +527,7 @@
/* PAM requested textual input with echo on */
case PAM_PROMPT_ECHO_ON:
if (strcmp (m, _("Username:")) == 0) {
- if ( ! ve_string_empty (selected_user)) {
- /* Sometimes we are just completely on crack,
- and pam asks for the username even if we
- already gave it. PAM is on better crack,
- then I can afford. */
- s = g_strdup (selected_user);
- } else {
+ if ( ve_string_empty (selected_user)) {
/* this is an evil hack, but really there is no way we'll
know this is a username prompt. However we SHOULD NOT
rely on this working. The pam modules can set their
- remove the tmp_PAM_USER hack
- redo auth with same username only until LOGIN_RETRIES is reached
Index: gdm2/daemon/verify-pam.c
===================================================================
--- gdm2.orig/daemon/verify-pam.c
+++ gdm2/daemon/verify-pam.c
@@ -52,8 +52,11 @@ static pam_handle_t *pamh = NULL;
static GdmDisplay *cur_gdm_disp = NULL;
-/* this is a hack */
-static char *tmp_PAM_USER = NULL;
+/* Hack. Used so user does not need to select username in face
+ * browser again if pw was wrong. Not used if username was typed
+ * manually */
+static char* prev_user;
+static unsigned auth_retries;
/* this is another hack */
static gboolean did_we_ask_for_password = FALSE;
@@ -504,13 +507,8 @@ gdm_verify_pam_conv (int num_msg, struct
/* Here we set the login if it wasn't already set,
* this is kind of anal, but this way we guarantee that
* the greeter always is up to date on the login */
- if (pam_get_item (pamh, PAM_USER, (void **)&p) == PAM_SUCCESS ||
- tmp_PAM_USER != NULL) {
+ if (pam_get_item (pamh, PAM_USER, (void **)&p) == PAM_SUCCESS) {
login = (const char *)p;
-
- /* FIXME: this is a HACK HACK HACK, for some reason needed */
- if (tmp_PAM_USER != NULL)
- login = tmp_PAM_USER;
gdm_slave_greeter_ctl_no_ret (GDM_SETLOGIN, login);
}
@@ -845,9 +843,16 @@ verify_user_again:
authenticate_again:
- /* hack */
- g_free (tmp_PAM_USER);
- tmp_PAM_USER = g_strdup (login);
+ if(prev_user && !login) {
+ login = g_strdup(prev_user);
+ } else if(login && !prev_user) {
+ prev_user = g_strdup(login);
+ auth_retries = 0;
+ } else if(login && prev_user && strcmp(login, prev_user)) {
+ g_free(prev_user);
+ prev_user = g_strdup(login);
+ auth_retries = 0;
+ }
/*
* Initialize a PAM session for the user...
@@ -865,6 +870,9 @@ authenticate_again:
}
g_free (pam_stack);
+ g_free (login);
+ login = NULL; // have to unset it otherwise there is no chance to ever enter a different user
+
pam_set_item (pamh, PAM_USER_PROMPT, _("Username:"));
#if 0
@@ -911,7 +919,11 @@ authenticate_again:
/* FIXME: what about errors */
/* really this has been a sucess, not a failure */
- pam_end (tmp_pamh, PAM_SUCCESS);
+ pam_end (tmp_pamh, pamerr);
+
+ g_free(prev_user);
+ prev_user = NULL;
+ auth_retries = 0;
gdm_slave_greeter_ctl_no_ret (GDM_SETLOGIN, login);
@@ -919,6 +931,7 @@ authenticate_again:
}
if (started_timer)
gdm_slave_greeter_ctl_no_ret (GDM_STOPTIMER, "");
+
if (gdm_slave_action_pending ()) {
/* FIXME: see note above about PAM_FAIL_DELAY */
/* #ifndef PAM_FAIL_DELAY */
@@ -927,7 +940,28 @@ authenticate_again:
usleep (g_random_int_range (0, 100000));
/* #endif */ /* PAM_FAIL_DELAY */
gdm_error (_("Couldn't authenticate user"));
+
+ if(prev_user) {
+ unsigned max_auth_retries = 3;
+ char *val = gdm_read_default("LOGIN_RETRIES=");
+ if(val) {
+ max_auth_retries = atoi(val);
+ g_free(val);
+ }
+ if(pamerr == PAM_MAXTRIES || ++auth_retries >= max_auth_retries) {
+ g_free(prev_user);
+ prev_user = NULL;
+ auth_retries = 0;
+ }
+ }
+ } else {
+ /* cancel, configurator etc pressed */
+ g_free(prev_user);
+ prev_user = NULL;
+ auth_retries = 0;
}
+
+
goto pamerr;
}
@@ -937,6 +971,8 @@ authenticate_again:
g_free (login);
login = NULL;
+ g_free (prev_user);
+ prev_user = NULL;
if ((pamerr = pam_get_item (pamh, PAM_USER, (void **)&p)) != PAM_SUCCESS) {
login = NULL;
@@ -1096,9 +1132,6 @@ authenticate_again:
cur_gdm_disp = NULL;
- g_free (tmp_PAM_USER);
- tmp_PAM_USER = NULL;
-
#ifdef HAVE_LOGINDEVPERM
if (d->attached)
(void) di_devperm_login ("/dev/console", pwent->pw_uid,
@@ -1183,9 +1216,6 @@ authenticate_again:
g_free (login);
- g_free (tmp_PAM_USER);
- tmp_PAM_USER = NULL;
-
cur_gdm_disp = NULL;
return NULL;
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
