[OpenAFS-devel] Bug in uss_kauth.c

[Dr.Dieter Mack]

We have recently introduced account locking after a number (actually 10) of
failed password attempts. As we create new accounts using uss bulk from
within a lengthy script, the "A" directive in the template file is the
obvious way to achieve this. Apparently nobody has ever used this, or at
least the lockout time parameter, as this is just not read in correctly.
Luckily, at these times we are now able to look at the code and fix it.
The function ktime_Str2int32 used in the original code converts a time
stamp, but not an duration of time. As the "kas setfields" command works,
the best thing is to borrow the corresponding code from kauth/admin_tools.c.
This is essentially what I did besides streamlining the code during this
process. And of course I tested it, and by now it is in production use
here in Hohemheim for about two weeks.

Here are the diffs:

$ diff -u uss_kauth.c.org uss_kauth.c
--- uss_kauth.c.org	Sat Jan 11 08:42:38 2003
+++ uss_kauth.c	Fri Feb 27 17:39:13 2004
@@ -604,7 +604,7 @@
     afs_int32 was_spare = 0;
     char instance = '\0';
     int pwexpiry;
-    int nfailures, locktime;
+    int nfailures, locktime, hrs, mins;

     if (strlen (username) > uss_UserLen) {
       	fprintf(stderr,
@@ -627,21 +627,16 @@
       fprintf(stderr,"Zero represents an unlimited lifetime.\n");
       fprintf(stderr,"Continuing with default lifetime == 0 for user %s.\n",
 	      username);
+      pwexpiry = 0;
     }
-    else {
-      misc_auth_bytes[0] = pwexpiry+1;
-    }
+    misc_auth_bytes[0] = pwexpiry+1;

-    if (!strcmp(reuse, "reuse")) {
-       misc_auth_bytes[1] = KA_REUSEPW;
-     }
-    else if (!strcmp(reuse, "noreuse")) {
+    if (!strcmp(reuse, "noreuse"))
       misc_auth_bytes[1] = KA_NOREUSEPW;
-      }
     else {
       misc_auth_bytes[1] = KA_REUSEPW;
-      fprintf(stderr,
-	      "must specify \"reuse\" or \"noreuse\": \"reuse\" assumed\n");
+      if (strcmp(reuse, "reuse"))
+	fprintf(stderr, "must specify \"reuse\" or \"noreuse\": \"reuse\" assumed\n");
     }

     nfailures = atoi(failures);
@@ -655,16 +650,28 @@
     else
       misc_auth_bytes[2] = nfailures+1;

-    locktime = ktime_Str2int32(lockout);
-    if (locktime < 0 || locktime > 36*60 ) {
+    hrs = 0;
+    if (strchr(lockout, ':'))
+      sscanf(lockout, "%d:%d", &hrs, &mins);
+    else
+      sscanf(lockout, "%d", &mins);
+
+    locktime = hrs*60 + mins;
+    if (hrs < 0 || hrs > 36 || mins < 0) {
+      fprintf(stderr,"Lockout times must be either minutes or hh:mm.\n");
+      fprintf(stderr,"Lockout times must be less than 36 hours.\n");
+      return KABADCMD;
+    }
+    else if (locktime > 36*60) {
       fprintf(stderr,"Lockout times must be either minutes or hh:mm.\n");
       fprintf(stderr,"Lockout times must be less than 36 hours.\n");
       fprintf(stderr,"Continuing with lock time == forever for user %s.\n",
 	      username);
+      misc_auth_bytes[3] = 1;
     }
     else {
-      locktime = (locktime * 60) >> 9;
-      misc_auth_bytes[3] = locktime +1;
+      locktime = (locktime * 60 + 511) >> 9;  /* ceil(l*60/512) */
+      misc_auth_bytes[3] = locktime + 1;
     }

     if (uss_SkipKaserver) {
$


And while browsing through these uss sources I found what is not really a bug
but perhaps one of the most efficient ways to copy a string:

$ diff -u uss.c.org uss.c
--- uss.c.org   Sat Oct 13 06:22:07 2001
+++ uss.c       Fri Feb 27 15:44:53 2004
@@ -1405,7 +1405,7 @@
         */
        uss_common_Reset();

-       sprintf(tbuf,"%s",buf);
+       strcpy(tbuf, buf);

        /*
        First line of file = line 1.
$


Thats all there is to it.

Have a nice weekend.

Dieter.


-------------
Dr. Dieter Mack                           Phone : +49 711 459 2838
                                          Fax   : +49 711 459 3449
Computer Center                           Email : Mack@uni-hohenheim.de
University of Hohenheim                   Mail  : Schloss, Westhof-Sued
                                                  70593 Stuttgart
http://www.uni-hohenheim.de/rz/mack               Germany
-------------
Disclaimer:  My views and those of my employer have been known to differ.