[Cvs-dev] Re: cvs-passwd patch

[Mark D. Baushke]

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

Prasad J Pandit <address@hidden> writes:

> On Mon, 25 Sep 2006, Mark D. Baushke wrote:
> > No, I tried to patch the top-of-tree and compile
> > that instead. The 1.12.13 code is too stale to be
> > useful to me.
> 
>    hmmn, fair enough. I think, I'm at quite an interesting point here.
> See, yesterday I saw my base source(for which I created that patch),
> and found that, the is_admin() and other changes which you pointed
> out, were just not there. Then I checked the top-of-tree source, which
> yesterday you gave me, and found that, the is_admin() function was
> added by Derek by about May 16'th 2006 or so, and I just didn't have
> those changes.

Yeah. The top-of-tree is the FEATURE branch and has had a fair amount of
churn recently. Right now I am syncing the GNULIB changes about once per
week on average as we try to get things stable on all of the platforms
for the 1.12.14 release. There is still a fair amount to do with the
windows-NT target and a few minor problems in some of the others as
well. Mostly the GNULIB changes should not impact you as your stuff
is a purely with the src and doc directories.

>   You know, It'll be really nice, if you could tell me about what
> practices you guys follow. I mean, do you check-out & check-in every
> morning, and evening, or every other day, or what?. And in my case,
> since I cann't do such check-out & check-ins, what should I do to
> avoid these things in future.

I'll try to remember to keep a copy of the source
tarball I gave you so that merging your patches
should be easier. In addition, if you are about to
do any serious additions to other parts of the
code, let me know and I'll spin a new source
tarball for you. I do understand that you are not
able to access the
:pserver:address@hidden/cvsroot/cvs
server and I am trying to help you out as needed.

If we can get your 'passwd' patch into the next
release, that would be nice. However, I don't know
what timeframe Derek will be looking to use. So,
if it does not make it for 1.12.14, it may still
be put into the tree shortly after 1.12.14 is
released.

Things to consider given what Tony said in his
last message, is that we apparently should make
certain that the current user is in the
CVSROOTADM_WRITERS file (CVSROOT/writers) before
allowing them to change a password.

If we look at the server.c check_command_valid_p()
function, we can see that it is already set to do
this for us if we only make the command table have
the correct entry.

So, YOU need to change your patch to main.c for this line:

    { "passwd",   "pw",       NULL,        passwd,    0 },

to this:

    { "passwd",   "pw",       NULL,        passwd,    
CVS_CMD_MODIFIES_REPOSITORY },

this will automagically prohibit non-writers from
hacking their password in the repository.

It is also the case that we may need to do
something to allow for password attempts that are
wrong to be properly logged for the cvs
administrator. It appears right now that you just
send the user an error() without any other logging
when they try to do something that they are not
allowed to do.

> > I live in a world where there are many different
> > versions of CVS clients and servers all trying
> > to interoperate amongst themselves.
> 
>    I understand your point. but that means(if
> I'm not wrong), I should create patch for all
> the versions available out there, isn't it? I
> don't know, how could/should I do that, let's
> see. :)

No, not at all, but I think you understood at
least that much.

However, you are providing two separable
components:

  1) a CVS client that should be able to properly
     speak the wire protocol to any existing CVS
     server and adapt your arguments to maximize
     the possibility of success.

  2) a CVS server that should be able to properly
     speak the wire protocol to any existing CVS
     client and to inform that client as to which
     features you support such that a CVSNT client
     or a CVS client would be able to interact
     properly with you.

I think this may be the fundamental problem with
the way you have been making your approach. I
suspect you keep thinking of this as if a single
client/server is all you are producing and that is
just not entirely the case.

So, given that a CVSNT server is going to
advertise via the 'passwd' command, if that is all
your client sees, it should only try to provide
that subset of command arguments that CVSNT expects.

If the CVS server adverstises that it understands
how to deal with an encrypted password, then so
much the better as your client will be able to
properly respond to it. You may also then use any
of the other forms that you wish to support as
long as a given REQ_LINE() entry has a documented
set of functionality (in the cvsclient.texi file
as well as the sources), everyone who wants to
change their own CVS clients to support this would
be able to do it. (Like the jCVS folks for
example.)

We are also including address@hidden on
this thread. At such time as we have a stable
addition to the CVS sources, he will probably
consider if picking up the changes to the CVSNT
server for a future release makes sense from their
piont of view. I also hope that he will speak up
if/when your proposed wire protocol responses to a
CVSNT server are not correct or interoperable so
that we do not cause more problems for the
CVS/CVSNT community than they already have...

Once again it is late and I fear my points may not
be well written and understandable. However, I hope
you get the general gist.

a) Be paranoid and validate all possible
command-line arguments at both sets of interfaces
(client and server) as the users are out to break
into your system using :pserver: and they will
focus on your new code just because it is new.

b) Try to make use of the existing sources where
possible rather than re-inventing the wheel.

c) Try to provide unit tests for both the client
and the server sides to do as much exercising of
them as is reasonable.

I hope the above makes sense...

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

iD8DBQFFGMt3Cg7APGsDnFERAiW+AJ4+WaK5XjaEoAgK2fr9OOV0tXVLEgCghLM4
mPVdHjQAhbPYNxfmk2ODKKY=
=B5aX
-----END PGP SIGNATURE-----