On Wed, Dec 09, 2020 at 06:28:10PM +0100, Stefan Sperling wrote:
> On Wed, Dec 09, 2020 at 09:00:00AM -0800, Yang Zhong wrote:
> > Here's the updated diff. With regard to committing, am I OK to
> > commit the initial patch (the one that adds the fd field to got_worktree)
> > at this point?
> 
> I didn't realize the new patch you posted wasn't self-contained.
> 
> You mean the initial patch quoted below?
> 
> My point of view on this is that you should not actually need most of
> these changes if you call cap_enter() after got_worktree_open() and
> got_repo_open() are done.
> Adding a root_fd to struct got_worktree is fine, of course. I would suggest
> to roll a new patch that is self-contained and doesn't assume that your
> sandbox is active during got_worktree_init/open() and got_repo_open().

To clarify, I imagine this could work somewhat as follows:

got_worktree_init() stays as-is (it does not need sandbox protection
since all the data involved is created locally.)

open_worktree() opens the wt_fd and caches it in struct got_worktree,
but otherwise doesn't use it. All the read_meta_file() calls etc. stay
as they were (no need for sandbox protection).

This should allow capsicum to access worktree->root_fd after
got_open_worktree() is done, and configure appropriate rights on the fd.

Once the work tree has been opened, and the repository fd and tmpfd have
also been opened, the code can enter a capsicum sandbox.

It looks to me like this could work.
Perhaps I am missing something?

I am suggesting this simply in an effort to keep the amount of required
changes small. I'd be fine with the original diffs if my suggestions
somehow conflict with what capsicum needs.