bug-cvs
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: user.c, user.h


From: Derek Robert Price
Subject: Re: user.c, user.h
Date: Tue, 13 Aug 2002 09:57:14 -0400
User-agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.0.0) Gecko/20020606

Hrm. I should probably point out that the changes I'm requesting won't guarantee that your patch is accepted, but they will make it more likely. Please read the `HACKING' file in the top level of the CVS source distribution for more: <http://ccvs.cvshome.org/source/browse/ccvs/HACKING>.

Also, just the standard reminder, sumbission of your work to address@hidden grants the right to use your code in CVS and distribute it under the GNU GPL.

Andrey Aristarkhov wrote:

Hi All!

Here my implementation of "user" & "pass(word)" commands.
Patches to other files in "src" directory also applied.

Regards,
Andrey Aristarkhov
BiTechnology

Please see the `HACKING' file in the top level of the CVS source distribution for instructions on the submission of patches. You are missing documentation (`cvs.texinfo') changes and test suite (`src/sanity.sh') changes. These are required for every change to the CVS source because otherwise those sections of the code tend to deteriorate rapidly.

[ . . . snip . . . ]

static int
extendline(char **buf, int * buflen, int needed)
{
        if (needed > *buflen) {
                char    *tmp = realloc(*buf, needed);
                if (tmp == NULL)
                        return -1;
                *buf = tmp;
                *buflen = needed;
        }
        return *buflen;
}

static int
extendarray(char ***buf, int * buflen, int needed)
{
        if (needed > *buflen) {
                char    **tmp = realloc(*buf, needed * sizeof(char *));
                if (tmp == NULL)
                        return -1;
                *buf = tmp;
                *buflen = needed;
        }
        return *buflen;
}

extendline() has an analogue in `src/subr.c'. I'll grant that expand_string() may allocate more space than you really wanted, but please at least use xrealloc() from `src/subr.c' in favor of the system realloc() in both these functions if you keep extendline().

static char * get_password(const char * user) {
 static char pwd[MAX_STRING_LEN];
 char line[MAX_STRING_LEN];
 FILE * fpw=NULL;
 char * token;
 fpw = fopen (getpwpath(), "r");
 if (fpw == NULL) {
        return NULL;
 }
 memset(pwd,0,MAX_STRING_LEN);
 while (!(getline (line, MAX_STRING_LEN, fpw))) {
   if (line[0] == '#') {
     continue;
   }
   token = strtok(line,":");
   if (token == NULL) {
     continue;
   }
   if (strcmp (user, token) != 0) {
     continue;
   }
   /* User found */
   token = strtok(NULL,":");
   strcpy(pwd,token);
fclose(fpw);
   return pwd;
 }
 fclose(fpw);
 return NULL;
}

This is being done in `login.c'. This functionality should be factored into a common function.

static char * get_alias(const char * user) {
 static char alias[MAX_STRING_LEN];
 char line[MAX_STRING_LEN];
 FILE * fpw;
 char * token;

 fpw = fopen (getpwpath(), "r");
 while (!(getline (line, sizeof (line), fpw))) {
   if (line[0] == '#') {
     continue;
   }
   token = strtok(line,":");
   if (strcmp (user, token) != 0) {
     continue;
   }
   /* User found */
   token = strtok(NULL,":");
   token = strtok(NULL,":");
   if (token == NULL) {
     return NULL;
   }
   strcpy(alias,token);
fclose(fpw);
   return alias;
 }
 fclose(fpw);
 return NULL;
}

This too. A single function shared with `server.c' should be returnning the whole line containing the passwd and alias (or a single struct with all the data) and you should use the data as you wish.

Oops, I almost missed getpwpath(). I deleted your code already, but there is already a construct_cvspass_filename() function in `login.c' as well.

[ . . . snip . . . ]

/*
* Reads and verifies user's password from an input
* Return password for the user
*/
#define safe_string(a) a ? a : "NULL"

static char * read_user_password(const char * user, int pwd_read_mode) {
 char prompt[128];
 char * pwd, * pwd_verify;
 static char password[_PASSWORD_LEN];

memset(password,0,_PASSWORD_LEN); switch (pwd_read_mode) {
   case PRM_NEW:
     sprintf(prompt,"Enter New password for user '%s':",user);
     /* We need to strdup because GETPASS uses static object */
     pwd_verify = GETPASS(prompt);
     verify_password_phrase(pwd_verify,TRUE);
     pwd = strdup(pwd_verify);
     memset(pwd_verify,0,strlen(pwd_verify));
     sprintf(prompt,"Re-type New password for user '%s':",user);
     pwd_verify = GETPASS(prompt);
     verify_password_phrase(pwd_verify,FALSE);
     if (strcmp(pwd,pwd_verify)) {
       free(pwd);
       error(1,0,"Passwords are different");
     } else {
       free(pwd);
       strcpy(password,pwd_verify);
       memset(pwd_verify,0,strlen(pwd_verify));
     }
   break;
case PRM_CHECK:
   pwd = get_password(user);
   sprintf(prompt,"Enter current password for user '%s': ",user);
   pwd_verify = GETPASS(prompt);
   verify_password_phrase(pwd_verify,FALSE);
   if (strcmp(pwd,crypt(pwd_verify,pwd))==0) {
     strcpy(password,pwd);
   } else {
     error(1,0,"Wrong password",user);
   }
  break;
case PRM_ADMIN:
   pwd = get_password(CVS_ADMIN_USER);
if (pwd == NULL) error(1,0,"Administrator's user accout not found. Please contact your CVS admin.");
   if (strcmp(pwd,"*")==0)
     error(1,0,"Password for Administrator's user accout is not set. Please contact 
your CVS admin.");
   pwd_verify = GETPASS("Enter CVS Administrator password: ");
   verify_password_phrase(pwd_verify,FALSE);
   if (strcmp(pwd,crypt(pwd_verify,pwd))==0) {
     strcpy(password,pwd);
   } else {
     error(1,0,"Administrator password is invalid");
   }
  break;
 }
 return password;
}

Again, much of this happens in `server.c'. Please reuse code. And unless I'm mistaken, this asks for the admin password - that shouldn't be happening. If the user is an administrator they should already have authenticated.

As for allowing any user's password to be changed if you know the user's password, I'm against that. It's unneeded overhead. Go ahead and verify a user's password, but make other users log in to change their own password or let the admin do it.

Derek

--
               *8^)

Email: address@hidden

Get CVS support at http://ximbiot.com
--
It is error alone which needs the support of government.  Truth can stand by
itself.
                        - Thomas Jefferson







reply via email to

[Prev in Thread] Current Thread [Next in Thread]