Subject: Re: Using a +t /tmp for chpass(1)
To: None <tech-security@netbsd.org>
From: Christos Zoulas <christos@astron.com>
List: tech-security
Date: 10/09/2006 17:49:14
In article <452A864E.8050006@NetBSD.org>, Elad Efrat  <elad@NetBSD.org> wrote:
>Hi,
>
>Few days ago we discussed a change on #NetBSD-code that made it to our
>chpass(1), and someone suggested I bring it up on a public list such
>as this "just to make sure" -- so I am.
>
>We used to write the temporary files, pw.XXXXX, to /etc, which created
>a problem: a user who started chpass(1) could ^Z it, then SIGKILL it,
>leaving a temporary file in /etc, slowly filling it up.

I think that chpass should obey the /etc/ptmp lock.

>Solutions discussed varied from kernel changes to limit signals to
>set-id processes (so that, for example, SIGKILL can't be sent; like
>how we limit coredumps of set-id processes), but eventually we chose
>a different solution.
>
>What we did is this:
>
>	static char tempname[] = "/tmp/pw.XXXXXX";
>
>	[...]
>
>	dfd = mkstemp(tempname);
>	if (dfd < 0 || fcntl(dfd, F_SETFD, 1) < 0)
>		(*Pw_error)(tempname, 1, 1);
>	if (atexit(cleanup)) {
>		cleanup();
>		errx(1, "couldn't register cleanup");
>	}
>	if (stat(dirname(tempname), &sb) == -1)
>		err(1, "couldn't stat `%s'", dirname(tempname));
>	if (!(sb.st_mode & S_ISTXT))
>		errx(1, "temporary directory `%s' is not sticky",
>		    dirname(tempname));
>
>	display(tempname, dfd, pw);
>	edit(tempname, pw);
>
>Where we first create the temporary file using mkstemp(), guaranteeing
>we get a newly created temporary file with 0600 permissions, avoiding
>TOCTOU races. We then check to see that our temp dir, /tmp (hard-coded)
>is +t, so that nobody but us (and root) can modify the file.
>
>Since it seems every operating system has its own way of doing this, I
>ended up agreeing it's only reasonable to bring this up to see if any
>of the folks who wasn't present during the Bugathon and/or didn't read
>the log message can point out anything obviously flawed we did.
>
>So, here goes -- is the above okay?

The problem with editing the password file and creating a temp file
in /tmp is data loss from concurrent edits. How does this scheme
prevent this? Let's say I am editing the password file using vipw.
You run chpass; I save my edit; your changes are lost.

christos