[Cvs-dev] Re: cvs-passwd patch

[Mark D. Baushke]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

P J P <address@hidden> writes:

> On Fri, 20 Oct 2006, Mark D. Baushke wrote:
> > I do not understand. If you have done a start_server(), then you have
> > already authenticated the current user. So, if the password to be
> > changed is for the current user, then there should be no conflict...
> > (well, in theory it is possible that the :ext: method might use a
> > username that is not the same as the username they were assigned for
> > :pserver: use in which case they are out of luck unless their :ext: user
> > is a cvs administrator).
> 
>    As I said earlier, if I/We skip this one step of authentication, it
> would become as easy for someone to change others password as slicing
> through butter. I really don't think we should leave this step.

Let us consider the states

  a) With client/server methods :pserver:

     The client-side already authenticated when the start_connection()
     was made, so if (server_active) { ... } you know that the client
     has already connected via the normal method and received an 'I LOVE
     YOU' response. If the user is only trying to change their own
     password, it should be no problem to just change it.

  b) With the client/server methods: :server: :ext: :kserver: :gserver:

     The client-side has already authenticated with the server host and
     is running with a user-credential of a particular real userid. If
     that is the user they are trying to change in the CVSROOT/passwd
     file, then there should be no problems. If they are trying to
     change an alias, then you have a problem in this case as the alias
     may not be the same as the real username on the server. However,
     let us consider if the user were to do a commit to the repository,
     what will the author be for the change? It will be the
     authenticated username. How is this different for the alias? It is
     not different. There is no way to distinguish between the two kinds
     of users. So, there is little harm in allowing the user with the
     more stringent authentication method in #b to change the less
     stringent password alias of the :pserver: method.

  c) With the :local: method.

     Like #b, the user must have logged into the cvs server to run this
     command and used some system level of authentication. Changing the
     password for the :pserver: method will not be as stringent a check
     as the CVSROOT/passwd authentication method, so if the username of
     the currently running process is either a real entry or an alias in
     the CVSROOT/passwd file, they should be allowed to change the
     password.

> > If you are a local user, then the username associated with the current
> > process is the one that you will be wanting to use in the CVSROOT/passwd
> > file.
> 
>    But, we can/should not assume that, the user will be a local system
> user. It might not be the case, always!

While this is true, there is nothing to stop such a user who is able to
commit changes to the repository locally from looking just like the
CVSROOT/passwd user. One would therefore hope that the system
administrator and the cvs administrator would have some way to keep a
sane understanding of global authorship of changes in the repository.

> >> -----------------------------x cut here x--------------------------------
> >> diff -Naur ccvs-2006-09-25/src/main.c ccvs-2006-09-25.new/src/main.c
> >> @@ -30,6 +30,7 @@
> >>   /* CVS Headers.  */
> >>   #include "command_line_opt.h"
> >>   #include "gpg.h"
> >> +#include "passwd.h"
> >>   #include "sign.h"
> >>   #include "verify.h"
> > MDB:
> > The other #include lines are left justified in the main.c I have and yet
> > the diff looks like it believes they are indended by a space. This is
> > weird.
> 
>    hmmn true! but, I didn't even touch that part lately. The only
> change I did was to command alias, "password", & "setpass".

It could be that the problem was in the mail user agent. You may want to
put change diffs up on the web and send URLs in your message as was done
for previous changes.

> >> +            current_parsed_root->password = xstrdup (key);
> >> +            connect_to_pserver (current_parsed_root, NULL, NULL, 1, 0);
> >
> > MDB:
> > If the current user is the same as the authenticated user, then this
> > step (connect_to_pserver) should not be required.
> 
>    Ah, please!! I'll still insist, you please, think over it, we
> should not skip this. Think, why UNIX `/usr/bin/passwd' needs to read
> your (current) UNIX password, when you try to change it.

I could be wrong, but I think you need to make the case for it.

The :pserver: is the weakest of all authentication methods available for
CVS to use. In all cases the system passwords are better and less likely
to be compromised than something which is being sent in the clear over
the network.

> > MDB:
> > You are not handling hte case of a user using a non-client/server
> > method. You may wish to revisit the CVSNT passwd.cpp file for control
> > flow.
> 
>    what? I'm afraid, I didn't get this!

cvs -d :local:/path/to/repository passwd

your code will not handle this case at all. That is, it will not try to
set the user's password.

> >> +#include <stdlib.h>
> >
> > MDB:
> > Why is this #include needed? (I am mostly just curious about what is
> > somehow missing if it is not present.)
> 
>    Yes right! it's not required now!!
> 
> Thank you!
> -- 
> regards
>     -P J P
> PS: Please don't send me html/attachment/Fwd mails

        -- Mark
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.4 (FreeBSD)

iD8DBQFFOJsYCg7APGsDnFERAjm8AKDonyA6tvrUzQ/4+MocykjyZ/yKzgCfUYAA
Zb2zNy0PTm4qjrY6+NS9aWo=
=yiEO
-----END PGP SIGNATURE-----