[Cvs-dev] Re: cvs-passwd patch

[Mark D. Baushke]

P J P <address@hidden> writes:

> On Mon, 16 Oct 2006, Mark D. Baushke wrote:
> > Well, I would rather not put in a half formulated feature.
> >
> > I really do wish for reasonable things to happen when 'cvs passwd'
> > is given as the command regardless of method being used:
> >
> >   :local:/path/to/repository
> >   :ext:host.name/path/to/repository
> >   :fork:/path/to/repository
> >   :pserver:host.name/path/to/repository
> 
> 
>    I understand your concern! Also, I'm more than keen to do it. But,
> I don't know, I suppose, that's a lot of work, which means, a lot of
> time as well. Considering, I've never used any of them except the
> :pserver:.

Hmmm... yes. An example is worth a lot. Another path you could consider
would be to port the CVSNT passwd.cpp from C++ to C and extend it with
your own -e addition. If you do that, then you would also need to adjust
the copyright slightly. However, the GPL in the CVS 1.4 kit was the same
heritage as what we are using in CVS today, so there are no license
problems.

Some changes in functional style would also be needed. For example, CVS
uses to Xasprintf() instead of xmalloc+strcpy+strcat usage. In general,
CVS tries to avoid using fixed-length buffers for anything which is part
of the GNU coding standards, so you would use getpass() rather than the
lib.PromptForPassword("string: ",variable,sizeof(variable)) method that
CVSNT uses...

Still, I think you understand the mechanical formatting stuff by now.

>    Could you please tell me how & where should I start, to make it
> support the other methods. Let's start from :local: ...

Honestly, you will learn a lot by downloading the CVSNT sources from
cvsnt.org and looking at src/passwd.cpp ... I'll even include a copy
of the top-of-tree passwd.cpp as an example for you.

> > Among these will be the desire to avoid use of
> >
> >    connect_to_pserver (current_parsed_root, NULL, NULL, 1, 0);
> >
> > to validate the current password. Either they are connected to the
> > repository by having done a 'cvs login' or they are authenticated as a
> > regular user via the appropriate authentication method for their current
> > username.
> 
>     Yes, true! but, how do I know, if user has done 'cvs login' or
> not. I think, the above call is meant to validate that only. And I saw
> such use of it, somewhere in cvs code it self.

Sure. You see/saw it in login.c which is where the 'cvs login' command
is validated before updating the local .cvspass file.

However, any other command would have already validated the user via the
'start_server ();' call you made to even be able to ask if the server
supported the "passwd" request or not.

All you need to do to get the current password is fetch if from the
.cvspass file and descramble it, but you really do not even need to
do that unless you want to give an error about changing the password
to itself or something like that.

> > getline() which is used by getpass() will return an arbitrarily sized
> > password. All you would want to do is restrict it to an appropriate
> > length. I only suggested using the maximum length of a CVSNT password to
> > ensure interoperability with CVSNT.
> 
>    Then, why is there no such provision in getpass() to facilitate
> that. Some extra 'length' parameter or so. Didn't you ever encounter
> such situation(need) before 'cvs passwd'.

getpass () is a good function that will allow you to input as many bytes
as you wish. It is only the case that you should check on the length
after it returns and reject passwords that are too long for the maximum
that is allowed by CVS and/or CVSNT clients and servers. I may be wrong
about the 128 byte limit on the passwd file, you should check the latest
revision of the passwd.cpp file that has been released.

A copy of the passwd.cpp from CVSNT_2_6_01_2288 is included after my
.signature.

> > So, you don't want to create a password on the server that a CVSNT
> > client is unable to specify when it talks to your server.
> 
>    What? I didn't get it!

Example:

  CVS client & server
      - "cvs setpass" command is given. The user responds with
        a 400 byte pass phrase and somehow managed to type it the
        same on the second attempt.

  CVSNT client talking to a CVS server
     - user attempts to do a "cvs login" and type in their 400 byte
       pass phrase. Oops. The CVSNT client is unable to read past the
       128 byte limit and the user is therefore unable to login.

Does that help make it concrete?
        
> >>    Second, Initially, I too, had thought of warning the administrator,
> >> who wish to 'delete' or 'disable' herself; As, 'add' is anyway not
> >> going to happen, because of duplicate username(error: 'name `%s' is
> >> not unique').
> >> But, then thought(still think), it's kind of unnecessary; I mean, the
> >> administrator knows, what she is up to, isn't it?
> >
> > Really? I do not assume that anyone will always get things right via
> > a command-line interface.
> 
>    Oh don't say that. One can always get things right via a command
> line; okay, if not at the first time. Aa mistakes are just inevitable
> anywhere, and are independent of everything. Also, somtimes, I find it
> very annoying, when some program asks me or warns me about something,
> which I'm sure of doing (A typical Microsoft philosophy, 'The user is
> a moron').

Sure, and if the user deletes themselves instead of the user they were
trying to delete, they now have to go back to some other path than the
'cvs passwd' command to fix the CVSROOT/passwd file. Is that what you
want?

> >  1) test cases which do not require a :pserver: to exercise the code
> >     [Hint: Being able to use :local: or :fork: would make testing a
> >     lot easier and make the administrator who might only be allowing
> >     :pserver: for read-only users an easier job of getting in via
> >     :ext: over SSH or using :local: on the box]
> 
>     How do I do that? Any starting point??, referrences???

You will not like the suggestion... the control flow for the command
will be the one you hate that merges passwd() and spasswd() and
check_optioN() and checks to see if the server_active is set or not to
determine if it is acting as the server or as the client.

You parse all of the command-line options for both cases and then
determine

> >  2) Handling more error conditions. What happens when a user issues
> >     a 'cvs passwd' command and no CVSROOT/passwd file exists? Or,
> >     what happens if the file is read-only and the user is not able
> >     to modify it.
> 
>     Well, she'll get an error (1, 0, "could not open: `%s'", file-name);
> Isn't that enough?

Probably, you have a test case for that in sanity.sh right? (Yes, I know
that you do not have such a test case becuase you are using a live
:pserver: repository which is one of the problems with the sanity.sh
testing you are doing... it is dangerous to expose a live repository to
experimental code.)

> > If you are not actually going to finish passwd to work on other
> > transports (I think this would require a vote of the other cvs-dev folks
> > to agree to that subset of work), then you need to send some
> > CVS_CLIENT_LOG examples for your existing test cases that we can see if
> > we can engineer them to work. It might also be possible to setup the
> > pserver process to listen on a non-2401 port and wait for our client to
> > connect to that port. This might require some kind of debugging output
> > that lets the parent process know which port the child pserver is
> > listening on to get things right.
> 
>    It's not that I'm not going to finish it; I'd definitely want to do
> that. But at the same time, I'll really appreciate it, if you could
> consider accepting the :pserver: changes. You know, that can serve as
> a kind of motivation or encouragement for me.

As I said, such a choice would take a vote by the cvs-dev folks to
accept the new feature.

Even if we reject it for now, at least you know that the code is in much
better shape than it was previously...

> Thank you! :)
> -- 
> regards
>     -P J P
> PS: Please don't send me html/attachment/Fwd mails

 ------- cvsnt/src/passwd.cpp 1.40 2005-09-30 16:08:22 -------
/*
 * Copyright (c) 1992, Brian Berliner and Jeff Polk
 * Copyright (c) 1989-1992, Brian Berliner
 * 
 * You may distribute under the terms of the GNU General Public License as
 * specified in the README file that comes with the CVS 1.4 kit.
 * 
 * passwd
 * 
 * Changes the password of the caller
 *
 * Corey Minyard:                               Initial development
 * Tony Hoyle:          23/11/2001      Rewrite for cvsnt
 *
 */

#include "cvs.h"

#include "getline.h"

#if defined(_WIN32) && defined(SERVER_SUPPORT)
/* We use this for sanity checking */
extern int isDomainMember();
#endif

extern char *crypt (const char *, const char *);

#define ascii_to_bin(c) ((c)>='a'?(c-59):(c)>='A'?((c)-53):(c)-'.')
#define bin_to_ascii(c) ((c)>=38?((c)-38+'a'):(c)>=12?((c)-12+'A'):(c)+'.')

static passwd_entry *passwd_list = NULL;
static int passwd_list_size = 0;

char *crypt_password(char *typed_password)
{
        CVS_TIME_T tm;
        char salt[2];

        if(typed_password)
        {
                time(&tm);
                salt[0] = bin_to_ascii(tm & 0x3f);
                salt[1] = bin_to_ascii((tm >> 5) & 0x3f);
                strcpy(typed_password,crypt(typed_password, salt));
        }
        return typed_password;
}

void free_passwd_list()
{
        if(passwd_list)
        {
                int u;
                for(u=0; u<passwd_list_size; u++)
                {
                        xfree(passwd_list[u].username);
                        xfree(passwd_list[u].password);
                        xfree(passwd_list[u].real_username);
                }
                xfree(passwd_list);
        }
}

void init_passwd_list()
{
        if(passwd_list)
                free_passwd_list();
        passwd_list = NULL;
        passwd_list_size = 0;
}


passwd_entry *new_passwd_entry()
{
        passwd_list_size++;
        passwd_list = 
(passwd_entry*)xrealloc(passwd_list,sizeof(passwd_entry)*passwd_list_size);
        memset(passwd_list+passwd_list_size-1,0,sizeof(passwd_entry));
        return passwd_list+passwd_list_size-1;
}

passwd_entry *find_passwd_entry(const char *username)
{
        int u;
        for(u=0; u<passwd_list_size; u++)
                if(passwd_list[u].username && 
!usercmp(passwd_list[u].username,username))
                        return passwd_list+u;
        return NULL;
}

int does_passwd_user_exist(const char *username)
{
        if(find_passwd_entry(username))
                return 1;
        if(system_auth && getpwnam(username))
                return 1;
        return 0;
}

static int write_passwd_entry(FILE *fp, int entry_num)
{
        if(!passwd_list[entry_num].username)
                return 0;

        fprintf(fp,"%s",passwd_list[entry_num].username);
        if(passwd_list[entry_num].password)
                fprintf(fp,":%s",passwd_list[entry_num].password);
        else
                fprintf(fp,":");
        if(passwd_list[entry_num].real_username)
                fprintf(fp,":%s",passwd_list[entry_num].real_username);
        fprintf(fp,"\n");
        return 0;
}

int read_passwd_list()
{
        char *filename;
        FILE *fp;
        char *linebuf = NULL;
        size_t linebuf_len;
        passwd_entry *passnode;

        filename = (char*)xmalloc(strlen(current_parsed_root->directory)
                           + strlen("/CVSROOT/passwd")
                           + 1);
        strcpy (filename, current_parsed_root->directory);
        strcat (filename, "/CVSROOT/passwd");

        init_passwd_list();

        fp = CVS_FOPEN (filename, "r");
        if (fp != NULL)
        {
            while (getline (&linebuf, &linebuf_len, fp) >= 0)
            {
                        char *user;
                        if(linebuf[strlen(linebuf)-1]=='\n')
                                linebuf[strlen(linebuf)-1]='\0';
                        if(linebuf[0]=='#')
                                continue;
                        user = cvs_strtok(linebuf,":");
                        if(!user || !*user)
                                continue;
                        passnode = new_passwd_entry();
                        passnode->username=xstrdup(user);
                        passnode->password=xstrdup(cvs_strtok(NULL,":"));
                        passnode->real_username=xstrdup(cvs_strtok(NULL,":"));
            
                        xfree (linebuf);
                        linebuf = NULL;
            }
            if (ferror (fp))
            {
                        error (1, errno, "cannot read %s", filename);
            }
            if (fclose (fp) < 0)
                        error (0, errno, "cannot close %s", filename);
        }
        return 0;
}

int write_passwd_list()
{
        FILE *fp;
        char *filename;

        filename = (char*)xmalloc(strlen(current_parsed_root->directory)
                           + strlen("/CVSROOT/passwd")
                           + 1);
        strcpy (filename, current_parsed_root->directory);
        strcat (filename, "/CVSROOT/passwd");

        fp = CVS_FOPEN (filename, "w");
        if (fp == NULL)
        {
            error (0, errno, "cannot open %s for writing", filename);
        }
        else
        {
                int u;
                for(u=0; u<passwd_list_size; u++)
                        write_passwd_entry(fp, u);
            if (fclose (fp) < 0)
                        error (0, errno, "cannot close %s", filename);
        }

        xfree(filename);
        return 0;
}

static const char *const passwd_usage[] =
{
    "Usage: %s %s [-a] [-x] [-X] [-r real_user] [-R] [-D domain] [username]\n",
    "\t-a\tAdd user\n",
        "\t-x\tDisable user\n",
        "\t-X\tDelete user\n",
        "\t-r\tAlias username to real system user\n",
        "\t-R\tRemove alias to real system user\n",
        "\t-D\tUse domain password\n",
    NULL
};

int passwd (int argc, char **argv)
{
    int    c;
    int    err = 0;
        char   typed_password[128]={0}, typed_password2[128]={0};
    const char   *username, *user;
        passwd_entry *passnode;
        char *real_user = NULL;
        char *password_domain = NULL;
        int 
adduser=0,deluser=0,disableuser=0,realuser=0,remove_realuser=0,use_domain=0;
        int arg_specified = 0;

    if (argc == -1)
        usage (passwd_usage);

    optind = 0;
    while ((c = getopt (argc, argv, "+axXr:RD:")) != -1)
    {
        switch (c)
        {
        case 'a':
                if(arg_specified)
                        usage (passwd_usage);
                arg_specified = 1;
            adduser = 1;
            break;
        case 'x':
                if(arg_specified)
                        usage (passwd_usage);
                arg_specified = 1;
                disableuser = 1;
                break;
        case 'X':
                if(arg_specified)
                        usage (passwd_usage);
                arg_specified = 1;
                deluser = 1;
                break;
        case 'r':
                realuser = 1;
                real_user = xstrdup(optarg);
                break;
        case 'R':
                remove_realuser = 1;
                break;
        case 'D':
                use_domain = 1;
                password_domain = xstrdup(optarg);
                break;
        case '?':
        default:
            usage (passwd_usage);
            break;
        }
    }
    argc -= optind;
    argv += optind;

        if(!argc)
                user = NULL;
        else
                user=argv[0];

    if (current_parsed_root->isremote)
    {
        if (argc > 1)
            usage (passwd_usage);

        if (!supported_request ("passwd"))
            error (1, 0, "server does not support passwd");

        if(!user && adduser)
        {
                error(1,0,"You cannot add yourself");
        }
        if(!user && deluser)
        {
                error(1,0,"You cannot delete yourself");
        }

        if(user || current_parsed_root->username || 
current_parsed_root->hostname)
        {
                printf ("%s address@hidden",
                        (adduser) ? "Adding user" : (deluser) ? "Deleting user" 
: "Changing repository password for",
                        
user?user:current_parsed_root->username?current_parsed_root->username:getcaller(),current_parsed_root->hostname);
        }
        else
        {
                printf ("Changing repository password for %s\n",getcaller());
        }
        fflush (stdout);

        if(!use_domain && !deluser && !disableuser)
        {
                CProtocolLibrary lib;

                if(!lib.PromptForPassword("New Password: 
",typed_password,sizeof(typed_password)))
                        error(1,0,"Aborted");

                if(!lib.PromptForPassword("Verify Password: 
",typed_password2,sizeof(typed_password2)))
                        error(1,0,"Aborted");

                if (strcmp(typed_password, typed_password2) != 0)
                {
                        memset (typed_password, 0, strlen (typed_password));
                        memset (typed_password2, 0, strlen (typed_password2));
                        error (1, 0, "Passwords do not match, try again");
                }
                memset (typed_password2, 0, strlen (typed_password2));
                if(strlen(typed_password))
                        crypt_password(typed_password);
        }

        if (adduser)
            send_arg ("-a");
        if (disableuser)
                send_arg ("-x");
        if (deluser)
                send_arg ("-X");
        if (realuser)
        {
                send_arg ("-r");
                send_arg (real_user);
        }
        if (remove_realuser)
                send_arg ("-R");
        if(use_domain)
        {
                send_arg ("-D");
                send_arg (password_domain);
        }

        if (argc == 1)
            send_arg(user);
        else
                send_arg("*");

        if(typed_password)
        {
                send_arg (typed_password); /* Send the new password */
                memset (typed_password, 0, strlen (typed_password));
        }

        send_to_server ("passwd\n", 0);
        return get_responses_and_close ();
    }
        if(!server_active)
        {
                if(argc!=0 && argc!=1)
                        usage (passwd_usage);

                if(!user && adduser)
                {
                        error(1,0,"You cannot add yourself");
                }
                if(!user && deluser)
                {
                        error(1,0,"You cannot delete yourself");
                }

                if(user || current_parsed_root->username)
                {
                        printf ("%s %s\n",
                                (adduser) ? "Adding user" : (deluser) ? 
"Deleting user" : "Changing password for",
                                user?user:current_parsed_root->username);
                }
                else
                {
                        printf ("Changing repository password for 
%s\n",getcaller());
                }
                fflush (stdout);

                if (argc == 0)
                        username = CVS_Username;
                else
                {
                        username = user;
                }

                if(!use_domain && !deluser && !disableuser)
                {
                        CProtocolLibrary lib;

                        if(!lib.PromptForPassword("New Password: 
",typed_password,sizeof(typed_password)))
                                error(1,0,"Aborted");

                        if(!lib.PromptForPassword("Verify Password: 
",typed_password2,sizeof(typed_password2)))
                                error(1,0,"Aborted");

                        if (strcmp(typed_password, typed_password2) != 0)
                        {
                                memset (typed_password, 0, strlen 
(typed_password));
                                memset (typed_password2, 0, strlen 
(typed_password2));
                                error (1, 0, "Passwords do not match, try 
again");
                        }
                        memset (typed_password2, 0, strlen (typed_password2));
                        if(strlen(typed_password))
                                crypt_password(typed_password);
                }
        } 
#ifdef SERVER_SUPPORT
        if(server_active)
        {
                if ((argc != 1) && (argc != 2))
                        usage (passwd_usage);

                if(!strcmp(user,"*"))
                        username = CVS_Username;
                else
                        username = user;

                if(argc==2)
                        strncpy(typed_password,argv[1],sizeof(typed_password));
        }
#endif

    if (typed_password[0] && 
        (strcmp(username, CVS_Username) != 0) && 
        (! verify_admin ()))
                error (1, 0, "Only administrators can add or change another's 
password");

        read_passwd_list();
        passnode = find_passwd_entry(username);
        if (passnode == NULL)
        {
            if (!adduser)
                        error (1, 0, "Could not find %s in password file", 
username);

            if (! verify_admin())
                {
                        error (1, 0, "Only administrators can add users" );
            }

                passnode = new_passwd_entry();
                passnode->username=xstrdup(username);
                passnode->password=xstrdup(typed_password);
                passnode->real_username=NULL;
        }

        if(deluser)
        {
            if (! verify_admin())
                {
                        error (1, 0, "Only administrators can delete users" );
            }
                xfree(passnode->username);
                passnode->username = NULL;
        }
        else if(disableuser)
        {
            if (! verify_admin())
                {
                        error (1, 0, "Only administrators can disable users" );
            }
                xfree(passnode->password);
                passnode->password=xstrdup("#DISABLED#");
        }
        else
        {
                xfree(passnode->password);
#ifdef _WIN32 /* Unix servers can't make any sense of this */
                if(use_domain)
                {
                        passnode->password = 
(char*)xmalloc(strlen(password_domain)+2);
                        strcpy(passnode->password,"!");
                        strcat(passnode->password,password_domain);
                }
                else
#endif
                        passnode->password = xstrdup(typed_password);

                if(realuser)
                {
                        if(!getpwnam(real_user))
                                error(1, 0, "User '%s' is not a real user on 
the system.",real_user);

                        xfree(passnode->real_username);
                        passnode->real_username = xstrdup(real_user);
                }
                else if (remove_realuser)
                {
                        xfree(passnode->real_username);
                        passnode->real_username=NULL;
                }

                if(!runas_user && ((passnode->real_username && 
!getpwnam(passnode->real_username)) || (!passnode->real_username && 
passnode->username && !getpwnam(passnode->username))))
                {
                        error(0,0,"*WARNING* CVS user '%s' will not be able to 
log in until they are aliased to a valid system user.",username);
                }
        }

        write_passwd_list();
        free_passwd_list();
        xfree(real_user);
        xfree(password_domain);

    return (err);
}