Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1652725 > unrolled thread

[PATCH v7 2/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v7 2/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN Matt Brown <matt@nmatt.com> - 2017-05-29 23:40 +0200
    Re: [PATCH v7 2/2] security: tty: make TIOCSTI ioctl require  CAP_SYS_ADMIN Alan Cox <gnomes@lxorguk.ukuu.org.uk> - 2017-05-30 00:30 +0200
      Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI  ioctl require CAP_SYS_ADMIN Boris Lukashev <blukashev@sempervictus.com> - 2017-05-30 02:00 +0200
        Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI  ioctl require CAP_SYS_ADMIN Casey Schaufler <casey@schaufler-ca.com> - 2017-05-30 02:30 +0200
          Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI  ioctl require CAP_SYS_ADMIN Matt Brown <matt@nmatt.com> - 2017-05-30 04:10 +0200
            Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI  ioctl require CAP_SYS_ADMIN Casey Schaufler <casey@schaufler-ca.com> - 2017-05-30 04:50 +0200
              Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI  ioctl require CAP_SYS_ADMIN Matt Brown <matt@nmatt.com> - 2017-05-30 05:20 +0200
                Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make  TIOCSTI ioctl require CAP_SYS_ADMIN Alan Cox <gnomes@lxorguk.ukuu.org.uk> - 2017-05-30 14:30 +0200
                  Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI  ioctl require CAP_SYS_ADMIN Matt Brown <matt@nmatt.com> - 2017-05-30 18:30 +0200
                    Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI  ioctl require CAP_SYS_ADMIN Daniel Micay <danielmicay@gmail.com> - 2017-05-30 18:50 +0200
                    Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make  TIOCSTI ioctl require CAP_SYS_ADMIN Stephen Smalley <sds@tycho.nsa.gov> - 2017-05-30 20:30 +0200
                      Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI  ioctl require CAP_SYS_ADMIN Nick Kralevich <nnk@google.com> - 2017-05-30 20:50 +0200
                        Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI  ioctl require CAP_SYS_ADMIN Matt Brown <matt@nmatt.com> - 2017-05-30 21:00 +0200
                          Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make  TIOCSTI ioctl require CAP_SYS_ADMIN Daniel Micay <danielmicay@gmail.com> - 2017-05-30 22:30 +0200
                            Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI  ioctl require CAP_SYS_ADMIN Matt Brown <matt@nmatt.com> - 2017-05-31 01:10 +0200
                              Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make  TIOCSTI ioctl require CAP_SYS_ADMIN Daniel Micay <danielmicay@gmail.com> - 2017-05-31 01:50 +0200
                                Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI  ioctl require CAP_SYS_ADMIN Matt Brown <matt@nmatt.com> - 2017-05-31 02:00 +0200
                    Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make  TIOCSTI ioctl require CAP_SYS_ADMIN Alan Cox <gnomes@lxorguk.ukuu.org.uk> - 2017-05-31 01:00 +0200
                      Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI  ioctl require CAP_SYS_ADMIN Matt Brown <matt@nmatt.com> - 2017-05-31 01:20 +0200
                        Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make  TIOCSTI ioctl require CAP_SYS_ADMIN Alan Cox <gnomes@lxorguk.ukuu.org.uk> - 2017-05-31 02:00 +0200
                          Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI  ioctl require CAP_SYS_ADMIN Kees Cook <keescook@chromium.org> - 2017-06-01 04:40 +0200
                            Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make  TIOCSTI ioctl require CAP_SYS_ADMIN Alan Cox <gnomes@lxorguk.ukuu.org.uk> - 2017-06-01 15:10 +0200
                              Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make  TIOCSTI ioctl require CAP_SYS_ADMIN "Serge E. Hallyn" <serge@hallyn.com> - 2017-06-01 19:20 +0200
                                Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make  TIOCSTI ioctl require CAP_SYS_ADMIN Alan Cox <gnomes@lxorguk.ukuu.org.uk> - 2017-06-01 23:30 +0200
                              Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI  ioctl require CAP_SYS_ADMIN Kees Cook <keescook@chromium.org> - 2017-06-01 21:00 +0200
                                Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make  TIOCSTI ioctl require CAP_SYS_ADMIN Alan Cox <gnomes@lxorguk.ukuu.org.uk> - 2017-06-01 23:30 +0200
                                  Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI  ioctl require CAP_SYS_ADMIN Matt Brown <matt@nmatt.com> - 2017-06-02 16:50 +0200
                                    Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make  TIOCSTI ioctl require CAP_SYS_ADMIN "Serge E. Hallyn" <serge@hallyn.com> - 2017-06-02 17:40 +0200
                                      Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI  ioctl require CAP_SYS_ADMIN Matt Brown <matt@nmatt.com> - 2017-06-02 18:10 +0200
                                        Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make  TIOCSTI ioctl require CAP_SYS_ADMIN "Serge E. Hallyn" <serge@hallyn.com> - 2017-06-02 19:00 +0200
                                          Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI  ioctl require CAP_SYS_ADMIN Matt Brown <matt@nmatt.com> - 2017-06-02 19:40 +0200
                                            Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make  TIOCSTI ioctl require CAP_SYS_ADMIN "Serge E. Hallyn" <serge@hallyn.com> - 2017-06-02 20:20 +0200
                                              Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI  ioctl require CAP_SYS_ADMIN Matt Brown <matt@nmatt.com> - 2017-06-02 21:30 +0200
                                              Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI  ioctl require CAP_SYS_ADMIN Kees Cook <keescook@chromium.org> - 2017-06-02 21:30 +0200
                                              Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI  ioctl require CAP_SYS_ADMIN Matt Brown <matt@nmatt.com> - 2017-06-02 21:30 +0200
                                        Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make  TIOCSTI ioctl require CAP_SYS_ADMIN Alan Cox <gnomes@lxorguk.ukuu.org.uk> - 2017-06-02 22:10 +0200
                                          Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI  ioctl require CAP_SYS_ADMIN Nick Kralevich <nnk@google.com> - 2017-06-02 22:20 +0200
                                          Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI  ioctl require CAP_SYS_ADMIN Matt Brown <matt@nmatt.com> - 2017-06-02 22:50 +0200
                                            Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make  TIOCSTI ioctl require CAP_SYS_ADMIN Alan Cox <gnomes@lxorguk.ukuu.org.uk> - 2017-06-04 00:10 +0200
                                              Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI  ioctl require CAP_SYS_ADMIN Matt Brown <matt@nmatt.com> - 2017-06-04 00:30 +0200
                                                Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI  ioctl require CAP_SYS_ADMIN Peter Dolding <oiaohm@gmail.com> - 2017-06-04 05:40 +0200
                Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI  ioctl require CAP_SYS_ADMIN Casey Schaufler <casey@schaufler-ca.com> - 2017-05-30 17:30 +0200
                  Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI  ioctl require CAP_SYS_ADMIN Matt Brown <matt@nmatt.com> - 2017-05-30 18:10 +0200
          Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI  ioctl require CAP_SYS_ADMIN Boris Lukashev <blukashev@sempervictus.com> - 2017-06-04 08:30 +0200
        Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI  ioctl require CAP_SYS_ADMIN James Morris <jmorris@namei.org> - 2017-05-31 04:50 +0200
          Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI  ioctl require CAP_SYS_ADMIN Matt Brown <matt@nmatt.com> - 2017-05-31 06:20 +0200
      Re: [PATCH v7 2/2] security: tty: make TIOCSTI ioctl require  CAP_SYS_ADMIN Matt Brown <matt@nmatt.com> - 2017-05-30 02:20 +0200

Page 2 of 3 — ← Prev page 1 [2] 3  Next page →

#1654761 — Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

On Tue, May 30, 2017 at 4:56 PM, Alan Cox <gnomes@lxorguk.ukuu.org.uk> wrote:
>> This is my point. Apps will continue to shoot themselves in the foot. Of course
>> the correct response to one of these vulns is to not pass ttys across a
>> security boundary. We have an opportunity here to reduce the impact of this bug
>> class at the kernel level.
>
> Not really.
>
> If you pass me your console for example I can mmap your framebuffer and
> spy on you all day. Or I could reprogram your fonts, your keyboard, your
> video mode, or use set and paste selection to write stuff. If you are
> using X and you can't get tty handles right you'll no doubt pass me a
> copy of your X file descriptor in which case I own your display, your
> keyboard and your mouse and I don't need to use TIOCSTI there either.
>
> There are so many different attacks based upon that screwup that the
> kernel cannot defend against them. You aren't exactly reducing the impact.

I still cannot wrap my head around why providing users with a
protection is a bad thing. Yes, the other tty games are bad, but this
fixes a specific and especially bad case that is easy to kill. It's
got a Kconfig and a sysctl. It's not on by default. This protects the
common case of privileged ttys that aren't attached to consoles, etc,
so while the framebuffer thing is an issue, it's not always an issue,
etc.

I'd like to hear Greg's thoughts on this series...

-Kees

-- 
Kees Cook
Pixel Security

[toc] | [prev] | [next] | [standalone]

#1655119 — Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

> I still cannot wrap my head around why providing users with a
> protection is a bad thing. Yes, the other tty games are bad, but this
> fixes a specific and especially bad case that is easy to kill. It's
> got a Kconfig and a sysctl. It's not on by default. This protects the
> common case of privileged ttys that aren't attached to consoles, etc,

Which just leads to stuff not getting fixed. Like all the code out there
today which is still vulnerable to selection based attacks because people
didn't do the job right when "fixing" stuff because they are not
thinking about security at a systems level but just tickboxing CVEs.

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/...

Alan

[toc] | [prev] | [next] | [standalone]

#1655602 — Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

Quoting Alan Cox (gnomes@lxorguk.ukuu.org.uk):
> > I still cannot wrap my head around why providing users with a
> > protection is a bad thing. Yes, the other tty games are bad, but this
> > fixes a specific and especially bad case that is easy to kill. It's
> > got a Kconfig and a sysctl. It's not on by default. This protects the
> > common case of privileged ttys that aren't attached to consoles, etc,
> 
> Which just leads to stuff not getting fixed. Like all the code out there
> today which is still vulnerable to selection based attacks because people
> didn't do the job right when "fixing" stuff because they are not
> thinking about security at a systems level but just tickboxing CVEs.
> 
> 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

Whitelist of ioctls (at least using seccomp) is not sufficient because
then we have to turn the ioctl always-off.  But like you say we may want
to enable it for ptys which are created inside the container's user ns.

> this with a kernel hook lets do it properly.
> 
> Remember the namespace of the tty on creation

Matt's patch does this,

> If the magic security flag is set then
> 	Apply a whitelist to *any* tty ioctl call where the ns doesn't
> 		match

Seems sensible.

> 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/...
> 
> Alan

[toc] | [prev] | [next] | [standalone]

#1655775 — Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

On Thu, 1 Jun 2017 12:18:58 -0500
"Serge E. Hallyn" <serge@hallyn.com> wrote:

> Quoting Alan Cox (gnomes@lxorguk.ukuu.org.uk):
> > > I still cannot wrap my head around why providing users with a
> > > protection is a bad thing. Yes, the other tty games are bad, but this
> > > fixes a specific and especially bad case that is easy to kill. It's
> > > got a Kconfig and a sysctl. It's not on by default. This protects the
> > > common case of privileged ttys that aren't attached to consoles, etc,  
> > 
> > Which just leads to stuff not getting fixed. Like all the code out there
> > today which is still vulnerable to selection based attacks because people
> > didn't do the job right when "fixing" stuff because they are not
> > thinking about security at a systems level but just tickboxing CVEs.
> > 
> > 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  
> 
> Whitelist of ioctls (at least using seccomp) is not sufficient because
> then we have to turn the ioctl always-off.  But like you say we may want
> to enable it for ptys which are created inside the container's user ns.
> 
> > this with a kernel hook lets do it properly.
> > 
> > Remember the namespace of the tty on creation  
> 
> Matt's patch does this,
> 
> > If the magic security flag is set then
> > 	Apply a whitelist to *any* tty ioctl call where the ns doesn't
> > 		match  
> 
> Seems sensible.

I'm arguing that we need to swap the TIOCSTI test for a !whitelisted()
test to go with the namespace difference check. Then it makes sense
because we actually address the real problem.

Alan

[toc] | [prev] | [next] | [standalone]

#1655676 — Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

On Thu, Jun 1, 2017 at 6:08 AM, Alan Cox <gnomes@lxorguk.ukuu.org.uk> wrote:
>> I still cannot wrap my head around why providing users with a
>> protection is a bad thing. Yes, the other tty games are bad, but this
>> fixes a specific and especially bad case that is easy to kill. It's
>> got a Kconfig and a sysctl. It's not on by default. This protects the
>> common case of privileged ttys that aren't attached to consoles, etc,
>
> Which just leads to stuff not getting fixed. Like all the code out there
> today which is still vulnerable to selection based attacks because people
> didn't do the job right when "fixing" stuff because they are not
> thinking about security at a systems level but just tickboxing CVEs.

There's a difference between "bugs" and "security bugs". Letting
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.

> 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
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.

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.

-Kees

-- 
Kees Cook
Pixel Security

[toc] | [prev] | [next] | [standalone]

#1655774 — 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

[toc] | [prev] | [next] | [standalone]

#1656309 — Re: [kernel-hardening] 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
> 

[toc] | [prev] | [next] | [standalone]

#1656344 — Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

Quoting Matt Brown (matt@nmatt.com):
> 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?

I think you're focusing on the wrong thing.  Your capable check (apart
from the fact that I think I've been convinced CAP_SYS_ADMIN is wrong)
is fine.

The key point is to not only check for TIOCSTI, but instead check for
a whitelisted ioctl.

What would the whitelist look like?  Should configuing that be the way
that you enable/disable, instead of the sysctl in this patchset?  So
by default the whitelist includes all ioctls (no change), but things
like sandboxes/sudo/container-starts can clear out the whitelist?

[toc] | [prev] | [next] | [standalone]

#1656364 — Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

On 6/2/17 11:36 AM, Serge E. Hallyn wrote:
> Quoting Matt Brown (matt@nmatt.com):
>> 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?
> 
> I think you're focusing on the wrong thing.  Your capable check (apart
> from the fact that I think I've been convinced CAP_SYS_ADMIN is wrong)
> is fine.

Can't we also have a sysctl that toggles if CAP_SYS_ADMIN is involved in
this whitelist check? Otherwise someone might leave things out of the
whitelist just because they want to use those ioctls as a privileged
process. Also restricting a privileged user from ioctls with this
whitelist approach is going to be pointless because, if the whitelist is
configurable from userspace, they will just be able to modify the
whitelist.

> 
> The key point is to not only check for TIOCSTI, but instead check for
> a whitelisted ioctl.
> 
> What would the whitelist look like?  Should configuing that be the way
> that you enable/disable, instead of the sysctl in this patchset?  So
> by default the whitelist includes all ioctls (no change), but things
> like sandboxes/sudo/container-starts can clear out the whitelist?
> 

I'm fine with moving this to an LSM that whitelists ioctls. I also want
to understand what a whitelist would like look and how you would
configure it? Does a sysctl that is a list of allowed ioctls work? I
don't want to just have a static whitelist that you can't change without
recompiling your kernel.

just running a sysctl -a on a linux box shows me one thing that looks
like a list: net.core.flow_limit_cpu_bitmap

[toc] | [prev] | [next] | [standalone]

#1656396 — Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

Quoting Matt Brown (matt@nmatt.com):
> On 6/2/17 11:36 AM, Serge E. Hallyn wrote:
> > Quoting Matt Brown (matt@nmatt.com):
> >> 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?
> > 
> > I think you're focusing on the wrong thing.  Your capable check (apart
> > from the fact that I think I've been convinced CAP_SYS_ADMIN is wrong)
> > is fine.
> 

I'm cc:ing linux-api here because really we're designing an interesting
API.

> Can't we also have a sysctl that toggles if CAP_SYS_ADMIN is involved in
> this whitelist check?  Otherwise someone might leave things out of the
> whitelist just because they want to use those ioctls as a privileged
> process.

I'm not quite sure what you're asking for here.  Let me offer a precise
strawman design.  I'm sure there are problems with it, it's just a starting
point.

system-wide whitelist (for now 'may_push_chars') is full by default.

By default, nothing changes - you can use those on your own tty, need
CAP_SYS_ADMIN against init_user_ns otherwise.

Introduce a new CAP_TTY_PRIVILEGED.

When may_push_chars is removed from the whitelist, you lose the ability
to use TIOCSTI on a tty - even your own - if you do not have CAP_TTY_PRIVILEGED
against the tty's user_ns.

>  Also restricting a privileged user from ioctls with this
> whitelist approach is going to be pointless because, if the whitelist is
> configurable from userspace, they will just be able to modify the
> whitelist.
> 
> > 
> > The key point is to not only check for TIOCSTI, but instead check for
> > a whitelisted ioctl.
> > 
> > What would the whitelist look like?  Should configuing that be the way
> > that you enable/disable, instead of the sysctl in this patchset?  So
> > by default the whitelist includes all ioctls (no change), but things
> > like sandboxes/sudo/container-starts can clear out the whitelist?
> > 
> 
> I'm fine with moving this to an LSM that whitelists ioctls. I also want

Right -  what else would go into the whitelist?  may_mmap?

> to understand what a whitelist would like look and how you would
> configure it? Does a sysctl that is a list of allowed ioctls work? I
> don't want to just have a static whitelist that you can't change without
> recompiling your kernel.
> 
> just running a sysctl -a on a linux box shows me one thing that looks
> like a list: net.core.flow_limit_cpu_bitmap

[toc] | [prev] | [next] | [standalone]

#1656409 — Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

On 6/2/17 12:57 PM, Serge E. Hallyn wrote:
> Quoting Matt Brown (matt@nmatt.com):
>> On 6/2/17 11:36 AM, Serge E. Hallyn wrote:
>>> Quoting Matt Brown (matt@nmatt.com):
>>>> 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?
>>>
>>> I think you're focusing on the wrong thing.  Your capable check (apart
>>> from the fact that I think I've been convinced CAP_SYS_ADMIN is wrong)
>>> is fine.
>>
> 
> I'm cc:ing linux-api here because really we're designing an interesting
> API.
> 
>> Can't we also have a sysctl that toggles if CAP_SYS_ADMIN is involved in
>> this whitelist check?  Otherwise someone might leave things out of the
>> whitelist just because they want to use those ioctls as a privileged
>> process.
> 
> I'm not quite sure what you're asking for here.  Let me offer a precise
> strawman design.  I'm sure there are problems with it, it's just a starting
> point.
> 
> system-wide whitelist (for now 'may_push_chars') is full by default.
> 

So is may_push_chars just an alias for TIOCSTI? Or are there some
potential whitelist members that would map to multiple ioctls?

> By default, nothing changes - you can use those on your own tty, need
> CAP_SYS_ADMIN against init_user_ns otherwise.
> 
> Introduce a new CAP_TTY_PRIVILEGED.
> 

I'm fine with this.

> When may_push_chars is removed from the whitelist, you lose the ability
> to use TIOCSTI on a tty - even your own - if you do not have CAP_TTY_PRIVILEGED
> against the tty's user_ns.
> 

How do you propose storing/updating the whitelist? sysctl?

If it is a sysctl, would each whitelist member have a sysctl?
e.g.: kernel.ioctlwhitelist.may_push_chars = 1

Overall, I'm fine with this idea.

>>  Also restricting a privileged user from ioctls with this
>> whitelist approach is going to be pointless because, if the whitelist is
>> configurable from userspace, they will just be able to modify the
>> whitelist.
>>
>>>
>>> The key point is to not only check for TIOCSTI, but instead check for
>>> a whitelisted ioctl.
>>>
>>> What would the whitelist look like?  Should configuing that be the way
>>> that you enable/disable, instead of the sysctl in this patchset?  So
>>> by default the whitelist includes all ioctls (no change), but things
>>> like sandboxes/sudo/container-starts can clear out the whitelist?
>>>
>>
>> I'm fine with moving this to an LSM that whitelists ioctls. I also want
> 
> Right -  what else would go into the whitelist?  may_mmap?
> 
>> to understand what a whitelist would like look and how you would
>> configure it? Does a sysctl that is a list of allowed ioctls work? I
>> don't want to just have a static whitelist that you can't change without
>> recompiling your kernel.
>>
>> just running a sysctl -a on a linux box shows me one thing that looks
>> like a list: net.core.flow_limit_cpu_bitmap

[toc] | [prev] | [next] | [standalone]

#1656446 — Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

Quoting Matt Brown (matt@nmatt.com):
> On 6/2/17 12:57 PM, Serge E. Hallyn wrote:
> > I'm not quite sure what you're asking for here.  Let me offer a precise
> > strawman design.  I'm sure there are problems with it, it's just a starting
> > point.
> > 
> > system-wide whitelist (for now 'may_push_chars') is full by default.
> > 
> 
> So is may_push_chars just an alias for TIOCSTI? Or are there some
> potential whitelist members that would map to multiple ioctls?

<shrug>  I'm seeing it as only TIOCSTI right now.

> > By default, nothing changes - you can use those on your own tty, need
> > CAP_SYS_ADMIN against init_user_ns otherwise.
> > 
> > Introduce a new CAP_TTY_PRIVILEGED.
> > 
> 
> I'm fine with this.
> 
> > When may_push_chars is removed from the whitelist, you lose the ability
> > to use TIOCSTI on a tty - even your own - if you do not have CAP_TTY_PRIVILEGED
> > against the tty's user_ns.
> > 
> 
> How do you propose storing/updating the whitelist? sysctl?
> 
> If it is a sysctl, would each whitelist member have a sysctl?
> e.g.: kernel.ioctlwhitelist.may_push_chars = 1
> 
> Overall, I'm fine with this idea.

That sounds reasonable.  Or a securityfs file - I guess not everyone
has securityfs, but if it were to become part of YAMA then that would
work.

-serge

[toc] | [prev] | [next] | [standalone]

#1656481 — Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

On 6/2/17 3:25 PM, Kees Cook wrote:
> On Fri, Jun 2, 2017 at 12:22 PM, Matt Brown <matt@nmatt.com> wrote:
>> On 6/2/17 2:18 PM, Serge E. Hallyn wrote:
>>> Quoting Matt Brown (matt@nmatt.com):
>>>> On 6/2/17 12:57 PM, Serge E. Hallyn wrote:
>>>>> I'm not quite sure what you're asking for here.  Let me offer a precise
>>>>> strawman design.  I'm sure there are problems with it, it's just a starting
>>>>> point.
>>>>>
>>>>> system-wide whitelist (for now 'may_push_chars') is full by default.
>>>>>
>>>>
>>>> So is may_push_chars just an alias for TIOCSTI? Or are there some
>>>> potential whitelist members that would map to multiple ioctls?
>>>
>>> <shrug>  I'm seeing it as only TIOCSTI right now.
>>>
>>>>> By default, nothing changes - you can use those on your own tty, need
>>>>> CAP_SYS_ADMIN against init_user_ns otherwise.
>>>>>
>>>>> Introduce a new CAP_TTY_PRIVILEGED.
>>>>>
>>>>
>>>> I'm fine with this.
>>>>
>>>>> When may_push_chars is removed from the whitelist, you lose the ability
>>>>> to use TIOCSTI on a tty - even your own - if you do not have CAP_TTY_PRIVILEGED
>>>>> against the tty's user_ns.
>>>>>
>>>>
>>>> How do you propose storing/updating the whitelist? sysctl?
>>>>
>>>> If it is a sysctl, would each whitelist member have a sysctl?
>>>> e.g.: kernel.ioctlwhitelist.may_push_chars = 1
>>>>
>>>> Overall, I'm fine with this idea.
>>>
>>> That sounds reasonable.  Or a securityfs file - I guess not everyone
>>> has securityfs, but if it were to become part of YAMA then that would
>>> work.
>>>
>>
>> Yama doesn't depend on securityfs does it?
>>
>> What do other people think? Should this be an addition to YAMA or its
>> own thing?
>>
>> Alan Cox: what do you think of the above ioctl whitelisting scheme?
> 
> It's easy to stack LSMs, so since Yama is ptrace-focused, perhaps make
> a separate one for TTY hardening?
> 

Sounds good. I also like the idea of them being separate.

Matt Brown

[toc] | [prev] | [next] | [standalone]

#1656483 — Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

On Fri, Jun 2, 2017 at 12:22 PM, Matt Brown <matt@nmatt.com> wrote:
> On 6/2/17 2:18 PM, Serge E. Hallyn wrote:
>> Quoting Matt Brown (matt@nmatt.com):
>>> On 6/2/17 12:57 PM, Serge E. Hallyn wrote:
>>>> I'm not quite sure what you're asking for here.  Let me offer a precise
>>>> strawman design.  I'm sure there are problems with it, it's just a starting
>>>> point.
>>>>
>>>> system-wide whitelist (for now 'may_push_chars') is full by default.
>>>>
>>>
>>> So is may_push_chars just an alias for TIOCSTI? Or are there some
>>> potential whitelist members that would map to multiple ioctls?
>>
>> <shrug>  I'm seeing it as only TIOCSTI right now.
>>
>>>> By default, nothing changes - you can use those on your own tty, need
>>>> CAP_SYS_ADMIN against init_user_ns otherwise.
>>>>
>>>> Introduce a new CAP_TTY_PRIVILEGED.
>>>>
>>>
>>> I'm fine with this.
>>>
>>>> When may_push_chars is removed from the whitelist, you lose the ability
>>>> to use TIOCSTI on a tty - even your own - if you do not have CAP_TTY_PRIVILEGED
>>>> against the tty's user_ns.
>>>>
>>>
>>> How do you propose storing/updating the whitelist? sysctl?
>>>
>>> If it is a sysctl, would each whitelist member have a sysctl?
>>> e.g.: kernel.ioctlwhitelist.may_push_chars = 1
>>>
>>> Overall, I'm fine with this idea.
>>
>> That sounds reasonable.  Or a securityfs file - I guess not everyone
>> has securityfs, but if it were to become part of YAMA then that would
>> work.
>>
>
> Yama doesn't depend on securityfs does it?
>
> What do other people think? Should this be an addition to YAMA or its
> own thing?
>
> Alan Cox: what do you think of the above ioctl whitelisting scheme?

It's easy to stack LSMs, so since Yama is ptrace-focused, perhaps make
a separate one for TTY hardening?

-Kees

-- 
Kees Cook
Pixel Security

[toc] | [prev] | [next] | [standalone]

#1656486 — Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

On 6/2/17 2:18 PM, Serge E. Hallyn wrote:
> Quoting Matt Brown (matt@nmatt.com):
>> On 6/2/17 12:57 PM, Serge E. Hallyn wrote:
>>> I'm not quite sure what you're asking for here.  Let me offer a precise
>>> strawman design.  I'm sure there are problems with it, it's just a starting
>>> point.
>>>
>>> system-wide whitelist (for now 'may_push_chars') is full by default.
>>>
>>
>> So is may_push_chars just an alias for TIOCSTI? Or are there some
>> potential whitelist members that would map to multiple ioctls?
> 
> <shrug>  I'm seeing it as only TIOCSTI right now.
> 
>>> By default, nothing changes - you can use those on your own tty, need
>>> CAP_SYS_ADMIN against init_user_ns otherwise.
>>>
>>> Introduce a new CAP_TTY_PRIVILEGED.
>>>
>>
>> I'm fine with this.
>>
>>> When may_push_chars is removed from the whitelist, you lose the ability
>>> to use TIOCSTI on a tty - even your own - if you do not have CAP_TTY_PRIVILEGED
>>> against the tty's user_ns.
>>>
>>
>> How do you propose storing/updating the whitelist? sysctl?
>>
>> If it is a sysctl, would each whitelist member have a sysctl?
>> e.g.: kernel.ioctlwhitelist.may_push_chars = 1
>>
>> Overall, I'm fine with this idea.
> 
> That sounds reasonable.  Or a securityfs file - I guess not everyone
> has securityfs, but if it were to become part of YAMA then that would
> work.
> 

Yama doesn't depend on securityfs does it?

What do other people think? Should this be an addition to YAMA or its
own thing?

Alan Cox: what do you think of the above ioctl whitelisting scheme?

[toc] | [prev] | [next] | [standalone]

#1656513 — Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

> Can't we also have a sysctl that toggles if CAP_SYS_ADMIN is involved in
> this whitelist check? Otherwise someone might leave things out of the
> whitelist just because they want to use those ioctls as a privileged
> process. Also restricting a privileged user from ioctls with this
> whitelist approach is going to be pointless because, if the whitelist is
> configurable from userspace, they will just be able to modify the
> whitelist.

Something like CAP_SYS_ADMIN is fine if you must have it configurable on
the grounds that CAP_SYS_ADMIN will let you override the list anyway.

> I'm fine with moving this to an LSM that whitelists ioctls. I also want
> to understand what a whitelist would like look and how you would
> configure it? Does a sysctl that is a list of allowed ioctls work? I
> don't want to just have a static whitelist that you can't change without
> recompiling your kernel.

That's odd because your previous argument was for a fixed one entry
whitelist containing TIOCSTI 8)

The list can probably be fixed. IMHO those wanting to do cleverer stuff
sre inevitably going to end up using a "grown up" LSM for the job because

a) they'll want to shape things like su not just containers
b) they will have cases where you want to lock down cleverly - eg you can
disable TIOCSTI use except by certain apps, and make those apps non
ptraceable so only existing real users of it can continue to use it.

For the whitelist you want most of the standard tty ioctls, but none of
the device specific ones, none of the hardware configuration ones, no
TIOCSTI, no selection, no line discipline change (or you can set a
network ldisc or various other evil and fascinating things).

I really think that for container type uses the whitelist can be fairly
clearly defined because we know a container isn't supposed to be screwing
with the hardware of the parent, configuring network interfaces or
pushing data to trip people up.

If we are prepared to accept nuisance attacks (messing up baud rate,
hangup, etc) then it's fairly easy to fix.

So I'd say it's all the generic tty ioctls except TIOCSTI and TIOCSETD
but it would be good to see what Android is going with and why.

You can still do some typeback stuff even then because the consoles and
terminals have ranges of query and requests you can trigger. As you can
use termios to absorb some symbols in the reply and map which one to use
for newline you can even type stuff. However it's very limited - hex
digits, [, c and some other oddments. People have looked hard at that and
I've yet to see a successful attack. Yes I can make you run test (aka
'[') but I've yet to see a way to use it for anything.

Alan

[toc] | [prev] | [next] | [standalone]

#1656515 — Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

On Fri, Jun 2, 2017 at 1:05 PM, Alan Cox <gnomes@lxorguk.ukuu.org.uk> wrote:
> So I'd say it's all the generic tty ioctls except TIOCSTI and TIOCSETD
> but it would be good to see what Android is going with and why.

Android limits tty ioctls to the following whitelist:
  TIOCOUTQ
  FIOCLEX
  TCGETS
  TCSETS
  TIOCGWINSZ
  TIOCSWINSZ
  TIOCSCTTY
  TCSETSW
  TCFLSH
  TIOCSPGRP
  TIOCGPGRP

See unpriv_tty_ioctls at
https://android.googlesource.com/platform/system/sepolicy/+/34b4b73729b288b4109b2225c1445eb58393b8cb/public/ioctl_macros#51


-- 
Nick Kralevich | Android Security | nnk@google.com | 650.214.4037

[toc] | [prev] | [next] | [standalone]

#1656539 — Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

On 6/2/17 4:05 PM, Alan Cox wrote:
>> Can't we also have a sysctl that toggles if CAP_SYS_ADMIN is involved in
>> this whitelist check? Otherwise someone might leave things out of the
>> whitelist just because they want to use those ioctls as a privileged
>> process. Also restricting a privileged user from ioctls with this
>> whitelist approach is going to be pointless because, if the whitelist is
>> configurable from userspace, they will just be able to modify the
>> whitelist.
> 
> Something like CAP_SYS_ADMIN is fine if you must have it configurable on
> the grounds that CAP_SYS_ADMIN will let you override the list anyway.
> 
>> I'm fine with moving this to an LSM that whitelists ioctls. I also want
>> to understand what a whitelist would like look and how you would
>> configure it? Does a sysctl that is a list of allowed ioctls work? I
>> don't want to just have a static whitelist that you can't change without
>> recompiling your kernel.
> 
> That's odd because your previous argument was for a fixed one entry
> whitelist containing TIOCSTI 8)
> 

I just take some convincing that's all ;)

> The list can probably be fixed. IMHO those wanting to do cleverer stuff
> sre inevitably going to end up using a "grown up" LSM for the job because
> 

I really want it to be more flexible if we are making this into a full
blown LSM. From drivers/tty/tty_ioctl.c I gather the following tty
ioctls:

TCGETA
TCGETS
TCGETS2
TCGETX
TCSETA
TCSETAF
TCSETAW
TCSETS
TCSETS2
TCSETSF
TCSETSF2
TCSETSW
TCSETSW2
TCSETX
TCSETXF
TCSETXW
TIOCGETC
TIOCGETP
TIOCGLCKTRMIOS
TIOCGLTC
TIOCGSOFTCAR
TIOCSETC
TIOCSETN
TIOCSETP
TIOCSLCKTRMIOS
TIOCSLTC
TIOCSSOFTCAR

would it be overkill to have a sysctl kernel.ttyioctlwhitelist.X where X
is one of the ioctls above?

(definitely will work on a better name than ttyioctlwhitelist)


> a) they'll want to shape things like su not just containers
> b) they will have cases where you want to lock down cleverly - eg you can
> disable TIOCSTI use except by certain apps, and make those apps non
> ptraceable so only existing real users of it can continue to use it.

I agree. When you have those kinds of requirements, a MAC is required.

> 
> For the whitelist you want most of the standard tty ioctls, but none of
> the device specific ones, none of the hardware configuration ones, no
> TIOCSTI, no selection, no line discipline change (or you can set a
> network ldisc or various other evil and fascinating things).
> 
> I really think that for container type uses the whitelist can be fairly
> clearly defined because we know a container isn't supposed to be screwing
> with the hardware of the parent, configuring network interfaces or
> pushing data to trip people up.
> 
> If we are prepared to accept nuisance attacks (messing up baud rate,
> hangup, etc) then it's fairly easy to fix.
> 
> So I'd say it's all the generic tty ioctls except TIOCSTI and TIOCSETD
> but it would be good to see what Android is going with and why.
> 
> You can still do some typeback stuff even then because the consoles and
> terminals have ranges of query and requests you can trigger. As you can
> use termios to absorb some symbols in the reply and map which one to use
> for newline you can even type stuff. However it's very limited - hex
> digits, [, c and some other oddments. People have looked hard at that and
> I've yet to see a successful attack. Yes I can make you run test (aka
> '[') but I've yet to see a way to use it for anything.
> 
> Alan
> 

[toc] | [prev] | [next] | [standalone]

#1656942 — Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

> TIOCSLCKTRMIOS

That one I'm more dubious about

> TIOCSLTC
> TIOCSSOFTCAR

tty_io.c also has a few and n_tty has a couple we'd want.

> 
> would it be overkill to have a sysctl kernel.ttyioctlwhitelist.X where X
> is one of the ioctls above?

Why would anyone want to change the entries on that list

Alan

[toc] | [prev] | [next] | [standalone]

#1656948 — Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

On 06/03/2017 06:00 PM, Alan Cox wrote:
>> TIOCSLCKTRMIOS
>
> That one I'm more dubious about
>
>> TIOCSLTC
>> TIOCSSOFTCAR
>
> tty_io.c also has a few and n_tty has a couple we'd want.
>
>>
>> would it be overkill to have a sysctl kernel.ttyioctlwhitelist.X where X
>> is one of the ioctls above?
>
> Why would anyone want to change the entries on that list
>

Did you see Serge's proposed solution? I want us to not be talking past
each other. Serge proposed the following:

| By default, nothing changes - you can use those on your own tty, need
| CAP_SYS_ADMIN against init_user_ns otherwise.
|
| Introduce a new CAP_TTY_PRIVILEGED.
|
| When may_push_chars is removed from the whitelist, you lose the
| ability to use TIOCSTI on a tty - even your own - if you do not have
| CAP_TTY_PRIVILEGED against the tty's user_ns.

The question is how do you add/remove something from this whitelist? I
assume by add/remove we don't mean that you have to recompile your
kernel to change the whitelist!

you earlier said you wanted the check to look like this:

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

I want to know which namespace you are talking about here. Did you mean
user_namespace? (the namespace I added tracking for in the tty_struct)

[toc] | [prev] | [next] | [standalone]

Page 2 of 3 — ← Prev page 1 [2] 3  Next page →

Back to top | Article view | linux.kernel

csiph-web