RE: Patch: Fix user authentication + MKDB

["Pankaj K Garg" <gargp@earthlink.net>]

[-- Attachment #1: Type: text/plain, Size: 1902 bytes --]

Thanks for testing the patch.

> ...
> signal Segmentation fault at >*[strlen, 0x3ff800d1d30]  ldq_u 
>   t0, 0(a0)
> (dbx) where
> >  0 strlen(0x120033f58, 0xf, 0xf, 0x140032660, 0x1200340d4) 
> [0x3ff800d1d30]
>    1 xstrdup() ["../../gnats-4/libiberty/xstrdup.c":6, 0x1200340b4]
>    2 copy_adm_entry() ["../../gnats-4/gnats/adm.c":6, 0x12001a1f8]
>    3 get_responsible_address() 
> ["../../gnats-4/gnats/mail.c":6, 0x120026504]
>    4 get_one_responsible_addr() 
> ["../../gnats-4/gnats/mail.c":6, 0x120026738]
>    5 get_responsible_addr() ["../../gnats-4/gnats/mail.c":6, 
> 0x120026b60]
>    6 gnatsdChdb() ["../../gnats-4/gnats/cmds.c":6, 0x12000b5a0]
>    7 GNATS_user() ["../../gnats-4/gnats/cmds.c":6, 0x12000aae8]
>    8 serverMainLoop() ["../../gnats-4/gnats/gnatsd.c":6, 0x120009ca0]
>    9 main() ["../../gnats-4/gnats/gnatsd.c":6, 0x12000a0e8]
> (dbx)
>

I was not able to replicate this problem even with LIB_CRYPT set
to 0. But, I suspect this is happening because I changed the
fields routines to return a NULL in case of an empty field. Sounds
like a bad idea, as obviously some routines depend on it being
non-NULL.

So, I've changed this in a new patch. Now, for an empty field
we return an empty string (""). Hopefully, this should fix
things. Please try the attached patch and let me know.

> As nn aside: if gnatsd for some reason cannot access the 
> responsible file
> (or the gnatsd.user_access file for that matter) it silently 
> ignores the fact
> which seems rather strange (but has nothing to do with your patch).

Yes, we should log it somewhere. I can take care of this after this
round of changes stabilize.

> By the way, don't forget the GNATS_help() function at the end 
> of cmds.c
> and the manual of course (I'm willing to help with that).

I made some changes to the GNATS_help() function. We can fix the manual
once we stabilize the code.

Regards

Pankaj
 

[-- Attachment #2: patchfile.out --]
[-- Type: text/plain, Size: 6463 bytes --]

Index: cmds.c
===================================================================
RCS file: /cvsroot/gnats/gnats/gnats/cmds.c,v
retrieving revision 1.69
diff -u -p -r1.69 cmds.c
--- cmds.c	12 Aug 2002 12:33:30 -0000	1.69
+++ cmds.c	27 Sep 2002 17:02:55 -0000
@@ -318,11 +318,11 @@ GNATS_user (int ac, char **av)
       printf ("%d %s\r\n", CODE_INFORMATION,
 	      access_level_str (user_access));
     }
-  else if (ac == 2)
+  else if ((ac == 1) || (ac == 2))
     {
       if (databaseValid (currentDatabase))
 	{
-	  if (gnatsdChdb (databaseName (currentDatabase), av[0], av[1], 0,
+	  if (gnatsdChdb (databaseName (currentDatabase), av[0], ac == 2 ? av[1] : "", 0,
 			  &err) != 0)
 	    {
 	      print_server_errors (err);
@@ -339,14 +339,21 @@ GNATS_user (int ac, char **av)
 	      free (currentPassword);
 	    }
 	  currentUsername = xstrdup (av[0]);
-	  currentPassword = xstrdup (av[1]);
+	  if (ac == 2)
+	    {
+	      currentPassword = xstrdup (av[1]);
+	    }
+	  else
+	    {
+	      currentPassword = (char *)"";
+	    }
 	  printf ("%d Current database is not valid; use CHDB to set the database\r\n",
 		  CODE_OK);
 	}
     }
   else
     {
-      printf ("%d Need two arguments, username and password\r\n", 
+      printf ("%d Need one or two arguments, username and optionally a password\r\n", 
 	      CODE_CMD_ERROR);
     }
 }
@@ -593,14 +600,18 @@ gnatsdChdb (const char *nameOfDb, const 
       currentUsername = xstrdup (username);
     }
 
+  if (currentPassword != NULL)
+    {
+      free (currentPassword);
+    }
   if (passwd != NULL)
     {
-      if (currentPassword != NULL)
-	{
-	  free (currentPassword);
-	}
       currentPassword = xstrdup (passwd);
     }
+  else
+    {
+      currentPassword = NULL;
+    }
 
   if (currentUsername == NULL)
     {
@@ -670,9 +681,9 @@ GNATS_chdb (int ac, char **av)
   const char *user = NULL;
   const char *passwd = NULL;
 
-  if (ac != 1 && ac != 3)
+  if (ac != 1 && ac != 2 && ac != 3)
     {
-      printf ("%d One or three arguments required.\r\n", CODE_CMD_ERROR); 
+      printf ("%d One, two, or three arguments required.\r\n", CODE_CMD_ERROR); 
       return;
     }
 
@@ -681,6 +692,10 @@ GNATS_chdb (int ac, char **av)
       user = av[1];
       passwd = av[2];
     }
+  else if (ac == 2)
+    {
+      user = av[1];
+    }
 
   if (gnatsdChdb (av[0], user, passwd, 0, &err) != 0)
     {
@@ -1786,11 +1801,11 @@ GNATS_help (int ac ATTRIBUTE_UNUSED, cha
  	  CODE_INFORMATION);
   printf ("%d-   SUBM                    submit a new PR\r\n",
 	  CODE_INFORMATION);
-  printf ("%d-   CHDB <database> [<user> <passwd>]\r\n",
+  printf ("%d-   CHDB <database> [<user> [<passwd>]]\r\n",
 	  CODE_INFORMATION);
   printf ("%d-                           change GNATS ROOT to <database>\r\n",
 	  CODE_INFORMATION);
-  printf ("%d-   USER <name> <passwd>    Sets the current user\r\n",
+  printf ("%d-   USER <name> [<passwd>]  Sets the current user\r\n",
 	  CODE_INFORMATION);
   printf ("%d-   USER                    Report current access level\r\n",
 	  CODE_INFORMATION);
Index: gnatsd.access
===================================================================
RCS file: /cvsroot/gnats/gnats/gnats/gnatsd.access,v
retrieving revision 1.5
diff -u -p -r1.5 gnatsd.access
--- gnatsd.access	16 Oct 2001 15:06:56 -0000	1.5
+++ gnatsd.access	27 Sep 2002 17:02:55 -0000
@@ -17,6 +17,8 @@
 #               assumed to be encrypted with standard crypt(), while passwords
 #               prefixed with $1$ are assumed to be MD5 encrypted.
 #               MD5 and crypt() encryption may not be available on all systems.
+#		An empty field value means that the user should not supply any
+#		password.
 #       * access-level: (default = edit)
 #               deny     - gnatsd closes the connection
 #               none     - no further access until userid and password given
@@ -33,4 +35,4 @@
 #         It's ignored in gnatsd-adm/gnatsd.access since this file is already
 #         database specific.
 #
-#*:*:view:
+#*::view:
Index: gnatsd.c
===================================================================
RCS file: /cvsroot/gnats/gnats/gnats/gnatsd.c,v
retrieving revision 1.47
diff -u -p -r1.47 gnatsd.c
--- gnatsd.c	4 Aug 2002 10:58:29 -0000	1.47
+++ gnatsd.c	27 Sep 2002 17:02:55 -0000
@@ -253,21 +253,45 @@ match (const char *line, const char *pat
 static int
 password_match (const char *password, const char *hash)
 {
-  if (! strncmp (hash, "$0$", 3))
+  if (strlen(password) && strlen(hash))
     {
-      /* explicit plain-text password */
-      return ! match (password, hash, TRUE);
-    }
-  else
-    {
-      /* DES crypt or MD5 hash of the password */
+      if (! strncmp (hash, "$0$", 3))
+	{
+	  /* explicit plain-text password */
+	  return match (password, hash+3, TRUE);
+	}
+      else
+	{
 #ifdef HAVE_LIBCRYPT
-      char *encrypted = crypt (password, hash);
-      return encrypted && ! strcmp (encrypted, hash);
+	  char *hashvalue, *encrypted;
+
+	  if (! strncmp (hash, "$1$", 3))
+	    {
+	      hashvalue = (char *)hash+3;
+	    }
+	  else
+	    {
+	      hashvalue = (char *)hash;
+	    }
+	  /* DES crypt or MD5 hash of the password */
+	  encrypted = crypt (password, hashvalue);
+	  return encrypted && ! strcmp (encrypted, hashvalue);
 #else
-      /* TODO: log some warning */
-      return FALSE;
+	  /* TODO: log some warning */
+	  return FALSE;
 #endif
+	}
+    }
+  else
+    {
+      if (strlen(password))
+	{
+	  return FALSE;
+	}
+      else
+	{
+	  return ! strlen(hash) ;
+	}
     }
 }
 
@@ -450,8 +474,11 @@ findUserAccessLevel (const char *file, c
 	      if (! password_match (passwd, ent->admFields[1]))
 		{
 		  /* Username matched but password didn't.  */
-		  *access = ACCESS_NONE;
-		  found = 1;
+		  if (strlen(ent->admFields[1]) && strlen(passwd))
+		    {
+		      *access = ACCESS_NONE;
+		      found = 1;
+		    }
 		}
 	      else
 		{
@@ -460,7 +487,10 @@ findUserAccessLevel (const char *file, c
 		      /* Compare all given names against the name of the
 			 requested database. */
 		      const char *l2 = ent->admFields[3];
-
+		      
+		      if (! strlen(l2))
+			found = 1;
+		      
 		      while (l2 != NULL && ! found)
 			{
 			  char *token = get_next_field (&l2, ',');