About passwd, deluser and delgroup

[Tito]

On Thursday 6 April 2006 16:28, Rob Landley wrote:
> On Wednesday 05 April 2006 3:09 am, Tito wrote:
> > Hi Rob,
> >
> > I was thinking about the fact if it would be possible to
> > reuse these functions (maybe with some minor modifications)
> >
> > static int update_passwd(const struct passwd *pw)
> > static int update_passwd_file(FILE *fp, const char *dest, const char
> > *username)
> >
> > of your pending passwd rewrite to be used also
> > by deluser and delgroup.
> > In this case they could be moved to libbb and
> > the behaviour of all applets writing to password files would be
> > consistent and more secure.
> >
> > BTW, by looking at the cleaned up versions of adduser
> > and addgroup it seems to me that they open the password
> 
> They weren't that cleaned up.  And it turns out it wasn't all the files I've 
> touched (didn't think so).  At the first deleted file that's still under 
> source control, sed stops with an error.  And I've deleted a lot of files.  
> And I'm maybe half done at this point...
> 
> > files in an insecure way.
> > What would happen if two
> > programs try to write to the file
> > at the same time?
> 
> If the new file is a constant filename (which is not a security problem 
> if /etc isn't world writeable) then O_EXCL should fail if it already exists.  

OK.

> The theory is to create a new version and then atomic rename it over the old 
> one.  (The backup is done via a hard link first, so the old one should remain 
> unmodified.  No need to actually copy the data and set the date and all 
> that.)
> 
>     // Create new file, as one atomic operation with the right permissions and
>     // ownership because doing chmod or chown after create opens a race.
> 
>     if ((out_fd = bb_xopen(dest, O_CREAT|O_EXCL,
>             ENABLE_FEATURE_SHADOWPASSWDS ? 0600 : 0644)) < 0) return 0;

I' m trying to use about the same routine in deluser and delgroup which
by the way I'm unifying in one file together with delline.c.
The code is:

static void del_line_matching(const char *login, const char *filename)
{
	char *line;
	FILE *passwd;
	FILE *temp;
	char *new_file = "/etc/ptmp";
	int fd;
	int found = 0;
	struct stat statbuf;
	struct flock lock;

	/* open file */
	passwd = bb_xfopen(filename, "r+");

	/*get mode */
	xstat(filename, &statbuf);

	/* Lock the password file before updating */
	lock.l_type = F_WRLCK;
	lock.l_whence = SEEK_SET;
	lock.l_start = 0;
	lock.l_len = 0;
	if (fcntl(fileno(passwd), F_SETLK, &lock) == 0) {
		/* Be paranoid about who's going to own this file. */
		setuid(0);
		setgid(0);	
		/* get fd */
		if ((fd = open(new_file, O_WRONLY | O_CREAT | O_EXCL, statbuf.st_mode)) != -1) {
			/* get stream */
			if ((temp = fdopen(fd,"w"))) {
				/* Loop through entries, copying each one except the one we want to remove */
				while ((line = bb_get_line_from_file(passwd)) != NULL) {
					if ((strncmp(login, line, strlen(login)) == 0) && line[strlen(login)] == ':') {
						found++;
					} else {
						fputs(line, temp);
					}
					free(line);
				}
				if (!found) {
					bb_error_msg("'%s' not found in '%s'", login, filename);
				} else {
					/* Be sure the new file was written to disk */
					if (!fflush(temp) && !fsync(fileno(temp)) && !fclose(temp)) {
						if (rename(new_file, filename)) {
							bb_error_msg("Can't rename %s to %s", new_file, filename);
						}
					}
				}
			}
		}
		/* Delete new copy in case it didn't get renamed. */
		unlink(new_file);
	}
	/* Remove lock */
	lock.l_type = F_UNLCK;
	fcntl(fileno(passwd), F_SETLK, &lock);
	/* close original file */
	fclose(passwd);
}

This should work with /etc/passwd /etc/shadow, /etc/group and /etc/gshadow

Comments and hints form the list members about the security of 
this code are wellcome as I'm not that good in this things....

TIA 

Ciao,
Tito

  
> I've pondered putting in some kind of recovery code there (check to see if the 
> date on the file that exists is more than X seconds in the past, and delete 
> it if so), but I haven't convinced myself it's worth the bytes.  Still 
> working on it, slowly, in the background...
> 
> Rob