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
[EMAIL PROTECTED] 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: [EMAIL PROTECTED]
Get CVS support at http://ximbiot.com
--
It is error alone which needs the support of government. Truth can stand by
itself.
- Thomas Jefferson
_______________________________________________
Bug-cvs mailing list
[EMAIL PROTECTED]
http://mail.gnu.org/mailman/listinfo/bug-cvs
