Re: Patch: Fix user authentication + MKDB

[Lars Henriksen]

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