Add a new parameter to restrict jails from binding to privileged ports
ClosedPublic

Authored by allanjude on Mar 30 2017, 3:20 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.
allanjude created this revision.Mar 30 2017, 3:20 PM
smh accepted this revision.Mar 30 2017, 3:23 PM

Some style nits but otherwise LGM

sys/kern/kern_jail.c
3314 ↗(On Diff #26830)

Personally I would remove the else, but this follows the existing file style.

usr.sbin/jail/config.c
101 ↗(On Diff #26830)

missing tab?

This revision is now accepted and ready to land.Mar 30 2017, 3:23 PM
cem accepted this revision.Mar 30 2017, 4:01 PM

LGTM, modulo the same nits as smh@.

allanjude marked 2 inline comments as done.Mar 30 2017, 11:07 PM
allanjude added inline comments.
sys/kern/kern_jail.c
3314 ↗(On Diff #26830)

keeping this as-is, as it the style in the rest of this switch block

usr.sbin/jail/config.c
101 ↗(On Diff #26830)

it does line up with all of the other tabs, but fixing anyway

allanjude updated this revision to Diff 26848.Mar 30 2017, 11:08 PM
allanjude marked 2 inline comments as done.

Fix style nits from smh@

This revision now requires review to proceed.Mar 30 2017, 11:08 PM
cem accepted this revision.Mar 30 2017, 11:12 PM
cem added inline comments.
usr.sbin/jail/config.c
101 ↗(On Diff #26848)

Was this really too wide?

This revision is now accepted and ready to land.Mar 30 2017, 11:12 PM
allanjude added inline comments.Mar 30 2017, 11:29 PM
usr.sbin/jail/config.c
101 ↗(On Diff #26848)

If you look at the file in !phabricator, all of the 0s line up perfectly at the tab stop. due to the length of the allow MIB, there was no room for a tab without shifting to the next tab stop. so I did what the file had done for exec.system_jail_user

smh accepted this revision.Mar 30 2017, 11:58 PM
jamie added a comment.Mar 31 2017, 2:42 AM

You don't need the KP_ALLOW_RESERVED_PORTS in jailp.h and config.c - you can just leave these files untouched. The KP_* defines are for parameters that are internally referenced somewhere within jail(8). That includes most of the allow.* parameters, only to handle back-compatibility with the security.jail.*_allowed sysctls.

The other changes (the kernel part) look good.

allanjude updated this revision to Diff 27216.Apr 8 2017, 1:38 AM

Update with feedback from jamie@

This revision now requires review to proceed.Apr 8 2017, 1:38 AM
jamie accepted this revision.Apr 8 2017, 3:25 AM
This revision is now accepted and ready to land.Apr 8 2017, 3:25 AM
wblock added a subscriber: wblock.May 19 2017, 4:09 PM

Please update .Dd also.

This revision was automatically updated to reflect the committed changes.