Page MenuHomeFreeBSD

Capsicumize strings
Needs ReviewPublic

Authored by oshogbo on Nov 18 2018, 3:08 PM.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

oshogbo created this revision.Nov 18 2018, 3:08 PM
oshogbo updated this revision to Diff 50563.Nov 18 2018, 3:12 PM

Simplify.

oshogbo updated this revision to Diff 50564.Nov 18 2018, 3:13 PM

Simplify #2.

cem accepted this revision.Nov 18 2018, 11:38 PM
cem added inline comments.
contrib/elftoolchain/strings/strings.c
200 ↗(On Diff #50564)

I would maybe print "unable to initialize casper fileargs."

I expect emaste will say, "what do we expect a user to do differently?", and from that perspective, sure, the two failures are essentially the same problem. But as a developer who may occasionally debug capsicumized programs, I greatly prefer that error messages are unique in code, so it is clear where the fault occurred.

I do not feel strongly about it.

204 ↗(On Diff #50564)

Why free before err() exit?

218 ↗(On Diff #50564)

Why free before return exit? :-)

usr.bin/strings/Makefile
13–17 ↗(On Diff #50564)

If !defined(WITH_CASPER), does caph_enter_casper() not enter the capsicum sandbox?

(It seems that this program will be totally broken if the sandbox is entered with MK_CASPER=no.)

This revision is now accepted and ready to land.Nov 18 2018, 11:38 PM

As with the other strings reviews, adding @kaiw and @jkoshy_users.sourceforge.net to CC

I think we want to rework this change slightly so that it could be changed upstream and still buid out of the box on Linux and older FreeBSD.

contrib/elftoolchain/strings/strings.c
200 ↗(On Diff #50564)

That is what I might say, but I don't feel strongly either.

emaste added inline comments.Nov 23 2018, 2:04 AM
contrib/elftoolchain/strings/strings.c
202–203 ↗(On Diff #50564)

@jkoshy_users.sourceforge.net this is the "sandbox setup & enter" entry; for OpenBSD this would be a pledge() call.

The fileargs_init() and fileargs_fopen() are the magic bits that perform the privilege separated file opening and fd shuffling that do not have an equivalent on !FreeBSD.

Also depends on D18037

Adding @arichardson for comment on Linux bootstrapping.

oshogbo added inline comments.Mar 21 2019, 6:40 AM
contrib/elftoolchain/strings/strings.c
200 ↗(On Diff #50564)

Ok I will address that.

204 ↗(On Diff #50564)

It's just nice to clean up after yourself, and I wanted to do another good example just to fallow.

218 ↗(On Diff #50564)

It's just nice to clean up after yourself, and I wanted to do another good example just to fallow.

usr.bin/strings/Makefile
13–17 ↗(On Diff #50564)

Yes.

This revision was automatically updated to reflect the committed changes.
reg reopened this revision.Jun 17 2019, 5:48 PM
reg added a subscriber: reg.

Fails with WITHOUT_CAPSICUM...

In D18038#446790, @reg wrote:

Fails with WITHOUT_CAPSICUM...

Please submit a PR with the build failure

markj added inline comments.Tue, Jul 9, 1:16 AM
head/contrib/elftoolchain/strings/strings.c
203

This should be ||.