[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <c5d9dd814f14467a8b756ebd27a07a75@AcuMS.aculab.com>
Date: Fri, 1 Dec 2017 09:46:28 +0000
From: David Laight <David.Laight@...LAB.COM>
To: 'Solar Designer' <solar@...nwall.com>
CC: 'Salvatore Mesoraca' <s.mesoraca16@...il.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, "Kernel
Hardening" <kernel-hardening@...ts.openwall.com>,
"linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>, "Alexander
Viro" <viro@...iv.linux.org.uk>, Jann Horn <jannh@...gle.com>, Kees Cook
<keescook@...omium.org>, "Eric W. Biederman" <ebiederm@...ssion.com>
Subject: RE: [PATCH v3 2/2] Protected O_CREAT open in sticky directories
From: Solar Designer
> Sent: 30 November 2017 17:52
>
> On Thu, Nov 30, 2017 at 04:53:06PM +0000, David Laight wrote:
> > From: Salvatore Mesoraca
> > > if a program tries to open a file, in a sticky directory,
> > > with the O_CREAT flag and without the O_EXCL, it probably has a bug.
> > > This feature allows to detect and potentially block programs that
> > > act this way, it can be used to find vulnerabilities (like those
> > > prevented by patch #1) and to do policy enforcement.
>
> > I presume the 'vulnerabilities' are related to symlinks being created
> > just before the open?
>
> That's one way to exploit them. We already have a sysctl to try and
> block that specific attack, and we're considering adding more, for other
> attacks. But we'd also like an easy way to learn of the vulnerabilities
> without anyone trying to exploit them yet, hence this patch.
>
> > Trouble is this change breaks a lot of general use of /tmp.
>
> That's general misuse of /tmp. Things like "command > /tmp/file"
> without having pre-created the file with O_EXCL e.g. by mktemp(1).
I'm sorry, I've been using Unix for over 30 years.
/tmp is a place that temporary files were created - nothing special.
Traditionally it was emptied on every boot.
There was never anything that required files be created in any
specific way.
> > I always assumed that code that cared would use O_EXCL and
> > everything else wasn't worth subverting.
>
> Opinions will vary on whether it's worth subverting or not, and someone
> may reasonably want to configure this differently on different systems.
>
> > I found code in vi (and elsewhere) that subverted these checks
> > by opening with O_WRONLY if stat() showed the file existed and
> > O_CREAT|O_EXCL if it didn't.
>
> Yes, such misuses of /tmp and the like by use of those programs - where
> the potential consequences would often be less severe (if your example
> above is correct, it sounds like it'd be data spoofing but not
> overwriting another file over a link) - could remain unnoticed.
It seemed to me that code had been added to avoid issues caused by
this policing of opens.
But the code added is itself racy - so makes the whole thing pointless.
> > I'm pretty sure that traditionally a lot of these opens were done
> > with O_CREAT|O_TRUNC.
> > Implementing that as unlink() followed by a create would stop
> > 'random' (ok all) symlinks being followed.
>
> That's racy. We have O_EXCL, and it must be used along with O_CREAT
> when the directory is untrusted. (If it were only about symlinks, we
> also already have O_NOFOLLOW.)
Not racy if done in the kernel itself.
> > Overall I'm pretty sure this change will break things badly somewhere.
>
> Sure. We want it to visibly break what was subtly broken, so that the
> breakage can be seen and corrected without an attempted attack.
Maybe, but it will break some user system when they do a kernel update.
It won't necessarily break a developer's system first.
And, AFAICT, there is already code that subverts any tests by doing
a check for the file existing and then selecting between two different
types on open.
David
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
