[Pkg-shadow-devel] Bug#87648: Old proposed patches #2: 463_login_delay_obeys_to_PAM

[Nicolas François]

Hello,

I tend to agree.
There is no need to have a FAIL_DELAY in login if another service do not
have a fail delay. So it is preferable to have the delay in PAM.

Currently FAIL_DELAY imposes a minimum delay. Other PAM modules may also
set another minimum delay (e.g. pam_unix imposes a 2s delay when nodelay
is not set). The resulting delay will be the maximum of these delays
plus/minus 25%.

The patch could be modified to:
 * remove FAIL_DELAY from debian/login.defs
 * move FAIL_DELAY to the ifndef USE_PAM section of lib/getdef.c
 * move int delay; to the ifndef USE_PAM section of src/login.c
(patch attached)


One possible issue would be if a pam module do not set a minimum delay.
FAIL_DELAY could be used as a configurable delay, but I don't think having
a non-configurable delay (0 or 2s in pam_unix) is an issue (i.e. no need
for a delay= pam_unix option, or at least no need to wait for such an
option).

Tomasz, the patch will probably not apply to your CVS. Please just comment
on the goal.

Another way to fix this bug is to document the PAM behavior in login.defs.
(nodelay will be respected when FAIL_DELAY is set to 0)


-- 
Nekral
-------------- next part --------------
Goal: Do not hardcode pam_fail_delay and let pam_unix do its
      job to set a delay...or not

Fixes: #87648

Status wrt upstream: Forwarded but not applied yet

Index: shadow-4.0.14/src/login.c
===================================================================
--- shadow-4.0.14.orig/src/login.c	2006-01-11 00:03:20.000000000 +0100
+++ shadow-4.0.14/src/login.c	2006-01-11 01:07:23.000000000 +0100
@@ -327,7 +327,6 @@
 	char ptime[80];
 #endif
 	int reason = PW_LOGIN;
-	int delay;
 	int retries;
 	int failed;
 	int flag;
@@ -346,6 +345,7 @@
 	pid_t child;
 	char *pam_user;
 #else
+	int delay;
 	struct spwd *spwd = NULL;
 #endif
 	/*
@@ -568,7 +568,6 @@
 			alarm (timeout);
 
 		environ = newenvp;	/* make new environment active */
-		delay = getdef_num ("FAIL_DELAY", 1);
 		retries = getdef_num ("LOGIN_RETRIES", RETRIES);
 
 #ifdef USE_PAM
@@ -584,17 +583,12 @@
 
 		/*
 		 * hostname & tty are either set to NULL or their correct values,
-		 * depending on how much we know. We also set PAM's fail delay to
-		 * ours.
+		 * depending on how much we know.
 		 */
 		retcode = pam_set_item (pamh, PAM_RHOST, hostname);
 		PAM_FAIL_CHECK;
 		retcode = pam_set_item (pamh, PAM_TTY, tty);
 		PAM_FAIL_CHECK;
-#ifdef HAVE_PAM_FAIL_DELAY
-		retcode = pam_fail_delay (pamh, 1000000 * delay);
-		PAM_FAIL_CHECK;
-#endif
 		/* if fflg == 1, then the user has already been authenticated */
 		if (!fflg || (getuid () != 0)) {
 			int failcount = 0;
@@ -635,8 +629,6 @@
 			  failed = 0;
 
 			  failcount++;
-			  if (delay > 0)
-			    retcode = pam_fail_delay(pamh, 1000000*delay);
 
 			  retcode = pam_authenticate (pamh, 0);
 
@@ -931,13 +923,16 @@
 		if (pwent.pw_passwd[0] == '\0')
 			pw_auth ("!", username, reason, (char *) 0);
 
+#ifndef USE_PAM
 		/*
 		 * Wait a while (a la SVR4 /usr/bin/login) before attempting
 		 * to login the user again. If the earlier alarm occurs
 		 * before the sleep() below completes, login will exit.
 		 */
+		delay = getdef_num ("FAIL_DELAY", 1);
 		if (delay > 0)
 			sleep (delay);
+#endif
 
 		puts (_("Login incorrect"));
 
Index: shadow-4.0.14/debian/login.defs
===================================================================
--- shadow-4.0.14.orig/debian/login.defs	2006-01-11 00:03:20.000000000 +0100
+++ shadow-4.0.14/debian/login.defs	2006-01-11 01:07:15.000000000 +0100
@@ -36,11 +36,6 @@
 #MAIL_FILE      .mail
 
 #
-# Delay in seconds before being allowed another attempt after a login failure
-#
-FAIL_DELAY		3
-
-#
 # Enable logging and display of /var/log/faillog login failure info.
 # This option conflicts with the pam_tally PAM module.
 #
@@ -321,6 +316,7 @@
 #ENV_HZ
 #CHFN_AUTH
 #CHSH_AUTH
+#FAIL_DELAY
 
 ################# OBSOLETED #######################
 #						  #
Index: shadow-4.0.14/lib/getdef.c
===================================================================
--- shadow-4.0.14.orig/lib/getdef.c	2006-01-11 00:03:20.000000000 +0100
+++ shadow-4.0.14/lib/getdef.c	2006-01-11 01:07:15.000000000 +0100
@@ -55,7 +55,6 @@
 	{"ENV_PATH", NULL},
 	{"ENV_SUPATH", NULL},
 	{"ERASECHAR", NULL},
-	{"FAIL_DELAY", NULL},
 	{"FAILLOG_ENAB", NULL},
 	{"FAKE_SHELL", NULL},
 	{"FTMP_FILE", NULL},
@@ -91,6 +90,7 @@
 	{"ENV_HZ", NULL},
 	{"ENV_TZ", NULL},
 	{"ENVIRON_FILE", NULL},
+	{"FAIL_DELAY", NULL},
 	{"ISSUE_FILE", NULL},
 	{"LASTLOG_ENAB", NULL},
 	{"LOGIN_STRING", NULL},