Re: Daily reports: Friday (threaded setuid testing)

[Rich Felker <dalias@aerifal.cx>]

On Sat, Jul 09, 2011 at 03:53:01PM +0400, Solar Designer wrote:
> Luka, Rich -
> 
> On Sat, Jul 09, 2011 at 12:41:02AM +0200, Luka Mar??eti?? wrote:
> > I wanted to move on to task number 8, but I had some questions. I asked 
> > Rich via XMPP about them, but I guess he's still out.
> 
> OK, let's wait for Rich's comments on this.  BTW, chances are that the
> RLIMIT_NPROC check on setuid(2) and friends will be removed from future
> kernels: http://www.openwall.com/lists/kernel-hardening/2011/07/06/8

I wonder if I should jump in that thread and mention that, even if the
application checks the return value of setuid, it can be wrong for
multi-threaded applications... Or at least I believe it can. I think
proving that it can be wrong with glibc would still be a really nice
task to get done in cluts, and it would establish the usefulness of
cluts to the community outside of just advancing musl towards 1.0.

Luka, to answer your questions from chat:

1. Yes, an otherwise-unused account (actually just uid number; it
doesn't need any files, homedir, etc.) is required for testing. This
should be configurable eventually, but hard-coding some oddball value
near the max uid would probably be sufficient for now.

2. An ordinary user can lower their RLIMIT_NPROCS, but you'll be doing
this before dropping root, not after.

3. Each thread does not need to call setuid (the library function);
rather setuid (the library function itself) jumps through hoops to
make every thread call the setuid syscall. The unfortunate situation
is that each thread's syscall can independently succeed or fail, and
glibc only reports the status of one of them (the last one, I believe,
which is also the thread that called the setuid library function).

4. Your goal (at least for catching the bug believed to exist in glibc
and which musl hopefully does not shared) is to cause some of the
setuid syscalls to fail, but the last one to succeed. This will
require very [un]lucky timing, so the test probably needs to run many
times.

- RLIMIT_NPROCS should be greater than the number of threads you'll be
  testing with.

- When setuid (the library function) is first called, the number of
  processes with the target uid should already be equal (or just
  under) RLIMIT_NPROCS due to having forked some children in advance
  and set their uid to the target uid.

- These children should attempt to terminate themselves after the
  parent (with all its threads) calls setuid, but before all threads
  get scheduled and have a chance to make the setuid syscall. This
  way, some threads may fail, but the last one should succeed.

- After setuid (library function) returns, you check its return value
  and have each thread call getuid to check that they all have the
  target uid or that the library function returned failure. If the
  library function returned success but some threads still have uid=0,
  this is a potentially-serious security bug.

Does this make sense? By the way, there's no exact science to when the
children should terminate. The best I can think of is to run the test
repeatedly with varying (extremely short) delays, probably just
spinning in userspace or making a single call to sched_yield. You
might also want to vary the number of threads, and note that it may be
easier or harder with the number very small (e.g. equal to the number
of cores).

> I understand that Rich's proposed tests are about the libc wrapper
> functions that are thread-aware rather than about syscalls, yet I felt
> the above was relevant to the tests.

If the syscalls were correct (process-global) it would be a non-issue,
and if they could not fail due to RLIMIT_NPROC, it would also be a
non-issue. But as the kernel currently works (or rather doesn't work),
the userspace has to put a lot of effort into changing uids safely...

Rich