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