Subject: Re: pwd_mkdb
To: None <ad@netbsd.org>
From: enami tsugutomo <enami@sm.sony.co.jp>
List: tech-userlevel
Date: 09/06/2000 12:06:20
Andy Doran <ad@netbsd.org> writes:

> Somebody (enami?) wanted to review my modifications to pwd_mkdb; they
> support adding or modifying records pertaining only to a specified user.
> 
> It's here: ftp.netbsd.org:/pub/incoming/ad/pwd_mkdb/pwd_mkdb.tgz.

Thanks.  Here is my comments (some of them might be for original code
though).  I don't run this, just UTSL.

enami.

> diff -c /usr/src/usr.sbin/pwd_mkdb/pwd_mkdb.c ./pwd_mkdb.c
> *** /usr/src/usr.sbin/pwd_mkdb/pwd_mkdb.c	Wed Aug  9 10:43:28 2000
> --- ./pwd_mkdb.c	Tue Sep  5 00:45:38 2000
> ***************
> *** 77,124 ****
>   	0		/* lorder */
>   };
>   
> ! static enum { FILE_INSECURE, FILE_SECURE, FILE_ORIG } clean;
> ! static char *pname;				/* password file name */
> ! static char prefix[MAXPATHLEN];
> ! static char oldpwdfile[MAX(MAXPATHLEN, LINE_MAX * 2)];
> ! static char pwd_db_tmp[MAX(MAXPATHLEN, LINE_MAX * 2)];
> ! static char pwd_Sdb_tmp[MAX(MAXPATHLEN, LINE_MAX * 2)];
> ! static int lorder = BYTE_ORDER;
>   
>   void	cleanup(void);
> ! void	error(const char *);
> ! void	wr_error(const char *);
> ! int	main(int, char **);
>   void	install(const char *, const char *);
>   void	rm(const char *);
>   int	scan(FILE *, struct passwd *, int *, int *);
>   void	usage(void);
> ! void	putdbent(DB *, struct passwd *, const char *, int, const char *, int);
> ! void	putyptoken(DB *dp, const char *fn);
>   
>   int
>   main(int argc, char *argv[])
>   {
> - 	DB *dp, *edp;
> - 	FILE *fp, *oldfp;
>   	sigset_t set;
> ! 	int ch, makeold, tfd, hasyp, flags, lineno;
> ! 	struct passwd pwd;
>   
> - 	hasyp = 0;
> - 	oldfp = NULL;
> - 	strcpy(prefix, "/");
>   	makeold = 0;
>   	
> ! 	while ((ch = getopt(argc, argv, "d:pvBL")) != -1)
>   		switch(ch) {
>   		case 'd':			/* set prefix */
> ! 			strncpy(prefix, optarg, sizeof(prefix));
> ! 			prefix[sizeof(prefix)-1] = '\0';
>   			break;
>   		case 'p':			/* create V7 "file.orig" */
>   			makeold = 1;
>   			break;
>   		case 'v':			/* backward compatible */
>   			break;
>   		case 'B':			/* big-endian output */
> --- 117,172 ----
>   	0		/* lorder */
>   };
>   
> ! enum	{ STATE_DONE, STATE_INSECURE, STATE_SECURE, STATE_ORIG } state;
> ! int	lorder = BYTE_ORDER;
> ! int	makeold;
> ! FILE	*pfd;

Since it is not file descriptor but file pointer, pfp may better.
Ditto for v7fd.

> ! char	*pname;
> ! char	*prefix = "/";
> ! 
> ! char	idbfn[MAXPATHLEN];
> ! char	sdbfn[MAXPATHLEN];
> ! char	v7fn[MAXPATHLEN];
> ! char	tmpidbfn[MAXPATHLEN];
> ! char	tmpsdbfn[MAXPATHLEN];
> ! char	tmpv7fn[MAXPATHLEN];
>   
>   void	cleanup(void);
> ! void	doall(void);
> ! int	dosingle(const char *);
>   void	install(const char *, const char *);
> + int	main(int, char **);
>   void	rm(const char *);
>   int	scan(FILE *, struct passwd *, int *, int *);
>   void	usage(void);
> ! void	putdbent(DB *, struct passwd *, const char *, int, const char *, int,
> ! 		 int);
> ! void	putv7ent(FILE *, struct passwd *, const char *);
> ! void	putyptoken(DB *, const char *);
>   
>   int
>   main(int argc, char *argv[])
>   {
>   	sigset_t set;
> ! 	int ch;
> ! 	const char *username;
>   
>   	makeold = 0;
> + 	openinfo.lorder = lorder;
> + 	state = STATE_DONE;
> + 	atexit(cleanup);

atexit() may fail.

>   	
> ! 	while ((ch = getopt(argc, argv, "d:pu:vBL")) != -1)
>   		switch(ch) {

A keyword is followed by space, btw.

>   		case 'd':			/* set prefix */
> ! 			prefix = optarg;
>   			break;
>   		case 'p':			/* create V7 "file.orig" */
>   			makeold = 1;
>   			break;
> + 		case 'u':			/* specified user only */
> + 			username = optarg;
> + 			break;
>   		case 'v':			/* backward compatible */
>   			break;
>   		case 'B':			/* big-endian output */
> ***************
> *** 136,145 ****
>   
>   	if (argc != 1)
>   		usage();
>   
>   	/*
> ! 	 * This could be changed to allow the user to interrupt.
> ! 	 * Probably not worth the effort.
>   	 */
>   	sigemptyset(&set);
>   	sigaddset(&set, SIGTSTP);
> --- 184,194 ----
>   
>   	if (argc != 1)
>   		usage();
> + 	pname = *argv;
>   
>   	/*
> ! 	 * This could be changed to allow the user to interrupt.  Probably
> ! 	 * not worth the effort.
>   	 */
>   	sigemptyset(&set);
>   	sigaddset(&set, SIGTSTP);
> ***************
> *** 147,189 ****
>   	sigaddset(&set, SIGINT);
>   	sigaddset(&set, SIGQUIT);
>   	sigaddset(&set, SIGTERM);
> ! 	(void)sigprocmask(SIG_BLOCK, &set, (sigset_t *)NULL);
>   
>   	/* We don't care what the user wants. */
> ! 	(void)umask(0);
> ! 
> ! 	pname = *argv;
> ! 	/* Open the original password file */
> ! 	if ((fp = fopen(pname, "r")) == NULL)
> ! 		error(pname);
>   
> ! 	openinfo.lorder = lorder;
>   
> ! 	/* Open the temporary insecure password database. */
> ! 	(void)snprintf(pwd_db_tmp, sizeof(pwd_db_tmp), "%s%s.tmp", prefix,
> ! 	    _PATH_MP_DB);
> ! 	dp = dbopen(pwd_db_tmp, O_RDWR | O_CREAT | O_EXCL, PERM_INSECURE,
> ! 	    DB_HASH, &openinfo);
> ! 	if (dp == NULL)
> ! 		error(pwd_db_tmp);
> ! 	clean = FILE_INSECURE;
>   
>   	/*
> ! 	 * Open file for old password file.  Minor trickiness -- don't want to
> ! 	 * chance the file already existing, since someone (stupidly) might
> ! 	 * still be using this for permission checking.  So, open it first and
> ! 	 * fdopen the resulting fd.  The resulting file should be readable by
> ! 	 * everyone.
>   	 */
> ! 	if (makeold) {
> ! 		(void)snprintf(oldpwdfile, sizeof(oldpwdfile), "%s.orig",
> ! 		    pname);
> ! 		if ((tfd = open(oldpwdfile, O_WRONLY | O_CREAT | O_EXCL,
> ! 		    PERM_INSECURE)) < 0)
> ! 			error(oldpwdfile);
> ! 		if ((oldfp = fdopen(tfd, "w")) == NULL)
> ! 			error(oldpwdfile);
> ! 		clean = FILE_ORIG;
>   	}
>   
>   	/*
> --- 196,264 ----
>   	sigaddset(&set, SIGINT);
>   	sigaddset(&set, SIGQUIT);
>   	sigaddset(&set, SIGTERM);
> ! 	sigprocmask(SIG_BLOCK, &set, (sigset_t *)NULL);
>   
>   	/* We don't care what the user wants. */
> ! 	umask(0);

Those casts there were for lint, weren't them?

>   
> ! 	snprintf(idbfn, sizeof(idbfn), "%s"_PATH_MP_DB, prefix);
> ! 	snprintf(sdbfn, sizeof(sdbfn), "%s"_PATH_SMP_DB, prefix);
> ! 	snprintf(v7fn, sizeof(v7fn), "%s"_PATH_PASSWD, prefix);
> ! 	snprintf(tmpidbfn, sizeof(tmpidbfn), "%s.tmp", idbfn);
> ! 	snprintf(tmpsdbfn, sizeof(tmpsdbfn), "%s.tmp", sdbfn);
> ! 	snprintf(tmpv7fn, sizeof(tmpv7fn), "%s.orig", v7fn);	/* XXX ? */
> ! 
> ! 	/* Open the original password file. */
> ! 	if ((pfd = fopen(pname, "r")) == NULL)
> ! 		err(EXIT_FAILURE, pname);
> ! 
> ! 	if (username == NULL)
> ! 		doall();
> ! 	else if (dosingle(username)) {
> ! 		warnx("inconsistancies exist; rebuilding databases");
> ! 		doall();
> ! 	} 
>   
> ! 	/* Set master.passwd permissions, in case caller forgot. */
> ! 	fchmod(fileno(pfd), S_IRUSR | S_IWUSR);

Is it better to use PERM_SECURE?

> ! 	fclose(pfd);
>   
>   	/*
> ! 	 * Move the master password LAST -- chpass(1), passwd(1) and vipw(8)
> ! 	 * all use flock(2) on it to block other incarnations of themselves.
> ! 	 * The rename means that everything is unlocked, as the original
> ! 	 * file can no longer be accessed.
>   	 */
> ! 	install(pname, _PATH_MASTERPASSWD);
> ! 	exit(EXIT_SUCCESS);
> ! 	/* NOTREACHED */
> ! }
> ! 
> ! /*
> !  * Rebuild the databases.
> !  */
> ! void
> ! doall(void)
> ! {
> ! 	DB *idb, *sdb;
> ! 	FILE *v7fd;
> ! 	int hasyp, flags, lineno;
> ! 	struct passwd pw;
> ! 
> ! 	hasyp = 0;
> ! 	v7fd = NULL;
> ! 
> ! 	/* Open the temporary insecure password database. */
> ! 	idb = dbopen(tmpidbfn, O_RDWR | O_CREAT | O_EXCL, PERM_INSECURE,
> ! 	    DB_HASH, &openinfo);
> ! 	if (idb == NULL)
> ! 		err(EXIT_FAILURE, "%s", tmpidbfn);
> ! 	state = STATE_INSECURE;
> ! 
> ! 	/* Open the temporary V7-style password database. */

Even if !makeold?

> ! 	if ((v7fd = fopen(tmpv7fn, "w")) == NULL) {

I think original code creates this file with 0644, but this creates
with 0666.

> ! 		err(EXIT_FAILURE, tmpv7fn);
> ! 		state = STATE_ORIG;
>   	}
>   
>   	/*
> ***************
> *** 191,292 ****
>   	 * pointer record, which if YP is enabled in the C lib, will speed
>   	 * things up.
>   	 */
> ! 	for (lineno = 0; scan(fp, &pwd, &flags, &lineno);) {
> ! 		/* look like YP? */
> ! 		if (pwd.pw_name[0] == '+' || pwd.pw_name[0] == '-')
> ! 			hasyp++;
> ! 
> ! 		/*
> ! 		 * Warn about potentially unsafe uid/gid overrides.
> ! 		 */
> ! 		if (pwd.pw_name[0] == '+') {
> ! 			if ((flags & _PASSWORD_NOUID) == 0 && pwd.pw_uid == 0)
> ! 				warnx(
> ! 				"line %d: superuser override in YP inclusion",
> ! 				    lineno);
> ! 			if ((flags & _PASSWORD_NOGID) == 0 && pwd.pw_gid == 0)
> ! 				warnx(
> ! 				    "line %d: wheel override in YP inclusion",
> ! 				    lineno);
> ! 		}
> ! 
> ! 		putdbent(dp, &pwd, "*", flags, pwd_db_tmp, lineno);
> ! 
> ! 		/* Create original format password file entry */
> ! 		if (makeold) {
> ! 			(void)fprintf(oldfp, "%s:*:%d:%d:%s:%s:%s\n",
> ! 			    pwd.pw_name, pwd.pw_uid, pwd.pw_gid, pwd.pw_gecos,
> ! 			    pwd.pw_dir, pwd.pw_shell);
> ! 			if (ferror(oldfp))
> ! 				wr_error(oldpwdfile);
> ! 		}
>   	}
>   
>   	/* Store YP token, if needed. */
>   	if (hasyp)
> ! 		putyptoken(dp, pwd_db_tmp);
>   
> ! 	if ((dp->close)(dp) < 0)
> ! 		wr_error(pwd_db_tmp);
>   	if (makeold) {
> ! 		if (fflush(oldfp) == EOF)
> ! 			wr_error(oldpwdfile);
> ! 		if (fclose(oldfp) == EOF)
> ! 			wr_error(oldpwdfile);
>   	}
>   
>   	/* Open the temporary encrypted password database. */
> ! 	(void)snprintf(pwd_Sdb_tmp, sizeof(pwd_Sdb_tmp), "%s%s.tmp", prefix,
> ! 		_PATH_SMP_DB);
> ! 	edp = dbopen(pwd_Sdb_tmp, O_RDWR | O_CREAT | O_EXCL, PERM_SECURE,
> ! 	   DB_HASH, &openinfo);
> ! 	if (!edp)
> ! 		error(pwd_Sdb_tmp);
> ! 	clean = FILE_SECURE;
> ! 
> ! 	rewind(fp);
> ! 	for (lineno = 0; scan(fp, &pwd, &flags, &lineno);)
> ! 		putdbent(edp, &pwd, pwd.pw_passwd, flags, pwd_Sdb_tmp,
> ! 		    lineno);
>   
>   	/* Store YP token, if needed. */
>   	if (hasyp)
> ! 		putyptoken(edp, pwd_Sdb_tmp);
>   
> ! 	if ((edp->close)(edp) < 0)
> ! 		wr_error(_PATH_SMP_DB);
> ! 
> ! 	/* Set master.passwd permissions, in case caller forgot. */
> ! 	(void)fchmod(fileno(fp), S_IRUSR|S_IWUSR);
> ! 	if (fclose(fp) == EOF)
> ! 		wr_error(pname);
>   
>   	/* Install as the real password files. */
> ! 	install(pwd_db_tmp, _PATH_MP_DB);
> ! 	install(pwd_Sdb_tmp, _PATH_SMP_DB);
>   	if (makeold)
> ! 		install(oldpwdfile, _PATH_PASSWD);
>   
>   	/*
> ! 	 * Move the master password LAST -- chpass(1), passwd(1) and vipw(8)
> ! 	 * all use flock(2) on it to block other incarnations of themselves.
> ! 	 * The rename means that everything is unlocked, as the original
> ! 	 * file can no longer be accessed.
>   	 */
> ! 	install(pname, _PATH_MASTERPASSWD);
> ! 	exit(EXIT_SUCCESS);
> ! 	/* NOTREACHED */
>   }
>   
>   int
> ! scan(FILE *fp, struct passwd *pw, int *flags, int *lineno)
>   {
>   	static char line[LINE_MAX];
>   	char *p;
> - 	int oflags;
>   
> ! 	if (fgets(line, sizeof(line), fp) == NULL)
> ! 		return (0);
>   	(*lineno)++;
>   
>   	/*
> --- 266,528 ----
>   	 * pointer record, which if YP is enabled in the C lib, will speed
>   	 * things up.
>   	 */
> ! 	for (lineno = 0; !scan(pfd, &pw, &flags, &lineno);) {
> ! 		/* Look like YP? */
> ! 		if (pw.pw_name[0] == '+' || pw.pw_name[0] == '-')
> ! 			hasyp = 1;
> ! 
> ! 		putdbent(idb, &pw, "*", flags, tmpidbfn, lineno,
> ! 		    R_NOOVERWRITE);
> ! 
> ! 		/* Create original format password file entry. */
> ! 		if (makeold)
> ! 			putv7ent(v7fd, &pw, tmpv7fn);
> ! 
> ! 		/* Warn about potentially unsafe uid/gid overrides. */
> ! 		if (pw.pw_name[0] != '+')
> ! 			continue;
> ! 
> ! 		if ((flags & _PASSWORD_NOUID) == 0 && pw.pw_uid == 0)
> ! 			warnx("line %d: superuser override in YP inclusion",
> ! 			    lineno);
> ! 		if ((flags & _PASSWORD_NOGID) == 0 && pw.pw_gid == 0)
> ! 			warnx("line %d: wheel override in YP inclusion",
> ! 			    lineno);
>   	}
>   
>   	/* Store YP token, if needed. */
>   	if (hasyp)
> ! 		putyptoken(idb, tmpidbfn);
>   
> ! 	if ((*idb->close)(idb) < 0)
> ! 		err(EXIT_FAILURE, "closing %s", tmpidbfn);
>   	if (makeold) {
> ! 		if (fflush(v7fd) == EOF)
> ! 			err(EXIT_FAILURE, "writing to %s", tmpv7fn);
> ! 		if (fclose(v7fd) == EOF)
> ! 			err(EXIT_FAILURE, "closing %s", tmpv7fn);
>   	}
>   
>   	/* Open the temporary encrypted password database. */
> ! 	sdb = dbopen(tmpsdbfn, O_RDWR | O_CREAT | O_EXCL, PERM_SECURE, DB_HASH,
> ! 	    &openinfo);
> ! 	if (!sdb)
> ! 		err(EXIT_FAILURE, "%s", tmpsdbfn);
> ! 	state = STATE_SECURE;
> ! 
> ! 	rewind(pfd);
> ! 	for (lineno = 0; !scan(pfd, &pw, &flags, &lineno);)
> ! 		putdbent(sdb, &pw, pw.pw_passwd, flags, tmpsdbfn,
> ! 		    lineno, R_NOOVERWRITE);
>   
>   	/* Store YP token, if needed. */
>   	if (hasyp)
> ! 		putyptoken(sdb, tmpsdbfn);
>   
> ! 	if ((*sdb->close)(sdb) < 0)
> ! 		err(EXIT_FAILURE, "closing %s", tmpsdbfn);
>   
>   	/* Install as the real password files. */
> ! 	install(tmpsdbfn, _PATH_SMP_DB);
> ! 	state = STATE_INSECURE;
> ! 	install(tmpidbfn, _PATH_MP_DB);

Do temporary files leak?  Suppose if this install() fails...

> ! 	state = STATE_SECURE;
>   	if (makeold)
> ! 		install(tmpv7fn, _PATH_PASSWD);
> ! 	state = STATE_DONE;
> ! }
>   
> + /*
> +  * Attempt to add or modify entries only for the specified user alone.  If
> +  * that's not possible, return non-zero to indicate that the entire database
> +  * should be rebuilt.
> +  */
> + int
> + dosingle(const char *username)
> + {
> + 	DB *idb, *sdb;
> + 	DBT data, key;
> + 	char *p, buf[MAX_KEY_SIZE];
> + 	u_int32_t x;
> + 	int found, newuser, rmolduid, flags, lineno, len, rv;
> + 	uid_t olduid;
> + 	struct passwd pw;
> + 	FILE *v7fd;
> + 	
>   	/*
> ! 	 * Get user's entry from /etc/master.passwd.
>   	 */
> ! 	for (lineno = 0, found = 0; !scan(pfd, &pw, &flags, &lineno);)
> ! 		if (strcmp(pw.pw_name, username) == 0) {
> ! 			found = 1;
> ! 			break;
> ! 		}
> ! 
> ! 	if (!found)
> ! 		errx(EXIT_FAILURE, "user ``%s'' does not exist", username);
> ! 
> ! 	/* Open the insecure database read-only. */
> ! 	if ((idb = dbopen(idbfn, O_RDONLY, 0, DB_HASH, NULL)) == NULL)
> ! 		err(EXIT_FAILURE, "%s", idbfn);
> ! 
> ! 	/*
> ! 	 * Get the by-name record.  If it's non-existant, this is a new
> ! 	 * user.  If it exists and the UID is changing, the by-UID record
> ! 	 * for the old UID must exist and must reference the same user name.
> ! 	 */
> ! 	newuser = 0;
> ! 	rmolduid = 0;
> ! 
> ! 	buf[0] = _PW_KEYBYNAME;
> ! 	len = strlen(username);
> ! 	memmove(buf + 1, username, MIN(len, sizeof(buf) - 1));
> ! 	key.data = (u_char *)buf;
> ! 	key.size = len + 1;
> ! 
> ! 	if ((*idb->get)(idb, &key, &data, 0) == 0) {
> ! 		p = (char *)data.data;
> ! 
> ! 		/* Jump over pw_name and pw_passwd, to get to pw_uid. */
> ! 		while (*p++ != '\0')
> ! 			;
> ! 		while (*p++ != '\0')
> ! 			;
> ! 
> ! 		if (lorder != BYTE_ORDER) {
> ! 			P_32_COPY(p, x);
> ! 			olduid = (uid_t)x;
> ! 		} else
> ! 			olduid = (uid_t)*(u_int32_t *)p;
> ! 			
> ! 		if (olduid != pw.pw_uid) {
> ! 			buf[0] = _PW_KEYBYUID;
> ! 			memmove(buf + 1, p, sizeof(u_int32_t));
> ! 			key.data = (u_char *)buf;
> ! 			key.size = sizeof(u_int32_t) + 1;
> ! 
> ! 			if ((*idb->get)(idb, &key, &data, 0) == 0) {
> ! 				if (strcmp(data.data, username) != 0) {
> ! 					(*idb->close)(idb);
> ! 					return (1);
> ! 				}
> ! 			} else {
> ! 				(*idb->close)(idb);
> ! 				return (1);
> ! 			}

Just writing

if (no old entry for the uid ||
    username for the uid doesn't match) {
	close db;
	return (hey, it's inconsistient);
}

isn't so complex.

> ! 				
> ! 			rmolduid = 1;
> ! 		}
> ! 	} else
> ! 		newuser = 1;
> ! 
> ! 	/*
> ! 	 * The by-UID record must be non-existant or must reference the same
> ! 	 * username.
> ! 	 */
> ! 	buf[0] = _PW_KEYBYUID;
> ! 	x = pw.pw_uid;
> ! 	if (lorder != BYTE_ORDER)
> ! 		M_32_SWAP(x);
> ! 	memmove(buf + 1, &x, sizeof(u_int32_t));
> ! 	key.data = (u_char *)buf;
> ! 	key.size = sizeof(u_int32_t) + 1;
> ! 
> ! 	if ((*idb->get)(idb, &key, &data, 0) == 0)
> ! 		if (strcmp(data.data, username) != 0) {
> ! 			(*idb->close)(idb);
> ! 			return (1);
> ! 		}
> ! 
> ! 	/*
> ! 	 * The by-number record for the new line number must reference the
> ! 	 * same user name.  If this is a new user name, it musn't exist.
> ! 	 */
> ! 	buf[0] = _PW_KEYBYNUM;
> ! 	x = lineno;
> ! 	if (lorder != BYTE_ORDER)
> ! 		M_32_SWAP(x);
> ! 	memmove(buf + 1, &x, sizeof(u_int32_t));
> ! 	key.data = (u_char *)buf;
> ! 	key.size = sizeof(u_int32_t) + 1;
> ! 
> ! 	rv = (*idb->get)(idb, &key, &data, 0);
> ! 	(*idb->close)(idb);
> ! 	if (rv == 0) {
> ! 		if (newuser || strcmp(data.data, username) != 0)
> ! 			return (1);
> ! 	} else if (!newuser)
> ! 		return (1);
> ! 	
> ! 	/*
> ! 	 * We have now verified that the changes can be made.
> ! 	 */
> ! 
> ! 	if ((idb = dbopen(idbfn, O_RDWR, 0, DB_HASH, NULL)) == NULL)
> ! 		err(EXIT_FAILURE, "%s", idbfn);
> ! 	if ((sdb = dbopen(sdbfn, O_RDWR, 0, DB_HASH, NULL)) == NULL)
> ! 		err(EXIT_FAILURE, "%s", sdbfn);
> ! 
> ! 	if (rmolduid) {
> ! 		buf[0] = _PW_KEYBYUID;
> ! 		x = olduid;
> ! 		if (lorder != BYTE_ORDER)
> ! 			M_32_SWAP(x);
> ! 		memmove(buf + 1, &x, sizeof(u_int32_t));
> ! 		key.data = (u_char *)buf;
> ! 		key.size = sizeof(u_int32_t) + 1;
> ! 
> ! 		if ((*idb->del)(idb, &key, 0) || (*sdb->del)(sdb, &key, 0))
> ! 			errx(EXIT_FAILURE,
> ! 			   "unable to remove old _PW_KEYBYUID record");
> ! 	}
> ! 
> ! 	putdbent(idb, &pw, "*", flags, idbfn, lineno, 0);
> ! 	putdbent(sdb, &pw, pw.pw_passwd, flags, sdbfn, lineno, 0);
> ! 	if ((*idb->close)(idb))
> ! 		err(EXIT_FAILURE, "closing %s", idbfn);
> ! 	if ((*sdb->close)(sdb))
> ! 		err(EXIT_FAILURE, "closing %s", sdbfn);
> ! 
> ! 	/*
> ! 	 * Re-build /etc/passwd.  If this is a new user, we simply append
> ! 	 * the entry.
> ! 	 */
> ! 	if (newuser) {
> ! 		if ((v7fd = fopen(v7fn, "a")) == NULL)
> ! 			err(EXIT_FAILURE, "%s", v7fn);
> ! 		putv7ent(v7fd, &pw, v7fn);
> ! 		fflush(v7fd);
> ! 		if (ferror(v7fd) || fclose(v7fd))
> ! 			err(EXIT_FAILURE, "%s", v7fn);
> ! 		fclose(v7fd);

Why fclose twice?  The way to test if fflush or fclose is failed is
different from above.

> ! 	} else {
> ! 		if ((v7fd = fopen(tmpv7fn, "w")) == NULL)
> ! 			err(EXIT_FAILURE, "%s", tmpv7fn);
> ! 		rewind(pfd);
> ! 		for (lineno = 0; !scan(pfd, &pw, &flags, &lineno); )
> ! 			putv7ent(v7fd, &pw, tmpv7fn);
> ! 		fflush(v7fd);
> ! 		if (ferror(v7fd) || fclose(v7fd))
> ! 			err(EXIT_FAILURE, "%s", tmpv7fn);
> ! 		fclose(v7fd);
> ! 		install(tmpv7fn, _PATH_PASSWD);

Ditto.  And file mode.

> ! 	}
> ! 
> ! 	return (0);
>   }
>   
> + /*
> +  * Read a single line from the password file and parse it.  Return non-zero
> +  * on EOF.
> +  */
>   int
> ! scan(FILE *fd, struct passwd *pw, int *flags, int *lineno)
>   {
>   	static char line[LINE_MAX];
>   	char *p;
>   
> ! 	if (fgets(line, sizeof(line), fd) == NULL)
> ! 		return (-1);
>   	(*lineno)++;
>   
>   	/*
> ***************
> *** 294,360 ****
>   	 * throat...''
>   	 *	-- The Who
>   	 */
> ! 	if ((p = strchr(line, '\n')) == NULL) {
> ! 		warnx("line too long");
> ! 		errno = EFTYPE;	/* XXX */
> ! 		error(pname);
> ! 	}
>   	*p = '\0';
>   	if (strcmp(line, "+") == 0)
> ! 		strcpy(line, "+:::::::::");	/* pw_scan() can't handle "+" */
> ! 	oflags = 0;
> ! 	if (!pw_scan(line, pw, &oflags)) {
> ! 		warnx("at line #%d", *lineno);
> ! 		errno = EFTYPE;	/* XXX */
> ! 		error(pname);
> ! 	}
> ! 	*flags = oflags;
>   
> ! 	return (1);
>   }
>   
>   void
>   install(const char *from, const char *to)
>   {
>   	char buf[MAXPATHLEN];
> - 	int sverrno;
>   
>   	snprintf(buf, sizeof(buf), "%s%s", prefix, to);
> ! 	if (rename(from, buf)) {
> ! 		sverrno = errno;
> ! 		(void)snprintf(buf, sizeof(buf), "%s to %s", from, buf);
> ! 		errno = sverrno;
> ! 		error(buf);
> ! 	}
> ! }
> ! 
> ! void
> ! wr_error(const char *str)
> ! {
> ! 	char errbuf[BUFSIZ];
> ! 	int sverrno;
> ! 
> ! 	sverrno = errno;
> ! 
> ! 	(void)snprintf(errbuf, sizeof(errbuf),
> ! 		"attempt to write %s failed", str);
> ! 
> ! 	errno = sverrno;
> ! 	error(errbuf);
> ! }
> ! 
> ! void
> ! error(const char *str)
> ! {
> ! 
> ! 	warn("%s", str);
> ! 	cleanup();
> ! #ifdef think_about_this_a_while_longer
> ! 	fprintf(stderr, "NOTE: possible inconsistencies between "
> ! 	    "text files and databases\n"
> ! 	    "re-run pwd_mkdb when you have fixed the problem.\n");
> ! #endif
> ! 	exit(EXIT_FAILURE);
>   }
>   
>   void
> --- 530,557 ----
>   	 * throat...''
>   	 *	-- The Who
>   	 */
> ! 	if ((p = strchr(line, '\n')) == NULL)
> ! 		errx(EXIT_FAILURE, "line %d: line too long", *lineno);
>   	*p = '\0';
> + 
> + 	/* pw_scan() can't handle "+". */
>   	if (strcmp(line, "+") == 0)
> ! 		strcpy(line, "+:::::::::");
>   
> ! 	if (!pw_scan(line, pw, flags)) 
> ! 		errx(EXIT_FAILURE, "line %d: parse error", *lineno);
> ! 
> ! 	return (0);
>   }
>   
>   void
>   install(const char *from, const char *to)
>   {
>   	char buf[MAXPATHLEN];
>   
>   	snprintf(buf, sizeof(buf), "%s%s", prefix, to);
> ! 	if (rename(from, buf))
> ! 		err(EXIT_FAILURE, "mv %s to %s", from, buf);
>   }
>   
>   void
> ***************
> *** 369,385 ****
>   cleanup(void)
>   {
>   
> ! 	switch(clean) {
> ! 	case FILE_ORIG:
> ! 		rm(oldpwdfile);
>   		/* FALLTHROUGH */
> ! 	case FILE_SECURE:
> ! 		rm(pwd_Sdb_tmp);
>   		/* FALLTHROUGH */
> ! 	case FILE_INSECURE:
> ! 		rm(pwd_db_tmp);
>   		/* FALLTHROUGH */
>   	}
>   }
>   
>   /*
> --- 566,587 ----
>   cleanup(void)
>   {
>   
> ! 	switch(state) {

A space after keyword.

> ! 	case STATE_DONE:
> ! 		return;
> ! 	case STATE_ORIG:
> ! 		rm(tmpv7fn);
>   		/* FALLTHROUGH */
> ! 	case STATE_SECURE:
> ! 		rm(tmpsdbfn);
>   		/* FALLTHROUGH */
> ! 	case STATE_INSECURE:
> ! 		rm(tmpidbfn);
>   		/* FALLTHROUGH */
>   	}
> + 
> + 	warnx("possible inconsistencies between text files and databases");
> + 	warnx("re-run pwd_mkdb when you have fixed the problem");
>   }
>   
>   /*
> ***************
> *** 398,440 ****
>   #define	COMPACT(e)	for (t = e; (*p++ = *t++) != '\0';)
>   
>   void
> ! putdbent(DB *dp, struct passwd *pw, const char *passwd, int flags,
> ! 	 const char *fn, int lineno)
>   {
> ! 	struct passwd pwd;
> ! 	char buf[MAX(MAXPATHLEN, LINE_MAX * 2)], tbuf[1024], *p;
>   	DBT data, key;
>   	const char *t;
>   	u_int32_t x;
>   	int len;
>   
> ! 	memcpy(&pwd, pw, sizeof(pwd));
>   	data.data = (u_char *)buf;
>   	key.data = (u_char *)tbuf;
>   
>   	if (lorder != BYTE_ORDER) {
> ! 		M_32_SWAP(pwd.pw_uid);
> ! 		M_32_SWAP(pwd.pw_gid);
> ! 		M_32_SWAP(pwd.pw_change);
> ! 		M_32_SWAP(pwd.pw_expire);
>   	}
>   
>   	/* Create insecure data. */
>   	p = buf;
> ! 	COMPACT(pwd.pw_name);
>   	COMPACT(passwd);
> ! 	memmove(p, &pwd.pw_uid, sizeof(pwd.pw_uid));
> ! 	p += sizeof(pwd.pw_uid);
> ! 	memmove(p, &pwd.pw_gid, sizeof(pwd.pw_gid));
> ! 	p += sizeof(pwd.pw_gid);
> ! 	memmove(p, &pwd.pw_change, sizeof(pwd.pw_change));
> ! 	p += sizeof(pwd.pw_change);
> ! 	COMPACT(pwd.pw_class);
> ! 	COMPACT(pwd.pw_gecos);
> ! 	COMPACT(pwd.pw_dir);
> ! 	COMPACT(pwd.pw_shell);
> ! 	memmove(p, &pwd.pw_expire, sizeof(pwd.pw_expire));
> ! 	p += sizeof(pwd.pw_expire);
>   	x = flags;
>   	if (lorder != BYTE_ORDER)
>   		M_32_SWAP(x);
> --- 600,642 ----
>   #define	COMPACT(e)	for (t = e; (*p++ = *t++) != '\0';)
>   
>   void
> ! putdbent(DB *db, struct passwd *pw, const char *passwd, int flags,
> ! 	 const char *fn, int lineno, int method)

You forget to pass this ``method'' to the database's put function.

>   {
> ! 	struct passwd mpw;

Um, what's wrong with ``pwd''?

> ! 	char buf[MAX_RECORD_SIZE], tbuf[MAX_KEY_SIZE], *p;
>   	DBT data, key;
>   	const char *t;
>   	u_int32_t x;
>   	int len;
>   
> ! 	memcpy(&mpw, pw, sizeof(mpw));
>   	data.data = (u_char *)buf;
>   	key.data = (u_char *)tbuf;
>   
>   	if (lorder != BYTE_ORDER) {
> ! 		M_32_SWAP(mpw.pw_uid);
> ! 		M_32_SWAP(mpw.pw_gid);
> ! 		M_32_SWAP(mpw.pw_change);
> ! 		M_32_SWAP(mpw.pw_expire);
>   	}
>   
>   	/* Create insecure data. */
>   	p = buf;
> ! 	COMPACT(mpw.pw_name);
>   	COMPACT(passwd);
> ! 	memmove(p, &mpw.pw_uid, sizeof(mpw.pw_uid));
> ! 	p += sizeof(mpw.pw_uid);
> ! 	memmove(p, &mpw.pw_gid, sizeof(mpw.pw_gid));
> ! 	p += sizeof(mpw.pw_gid);
> ! 	memmove(p, &mpw.pw_change, sizeof(mpw.pw_change));
> ! 	p += sizeof(mpw.pw_change);
> ! 	COMPACT(mpw.pw_class);
> ! 	COMPACT(mpw.pw_gecos);
> ! 	COMPACT(mpw.pw_dir);
> ! 	COMPACT(mpw.pw_shell);
> ! 	memmove(p, &mpw.pw_expire, sizeof(mpw.pw_expire));
> ! 	p += sizeof(mpw.pw_expire);
>   	x = flags;
>   	if (lorder != BYTE_ORDER)
>   		M_32_SWAP(x);
> ***************
> *** 444,455 ****
>   
>   	/* Store insecure by name. */
>   	tbuf[0] = _PW_KEYBYNAME;
> ! 	len = strlen(pwd.pw_name);
> ! 	memmove(tbuf + 1, pwd.pw_name, len);
>   	key.size = len + 1;
> ! 	if ((dp->put)(dp, &key, &data, R_NOOVERWRITE) == -1)
> ! 		wr_error(fn);
> ! 		
>   	/* Store insecure by number. */
>   	tbuf[0] = _PW_KEYBYNUM;
>   	x = lineno;
> --- 646,657 ----
>   
>   	/* Store insecure by name. */
>   	tbuf[0] = _PW_KEYBYNAME;
> ! 	len = strlen(mpw.pw_name);
> ! 	memmove(tbuf + 1, mpw.pw_name, len);
>   	key.size = len + 1;
> ! 	if ((db->put)(db, &key, &data, R_NOOVERWRITE) == -1)
> ! 		err(EXIT_FAILURE, "writing to %s", fn);
> ! 
>   	/* Store insecure by number. */
>   	tbuf[0] = _PW_KEYBYNUM;
>   	x = lineno;
> ***************
> *** 457,476 ****
>   		M_32_SWAP(x);
>   	memmove(tbuf + 1, &x, sizeof(x));
>   	key.size = sizeof(x) + 1;
> ! 	if ((dp->put)(dp, &key, &data, R_NOOVERWRITE) == -1)
> ! 		wr_error(fn);
>   
>   	/* Store insecure by uid. */
>   	tbuf[0] = _PW_KEYBYUID;
> ! 	memmove(tbuf + 1, &pwd.pw_uid, sizeof(pwd.pw_uid));
> ! 	key.size = sizeof(pwd.pw_uid) + 1;
> ! 	if ((dp->put)(dp, &key, &data, R_NOOVERWRITE) == -1)
> ! 		wr_error(fn);
>   }
>   
>   void
> ! putyptoken(DB *dp, const char *fn)
>   {
>   	DBT data, key;
>   
>   	key.data = (u_char *)__yp_token;
> --- 659,689 ----
>   		M_32_SWAP(x);
>   	memmove(tbuf + 1, &x, sizeof(x));
>   	key.size = sizeof(x) + 1;
> ! 	if ((db->put)(db, &key, &data, R_NOOVERWRITE) == -1)

* is used to deference pointer to function in other place.

> ! 		err(EXIT_FAILURE, "writing to %s", fn);
>   
>   	/* Store insecure by uid. */
>   	tbuf[0] = _PW_KEYBYUID;
> ! 	memmove(tbuf + 1, &mpw.pw_uid, sizeof(mpw.pw_uid));
> ! 	key.size = sizeof(mpw.pw_uid) + 1;
> ! 	if ((db->put)(db, &key, &data, R_NOOVERWRITE) == -1)
> ! 		err(EXIT_FAILURE, "writing to %s", fn);
> ! }
> ! 
> ! void
> ! putv7ent(FILE *fd, struct passwd *pw, const char *fn)
> ! {
> ! 
> ! 	fprintf(fd, "%s:*:%d:%d:%s:%s:%s\n", pw->pw_name, pw->pw_uid,
> ! 	    pw->pw_gid, pw->pw_gecos, pw->pw_dir, pw->pw_shell);
> ! 	if (ferror(fd))
> ! 		err(EXIT_FAILURE, "writing to %s", fn);
>   }
>   
>   void
> ! putyptoken(DB *db, const char *fn)
>   {
> + 	extern const char __yp_token[];		/* XXX From libc. */
>   	DBT data, key;
>   
>   	key.data = (u_char *)__yp_token;
> ***************
> *** 478,491 ****
>   	data.data = (u_char *)NULL;
>   	data.size = 0;
>   
> ! 	if ((dp->put)(dp, &key, &data, R_NOOVERWRITE) == -1)
> ! 		wr_error(fn);
>   }
>   
>   void
>   usage(void)
>   {
>   
> ! 	(void)fprintf(stderr, "usage: pwd_mkdb [-pBL] [-d directory] file\n");
>   	exit(EXIT_FAILURE);
>   }
> --- 691,706 ----
>   	data.data = (u_char *)NULL;
>   	data.size = 0;
>   
> ! 	if ((db->put)(db, &key, &data, R_NOOVERWRITE) == -1)
> ! 		err(EXIT_FAILURE, "writing to %s", fn);
>   }
>   
>   void
>   usage(void)
>   {
> + 	extern char *__progname;
>   
> ! 	fprintf(stderr, "usage: %s [-pBL] [-d directory] [-u user] file\n",
> ! 	    __progname);
>   	exit(EXIT_FAILURE);
>   }
>