Relevant BUGIDs: 493294

Purpose of commit: bugfix and document

Commit summary:
---------------
The login limit counting seemed to have its math incorrect.
The default has been changed to the sane one, and a new module argument
has been added "utmp_early" that returns the module to its old behavior.

Thanks to Berend De Schouwe for getting to the bottom of this issue.

4 files changed, 73 insertions, 32 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 14cc890f..a85b4e9f 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -55,6 +55,11 @@ bug report - outstanding bugs are listed here:
 0.76: please submit patches for this section with actual code/doc
       patches!
 
+* pam_limits can handle negative priority limits now (which can apply
+  to the superuser too) - based on patch from Nalin. Also cleanup the
+  error handling that was very sloppy before. Also, courtesy of Berend
+  De Schouwe get the math right on login counting (Bug 476990, 476987,
+  493294 - agmorgan)
 * documentation: random typo fixes from Nalin and more stuff from me
   (Bug 476949, Tasks 43507, 17426 - agmorgan)
 * A Tru64 fix (given other stuff has already resolved this, it
@@ -110,10 +115,6 @@ bug report - outstanding bugs are listed here:
   from Nalin (Bug 476940 - agmorgan)
 * pam_filter cleanup (including moving the filter directory) Nalin
   and Harald Welte (Bugs 436057, 476970 - agmorgan)
-* pam_limits can handle negative priority limits now (which can apply
-  to the superuser too) - based on patch from Nalin. Also cleanup the
-  error handling that was very sloppy before (Bug 476990, 476987 -
-  agmorgan)
 * db3 is now recognized as a libdb candidate (Bug 435764 - agmorgan)
 * more changes (extracted from redhat version) courtesy of
   Harald Welte (Bugs pam_limits=436061, pam_lastlog=436060,
diff --git a/doc/modules/pam_limits.sgml b/doc/modules/pam_limits.sgml
index 961f0b55..eaa16bd9 100644
--- a/doc/modules/pam_limits.sgml
+++ b/doc/modules/pam_limits.sgml
@@ -54,7 +54,8 @@ configuration file discussed below.
 <descrip>
 
 <tag><bf>Recognized arguments:</bf></tag>
-<tt/debug/; <tt>conf=/path/to/file.conf</tt>
+<tt/debug/; <tt>conf=/path/to/file.conf</tt>; <tt>change_uid</tt>;
+<tt>utmp_early</tt>
 
 <tag><bf>Description:</bf></tag>
 
@@ -80,6 +81,13 @@ option if you have problems like login not forking a shell for user
 who has no processes. Be warned that something else may break when
 you do this.
 
+<item><tt/utmp_early/ - 
+some broken applications actually allocate a utmp entry for the user
+before the user is admitted to the system. If some of the services you
+are configuring PAM for do this, you can selectively use this module
+argument to compensate for this behavior and at the same time maintain
+system-wide consistency with a single limits.conf file.
+
 </itemize>
 
 <tag><bf>Examples/suggested usage:</bf></tag>
diff --git a/modules/pam_limits/README b/modules/pam_limits/README
index c0dba581..023b9575 100644
--- a/modules/pam_limits/README
+++ b/modules/pam_limits/README
@@ -81,6 +81,13 @@ ARGUMENTS RECOGNIZED:
 			no processes.  Be warned that something else
 			may break when you do this.
 
+    utmp_early		some broken applications actually allocate a
+			utmp entry for the user before the user is
+			admitted to the system. If the service you are
+			configuring PAM for does this, you can use
+			this module argument to compensate for this
+			brokenness.
+
 MODULE SERVICES PROVIDED:
 	session            _open_session and _close_session (blank)
 
diff --git a/modules/pam_limits/pam_limits.c b/modules/pam_limits/pam_limits.c
index 303ed661..f85c5f05 100644
--- a/modules/pam_limits/pam_limits.c
+++ b/modules/pam_limits/pam_limits.c
@@ -2,7 +2,7 @@
  * pam_limits - impose resource limits when opening a user session
  *
  * 1.6 - modified for PLD (added process priority settings) 
- *       by Marcin Korzonek <mkorz@shadow.eu.org
+ *       by Marcin Korzonek <mkorz@shadow.eu.org>
  * 1.5 - Elliot Lee's "max system logins patch"
  * 1.4 - addressed bug in configuration file parser
  * 1.3 - modified the configuration file format
@@ -71,6 +71,7 @@ struct pam_limit_s {
     int supported[RLIM_NLIMITS];
     struct user_limits_struct limits[RLIM_NLIMITS];
     char conf_file[BUFSIZ];
+    int utmp_after_pam_call;
 };
 
 #define LIMIT_LOGIN RLIM_NLIMITS+1
@@ -102,29 +103,32 @@ static void _pam_log(int err, const char *format, ...)
 
 #define PAM_DEBUG_ARG       0x0001
 #define PAM_DO_SETREUID     0x0002
+#define PAM_UTMP_EARLY      0x0004
 
 static int _pam_parse(int argc, const char **argv, struct pam_limit_s *pl)
 {
-     int ctrl=0;
-
-     /* step through arguments */
-     for (ctrl=0; argc-- > 0; ++argv) {
-
-          /* generic options */
-
-          if (!strcmp(*argv,"debug"))
-               ctrl |= PAM_DEBUG_ARG;
-          else if (!strncmp(*argv,"conf=",5))
-                strncpy(pl->conf_file,*argv+5,sizeof(pl->conf_file)-1);
-	  else if (!strncmp(*argv,"change_uid",10))
-		ctrl |= PAM_DO_SETREUID;
-          else {
-               _pam_log(LOG_ERR,"pam_parse: unknown option; %s",*argv);
-          }
-     }
-     pl->conf_file[sizeof(pl->conf_file) - 1] = '\0';
+    int ctrl=0;
+
+    /* step through arguments */
+    for (ctrl=0; argc-- > 0; ++argv) {
+
+	/* generic options */
 
-     return ctrl;
+	if (!strcmp(*argv,"debug")) {
+	    ctrl |= PAM_DEBUG_ARG;
+	} else if (!strncmp(*argv,"conf=",5)) {
+	    strncpy(pl->conf_file,*argv+5,sizeof(pl->conf_file)-1);
+	} else if (!strncmp(*argv,"change_uid",10)) {
+	    ctrl |= PAM_DO_SETREUID;
+	} else if (!strcmp(*argv,"utmp_early")) {
+	    ctrl |= PAM_UTMP_EARLY;
+	} else {
+	    _pam_log(LOG_ERR,"pam_parse: unknown option; %s",*argv);
+	}
+    }
+    pl->conf_file[sizeof(pl->conf_file) - 1] = '\0';
+
+    return ctrl;
 }
 
 
@@ -212,24 +216,45 @@ static int check_logins(const char *name, int limit, int ctrl,
     }
 
     setutent();
-    count = 0;
+
+    /* Because there is no definition about when an application
+       actually adds a utmp entry, some applications bizarrely do the
+       utmp call before the have PAM authenticate them to the system:
+       you're logged it, sort of...? Anyway, you can use the
+       "utmp_early" module argument in your PAM config file to make
+       allowances for this sort of problem. (There should be a PAM
+       standard for this, since if a module wants to actually map a
+       username then any early utmp entry will be for the unmapped
+       name = broken.) */
+    
+    if (ctrl & PAM_UTMP_EARLY) {
+	count = 0;
+    } else {
+	count = 1;
+    }
+
     while((ut = getutent())) {
 #ifdef USER_PROCESS
-        if (ut->ut_type != USER_PROCESS)
+        if (ut->ut_type != USER_PROCESS) {
             continue;
+	}
 #endif
-        if (ut->UT_USER[0] == '\0')
+        if (ut->UT_USER[0] == '\0') {
             continue;
+	}
         if (!pl->flag_numsyslogins) {
 	    if ((pl->login_limit_def == LIMITS_DEF_USER)
-		&& strncmp(name, ut->UT_USER, sizeof(ut->UT_USER)) != 0)
+		&& strncmp(name, ut->UT_USER, sizeof(ut->UT_USER)) != 0) {
                 continue;
+	    }
 	    if ((pl->login_limit_def == LIMITS_DEF_GROUP)
-		&& !is_in_group(ut->UT_USER, name))
+		&& !is_in_group(ut->UT_USER, name)) {
                 continue;
+	    }
+	}
+	if (++count > limit) {
+	    break;
 	}
-        if (++count > limit)
-            break;
     }
     endutent();
     if (count > limit) {