Page MenuHomeFreeBSD

inetd: a light introduction to capsicum
Needs ReviewPublic

Authored by kevans on Jan 3 2020, 4:32 PM.

Details

Reviewers
markj
oshogbo
Summary

This isn't a complete solution, but perhaps roughly the first half of one. This should effectively limit all fds opened by inetd, even those we hook up to child stdin/out/err, and enters capability mode for some of the builtin handlers.

I think the other half of the solution is to devise a syntax for inetd.conf to indicate that we should go ahead and enter capability mode for non-builtins.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Lint Skipped
Unit
Unit Tests Skipped
Build Status
Buildable 28460

Event Timeline

Thank you for working on this!
This doesn't seems like simple peace of code.

usr.sbin/inetd/builtins.c
214

Is any of those time can be run in the capability program?
If so does we need caph_cache_tzdata ?

usr.sbin/inetd/inetd.c
348

Up to you: but this can be writen shorten:
caph_rights_limit(signalpipe[1], cap_rights_init(&sigtxpipe_rights, CAP_WRITE));

378

This is my private preference, so it's up to you what you will do with it:
I just prefer to have a fixed list of capability rights for the descriptors other ways it's getting compacted to say from the code what capabilities they are.

423

Why don't lmit it in the first if?

912

We don't need a caph_cache_catpages ?

916

Should we use casper service here for the syslog?

Thank you for working on this!
This doesn't seems like simple peace of code.

Thanks for the review! Unfortunately it is a bit windy, and I'm not all that experienced with capsicum'izing things, but we'll get there. =-)

usr.sbin/inetd/builtins.c
214

Nope- those ones are confined to builtins that don't fork

usr.sbin/inetd/inetd.c
378

So, the problem is that I don't want to be duplicating the caph_stream_rights() because then that would need to be maintained, but I need them since the invoked program may attempt to caph_limit_stdio().

423

Will do

916

This branch doesn't get hit in the caph_enter() path, but both are things we'll need to consider going forward (that I hadn't previously) when we allow inetd.conf specifying that a service should be capsicumized.

This looks like a good start. If you are not already doing so, please try testing with kern.trap_enotcap set to 1.

usr.sbin/inetd/builtins.c
79

I'd consider combining bi_fork and bi_capenter into a bi_flags variable. That'd make it easier to read the table.

84

Why do we not support cap mode for any of the UDP services? Seems like discard_dg should be trivial.

634

It reads a bit strangely to check fakeid_fd == -1 after calling caph_limit_stream(). You might consider extending the existing if-statement instead, i.e.

if (fakeid_fd == -1 ||
    caph_limit_stream(...) == -1 ||
    fstat(...) == -1 ||
    ...
usr.sbin/inetd/inetd.c
907

This could be collapsed into the following if-statement (for now). You are duplicating the sep->se_bi != NULL test.