kernel-hardening.lists.openwall.com archive mirror
 help / color / mirror / Atom feed
From: Alan Cox <gnomes@lxorguk.ukuu.org.uk>
To: Matt Brown <matt@nmatt.com>
Cc: serge@hallyn.com, gregkh@linuxfoundation.org, jslaby@suse.com,
	akpm@linux-foundation.org, jannh@google.com,
	keescook@chromium.org, jmorris@namei.org,
	kernel-hardening@lists.openwall.com,
	linux-security-module@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: [kernel-hardening] Re: [PATCH v6 0/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN
Date: Mon, 15 May 2017 21:57:52 +0100	[thread overview]
Message-ID: <20170515215752.4e9f3826@alans-desktop> (raw)
In-Reply-To: <e73698a7-7b6a-a916-3270-82dcfe0a558b@nmatt.com>

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

  parent reply	other threads:[~2017-05-15 20:57 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-05-05 23:20 [kernel-hardening] [PATCH v6 0/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN Matt Brown
2017-05-05 23:20 ` [kernel-hardening] [PATCH v6 1/2] security: tty: Add owner user namespace to tty_struct Matt Brown
2017-05-05 23:20 ` [kernel-hardening] [PATCH v6 2/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN Matt Brown
2017-05-18 13:31   ` [kernel-hardening] " Greg KH
2017-05-19  4:51     ` Matt Brown
2017-05-10 20:29 ` [kernel-hardening] Re: [PATCH v6 0/2] " Alan Cox
2017-05-10 21:02   ` Daniel Micay
2017-05-13 19:52   ` Matt Brown
2017-05-15  4:45     ` Nicolas Belouin
2017-05-15 20:57     ` Alan Cox [this message]
2017-05-15 23:10       ` Peter Dolding
2017-05-16  4:15         ` Matt Brown
2017-05-16  9:01           ` Peter Dolding
2017-05-16 12:22             ` Matt Brown
2017-05-16 14:28               ` Kees Cook
2017-05-16 15:48                 ` Serge E. Hallyn
2017-05-16 22:05                   ` Peter Dolding
2017-05-16 21:43                 ` Peter Dolding
2017-05-16 21:54                   ` Matt Brown
2017-05-17 16:41                 ` Alan Cox
2017-05-17 18:25                   ` Daniel Micay
2017-05-17 23:04                     ` Boris Lukashev
2017-05-18  3:18                     ` Kees Cook
2017-05-19  2:48                       ` Peter Dolding
2017-05-19  4:08                         ` Boris Lukashev
2017-05-19 14:33                         ` Serge E. Hallyn
2017-05-29 10:42                           ` Peter Dolding
2017-05-30 15:52                             ` Serge E. Hallyn
2017-05-30 21:52                               ` Alan Cox
2017-05-31 11:27                                 ` Peter Dolding
2017-05-31 14:36                                   ` Alan Cox
2017-05-31 15:32                                     ` Serge E. Hallyn

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170515215752.4e9f3826@alans-desktop \
    --to=gnomes@lxorguk.ukuu.org.uk \
    --cc=akpm@linux-foundation.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=jannh@google.com \
    --cc=jmorris@namei.org \
    --cc=jslaby@suse.com \
    --cc=keescook@chromium.org \
    --cc=kernel-hardening@lists.openwall.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=matt@nmatt.com \
    --cc=serge@hallyn.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).