public inbox for gentoo-dev@lists.gentoo.org
 help / color / mirror / Atom feed
From: William Hubbs <williamh@gentoo.org>
To: gentoo-dev@lists.gentoo.org
Cc: mjo@gentoo.org
Subject: Re: [gentoo-dev] rfc: ideas for fixing OpenRC checkpath issue
Date: Fri, 19 Jan 2018 18:16:48 -0600	[thread overview]
Message-ID: <20180120001648.GA24415@linux1.home> (raw)
In-Reply-To: <03558fda-26b3-2e3a-ad42-c94848f49955@gentoo.org>

[-- Attachment #1: Type: text/plain, Size: 3261 bytes --]

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


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

  reply	other threads:[~2018-01-20  0:16 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-10  0:07 [gentoo-dev] rfc: ideas for fixing OpenRC checkpath issue William Hubbs
2018-01-10  1:19 ` Michael Orlitzky
2018-01-10 18:04   ` William Hubbs
2018-01-10 20:25     ` Michael Orlitzky
2018-01-10 21:54       ` William Hubbs
2018-01-13 20:48         ` Michael Orlitzky
2018-01-17 15:21           ` William Hubbs
2018-01-17 15:41             ` Michael Orlitzky
2018-01-17 17:14               ` William Hubbs
2018-01-19  0:19                 ` Michael Orlitzky
2018-01-20  0:16                   ` William Hubbs [this message]
2018-01-20  0:53                     ` Michael Orlitzky
2018-01-20  1:14                       ` William Hubbs
2018-01-20  1:20                         ` Michael Orlitzky
2018-01-10 18:17   ` Kristian Fiskerstrand
2018-01-12 16:33     ` Michael Orlitzky
2018-01-10 22:18 ` James Le Cuirot
2018-01-10 23:31   ` Michael Orlitzky

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180120001648.GA24415@linux1.home \
    --to=williamh@gentoo.org \
    --cc=gentoo-dev@lists.gentoo.org \
    --cc=mjo@gentoo.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox