Re: [PATCH v6] fs: allow protected cross-uid sticky symlinks

[ebiederm@xmission.com (Eric W. Biederman)]

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

> On Mon, Jun 07, 2010 at 12:18:56PM -0400, Valdis.Kletnieks@vt.edu wrote:
>> On Thu, 03 Jun 2010 14:00:51 PDT, Kees Cook said:
>> > On Thu, Jun 03, 2010 at 01:02:48PM -0700, Eric W. Biederman wrote:
>> > > Kees Cook <kees.cook@canonical.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
>> > > 
>> > > Nacked-by: "Eric W. Biederman" <ebiederm@xmission.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.
>> > 
>> > How to safely deal with /tmp has been well understood for well over
>> > a decade.  I don't think this change would "encourage" poor code.
>> 
>> The fact that you're proposing this patch a decade after we "well understood"
>> the problem should suggest that it *will* encourage poor code, as the same
>> programmers who don't currently get it right (and are thus the targets of your
>> patch) will quite likely just say "Oh, I saw a patch for that, I don't have to
>> try to do it right..."
>
> This objection and its rebuttal are entirely conjecture and I don't think
> it matters since we can never know the objective truth about the education
> or motivation of nameless coders. 

But we can know about distributions.

I see a patch put forward with the argument that it solves all the
problems of /tmp.  The patch does not actually solve all of the
problems of /tmp but more refined claims have not been made.

With the existence of that patch I see no interest in actually solving
the slightly more challenging problem at the distribution level of
unnecessary sharing in /tmp because a handful of uses of /tmp will
have to be fixed.

> That said, I would assume that anyone
> well-educated enough about /tmp races to think "oh I saw a kernel patch
> for that" was either going to get it right to begin with or was going to
> ignore best practices anyway.  The issue is more about the people that
> just don't think about it at all.  I would argue that people are still
> doing it, for over a decade, when it's still vulnerable, is evidence
> that it is not something that will improve.

Furthermore making it impossible to exploit the races with symlinks
(the best your patch can do).  Is not going to make the races impossible
to exploit by placing a different file in place of the one to be
created, or using hard links instead of symlinks.

So I would be surprised if after just your patch a system is actually
more secure from attack, once the attackers notice the change.

Eric