Page MenuHomeFreeBSD

bspatch: enter capability mode after dropping fd rights
AbandonedPublic

Authored by emaste on Sep 24 2020, 2:15 PM.

Details

Reviewers
None
Group Reviewers
capsicum
Summary

This change has no functional difference, but this order might discourage mistakes in the future.

One could argue that entering capability mode as early as possible is preferred. However, a future developer may assume the`cap_enter()` indicates where the sandbox has been entered and is ready for potentially unsafe operations, so I feel it makes sense to have fd rights limited before that point.

Diff Detail

Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

emaste created this revision.
emaste edited the summary of this revision. (Show Details)
emaste added a subscriber: cperciva.
oshogbo added inline comments.
usr.bin/bsdiff/bspatch/bspatch.c
173

Is there any issue with that?
You can limit descriptors after entering the sandbox.

usr.bin/bsdiff/bspatch/bspatch.c
173

See the review description above - there's no issue with it as is, but IMO it's more clear to enter capability mode after limiting fd rights, because the cap_enter() servers as a marker that we're now in the sandbox and may use potentially-unsafe code.

pjd added inline comments.
usr.bin/bsdiff/bspatch/bspatch.c
173

To be honest, even after reading the reasoning behind this change I still would prefer to enter capability mode as soon as possible. Entering a sandbox doesn't have to be a one-step process. For example:

  1. Enter capability mode.
  2. Limit descriptors 1&2.
  3. Do some work.
  4. Limit descriptors 3&4; close descriptor 2.
  5. Do more work.
  6. Limit descriptor 4 even further.

etc.
I'd encourage this kind of examples wherever possible.
Sandboxing is about limiting TCB, so in my opinion entering the sandbox as soon as possible and then limit further is a better example.