Re: [gentoo-dev] rfc: ideas for fixing OpenRC checkpath issue

[William Hubbs]

On Thu, Jan 18, 2018 at 07:19:59PM -0500, Michael Orlitzky wrote:
> Not at all. I'm working this out as I go, so better to speak up if
> something looks fishy.
>
> There are a few risks that I see with the first approach...
>
>
> Risk #1: From what I can tell, the current implementation of checkpath
> first creates the path, and then adjusts the ownership and permissions
> on it. After /run/foo/bar has been created but before the permissions
> and ownership are changed, the "foo" user can replace "bar" with a hard
> link (because he owns /run/foo). The lchown/chmod will operate on the
> target of that link, and if he's fast enough, then the "foo" user can
> use that to take over root's files. (Limited to /run in this example,
> but that's beside the point.)
>
> Maybe that can be avoided. Is there a portable way to atomically create
> a file/directory with its ownership and permissions already set? Or is
> it possible to operate on the descriptor that you get back when creating
> the path?

Permissions are set as part of the mkdir/open/mkfifo call, so they are
set immediately when the file is created. but ownership is adjusted
separately by calling chown/fchown/lchown.

At a high level, checkpath looks like this:

1. Creating and setting permissions.
a. If the file doesn't exist, set the permissions when it is created.
b. If the file does exist, only fix the permissions if necessary.
c. At this point, the file's permissions are correct.
2. setting ownership
a. always set the ownership if the file's ownership doesn't match the
specified ownership.

It looks like we can't use your --as suggestion if we want to be
able to create paths in /var/lib and /var/spool that are owned by
non-privileged users because of the permissions on those paths. It is
possible that service scripts are doing this.

Is it worth changing the algorithm to do this instead:

0. test for existance by opening a read-only file descriptor to this
file.
1. Creating the file/directory/fifo.
a. If it doesn't exist, create it -- note that I'm not setting
permissions with the create call.
b. Open a read-only file descriptor that attaches to the newly created
file.
2. Setting Permissions.
a. Fix the permissions of the file if necessary.
3. setting ownership
a. Set the ownership if it doesn't match the specified ownership.

> Risk #2: Instead consider a four-component path /run/foo/bar/baz. If you
> start creating those directories with owner "foo", then when you get to
> creating "baz", it's possible that "bar" has been replaced by a symlink
> somewhere else.
 
 It is possible, sure, but the question I would ask is, could this also
 be a legit situation where a user would want /run/foo/bar to be a
 symlink to some other location? If it is, there's no way to tell the
 difference.

> Certain tricks exist to avoid that, but I'm not an expert on them and I
> don't know how portable or secure they really are. For example, you
> might get the descriptor of "bar" and then use openat(dirfd,...) instead
> of open(...) to create "baz".
>
> If both of those can be solved portably, then all you have to worry
> about is everything I haven't thought of =)

Thoughts?

William