[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <2d0ad49c-886e-1caf-771a-d251957f614c@nmatt.com>
Date: Fri, 2 Jun 2017 10:46:54 -0400
From: Matt Brown <matt@...tt.com>
To: Alan Cox <gnomes@...rguk.ukuu.org.uk>, Kees Cook <keescook@...omium.org>
Cc: 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: Re: [PATCH v7 2/2] security: tty: make TIOCSTI
ioctl require CAP_SYS_ADMIN
On 6/1/17 5:24 PM, Alan Cox wrote:
>> 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)
>
This is not what my patch does. Mine is like:
if (TIOCSTI && !ns_capable(tty->owner_user_ns, CAP_SYS_ADMIN) &&
magic_flag)
in other words:
if (TIOCSTI && (different_owner_user_ns || !CAP_SYS_ADMIN) &&
magic_flag)
can you specify what you mean by different_namespace? which namespace?
The reason I brought in the user namespace was to solve an edge case
with nested containers. capable() will never be true in a container, so
nested containers would break if we didn't do the ns_capable check. This
is also the reason for the addition of owner_user_ns to tty_struct.
The point of my patch is to prevent any type of tiocsti activity where
the tty is given to an unprivileged process/container. The emphasis of
my check is on the CAP_SYS_ADMIN part, not the namespace part.
> 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
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
