Message-ID: <20170601222432.6f593538@lxorguk.ukuu.org.uk>
Date:   Thu, 1 Jun 2017 22:24:32 +0100
From:   Alan Cox <gnomes@...rguk.ukuu.org.uk>
To:     Kees Cook <keescook@...omium.org>
Cc:     Matt Brown <matt@...tt.com>,
        Casey Schaufler <casey@...aufler-ca.com>,
        Boris Lukashev <blukashev@...pervictus.com>,
        Greg KH <gregkh@...uxfoundation.org>,
        "Serge E. Hallyn" <serge@...lyn.com>,
        "kernel-hardening@...ts.openwall.com" 
        <kernel-hardening@...ts.openwall.com>,
        linux-security-module <linux-security-module@...r.kernel.org>,
        linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make
 TIOCSTI ioctl require CAP_SYS_ADMIN

> There's a difference between "bugs" and "security bugs". Letting

Not really, it's merely a matter of severity of result. A non security
bug that hoses your hard disk is to anyone but security nutcases at
least as bad as a security hole.

> security bugs continue to get exploited because we want to flush out
> bugs seems insensitive to the people getting attacked. I'd rather
> protect against a class of bug than have to endless fix each bug.

The others are security bugs too to varying degree

> > I'm not against doing something to protect the container folks, but that
> > something as with Android is a whitelist of ioctls. And if we need to do
> > this with a kernel hook lets do it properly.
> >
> > Remember the namespace of the tty on creation
> > If the magic security flag is set then
> >         Apply a whitelist to *any* tty ioctl call where the ns doesn't
> >                 match
> >
> > and we might as well just take the Android whitelist since they've kindly
> > built it for us all!
> >
> > In the tty layer it ends up being something around 10 lines of code and
> > some other file somewhere in security/ that's just a switch or similar
> > with the whitelisted ioctl codes in it.
> >
> > That (or a similar SELinux ruleset) would actually fix the problem.
> > SELinux would be better because it can also apply the rules when doing
> > things like su/sudo/...  
> 
> Just to play devil's advocate, wouldn't such a system continue to not
> address your physical-console concerns? I wouldn't want to limit the

It would for the cases that a whitelist and container check covers -
because the whitelist wouldn't allow you to do anything but boring stuff
on the tty. TIOCSTI is just one of a whole range of differently stupid
and annoying opportunities. Containers do not and should not be able to
set the keymap, change the video mode, use console selection, make funny
beepy noises, access video I/O registers and all the other stuff like
that. Nothing is going to break if we have a fairly conservative
whitelist.

> protection to only containers (but it's a good start), since it
> wouldn't protect people not using containers that still have a
> privileged TTY attached badly somewhere.

How are you going to magically fix the problem. I'm not opposed to fixing
the real problem but right now it appears to be a product of wishful
thinking not programming. What's the piece of security code that
magically discerns the fact you are running something untrusted at the
other end of your tty. SELinux can do it via labelling but I don't see
any generic automatic way for the kernel to magically work out when to
whitelist and when not to. If there is a better magic rule than
differing-namespace then provide the code.

You can't just disable TIOCSTI, it has users deal with it. You can
get away with disabling it for namespace crossing I think but if you do
that you need to disable a pile of others.

(If it breaks containers blocking TIOCSTI then we need to have a good
look at algorithms for deciding when to flush the input queue on exiting
a container or somesuch)

> If you're talking about wholistic SELinux policy, sure, I could
> imagine a wholistic fix. But for the tons of people without a
> comprehensive SELinux policy, the proposed protection continues to
> make sense.

No it doesn't. It's completely useless unless you actually bother to
address the other exploit opportunities.

Right now the proposal is a hack to do 

	if (TIOCSTI && different_namespace && magic_flag)

the proper solution is

	if (!whitelisted(ioctl) && different_namespace && magic_flag)

The former is snake oil, the latter actually deals with the problem space
for namespaced stuff comprehensively and is only a tiny amount more code.

For non namespaced stuff it makes it no worse, if you don't allocate a
pty/tty pair properly then the gods are not going to magically save you
from on high sorry. And if you want to completely kill TIOCSTI even
though it's kind of pointless you can use seccomp.

We can make things a bit better for the non-namespaced cases by providing
a new ioctl that turns on/off whitelisting for that tty so that the
process can do

	ioctl(tty_fd, TIOCTRUST, &zero);
	execl("/home/foo/stupid-idea", "ooops", NULL);

that's a simple per tty flag and a trivial one condition extra check to
the test above. We do need some way to change it back however and that's
a bit trickier given we don't want the stupid-idea tool to be able to but
we do want the invoking shell to - maybe you have to be session leader ?

Alan

Powered by blists - more mailing lists