Enter Capsicum capability sandbox pretty early in this setuid program.
Some minor modifications were needed to cache directory fds and use
relative lookups.
Details
Details
- Reviewers
emaste allanjude ed oshogbo - Commits
- rS306761: write(1): Capsicumify
- 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.
Diff Detail
Diff Detail
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 5472 Build 5689: arc lint + arc unit
Event Timeline
Comment Actions
- Remove sprintf'ed full path
| usr.bin/write/write.c | ||
|---|---|---|
| 285 | Sure. I didn't feel the need to rototill working code — aiming for minimal diff — but there's nothing wrong with removing this. | |
Comment Actions
- Remove paired endutxent
- Close old stdout stream (doesn't matter too much)
| usr.bin/write/write.c | ||
|---|---|---|
| 95–97 | Not anymore it doesn't. | |
| usr.bin/write/write.c | ||
|---|---|---|
| 50 | 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. ;-) | |
| 118–119 | 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. | |
| usr.bin/write/write.c | ||
|---|---|---|
| 304–305 | Wait. This code will no longer work when sandboxed, right? | |
| usr.bin/write/write.c | ||
|---|---|---|
| 304–305 | Exactly. So we should move this code out of do_write() into main(), so that the username is obtained prior to calling cap_enter(). | |
Comment Actions
- 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@.
| usr.bin/write/write.c | ||
|---|---|---|
| 69 | 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 | Instead of introducing global state, could you please just add function arguments for these? Thanks! | |
| usr.bin/write/write.c | ||
|---|---|---|
| 69 | 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. :) | |
| usr.bin/write/write.c | ||
|---|---|---|
| 80 | This adds a lot of (IMO useless) churn because of all of the sites that need devfd. usr.bin/write/write.c | 29 ++++++++++++++--------------- Both of these globals are write-once. | |
Comment Actions
- Put capsicum_helpers header with other userspace ones
- Plumb devfd/login globals through a bunch of subroutines for no good reason