[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Mon, 15 May 2017 21:57:52 +0100
From: Alan Cox <gnomes@...rguk.ukuu.org.uk>
To: Matt Brown <matt@...tt.com>
Cc: serge@...lyn.com, gregkh@...uxfoundation.org, jslaby@...e.com,
akpm@...ux-foundation.org, jannh@...gle.com, keescook@...omium.org,
jmorris@...ei.org, kernel-hardening@...ts.openwall.com,
linux-security-module@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v6 0/2] security: tty: make TIOCSTI ioctl require
CAP_SYS_ADMIN
O> I'm not implying that my patch is supposed to provide safety for
> "hundreds of other" issues. I'm looking to provide a way to lock down a
> single TTY ioctl that has caused real security issues to arise. For
In other words you are not actually fixing anything.
> this reason, it's completely incorrect to say that this feature is
> snake oil. My patch does exactly what it claims to do. No more no less.
>
> > In addition your change to allow it to be used by root in the guest
> > completely invalidates any protection you have because I can push
> >
> > "rm -rf /\n"
> >
> > as root in my namespace and exit
> >
> > The tty buffers are not flushed across the context change so the shell
> > you return to gets the input and oh dear....
>
> This is precisely what my patch prevents! With my protection enabled, a
> container will only be able to use the TIOCSTI ioctl on a tty if that
> container has CAP_SYS_ADMIN in the user namespace in which the tty was
> created.
Which is not necessarily the namespace of the process that next issues a
read().
This is snake oil. There is a correct and proper process for this use
case. That proper process is to create a new pty/tty pair. There are two
cases
- processes that do it right in which case the attacker is screwed and we
don't need to mess up TIOCSTI handling for no reason.
- processes that do it wrong. If they do it wrong then they'll also use
all the other holes and attacks available via the same path which are
completely unfixable without making your system basically unusable.
So can we please apply the minimum of common sense reasoning to this and
drop the patch.
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.
