write(1): Capsicumify

[cem]

Summary

Enter Capsicum capability sandbox pretty early in this setuid program.

Some minor modifications were needed to cache directory fds and use
relative lookups.

Test Plan

• truss write <myuser>

Note that one feature is still broken: gethostname(3) doesn't work because
sysctl(kern.hostname) is forbidden. Probably that could just be cached before
entering the sandbox.

[ed]

usr.bin/write/write.c

270 ↗

(On Diff #20601)

What about getting rid of path entirely? If we really want to print the full pathname, we can just use warn("%s%s", _PATH_DEV, tty).

303 ↗

(On Diff #20601)

Same here.

[cem]

• Remove sprintf'ed full path

usr.bin/write/write.c

270 ↗

(On Diff #20601)

Sure. I didn't feel the need to rototill working code — aiming for minimal diff — but there's nothing wrong with removing this.

[emaste]

usr.bin/write/write.c

92–94 ↗

(On Diff #20653)

nitpick, this file uses the

if ((devfd = open(...)) < 0)
        err(...);

style (as with most of userland)

115–116 ↗

(On Diff #20653)

@ed's setutxent() comment from D8001
should drop the endutxent?

301 ↗

(On Diff #20653)

should we close old stdout fd?

[cem]

• Remove paired endutxent
• Close old stdout stream (doesn't matter too much)

usr.bin/write/write.c

92–94 ↗

(On Diff #20653)

Not anymore it doesn't.

[ed]

usr.bin/write/write.c

50 ↗

(On Diff #20659)

Do you by any chance know what this header file is used for? I've seen some applications outside of base also use this, but for some reason, nothing useful is ever used. Fairly annoying, as it's not part of POSIX and is only there to make the build on CloudABI fail. ;-)

115–116 ↗

(On Diff #20653)

In this case it may be exactly what we want. The write(1) utility only has to traverse the active sessions table, which is opened by setutxent(). The utility provides no way to override the pathname to the database, which is also what you'd want from a setgid utility.

[ed]

usr.bin/write/write.c

291 ↗

(On Diff #20659)

Wait. This code will no longer work when sandboxed, right?

[cem]

usr.bin/write/write.c

50 ↗

(On Diff #20659)

No idea. write.c compiles fine without the explicit inclusion of sys/file.h. (I had just moved it while sorting includes in this patch.)

291 ↗

(On Diff #20659)

getlogin() should work; getpwuid() does not.

[ed]

usr.bin/write/write.c

291 ↗

(On Diff #20659)

Exactly. So we should move this code out of do_write() into main(), so that the username is obtained prior to calling cap_enter().

[emaste]

Also, can now make use of capsicum helpers added in rS306657

[cem]

• Use capsicum helpers for caching catpages/tzdata
• Capsicum helpers were not especially useful for this one. To grab ttyname we need a privilege not in the default stdio set.
  • Maybe caph_ routines with variadic extra rights would work well?
  • And maybe a CAPH_ flag to skip restricting ioctls.
• Move login (username) fetch out of capability mode so it works, per ed@.

[ed]

usr.bin/write/write.c

69 ↗

(On Diff #21097)

If you want, you can just sort this #include with the others above. The list already contains BSD-specific headers (<err.h>, <paths.h>), so there is no need to keep this one separate.

80 ↗

(On Diff #21097)

Instead of introducing global state, could you please just add function arguments for these? Thanks!

[cem]

usr.bin/write/write.c

69 ↗

(On Diff #21097)

Sure; I figured the rest were part of libc, while this was separate. But my assumption might be false anyway. I don't feel strongly about it. :)

[cem]

usr.bin/write/write.c

80 ↗

(On Diff #21097)

This adds a lot of (IMO useless) churn because of all of the sites that need devfd.

usr.bin/write/write.c | 29 ++++++++++++++---------------
1 file changed, 14 insertions(+), 15 deletions(-)

Both of these globals are write-once.

[cem]

• Put capsicum_helpers header with other userspace ones
• Plumb devfd/login globals through a bunch of subroutines for no good reason