Remove chroot(2) call from dhclient.

[markj]

Summary

dhclient runs in capability mode, so its filesystem access is already
restricted. Moreover, libutil keeps open a directory descriptor for the
pidfile (/var/run by default). If kern.chroot_allow_open_directories is
not set, this descriptor will cause the chroot to fail. To fix this,
stop using chroot(), and store pidfiles under /var/run/dhclient/ instead
to restrict the scope of libutil's descriptor. This is not perfect
since different dhclient instances can still mess with each other's
pidfiles despite the use of capability mode, but I don't think that this
is a serious problem.

Also stop removing rights from the pidfile descriptor after writing the
PID. This interferes with pidfile_verify(), called from
pidfile_close(). We already restrict rights to the pidfile somewhat in
libutil; one possible follow-up enhancement would be drop the
CAP_PWRITE and CAP_TRUNCATE rights in pidfile_write(). Of course, this
would mean that pidfile_write() could only be called once, but that's
most likely fine.

[cem]

sbin/dhclient/dhclient.c

422 ↗

(On Diff #46267)

For whatever reason, _PATH_VARRUN has already got a trailing slash. Not that it matters too much for lookup, and I agree this is more clear.

I think spelling it without the %s might also be more clear?

asprintf(..., _PATH_VARRUN "/...", *argv)

The uppercase name implies it is a macro, rather than some extern variable. I'm not sure if that makes it reasonable to expect it to be a string literal or not.

531–534 ↗

(On Diff #46267)

I might have missed earlier discussion, but why not just wrap this in if (cap_getmode(&dummy) != 0) {? (I.e., kernel does not support CAPABILITY_MODE.)

[markj]

sbin/dhclient/dhclient.c

531–534 ↗

(On Diff #46267)

Hmm, we still have to handle the original problem then.

... is there any chance capsicum support will become non-optional at some point? :)

[cem]

(I guess I already marked "approved" so Phab won't let me again. But I just want to be clear that in spite of my line comments, I approve this change as-is.)

sbin/dhclient/dhclient.c

531–534 ↗

(On Diff #46267)

Given we're already creating /var/run/dhclient for capsicum, could we also use that for the chroot in place of /var/empty? The pid file doesn't seem especially sensitive for an attacker.

But I'm confused. How does chrooted (or capsicum'd) dhclient access its leases file in /var/db today?

$ sudo procstat -f $(cat /var/run/dhclient.igb0.pid)
  PID COMM                FD T V FLAGS    REF  OFFSET PRO NAME
  565 dhclient          text v r r-------   -       - -   /sbin/dhclient
  565 dhclient           cwd v d r-------   -       - -   /var/empty
  565 dhclient          root v d r-------   -       - -   /var/empty
  565 dhclient          jail v d r-------   -       - -   /var/empty
  565 dhclient             0 v c rw------   6       0 -   /dev/null
  565 dhclient             1 v c rw------   6       0 -   /dev/null
  565 dhclient             2 v c rw------   6       0 -   /dev/null
  565 dhclient             3 v d r----n--   2       0 -   /var/run
  565 dhclient             4 s - rw---n--   2       0 UDS 0 0 -
  565 dhclient             5 v r -w---n-l   2       0 -   /var/run/dhclient.igb0.pid
  565 dhclient             7 v c r-------   1   12880 -   /dev/bpf
  565 dhclient             8 s - rw------   1       0 ?
  565 dhclient             9 v r -w------   1    5495 -   /var/db/dhclient.leases.igb0
  565 dhclient            11 p - rw------   1       0 -   -

... is there any chance capsicum support will become non-optional at some point? :)

Yeah, that's the question. I expect laptop-class systems and better to have capsicum non-optionally. (I thought that was planned for 12, actually.)

But I am imagining small embedded systems (MIPS/riscv/arm32, e.g., routers) might not want to pay any penalty for capsicum, and still have some protection for dhclient. That said, I don't have any quantified sense of how costly capsicum support is.

I'm willing to assume we'll just require capsicum on everywhere and therefore the chroot can be discarded.

Do you have a good idea of how the capsicum sandboxing interacts with both the pidfile, /dev/bpf, and the lease file today?

[emaste]

I thought that was planned for 12, actually.

Capsicum is generally enabled in GENERIC config files now (for some time); there is an open question about removing the #ifdefs and kernel config goo.

[cem]

In D16584#352320, @emaste wrote:

I thought that was planned for 12, actually.

Capsicum is generally enabled in GENERIC config files now (for some time); there is an open question about removing the #ifdefs and kernel config goo.

What would it take to also enable in the myriad non-GENERIC embedded board configs, in a way that doesn't make them useless or annoy their users/developers? I think that's probably a necessary step towards removing the config(8) option and ifdefs entirely? If not adding the option to configs, at least researching what the pain level will be.

[markj]

In D16584#352320, @emaste wrote:

I thought that was planned for 12, actually.

Capsicum is generally enabled in GENERIC config files now (for some time); there is an open question about removing the #ifdefs and kernel config goo.

FWIW, I've hit this issue before when modifying an application to use Capsicum, and ended up making lack of support a fatal error. The lack of mandatory support makes Capsicum harder to use.

sbin/dhclient/dhclient.c

531–534 ↗

(On Diff #46267)

Given we're already creating /var/run/dhclient for capsicum, could we also use that for the chroot in place of /var/empty? The pid file doesn't seem especially sensitive for an attacker.

The issue is that we'd have to modify pidfile(3) to close the directory descriptor.

Do you have a good idea of how the capsicum sandboxing interacts with both the pidfile, /dev/bpf, and the lease file today?

For pidfiles, the only requirement today is that you call pidfile_open() before entering capability mode. pidfile_open() keeps rights to both the pidfile and the directory containing the pidfile. The latter is so that we can unlink the pidfile in capability mode when exiting.

dhclient uses some privilege separation and has a non-sandboxed process which can send raw packets. I'm not sure how it manages lease files.

[markj]

Fall back to a chroot if we are not sandboxed after calling
caph_enter_casper().

Document the problem with chroot_allow_open_directories in
the code and man page. Because capsicum is enabled in most
kernel configurations and chroot_allow_open_directories defaults
to 1, the scope of the problem reported in PR 223327 is
reduced significantly. If Capsicum is made non-optional,
the problem goes away.

[oshogbo]

sbin/dhclient/dhclient.c

543 ↗

(On Diff #46289)

I would add some comment here because this is not obvious why after entering cap_mode we check the mode.