* Patch: Fix user authentication + MKDB @ 2002-09-24 7:17 Pankaj K Garg 2002-09-25 8:25 ` Lars Henriksen 0 siblings, 1 reply; 18+ messages in thread From: Pankaj K Garg @ 2002-09-24 7:17 UTC (permalink / raw) To: help-gnats [-- Attachment #1: Type: text/plain, Size: 1184 bytes --] Please take a look at the following patch that fixes the following: 1) MKDB: creates all parent directories in case they did not exist. 2) MKDB: creates gnatsd.user_access instead of gnatsd.access 3) PASSWORD CHECKING: The password checking in the current CVS directory is broken. It was not working as someone else also recenlty noted on this list. The problems were: (a) it was using the opposite logic of match(), (b) it did not default to plain text passwords, (c) an empty database list was confusing it, and (d) there was no fall-through. The last point needs a bit of explanation: Suppose I had a entry like so: foo:test:edit: *:*:view: The desired behavior should be that in case a user fails the password check for 'foo' then he should be allowed to have a 'view' access as everybody else. The current code will default 'foo' with a bad password to 'no_access'. This should close BUG report number: 416 Pankaj P.S. Patch file created by `cvs -up gnatsd.c misc.c mkdb.sh > patchfile.out` --- Pankaj K Garg gargp@acm.org 1684 Nightingale Avenue 408-373-4027 Sunnyvale, CA 94304 http://home.earthlink.net/~gargp [-- Attachment #2: patchfile.out --] [-- Type: text/plain, Size: 3043 bytes --] 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 24 Sep 2002 00:04:50 -0000 @@ -256,9 +256,9 @@ password_match (const char *password, co if (! strncmp (hash, "$0$", 3)) { /* explicit plain-text password */ - return ! match (password, hash, TRUE); + return match (password, hash+3, TRUE); } - else + else if (! strncmp (hash, "$1", 3)) { /* DES crypt or MD5 hash of the password */ #ifdef HAVE_LIBCRYPT @@ -269,6 +269,9 @@ password_match (const char *password, co return FALSE; #endif } + else { + return match (password, hash, TRUE); + } } /* */ @@ -451,7 +454,6 @@ findUserAccessLevel (const char *file, c { /* Username matched but password didn't. */ *access = ACCESS_NONE; - found = 1; } else { @@ -460,7 +462,10 @@ findUserAccessLevel (const char *file, c /* Compare all given names against the name of the requested database. */ const char *l2 = ent->admFields[3]; - + + if (l2 == NULL) + found = 1; + while (l2 != NULL && ! found) { char *token = get_next_field (&l2, ','); Index: misc.c =================================================================== RCS file: /cvsroot/gnats/gnats/gnats/misc.c,v retrieving revision 1.36 diff -u -p -r1.36 misc.c --- misc.c 6 Jan 2002 16:13:20 -0000 1.36 +++ misc.c 24 Sep 2002 00:04:50 -0000 @@ -287,7 +287,10 @@ get_next_field (const char **line_ptr, i *line_ptr = NULL; } - return res; + if (end_line == line) + return NULL ; + else + return res; } /* Adds quote-marks (") around the string, and escapes any quotes that Index: mkdb.sh =================================================================== RCS file: /cvsroot/gnats/gnats/gnats/mkdb.sh,v retrieving revision 1.9 diff -u -p -r1.9 mkdb.sh --- mkdb.sh 4 Aug 2002 10:57:17 -0000 1.9 +++ mkdb.sh 24 Sep 2002 00:04:50 -0000 @@ -28,7 +28,7 @@ DATADIR=xSYSCONFDIRx/gnats/defaults LIBEXECDIR=xLIBEXECDIRx domkdir() { - mkdir "$1" || { echo "Can't create directory $1, exiting"; exit 1 ; } + mkdir -p "$1" || { echo "Can't create directory $1, exiting"; exit 1 ; } chown "${GNATS_USER}" "$1" } @@ -82,8 +82,8 @@ echo "Copying default files from ${DATAD docp categories "${dbdir}/gnats-adm/categories" docp submitters "${dbdir}/gnats-adm/submitters" docp responsible "${dbdir}/gnats-adm/responsible" -docp gnatsd.access "${dbdir}/gnats-adm/gnatsd.access" -chmod 600 "${dbdir}/gnats-adm/gnatsd.access" +docp gnatsd.access "${dbdir}/gnats-adm/gnatsd.user_access" +chmod 600 "${dbdir}/gnats-adm/gnatsd.user_access" docp addresses "${dbdir}/gnats-adm/addresses" docp states "${dbdir}/gnats-adm/states" docp classes "${dbdir}/gnats-adm/classes" ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: Patch: Fix user authentication + MKDB 2002-09-24 7:17 Patch: Fix user authentication + MKDB Pankaj K Garg @ 2002-09-25 8:25 ` Lars Henriksen 2002-09-25 8:25 ` Dirk Schenkewitz 2002-09-25 8:59 ` Pankaj K Garg 0 siblings, 2 replies; 18+ messages in thread From: Lars Henriksen @ 2002-09-25 8:25 UTC (permalink / raw) To: gargp; +Cc: help-gnats On Mon, Sep 23, 2002 at 05:49:21PM -0700, Pankaj K Garg wrote: > Please take a look at the following patch that fixes the > following: > > 1) MKDB: creates all parent directories in case they did > not exist. I'm not sure I agree. Let mkdb do what it's supposed to do: create the database, and let the gnats administrator do what he is supposed to do: establish the prerequisites for invoking mkdb. > 2) MKDB: creates gnatsd.user_access instead of gnatsd.access Agreed. > 3) PASSWORD CHECKING: The password checking in the current CVS > directory is broken. It was not working as someone else also > recenlty noted on this list. The problems were: (a) it was using > the opposite logic of match(), (b) it did not default to plain > text passwords, (c) an empty database list was confusing it, and > (d) there was no fall-through. I agree with (a) and (c), but not with (b); (d) should be considered. As for (b), the password checking behaves as described in version 4 of the gnats manual (Keeping Track), see section C.4. Yngve Svendsen put a lot of work into this and I believe it behaves as intended. There is no "default". You get the kind of password checking you ask for: plain text for passwords with a $0$ prefix, MD5 format for passwords with a $1$ prefix, and DES format for passwords without a prefix. Your example below will be interpreted as having traditional Unix DES-crypted passwords and will effectively be no-login entries. > The last point needs a bit of > explanation: > > Suppose I had a entry like so: > foo:test:edit: > *:*:view: Now for (d); I'll assume that gnats is kept as is with no "default" password checking. For the discussion I'll change your example to read foo:$0$test:edit: *:$0$*:view: > The desired behavior should be that in case a user fails the password > check for 'foo' then he should be allowed to have a 'view' access > as everybody else. The current code will default 'foo' with a bad > password to 'no_access'. This is new behavior, but worth consideration. As it is, the user access file is processed like the Unix password file: the first line with a matching user is used. But with an all-user entry (or several-user entries), it makes a lot of sense to continue processing. If adopted the behavior should of course be documented in the manual. What do others think? Lars Henriksen _______________________________________________ Help-gnats mailing list Help-gnats@gnu.org http://mail.gnu.org/mailman/listinfo/help-gnats ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: Patch: Fix user authentication + MKDB 2002-09-25 8:25 ` Lars Henriksen @ 2002-09-25 8:25 ` Dirk Schenkewitz 2002-09-25 8:43 ` Pankaj K Garg 2002-09-27 6:01 ` Pankaj K Garg 2002-09-25 8:59 ` Pankaj K Garg 1 sibling, 2 replies; 18+ messages in thread From: Dirk Schenkewitz @ 2002-09-25 8:25 UTC (permalink / raw) To: help-gnats Lars Henriksen wrote: > > On Mon, Sep 23, 2002 at 05:49:21PM -0700, Pankaj K Garg wrote: > ... > > 3) PASSWORD CHECKING: The password checking in the current CVS > > directory is broken. It was not working as someone else also > > recenlty noted on this list. The problems were: (a) it was using > > the opposite logic of match(), (b) it did not default to plain > > text passwords, (c) an empty database list was confusing it, and > > (d) there was no fall-through. > > I agree with (a) and (c), but not with (b); (d) should be considered. > > As for (b), the password checking behaves as described in version 4 of the > gnats manual (Keeping Track), see section C.4. Yngve Svendsen put a lot > of work into this and I believe it behaves as intended. There is no > "default". You get the kind of password checking you ask for: > > plain text for passwords with a $0$ prefix, > MD5 format for passwords with a $1$ prefix, and > DES format for passwords without a prefix. IMHO this is better than default=plaintext-passwords. > ... > Now for (d); I'll assume that gnats is kept as is with no "default" > password checking. For the discussion I'll change your example to read > > foo:$0$test:edit: > *:$0$*:view: > > > The desired behavior should be that in case a user fails the password > > check for 'foo' then he should be allowed to have a 'view' access > > as everybody else. The current code will default 'foo' with a bad > > password to 'no_access'. > > This is new behavior, but worth consideration. As it is, the user access > file is processed like the Unix password file: the first line with a > matching user is used. But with an all-user entry (or several-user entries), > it makes a lot of sense to continue processing. If adopted the behavior > should of course be documented in the manual. I believe, a fall-through/continued-processing would be a good thing. If you don't want anybody to read your database, you can have a password even for reading. But what if a user mis-types his password? If there is another possibility that fits him (without a proper password), he might be silently put in "anybody else"-status with read-only-access. That can be unwanted. Then again, if for "everybody" or "a group of users" read- only-access is desired, why not give them empty passwords ? As in: foo:$0$test:edit: *::view: Then the behavior IMHO should be: - user foo gives correct password --> 'edit' access - user foo gives wrong password --> no access - user foo gives no/empty password --> 'view' access - user bar gives any password --> no access - user bar gives no/empty password --> 'view' access Would that be possible ? > What do others think? :-) well, I'll repeat that :-) greetings dirk -- Dirk Schenkewitz InterFace AG fon: +49 (0)89 / 610 49 - 126 Leipziger Str. 16 fax: +49 (0)89 / 610 49 - 83 D-82008 Unterhaching http://www.interface-ag.de mailto:dirk.schenkewitz@interface-ag.de _______________________________________________ Help-gnats mailing list Help-gnats@gnu.org http://mail.gnu.org/mailman/listinfo/help-gnats ^ permalink raw reply [flat|nested] 18+ messages in thread
* RE: Patch: Fix user authentication + MKDB 2002-09-25 8:25 ` Dirk Schenkewitz @ 2002-09-25 8:43 ` Pankaj K Garg 2002-09-27 6:01 ` Pankaj K Garg 1 sibling, 0 replies; 18+ messages in thread From: Pankaj K Garg @ 2002-09-25 8:43 UTC (permalink / raw) To: 'Dirk Schenkewitz', help-gnats > > ... > > plain text for passwords with a $0$ prefix, > > MD5 format for passwords with a $1$ prefix, and > > DES format for passwords without a prefix. > > IMHO this is better than default=plaintext-passwords. Yes. I'll fix this in the patch tomorrow. > > ... > ... why not give them empty passwords ? As in: > > foo:$0$test:edit: > *::view: > > Then the behavior IMHO should be: > - user foo gives correct password --> 'edit' access > - user foo gives wrong password --> no access > - user foo gives no/empty password --> 'view' access > - user bar gives any password --> no access > - user bar gives no/empty password --> 'view' access > > Would that be possible ? I see your point: If the user mis-types his password, and we silently give them read access, they don't know what happened. I'm in favor of your solution--should be possible to implement. Regards Pankaj _______________________________________________ Help-gnats mailing list Help-gnats@gnu.org http://mail.gnu.org/mailman/listinfo/help-gnats ^ permalink raw reply [flat|nested] 18+ messages in thread
* RE: Patch: Fix user authentication + MKDB 2002-09-25 8:25 ` Dirk Schenkewitz 2002-09-25 8:43 ` Pankaj K Garg @ 2002-09-27 6:01 ` Pankaj K Garg 2002-09-27 10:06 ` Yngve Svendsen 2002-09-27 11:40 ` Lars Henriksen 1 sibling, 2 replies; 18+ messages in thread From: Pankaj K Garg @ 2002-09-27 6:01 UTC (permalink / raw) To: 'Dirk Schenkewitz', help-gnats [-- Attachment #1: Type: text/plain, Size: 1486 bytes --] > ... > foo:$0$test:edit: > *::view: > > Then the behavior IMHO should be: > - user foo gives correct password --> 'edit' access > - user foo gives wrong password --> no access > - user foo gives no/empty password --> 'view' access > - user bar gives any password --> no access > - user bar gives no/empty password --> 'view' access > > Would that be possible ? > ... I'm attaching a patch "gnatspatch.out" with this message that does this. Seems to work on my small tries here, but would appreciate if someone else can take a look at it also. The empty password bit required a change (backwards compatible) to the protocol. Previously, the client was required to give: USER <name> <password> CHDB <db> <name> <password> Now, clients can give: USER <name> [<password>] CHDB <db> <name> [<password>] Hence, the password is optional and will be assumed to be empty. For clients to take advantage of this, however, they may need some modification. For example, gnatsweb assumes that the user always requires some value for the password. So, I'm also attaching a patch for gnatsweb ("gnatswebpatch.out") that takes care of empty password situation. If the gnats gurus will accept the gnats patch, then I can submit the gnatsweb patch to the appropriate mailing list. BTW, the documentation in gnatsd.access states that $1$ implies use of MD5. In the code I did not find the use of MD5 hashes... am I missing something here? Regards Pankaj [-- Attachment #2: gnatspatch.out --] [-- Type: text/plain, Size: 6628 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 26 Sep 2002 22:19:02 -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; } @@ -680,6 +691,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) 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 26 Sep 2002 22:19:02 -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 26 Sep 2002 22:19:03 -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)) - { - /* explicit plain-text password */ - return ! match (password, hash, TRUE); - } - else + char *hashvalue, *encrypted; + + if (strlen(password) && hash) { - /* 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); + 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 ! hash ; + } } } @@ -437,10 +461,10 @@ findUserAccessLevel (const char *file, c while (! found) { char *buffer = read_line (acc, NULL); + if (buffer != NULL) { AdmEntry *ent = build_adm_entry (buffer, InvalidFieldIndex); - free (buffer); /* Is there a mistake on the setup of this line? (must have 3 or 4 fields) */ @@ -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 (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 (l2 == NULL) + found = 1; + while (l2 != NULL && ! found) { char *token = get_next_field (&l2, ','); Index: misc.c =================================================================== RCS file: /cvsroot/gnats/gnats/gnats/misc.c,v retrieving revision 1.36 diff -u -p -r1.36 misc.c --- misc.c 6 Jan 2002 16:13:20 -0000 1.36 +++ misc.c 26 Sep 2002 22:19:03 -0000 @@ -287,7 +287,10 @@ get_next_field (const char **line_ptr, i *line_ptr = NULL; } - return res; + if (end_line == line) + return NULL ; + else + return res; } /* Adds quote-marks (") around the string, and escapes any quotes that [-- Attachment #3: gnatswebpatch.out --] [-- Type: text/plain, Size: 1226 bytes --] Index: gnatsweb.pl =================================================================== RCS file: /cvsroot/gnatsweb/gnatsweb/gnatsweb.pl,v retrieving revision 1.101 diff -u -p -r1.101 gnatsweb.pl --- gnatsweb.pl 24 Sep 2002 20:39:17 -0000 1.101 +++ gnatsweb.pl 26 Sep 2002 22:20:26 -0000 @@ -4079,7 +4079,13 @@ sub init_prefs } %db_prefs = (); set_pref('user', \%db_prefs, \%cvals); - set_pref('password', \%db_prefs, \%cvals); + if (defined $q->param('password')) { + $db_prefs{'password'} = $q->param('password'); + } + else + { + set_pref('password', \%db_prefs, \%cvals); + } # Debug. warn "global_prefs = ", Dumper(\%global_prefs) if $debug; @@ -4189,10 +4195,9 @@ sub main ### Cookie-related code must happen before we print the HTML header. init_prefs(); - if(!$global_prefs{'database'} - || !$db_prefs{'user'} || !$db_prefs{'password'}) + if(!$global_prefs{'database'} || !$db_prefs{'user'}) { - # We don't have username/password/database; give login page then + # We don't have username/database; give login page then # redirect to the url they really want (self_url). print_header(); login_page($q->self_url()); ^ permalink raw reply [flat|nested] 18+ messages in thread
* RE: Patch: Fix user authentication + MKDB 2002-09-27 6:01 ` Pankaj K Garg @ 2002-09-27 10:06 ` Yngve Svendsen 2002-09-27 11:40 ` Lars Henriksen 1 sibling, 0 replies; 18+ messages in thread From: Yngve Svendsen @ 2002-09-27 10:06 UTC (permalink / raw) To: gargp, 'Dirk Schenkewitz', help-gnats At 16:16 26.09.2002 -0700, Pankaj K Garg wrote: >I'm attaching a patch "gnatspatch.out" with this message that >does this. Seems to work on my small tries here, but would >appreciate if someone else can take a look at it also. > >The empty password bit required a change (backwards compatible) >to the protocol. Previously, the client was required to >give: > > USER <name> <password> > CHDB <db> <name> <password> > >Now, clients can give: > > USER <name> [<password>] > CHDB <db> <name> [<password>] > >Hence, the password is optional and will be assumed to be >empty. > >For clients to take advantage of this, however, they may >need some modification. For example, gnatsweb assumes that >the user always requires some value for the password. >So, I'm also attaching a patch for gnatsweb ("gnatswebpatch.out") >that takes care of empty password situation. > >If the gnats gurus will accept the gnats patch, then I can >submit the gnatsweb patch to the appropriate mailing list. You don't need to. If we decide to apply the patch to GNATS, I will apply the necessary changes to Gnatsweb as well. I think the behaviour this patch would introduce is sensible, but I will wait for people's comments before possibly applying it. Be aware that I am not a seasoned C coder, so if someone would take a look at the tecnicalities of the patch, that would be excellent. Yngve Svendsen Gnatsweb maintainer _______________________________________________ Help-gnats mailing list Help-gnats@gnu.org http://mail.gnu.org/mailman/listinfo/help-gnats ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: Patch: Fix user authentication + MKDB 2002-09-27 6:01 ` Pankaj K Garg 2002-09-27 10:06 ` Yngve Svendsen @ 2002-09-27 11:40 ` Lars Henriksen 2002-09-27 14:28 ` Pankaj K Garg 1 sibling, 1 reply; 18+ messages in thread From: Lars Henriksen @ 2002-09-27 11:40 UTC (permalink / raw) To: gargp; +Cc: 'Dirk Schenkewitz', help-gnats On Thu, Sep 26, 2002 at 04:16:53PM -0700, Pankaj K Garg wrote: > > ... > > foo:$0$test:edit: > > *::view: > > > > Then the behavior IMHO should be: > > - user foo gives correct password --> 'edit' access > > - user foo gives wrong password --> no access > > - user foo gives no/empty password --> 'view' access > > - user bar gives any password --> no access > > - user bar gives no/empty password --> 'view' access Looks OK to me. > I'm attaching a patch "gnatspatch.out" with this message that > does this. Seems to work on my small tries here, but would > appreciate if someone else can take a look at it also. ... > BTW, the documentation in gnatsd.access states that $1$ implies > use of MD5. In the code I did not find the use of MD5 hashes... > am I missing something here? As I understand it, MD5 password encryption is built into some versions of libcrypt, again see the "Keep Track" manual (section C.4), it's not bad :-) My system doesn't support MD5 (HAVE_LIBCRYPT undefined) so your patch wouldn't compile. That was easily fixed by moving things around: *** gnatsd.c.patch 2002-09-27 14:16:49.000000000 +0200 --- gnatsd.c 2002-09-27 07:50:09.000000000 +0200 *************** *** 253,260 **** static int password_match (const char *password, const char *hash) { - char *hashvalue, *encrypted; - if (strlen(password) && hash) { if (! strncmp (hash, "$0$", 3)) --- 253,258 ---- *************** *** 265,270 **** --- 263,270 ---- else { #ifdef HAVE_LIBCRYPT + char *hashvalue, *encrypted; + if (! strncmp (hash, "$1$", 3)) { hashvalue = (char *)hash+3; Then from the command line: 130$ ./gnatsd -n 200 cluster2.netman.dk GNATS server 4.0-beta1 ready. user lh Segmentation fault (core dumped) 131$ dbx gnatsd core dbx version 5.1 Type 'help' for help. Core file created by program "gnatsd" 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) As an 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). 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). Lars Henriksen _______________________________________________ Help-gnats mailing list Help-gnats@gnu.org http://mail.gnu.org/mailman/listinfo/help-gnats ^ permalink raw reply [flat|nested] 18+ messages in thread
* RE: Patch: Fix user authentication + MKDB 2002-09-27 11:40 ` Lars Henriksen @ 2002-09-27 14:28 ` Pankaj K Garg 2002-09-29 2:43 ` Lars Henriksen 0 siblings, 1 reply; 18+ messages in thread From: Pankaj K Garg @ 2002-09-27 14:28 UTC (permalink / raw) To: 'Lars Henriksen', gargp; +Cc: 'Dirk Schenkewitz', help-gnats [-- 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, ','); ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: Patch: Fix user authentication + MKDB 2002-09-27 14:28 ` Pankaj K Garg @ 2002-09-29 2:43 ` Lars Henriksen 2002-09-29 12:06 ` Pankaj K Garg 0 siblings, 1 reply; 18+ messages in thread From: Lars Henriksen @ 2002-09-29 2:43 UTC (permalink / raw) To: gargp; +Cc: 'Dirk Schenkewitz', help-gnats On Fri, Sep 27, 2002 at 10:37:04AM -0700, Pankaj K Garg wrote: > 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. Yes, that helped. As you already know I'm only able to test plaintext and no password. They seem to work as agreed. I've tested with two databases, with users present in or absent from the responsible file, with and without password and with the site and database specific gnatsd.user_access files. I've spotted one difference in behaviour: gnatsd no longer seems to remember the user when you switch between databases (it still remembers the database when you switch user). In the following sessions there are two databases, both with anybody/any password edit access specified in the database specific user access file; the site access level is listdb. Two sessions follow. First the existing gnatsd: 15$ ./gnatsd -n 200 cluster2.netman.dk GNATS server 4.0-beta1 ready. chdb development x y 210-Now accessing GNATS database 'development' 210 User access level set to 'edit' chdb support 210-Now accessing GNATS database 'support' 210 User access level set to 'edit' quit 201 Later. Then the patched version: 17$ ./gnatsd -n 200 cluster2.netman.dk GNATS server 4.0-beta1 ready. chdb development x y 210-Now accessing GNATS database 'development' 210 User access level set to 'edit' chdb support 210-Now accessing GNATS database 'support' 210 User access level set to 'listdb' user x y 210-Now accessing GNATS database 'support' 210 User access level set to 'edit' quit 201 Later. Regards Lars _______________________________________________ Help-gnats mailing list Help-gnats@gnu.org http://mail.gnu.org/mailman/listinfo/help-gnats ^ permalink raw reply [flat|nested] 18+ messages in thread
* RE: Patch: Fix user authentication + MKDB 2002-09-29 2:43 ` Lars Henriksen @ 2002-09-29 12:06 ` Pankaj K Garg 2002-09-29 20:31 ` Lars Henriksen 0 siblings, 1 reply; 18+ messages in thread From: Pankaj K Garg @ 2002-09-29 12:06 UTC (permalink / raw) To: 'Lars Henriksen', gargp; +Cc: 'Dirk Schenkewitz', help-gnats [-- Attachment #1: Type: text/plain, Size: 557 bytes --] >... > I've spotted one difference in behaviour: gnatsd no longer seems to > remember the user when you switch between databases (it still > remembers > the database when you switch user). In the following sessions there > are two databases, both with anybody/any password edit access > specified in the database specific user access file; the site access > level is listdb. OOpss...yes, this should not have happened. There was a problem with the way I was handling NULL usernames and passwords. Should be fixed in the attached patch. Regards Pankaj [-- Attachment #2: patchfile.out --] [-- Type: text/plain, Size: 6491 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 21:22:25 -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); } } @@ -591,15 +598,19 @@ gnatsdChdb (const char *nameOfDb, const free (currentUsername); } currentUsername = xstrdup (username); - } - - if (passwd != NULL) - { + if (currentPassword != NULL) { free (currentPassword); } - currentPassword = xstrdup (passwd); + if (passwd != NULL) + { + 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 21:22:25 -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 21:22:25 -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, ','); ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: Patch: Fix user authentication + MKDB 2002-09-29 12:06 ` Pankaj K Garg @ 2002-09-29 20:31 ` Lars Henriksen 2002-09-30 3:21 ` Pankaj K Garg 0 siblings, 1 reply; 18+ messages in thread From: Lars Henriksen @ 2002-09-29 20:31 UTC (permalink / raw) To: gargp; +Cc: 'Dirk Schenkewitz', help-gnats On Fri, Sep 27, 2002 at 02:59:28PM -0700, Pankaj K Garg wrote: > >... > > I've spotted one difference in behaviour: gnatsd no longer seems to > > remember the user when you switch between databases (it still > > remembers > > OOpss...yes, this should not have happened. There was a problem with > the way I was handling NULL usernames and passwords. > > Should be fixed in the attached patch. It is! I believe your patch is OK now as far as plaintext/no password is concerned. Apart from making the password checking work, this is a convenient improvement. But as for DES/MD5 I believe the original code is correct: else { /* DES crypt or MD5 hash of the password */ #ifdef HAVE_LIBCRYPT char *encrypted = crypt (password, hash); return encrypted && ! strcmp (encrypted, hash); #else /* TODO: log some warning */ return FALSE; #endif } It is for crypt() to decide the form of password encryption based on the contents of hash: if hash begins with $1$ it will use MD5, if it doesn't begin with $<digit>$ it will use DES. The return value of crypt() is similarly adjusted with a starting $1$ for MD5. This assumes an MD5-supporting crypt(3) (e.g. FreeBSD or GNU). With a traditional Unix crypt() function you will of course get DES encryption. Regards Lars Henriksen _______________________________________________ Help-gnats mailing list Help-gnats@gnu.org http://mail.gnu.org/mailman/listinfo/help-gnats ^ permalink raw reply [flat|nested] 18+ messages in thread
* RE: Patch: Fix user authentication + MKDB 2002-09-29 20:31 ` Lars Henriksen @ 2002-09-30 3:21 ` Pankaj K Garg 2002-10-03 21:27 ` Lars Henriksen 2002-10-04 0:01 ` Lars Henriksen 0 siblings, 2 replies; 18+ messages in thread From: Pankaj K Garg @ 2002-09-30 3:21 UTC (permalink / raw) To: 'Lars Henriksen', gargp; +Cc: 'Dirk Schenkewitz', help-gnats [-- Attachment #1: Type: text/plain, Size: 743 bytes --] >... > But as for DES/MD5 I believe the original code is correct: > > else > { > /* DES crypt or MD5 hash of the password */ > #ifdef HAVE_LIBCRYPT > char *encrypted = crypt (password, hash); > return encrypted && ! strcmp (encrypted, hash); > #else > /* TODO: log some warning */ > return FALSE; > #endif > } > ... Thanks, yes. I stand corrected. I was thrown off by the man page for crypt on my Linux machine. But on a bit of searching found out that FreeBSD crypt will do this. BTW, the documentation with FreeBSD also mentioned the use of $2$ for BlowFish algorithm. Should we mention that in the documentation? I'm attaching a corrected patch with the original DES/MD5 code. Regards Pankaj [-- Attachment #2: patchfile.out --] [-- Type: text/plain, Size: 6594 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 30 Sep 2002 03:22:27 -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); } } @@ -591,15 +598,19 @@ gnatsdChdb (const char *nameOfDb, const free (currentUsername); } currentUsername = xstrdup (username); - } - - if (passwd != NULL) - { + if (currentPassword != NULL) { free (currentPassword); } - currentPassword = xstrdup (passwd); + if (passwd != NULL) + { + 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 30 Sep 2002 03:22:27 -0000 @@ -14,9 +14,11 @@ # * userid: a user id to gain access to gnatsd # * password: a password for the user. Passwords prefixed by $0$ are # assumed to be plain-text. Passwords without a prefix are -# assumed to be encrypted with standard crypt(), while passwords +# assumed to be encrypted with standard (DES) 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 30 Sep 2002 03:22:27 -0000 @@ -253,21 +253,35 @@ 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); + /* DES crypt or MD5 hash of the password */ + char *encrypted = crypt (password, hash); + return encrypted && ! strcmp (encrypted, hash); #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 +464,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 +477,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, ','); ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: Patch: Fix user authentication + MKDB 2002-09-30 3:21 ` Pankaj K Garg @ 2002-10-03 21:27 ` Lars Henriksen 2002-10-04 0:01 ` Lars Henriksen 1 sibling, 0 replies; 18+ messages in thread From: Lars Henriksen @ 2002-10-03 21:27 UTC (permalink / raw) To: gargp; +Cc: 'Dirk Schenkewitz', help-gnats I've mailed this once before, but it never reached the list. On Sun, Sep 29, 2002 at 09:01:58PM -0700, Pankaj K Garg wrote: > > BTW, the documentation with FreeBSD > also mentioned the use of $2$ for BlowFish algorithm. Should we mention > that in the documentation? I think so. Or maybe a general remark that gnats supports whatever algorithm the local crypt() supports, e.g. MD5 and BlowFish on FreeBSD. > I'm attaching a corrected patch with the original DES/MD5 code. Thanks. I believe this version is OK and may be comitted. Regards Lars Henriksen _______________________________________________ Help-gnats mailing list Help-gnats@gnu.org http://mail.gnu.org/mailman/listinfo/help-gnats ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: Patch: Fix user authentication + MKDB 2002-09-30 3:21 ` Pankaj K Garg 2002-10-03 21:27 ` Lars Henriksen @ 2002-10-04 0:01 ` Lars Henriksen 2002-10-04 10:25 ` Pankaj K Garg 1 sibling, 1 reply; 18+ messages in thread From: Lars Henriksen @ 2002-10-04 0:01 UTC (permalink / raw) To: gargp; +Cc: 'Dirk Schenkewitz', help-gnats On Sun, Sep 29, 2002 at 09:01:58PM -0700, Pankaj K Garg wrote: > > BTW, the documentation with FreeBSD > also mentioned the use of $2$ for BlowFish algorithm. Should we mention > that in the documentation? I think so. Or maybe a general remark that gnats supports whatever algorithm the local crypt() supports, e.g. MD5 and BlowFish on FreeBSD. > I'm attaching a corrected patch with the original DES/MD5 code. Thanks. I believe this version is OK and may be comitted. Regards Lars Henriksen _______________________________________________ Help-gnats mailing list Help-gnats@gnu.org http://mail.gnu.org/mailman/listinfo/help-gnats ^ permalink raw reply [flat|nested] 18+ messages in thread
* RE: Patch: Fix user authentication + MKDB 2002-10-04 0:01 ` Lars Henriksen @ 2002-10-04 10:25 ` Pankaj K Garg 2002-10-04 11:41 ` Yngve Svendsen 0 siblings, 1 reply; 18+ messages in thread From: Pankaj K Garg @ 2002-10-04 10:25 UTC (permalink / raw) To: 'Lars Henriksen', gargp; +Cc: 'Dirk Schenkewitz', help-gnats [-- Attachment #1: Type: text/plain, Size: 406 bytes --] > > Thanks. I believe this version is OK and may be comitted. > Thanks. Here's the final version of the patch for both Gnats and Gnatsweb. I've taken care of the appropriate documentation for Gnats also. Can someone please commit these patches. Regards Pankaj --- Pankaj K Garg gargp@acm.org 1684 Nightingale Avenue 408-373-4027 Sunnyvale, CA 94304 http://home.earthlink.net/~gargp [-- Attachment #2: gnatsweb.patch --] [-- Type: application/octet-stream, Size: 1225 bytes --] Index: gnatsweb.pl =================================================================== RCS file: /cvsroot/gnatsweb/gnatsweb/gnatsweb.pl,v retrieving revision 1.101 diff -u -p -r1.101 gnatsweb.pl --- gnatsweb.pl 24 Sep 2002 20:39:17 -0000 1.101 +++ gnatsweb.pl 4 Oct 2002 17:06:30 -0000 @@ -4079,7 +4079,13 @@ sub init_prefs } %db_prefs = (); set_pref('user', \%db_prefs, \%cvals); - set_pref('password', \%db_prefs, \%cvals); + if (defined $q->param('password')) { + $db_prefs{'password'} = $q->param('password'); + } + else + { + set_pref('password', \%db_prefs, \%cvals); + } # Debug. warn "global_prefs = ", Dumper(\%global_prefs) if $debug; @@ -4189,10 +4195,9 @@ sub main ### Cookie-related code must happen before we print the HTML header. init_prefs(); - if(!$global_prefs{'database'} - || !$db_prefs{'user'} || !$db_prefs{'password'}) + if(!$global_prefs{'database'} || !$db_prefs{'user'}) { - # We don't have username/password/database; give login page then + # We don't have username/database; give login page then # redirect to the url they really want (self_url). print_header(); login_page($q->self_url()); [-- Attachment #3: gnats.patch --] [-- Type: application/octet-stream, Size: 8481 bytes --] Index: doc/gnats.texi =================================================================== RCS file: /cvsroot/gnats/gnats/doc/gnats.texi,v retrieving revision 1.26 diff -u -p -r1.26 gnats.texi --- doc/gnats.texi 14 Jul 2002 17:27:47 -0000 1.26 +++ doc/gnats.texi 4 Oct 2002 17:04:55 -0000 @@ -1704,7 +1704,8 @@ OpenBSD.}. If the password is in plain text format, it must be prefixed by @samp{$0$} and if it is in MD5 format, it needs to be prefixed by the string @samp{$1$}. Passwords encrypted by @code{crypt()} should have no -prefix. +prefix. If no password is given then users can login with an empty +password string. A @code{gnats-passwd} tool to manage @file{gnatsd.user_access} files is planned. In the meantime, @code{crypt()} passwords can be generated by @@ -1739,20 +1740,20 @@ plain text passwords: @example rickm:$0$ruckm:edit pablo:$0$pueblo:view -*:*:none +*::none @end example @noindent And this is the same file with MD5-encrypted passwords: @example rickm:$1$92388613$D7ZIYikzTUqd./dODTFrI.:edit pablo:$1$92388652$QRfAhIBG5elT.FQjQKhj80:view -*:*:none +*::none @end example @noindent In these examples, anybody other than rickm and pablo get denied access, assuming that the host access level is also @code{none}. You could set -the catch-all rule at the end to be @code{*:*:view} to allow view access -to anyone. +the catch-all rule at the end to be @code{*::view} to allow view access +to anyone who does not supply a password. The overall user access file @w{@file{@var{SYSCONFDIR}/gnats/gnatsd.user_access}}, usually Index: gnats/cmds.c =================================================================== RCS file: /cvsroot/gnats/gnats/gnats/cmds.c,v retrieving revision 1.69 diff -u -p -r1.69 cmds.c --- gnats/cmds.c 12 Aug 2002 12:33:30 -0000 1.69 +++ gnats/cmds.c 4 Oct 2002 17:04:56 -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); } } @@ -591,15 +598,19 @@ gnatsdChdb (const char *nameOfDb, const free (currentUsername); } currentUsername = xstrdup (username); - } - - if (passwd != NULL) - { + if (currentPassword != NULL) { free (currentPassword); } - currentPassword = xstrdup (passwd); + if (passwd != NULL) + { + 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: gnats/gnatsd.access =================================================================== RCS file: /cvsroot/gnats/gnats/gnats/gnatsd.access,v retrieving revision 1.5 diff -u -p -r1.5 gnatsd.access --- gnats/gnatsd.access 16 Oct 2001 15:06:56 -0000 1.5 +++ gnats/gnatsd.access 4 Oct 2002 17:04:57 -0000 @@ -13,10 +13,12 @@ # # * userid: a user id to gain access to gnatsd # * password: a password for the user. Passwords prefixed by $0$ are -# assumed to be plain-text. Passwords without a prefix are -# 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. +# assumed to be plain-text. Passwords without a $0$ prefix are +# assumed to be encrypted with standard crypt(). For example, on +# FreeBSD systems, no prefix implies DES encryption, a prefix of $1$ +# implies MD5 encryption, while $2$ prefix implies Blowfish encryption. +# 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: gnats/gnatsd.c =================================================================== RCS file: /cvsroot/gnats/gnats/gnats/gnatsd.c,v retrieving revision 1.47 diff -u -p -r1.47 gnatsd.c --- gnats/gnatsd.c 4 Aug 2002 10:58:29 -0000 1.47 +++ gnats/gnatsd.c 4 Oct 2002 17:04:57 -0000 @@ -253,21 +253,35 @@ 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); + /* DES crypt or MD5 hash of the password */ + char *encrypted = crypt (password, hash); + return encrypted && ! strcmp (encrypted, hash); #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 +464,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 +477,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, ','); ^ permalink raw reply [flat|nested] 18+ messages in thread
* RE: Patch: Fix user authentication + MKDB 2002-10-04 10:25 ` Pankaj K Garg @ 2002-10-04 11:41 ` Yngve Svendsen 0 siblings, 0 replies; 18+ messages in thread From: Yngve Svendsen @ 2002-10-04 11:41 UTC (permalink / raw) To: gargp, 'Lars Henriksen', gargp Cc: 'Dirk Schenkewitz', help-gnats At 10:45 04.10.2002 -0700, Pankaj K Garg wrote: > > > > Thanks. I believe this version is OK and may be comitted. > > > >Thanks. Here's the final version of the patch for both >Gnats and Gnatsweb. > >I've taken care of the appropriate documentation for >Gnats also. > >Can someone please commit these patches. I hope to be able to take a good look at it next week, if no one else with commit rights wants to do it before that. - Yngve _______________________________________________ Help-gnats mailing list Help-gnats@gnu.org http://mail.gnu.org/mailman/listinfo/help-gnats ^ permalink raw reply [flat|nested] 18+ messages in thread
* RE: Patch: Fix user authentication + MKDB 2002-09-25 8:25 ` Lars Henriksen 2002-09-25 8:25 ` Dirk Schenkewitz @ 2002-09-25 8:59 ` Pankaj K Garg 2002-09-26 6:38 ` Lars Henriksen 1 sibling, 1 reply; 18+ messages in thread From: Pankaj K Garg @ 2002-09-25 8:59 UTC (permalink / raw) To: 'Lars Henriksen', gargp; +Cc: help-gnats > > 1) MKDB: creates all parent directories in case they did > > not exist. > I'm not sure I agree. Let mkdb do what it's supposed to do: create > the database, and let the gnats administrator do what he is supposed > to do: establish the prerequisites for invoking mkdb. I used this to fix the problem of the 'com' directory not being there for the creation of the default database. Another fix would be to create the <prefix>/com directory as part of the 'make install'...I can do that if people prefer that. This seemed like a more general fix, as why do things in two steps when you can do it in one? > > 3) PASSWORD CHECKING: The password checking in the current CVS > > directory is broken. It was not working as someone else also > > recenlty noted on this list. The problems were: (a) it was using > > the opposite logic of match(), (b) it did not default to plain > > text passwords, (c) an empty database list was confusing it, and > > (d) there was no fall-through. > > I agree with (a) and (c), but not with (b); (d) should be considered. > > As for (b), the password checking behaves as described in > version 4 of the > gnats manual (Keeping Track), see section C.4. Yngve Svendsen > put a lot > of work into this and I believe it behaves as intended. There is no > "default". You get the kind of password checking you ask for: > > plain text for passwords with a $0$ prefix, > MD5 format for passwords with a $1$ prefix, and > DES format for passwords without a prefix. > > Your example below will be interpreted as having traditional Unix > DES-crypted passwords and will effectively be no-login entries. OK, sorry I did not take a look at the manual:-) I was tripped off by the default line '#*:*:view' in the 'gnatsd.user_access' file and thought that the default behavior was for plain-text passwords. So,this requires fixing in my patch. I'll redo it such that it defaults to DES instead of plain-text. Regards Pankaj _______________________________________________ Help-gnats mailing list Help-gnats@gnu.org http://mail.gnu.org/mailman/listinfo/help-gnats ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: Patch: Fix user authentication + MKDB 2002-09-25 8:59 ` Pankaj K Garg @ 2002-09-26 6:38 ` Lars Henriksen 0 siblings, 0 replies; 18+ messages in thread From: Lars Henriksen @ 2002-09-26 6:38 UTC (permalink / raw) To: gargp; +Cc: help-gnats On Wed, Sep 25, 2002 at 08:56:07AM -0700, Pankaj K Garg wrote: > > > 1) MKDB: creates all parent directories in case they did > > > not exist. > > I used this to fix the problem of the 'com' directory not being there > for the creation of the default database. Another fix would be to create > the <prefix>/com directory as part of the 'make install'...I can do that > if people prefer that. This seemed like a more general fix, as why do > things in two steps when you can do it in one? Do you perform gnats administration as root or as the gnats user? I prefer doing it as the gnats user, but I believe others do it as root. And it's just that I don't like the thought of invoking "mkdir -p" as root. Anyway, this isn't very important and your patch has already been committed. > OK, sorry I did not take a look at the manual:-) I was tripped off > by the default line '#*:*:view' in the 'gnatsd.user_access' file and > thought that the default behavior was for plain-text passwords. Yes, that IS misleading and should be changed to '#*:$0$*:view'. It would be nice if you included a patch for that as well ;-) Lars Henriksen _______________________________________________ Help-gnats mailing list Help-gnats@gnu.org http://mail.gnu.org/mailman/listinfo/help-gnats ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2002-10-04 18:41 UTC | newest] Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2002-09-24 7:17 Patch: Fix user authentication + MKDB Pankaj K Garg 2002-09-25 8:25 ` Lars Henriksen 2002-09-25 8:25 ` Dirk Schenkewitz 2002-09-25 8:43 ` Pankaj K Garg 2002-09-27 6:01 ` Pankaj K Garg 2002-09-27 10:06 ` Yngve Svendsen 2002-09-27 11:40 ` Lars Henriksen 2002-09-27 14:28 ` Pankaj K Garg 2002-09-29 2:43 ` Lars Henriksen 2002-09-29 12:06 ` Pankaj K Garg 2002-09-29 20:31 ` Lars Henriksen 2002-09-30 3:21 ` Pankaj K Garg 2002-10-03 21:27 ` Lars Henriksen 2002-10-04 0:01 ` Lars Henriksen 2002-10-04 10:25 ` Pankaj K Garg 2002-10-04 11:41 ` Yngve Svendsen 2002-09-25 8:59 ` Pankaj K Garg 2002-09-26 6:38 ` Lars Henriksen
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).