Capsicum support for jot(1)

[cem]

Summary

Limit descriptors and enter capability mode in jot(1)

[rwatson]

For some reason, phabricator is not willing to show me full context on this file -- whereas it did for your elfdump patch. I wonder why?

[brueffer]

No changes, trying to get diff context working.

[rwatson]

Overall I like these changes a lot, but had a few questions about implied need for ambient authority relating to random number generation and localisation.

usr.bin/jot/jot.c

120 ↗

(On Diff #3011)

It makes me slightly nervous that we need CAP_WRITE | CAP_FSTAT | CAP_IOCTL for stdout, but only CAP_WRITE for stderr.

288 ↗

(On Diff #3011)

I guess srandom() doesn't access /dev/random (unlike srandomdev()), but it would be good to convince ourselves that it is able to properly initialise in capability mode. (I seem to recall we do allow access to the arc4random sysctl from capability mode for this reason .. but maybe srandom() doesn't use that?)

397 ↗

(On Diff #3011)

Is there any risk that various locale-related bits might need ambient authority to initialise?

[eadler]

usr.bin/jot/jot.c

397 ↗

(On Diff #3011)

why would they? Its just a table lookup?

[jonathan]

usr.bin/jot/jot.c

288 ↗

(On Diff #3011)

srandom() calls good_rand() (pure computation - lib/libc/stdlib/random.c:219), and random() (which only calls good_rand()). So, it's entirely numerical computation based on the provided seed.

On a side note, srandomdev() also seems to use sysctl() rather than opening /dev/random as a file (hooray!).

[brueffer]

kib was vehemently opposed, so I left it for now. Maybe something to discuss at BSDCan.

[jonathan]

So... any progress on @rwatson's CAP_IOCTL comment?

[jonathan]

I think we need an answer to the CAP_IOCTL question before this change proceeds further.

usr.bin/jot/jot.c

288 ↗

(On Diff #3011)

(so I believe that we can call this comment "done")

397 ↗

(On Diff #3011)

@brueffer: any response?

[cem]

• Restrict stderr identically to stdout.
• Preopen NLS database(s) for err() usage after cap_enter().

usr.bin/jot/jot.c

318 ↗

(On Diff #3011)

Note arc4random -> arc4_stir attempts to fall back to /dev/random if the kern.arandom sysctl fails. (I don't know why it would fail; maybe it's a fallback that can be removed.)

I don't think this is a problem (the sysctl is used first).

397 ↗

(On Diff #3011)

Yes. err and friends need the localization database(s) opened.

[jonathan]

LGTM

[rwatson]

usr.bin/jot/jot.c

318 ↗

(On Diff #3011)

We should ensure that srandomdev() failing leads to an application fail stop. Right now, srandomdev() does not have a return value -- which means we must ensure that libc triggers application exit if both the sysctl and /dev/random fail. This is especially important in Capsicum, where the latter is guaranteed to fail!

[emaste]

usr.bin/jot/jot.c

318 ↗

(On Diff #3011)

See D8077

[emaste]

usr.bin/jot/jot.c

318 ↗

(On Diff #3011)

And now D8133 for srandomdev.

[emaste]

usr.bin/jot/jot.c

114 ↗

(On Diff #20507)

Code often makes assumptions about fd 0/1/2. We may want to limit stdin to no rights rather than closing it (cap_rights_limit(STDIN_FILENO, cap_rights_init(&rights));) as a pattern to follow, even if it doesn't matter for jot.

See for example rS306056 in elfdump.

[cem]

usr.bin/jot/jot.c

114 ↗

(On Diff #20507)

Sure.

[emaste]

LGTM either way

[cem]

• Update to use capsicum_helpers header.
• Restrict stdin rather than closing it.