Bug 35895 - [PATCH] Make /run/lock (and thus /var/lock) world-writable and sticky, to avoid regression
Summary: [PATCH] Make /run/lock (and thus /var/lock) world-writable and sticky, to avo...
Status: RESOLVED WONTFIX
Alias: None
Product: systemd
Classification: Unclassified
Component: general (show other bugs)
Version: unspecified
Hardware: All All
: medium normal
Assignee: Lennart Poettering
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-04-01 20:30 UTC by Josh Triplett
Modified: 2011-04-03 04:17 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments
git patch: Make /run and /run/lock (and thus /var/run and /var/lock) world-writable and sticky (2.44 KB, patch)
2011-04-01 20:30 UTC, Josh Triplett
Details | Splinter Review

Description Josh Triplett 2011-04-01 20:30:55 UTC
Created attachment 45153 [details] [review]
git patch: Make /run and /run/lock (and thus /var/run and /var/lock) world-writable and sticky

/var/lock normally has mode 1777, so that users can create lockfiles
but cannot remove each others' lockfiles.  (For instance, programs using
serial devices normally create lockfiles in /var/lock, even though those
programs do not run as root.)  Thus, make /run/lock 1777 as well, to
preserve this.
    
/var/run should have mode 1777 as well, so that daemons running as
non-root users can create their sockets there.  This helps both daemons
which start as root and drop privilege later (now they can drop
privilege sooner) and daemons which run as an ordinary user in the first
place and currently have to create their sockets in /tmp (gnome-keyring,
authentication agents, gvfs).

Patch attached.
Comment 1 Josh Triplett 2011-04-02 01:38:17 UTC
(Also happy to split out the two portions of the patch, since I suspect /run might prove more controversial than /run/lock; the latter represents a regression, the former not.)
Comment 2 Lennart Poettering 2011-04-02 12:57:35 UTC
No, we definitely don't want to add two more /tmp like security holes. Also, the sticky bit doesn't work, since you need to allow users to remove stale lock files of other users. You don't want to allow arbitrary users fill up the tmpfs. You don't want to allow normal users to trigger DoS problems by creating a directory beneath /var/run then later on wants to be used by system daemon.

This all is discussed here:

http://lists.freedesktop.org/archives/systemd-devel/2011-March/001823.html
https://bugzilla.redhat.com/show_bug.cgi?id=581884
https://bugzilla.redhat.com/show_bug.cgi?id=145264#c1
http://bugs.freestandards.org/show_bug.cgi?id=719
Comment 3 Josh Triplett 2011-04-02 18:38:25 UTC
(In reply to comment #2)
> No, we definitely don't want to add two more /tmp like security holes. Also,
> the sticky bit doesn't work, since you need to allow users to remove stale lock
> files of other users. You don't want to allow arbitrary users fill up the
> tmpfs. You don't want to allow normal users to trigger DoS problems by creating
> a directory beneath /var/run then later on wants to be used by system daemon.
> 
> This all is discussed here:
> 
> http://lists.freedesktop.org/archives/systemd-devel/2011-March/001823.html
> https://bugzilla.redhat.com/show_bug.cgi?id=581884
> https://bugzilla.redhat.com/show_bug.cgi?id=145264#c1
> http://bugs.freestandards.org/show_bug.cgi?id=719

Sigh.  Ignoring /var/run for the moment (since I learned of XDG_RUNTIME_DIR and thus don't feel like arguing that point), making /var/lock not user-writable will cause a regression for existing programs which expect to create lock files there.  "stale lock files" don't seem like a justification for allowing user programs to remove locks legitimately created by other users (or root).  Conversely, allowing arbitrary users to remove files created by other users seems like the more critical security problem.  As for filling up the tmpfs, that can occur anyway via XDG_RUNTIME_DIR, with the same solution: use a quota if you care.  (They can also fill up RAM easily enough if you let them, with a similar solution.)
Comment 4 Josh Triplett 2011-04-02 18:39:03 UTC
Reopening with narrowed scope.
Comment 5 Lennart Poettering 2011-04-03 04:17:12 UTC
(In reply to comment #3)
> (In reply to comment #2)
> > No, we definitely don't want to add two more /tmp like security holes. Also,
> > the sticky bit doesn't work, since you need to allow users to remove stale lock
> > files of other users. You don't want to allow arbitrary users fill up the
> > tmpfs. You don't want to allow normal users to trigger DoS problems by creating
> > a directory beneath /var/run then later on wants to be used by system daemon.
> > 
> > This all is discussed here:
> > 
> > http://lists.freedesktop.org/archives/systemd-devel/2011-March/001823.html
> > https://bugzilla.redhat.com/show_bug.cgi?id=581884
> > https://bugzilla.redhat.com/show_bug.cgi?id=145264#c1
> > http://bugs.freestandards.org/show_bug.cgi?id=719
> 
> Sigh.  Ignoring /var/run for the moment (since I learned of XDG_RUNTIME_DIR and
> thus don't feel like arguing that point), making /var/lock not user-writable
> will cause a regression for existing programs which expect to create lock files
> there.

Well, in multiple distributions access to /var/lock was already limited to programs setgid lock. If apps use libraries like liblockdev which include a setgid helper (and they shoiuld), there is no real problem here.

>   "stale lock files" don't seem like a justification for allowing user
> programs to remove locks legitimately created by other users (or root). 

No doubt that LCK.. style lock files are broken. But they are cooperative by nature, and do not clean themselves up if an application dies. Due to that it must be possible to delete other people's lock files.

The right way to lock devices is probably using BSD locks. See 

http://lists.freedesktop.org/archives/systemd-devel/2011-April/001834.html

and the surrounding thread.

> Conversely, allowing arbitrary users to remove files created by other users
> seems like the more critical security problem.  As for filling up the tmpfs,
> that can occur anyway via XDG_RUNTIME_DIR, with the same solution: use a quota
> if you care.  (They can also fill up RAM easily enough if you let them, with a
> similar solution.)

There is no quota on tmpfs right now. Also, what you want here is a bit of an inverse quota which cannot even be expressed with the ext3 quota system: uids < 500 must always have space for a certain amount of storage.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.