Message-ID: <m11vcnlw9j.fsf@fess.ebiederm.org>
Date:	Thu, 03 Jun 2010 13:02:48 -0700
From:	ebiederm@...ssion.com (Eric W. Biederman)
To:	Kees Cook <kees.cook@...onical.com>
Cc:	Dave Young <hidave.darkstar@...il.com>,
	Al Viro <viro@...iv.linux.org.uk>,
	Eric Paris <eparis@...hat.com>,
	Christoph Hellwig <hch@...radead.org>,
	James Morris <jmorris@...ei.org>, linux-kernel@...r.kernel.org,
	linux-security-module@...r.kernel.org,
	linux-fsdevel@...r.kernel.org, linux-doc@...r.kernel.org,
	Randy Dunlap <rdunlap@...otime.net>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Jiri Kosina <jkosina@...e.cz>,
	Martin Schwidefsky <schwidefsky@...ibm.com>,
	David Howells <dhowells@...hat.com>,
	Ingo Molnar <mingo@...e.hu>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Tim Gardner <tim.gardner@...onical.com>,
	"Serge E. Hallyn" <serue@...ibm.com>, tytso@....edu,
	Alan Cox <alan@...rguk.ukuu.org.uk>
Subject: Re: [PATCH v6] fs: allow protected cross-uid sticky symlinks

Kees Cook <kees.cook@...onical.com> writes:

> A long-standing class of security issues is the symlink-based
> time-of-check-time-of-use race, most commonly seen in world-writable
> directories like /tmp. The common method of exploitation of this flaw
> is to cross privilege boundaries when opening a file through a given
> symlink (i.e. a root process opens a symlink belonging to another user).
> For a likely incomplete list of hundreds of examples across the years,
> please see: http://cve.mitre.org/cgi-bin/cvekey.cgi?keyword=/tmp
>
> The solution is to permit symlinks to only be opened when outside a sticky
> world-writable directory, or when the uid of the symlink and opener match,
> or when the directory owner matches the symlink's owner.
>
> Some pointers to the history of earlier discussion that I could find:
>
>  1996 Aug, Zygo Blaxell
>   http://marc.info/?l=bugtraq&m=87602167419830&w=2
>  1996 Oct, Andrew Tridgell
>   http://lkml.indiana.edu/hypermail/linux/kernel/9610.2/0086.html
>  1997 Dec, Albert D Cahalan
>   http://lkml.org/lkml/1997/12/16/4
>  2005 Feb, Lorenzo Hernández García-Hierro
>   http://lkml.indiana.edu/hypermail/linux/kernel/0502.0/1896.html
>
> Past objections and rebuttals could be summarized as:
>
>  - Violates POSIX.
>    - POSIX didn't consider this situation and it's not useful to follow
>      a broken specification at the cost of security.
>  - Might break unknown applications that use this feature.
>    - Applications that break because of the change are easy to spot and
>      fix. Applications that are vulnerable to symlink ToCToU by not having
>      the change aren't.
>  - Applications should just use mkstemp() or O_CREATE|O_EXCL.
>    - True, but applications are not perfect, and new software is written
>      all the time that makes these mistakes; blocking this flaw at the
>      kernel is a single solution to the entire class of vulnerability.
>
> This patch is based on the patch in Openwall and grsecurity, but with the
> scope changed to be only "opening" a symlink.  I have added a sysctl to
> enable the protected behavior, documentation, and a ratelimited warning.

Nacked-by: "Eric W. Biederman" <ebiederm@...ssion.com>

This approach to fix the problem to of /tmp looks to me like it
will have the opposite effect.  I think this patch will encourage
more badly written applications.

This patch does not actually fix the problem of recurring security
issues in /tmp.  Fundamentally /tmp is a design flaw. You have
evidence that even the introduction of the sticky bit, and O_NOFOLLOW
did not fix this design flaw, I don't see how yet another kernel feature
will make finally make /tmp secure.  The design flaw in /tmp is that we
encourage storing files that we care little about (temporary files) in
a location that is shared between everyone.  From a security point of
view sharing of files needs to be done very carefully, and as such
should only be done for files that we really care about and intend to
share.

Furthermore the set of applications with security problems you are intending
to address are privileged applications.   There is no excuse not to audit
and fix those applications.  If you include this patch I see no reason why
audits won't get more lax and other related security issues won't crop up.

The real solution to this problem of inadvertently sharing things that are
not safe to be shared is not to have a shared /tmp at all.

Per user tmp can be as simple as:
export TMPDIR=$HOME/tmp/
chmod 0500 /tmp

So is your interest in actually fixing this problem, or do you just
want to encourage more bad applications to be written without worrying
about inadvertent sharing in /tmp?

Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists