On Wed, May 17, 2017 at 11:25 AM, Daniel Micay [off-list ref] wrote:

On Wed, 2017-05-17 at 17:41 +0100, Alan Cox wrote:

quoted

quoted

If we're adjusting applications, they should be made to avoid
TIOSCTI
completely. This looks to me a lot like the symlink restrictions:
yes,
userspace should be fixed to the do the right thing, but why not
provide support to userspace to avoid the problem entirely?

We do it's called pty/tty. There isn't any other way to do this
correctly
because TIOCSTI is just one hundreds of things the attacker can do to
make your life miserable in the case you create a child process of
lower
security privilege and give it your tty file handle or worse (like
some
container crapware) your X11 socket fd.

Does it really matter any more or less if I reprogram your enter key,
use
TIOCSTI, set the baud rate, change all your fonts ?

The mainstream tools like sudo get this right (*). Blocking TIOCSTI
fixes
nothing and breaks apps. If it magically fixed the problem it might
make
sense but it doesn't. You actually have to get an adult to write the
relevant code.

It gets it right because it was reported as a vulnerability and fixed,
which is the cycle many of these tools have gone through. It looks like
sudo and coreutils / shadow su were fixed in 2005, but there are more of
these tools.

CVE-2005-4890 (coreutils su, shadow su, sudo), CVE-2016-7545 (SELinux
sandbox utility), CVE-2016-2781 (chroot --userspec), CVE-2016-2779
(util-linux runuser), CVE-2016-2568 (pkexec)

I am not sure if the pkexec vulnerability is even fixed:

https://bugzilla.redhat.com/show_bug.cgi?id=1300746

CVE-2017-5226 is for bubblewrap/flatpak this year, and I'm sure there
were a lot more as I didn't bother to dig very deep.

I completely agree that it should be solved by doing things properly in
each application. However, it isn't solved properly everywhere and each
new application is making the same mistake. Providing this feature does
not break anything that people use in practice and it doesn't need to
solve every issue to be quite useful, it only needs to prevent local
privilege escalation in the form of code execution in many cases. Is
there another way to get code execution via that bubblewrap/flatpak bug
with this feature implemented? As far as I can tell, there isn't. It's a
problem even with this feature but a significantly less serious one.

This patch solves a real problem when userspace does things wrong. For
those that do not want it, a sysctl exists (and is even default off).
Arguments about how userspace needs to be fixed is a total red
herring. Like symlink protections before, this should be available for
those that want it because it solves an interface problem that gets
regularly misused and causes real damage. The kernel has a
responsibility to protect userspace.

As Daniel mentions, there is a history of mistakes that appear to be
becoming even more common:
http://cve.mitre.org/cgi-bin/cvekey.cgi?keyword=TIOCSTI

There are people who want this feature that do not care about xemacs
breaking. The default cannot be enabled because of the golden rule of
never breaking userspace, but entirely refusing to fix a regularly
misused feature is wrong-headed. We must accept that not only are bug
lifetimes long, but that bugs will keep getting reintroduced. When
there is a chance to kill a class of bugs or exploit techniques, it's
the kernel responsibility to do so. This is a clear case of that, and
the solution is concise.

If there is some better solution that the kernel can provide to
mitigate processes misusing ttys, then by all means, we can add that
too, but has nothing to do with refusing this change. This solves a
specific problem that in many cases is the only path to privilege
escalation (as Daniel mentioned). Refusing this change is nonsense.
"Your car shouldn't have seat belts because maybe something will stab
you through the windshield" isn't a reasonable argument to make.

-Kees

-- 
Kees Cook
Pixel Security
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html