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

Groups > linux.kernel > #1629199 > unrolled thread

[PATCH v4 1/2] tiocsti-restrict : Add owner user namespace to tty_struct

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 v4 1/2] tiocsti-restrict : Add owner user namespace to tty_struct Matt Brown <matt@nmatt.com> - 2017-04-24 07:20 +0200
    Re: [PATCH v4 1/2] tiocsti-restrict : Add owner user namespace to  tty_struct "Serge E. Hallyn" <serge@hallyn.com> - 2017-04-24 16:00 +0200
      Re: [PATCH v4 1/2] tiocsti-restrict : Add owner user namespace to tty_struct Kees Cook <keescook@chromium.org> - 2017-05-03 21:40 +0200
        Re: [PATCH v4 1/2] tiocsti-restrict : Add owner user namespace to  tty_struct Greg KH <gregkh@linuxfoundation.org> - 2017-05-03 21:50 +0200
          Re: [PATCH v4 1/2] tiocsti-restrict : Add owner user namespace to  tty_struct Matt Brown <matt@nmatt.com> - 2017-05-03 22:10 +0200
            Re: [PATCH v4 1/2] tiocsti-restrict : Add owner user namespace to tty_struct Kees Cook <keescook@chromium.org> - 2017-05-03 22:30 +0200
              Re: [kernel-hardening] Re: [PATCH v4 1/2] tiocsti-restrict : Add  owner user namespace to tty_struct "Serge E. Hallyn" <serge@hallyn.com> - 2017-05-04 06:50 +0200
    Re: [PATCH v4 1/2] tiocsti-restrict : Add owner user namespace to  tty_struct Alan Cox <gnomes@lxorguk.ukuu.org.uk> - 2017-04-25 17:00 +0200

#1629199 — [PATCH v4 1/2] tiocsti-restrict : Add owner user namespace to tty_struct

This patch adds struct user_namespace *owner_user_ns to the tty_struct.
Then it is set to current_user_ns() in the alloc_tty_struct function.

This is done to facilitate capability checks against the original user
namespace that allocated the tty.

E.g. ns_capable(tty->owner_user_ns,CAP_SYS_ADMIN)

This combined with the use of user namespace's will allow hardening
protections to be built to mitigate container escapes that utilize TTY
ioctls such as TIOCSTI.

See: https://bugzilla.redhat.com/show_bug.cgi?id=1411256

Signed-off-by: Matt Brown <matt@nmatt.com>
---
 drivers/tty/tty_io.c | 2 ++
 include/linux/tty.h  | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index e6d1a65..c276814 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -171,6 +171,7 @@ static void free_tty_struct(struct tty_struct *tty)
 	put_device(tty->dev);
 	kfree(tty->write_buf);
 	tty->magic = 0xDEADDEAD;
+	put_user_ns(tty->owner_user_ns);
 	kfree(tty);
 }
 
@@ -3191,6 +3192,7 @@ struct tty_struct *alloc_tty_struct(struct tty_driver *driver, int idx)
 	tty->index = idx;
 	tty_line_name(driver, idx, tty->name);
 	tty->dev = tty_get_device(tty);
+	tty->owner_user_ns = get_user_ns(current_user_ns());
 
 	return tty;
 }
diff --git a/include/linux/tty.h b/include/linux/tty.h
index 1017e904..d902d42 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -12,6 +12,7 @@
 #include <uapi/linux/tty.h>
 #include <linux/rwsem.h>
 #include <linux/llist.h>
+#include <linux/user_namespace.h>
 
 
 /*
@@ -333,6 +334,7 @@ struct tty_struct {
 	/* If the tty has a pending do_SAK, queue it here - akpm */
 	struct work_struct SAK_work;
 	struct tty_port *port;
+	struct user_namespace *owner_user_ns;
 };
 
 /* Each of a tty's open files has private_data pointing to tty_file_private */
-- 
2.10.2

[toc] | [next] | [standalone]

#1629596 — Re: [PATCH v4 1/2] tiocsti-restrict : Add owner user namespace to tty_struct

Quoting Matt Brown (matt@nmatt.com):
> This patch adds struct user_namespace *owner_user_ns to the tty_struct.
> Then it is set to current_user_ns() in the alloc_tty_struct function.
> 
> This is done to facilitate capability checks against the original user
> namespace that allocated the tty.
> 
> E.g. ns_capable(tty->owner_user_ns,CAP_SYS_ADMIN)
> 
> This combined with the use of user namespace's will allow hardening
> protections to be built to mitigate container escapes that utilize TTY
> ioctls such as TIOCSTI.
> 
> See: https://bugzilla.redhat.com/show_bug.cgi?id=1411256
> 
> Signed-off-by: Matt Brown <matt@nmatt.com>

Acked-by: Serge Hallyn <serge@hallyn.com>

> ---
>  drivers/tty/tty_io.c | 2 ++
>  include/linux/tty.h  | 2 ++
>  2 files changed, 4 insertions(+)
> 
> diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
> index e6d1a65..c276814 100644
> --- a/drivers/tty/tty_io.c
> +++ b/drivers/tty/tty_io.c
> @@ -171,6 +171,7 @@ static void free_tty_struct(struct tty_struct *tty)
>  	put_device(tty->dev);
>  	kfree(tty->write_buf);
>  	tty->magic = 0xDEADDEAD;
> +	put_user_ns(tty->owner_user_ns);
>  	kfree(tty);
>  }
>  
> @@ -3191,6 +3192,7 @@ struct tty_struct *alloc_tty_struct(struct tty_driver *driver, int idx)
>  	tty->index = idx;
>  	tty_line_name(driver, idx, tty->name);
>  	tty->dev = tty_get_device(tty);
> +	tty->owner_user_ns = get_user_ns(current_user_ns());
>  
>  	return tty;
>  }
> diff --git a/include/linux/tty.h b/include/linux/tty.h
> index 1017e904..d902d42 100644
> --- a/include/linux/tty.h
> +++ b/include/linux/tty.h
> @@ -12,6 +12,7 @@
>  #include <uapi/linux/tty.h>
>  #include <linux/rwsem.h>
>  #include <linux/llist.h>
> +#include <linux/user_namespace.h>
>  
>  
>  /*
> @@ -333,6 +334,7 @@ struct tty_struct {
>  	/* If the tty has a pending do_SAK, queue it here - akpm */
>  	struct work_struct SAK_work;
>  	struct tty_port *port;
> +	struct user_namespace *owner_user_ns;
>  };
>  
>  /* Each of a tty's open files has private_data pointing to tty_file_private */
> -- 
> 2.10.2

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

#1635232

On Mon, Apr 24, 2017 at 6:57 AM, Serge E. Hallyn <serge@hallyn.com> wrote:
> Quoting Matt Brown (matt@nmatt.com):
>> This patch adds struct user_namespace *owner_user_ns to the tty_struct.
>> Then it is set to current_user_ns() in the alloc_tty_struct function.
>>
>> This is done to facilitate capability checks against the original user
>> namespace that allocated the tty.
>>
>> E.g. ns_capable(tty->owner_user_ns,CAP_SYS_ADMIN)
>>
>> This combined with the use of user namespace's will allow hardening
>> protections to be built to mitigate container escapes that utilize TTY
>> ioctls such as TIOCSTI.
>>
>> See: https://bugzilla.redhat.com/show_bug.cgi?id=1411256
>>
>> Signed-off-by: Matt Brown <matt@nmatt.com>
>
> Acked-by: Serge Hallyn <serge@hallyn.com>

This Ack didn't end up in the v5, but I think it stands, yes?

Greg, is the v5 okay to pull for you or would a v6 with Acks/Reviews
included be preferred?

-Kees

>
>> ---
>>  drivers/tty/tty_io.c | 2 ++
>>  include/linux/tty.h  | 2 ++
>>  2 files changed, 4 insertions(+)
>>
>> diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
>> index e6d1a65..c276814 100644
>> --- a/drivers/tty/tty_io.c
>> +++ b/drivers/tty/tty_io.c
>> @@ -171,6 +171,7 @@ static void free_tty_struct(struct tty_struct *tty)
>>       put_device(tty->dev);
>>       kfree(tty->write_buf);
>>       tty->magic = 0xDEADDEAD;
>> +     put_user_ns(tty->owner_user_ns);
>>       kfree(tty);
>>  }
>>
>> @@ -3191,6 +3192,7 @@ struct tty_struct *alloc_tty_struct(struct tty_driver *driver, int idx)
>>       tty->index = idx;
>>       tty_line_name(driver, idx, tty->name);
>>       tty->dev = tty_get_device(tty);
>> +     tty->owner_user_ns = get_user_ns(current_user_ns());
>>
>>       return tty;
>>  }
>> diff --git a/include/linux/tty.h b/include/linux/tty.h
>> index 1017e904..d902d42 100644
>> --- a/include/linux/tty.h
>> +++ b/include/linux/tty.h
>> @@ -12,6 +12,7 @@
>>  #include <uapi/linux/tty.h>
>>  #include <linux/rwsem.h>
>>  #include <linux/llist.h>
>> +#include <linux/user_namespace.h>
>>
>>
>>  /*
>> @@ -333,6 +334,7 @@ struct tty_struct {
>>       /* If the tty has a pending do_SAK, queue it here - akpm */
>>       struct work_struct SAK_work;
>>       struct tty_port *port;
>> +     struct user_namespace *owner_user_ns;
>>  };
>>
>>  /* Each of a tty's open files has private_data pointing to tty_file_private */
>> --
>> 2.10.2



-- 
Kees Cook
Pixel Security

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

#1635243 — Re: [PATCH v4 1/2] tiocsti-restrict : Add owner user namespace to tty_struct

On Wed, May 03, 2017 at 12:32:07PM -0700, Kees Cook wrote:
> On Mon, Apr 24, 2017 at 6:57 AM, Serge E. Hallyn <serge@hallyn.com> wrote:
> > Quoting Matt Brown (matt@nmatt.com):
> >> This patch adds struct user_namespace *owner_user_ns to the tty_struct.
> >> Then it is set to current_user_ns() in the alloc_tty_struct function.
> >>
> >> This is done to facilitate capability checks against the original user
> >> namespace that allocated the tty.
> >>
> >> E.g. ns_capable(tty->owner_user_ns,CAP_SYS_ADMIN)
> >>
> >> This combined with the use of user namespace's will allow hardening
> >> protections to be built to mitigate container escapes that utilize TTY
> >> ioctls such as TIOCSTI.
> >>
> >> See: https://bugzilla.redhat.com/show_bug.cgi?id=1411256
> >>
> >> Signed-off-by: Matt Brown <matt@nmatt.com>
> >
> > Acked-by: Serge Hallyn <serge@hallyn.com>
> 
> This Ack didn't end up in the v5, but I think it stands, yes?
> 
> Greg, is the v5 okay to pull for you or would a v6 with Acks/Reviews
> included be preferred?

v6 would be great, and we are dropping patch 2 from the series, right?
I was expecting this to be resent.  I'll start looking at new patches
like this after 4.12-rc1 is out.

thanks,

greg k-h

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

#1635252 — Re: [PATCH v4 1/2] tiocsti-restrict : Add owner user namespace to tty_struct

On 05/03/2017 03:45 PM, Greg KH wrote:
> On Wed, May 03, 2017 at 12:32:07PM -0700, Kees Cook wrote:
>> On Mon, Apr 24, 2017 at 6:57 AM, Serge E. Hallyn <serge@hallyn.com> wrote:
>>> Quoting Matt Brown (matt@nmatt.com):
>>>> This patch adds struct user_namespace *owner_user_ns to the tty_struct.
>>>> Then it is set to current_user_ns() in the alloc_tty_struct function.
>>>>
>>>> This is done to facilitate capability checks against the original user
>>>> namespace that allocated the tty.
>>>>
>>>> E.g. ns_capable(tty->owner_user_ns,CAP_SYS_ADMIN)
>>>>
>>>> This combined with the use of user namespace's will allow hardening
>>>> protections to be built to mitigate container escapes that utilize TTY
>>>> ioctls such as TIOCSTI.
>>>>
>>>> See: https://bugzilla.redhat.com/show_bug.cgi?id=1411256
>>>>
>>>> Signed-off-by: Matt Brown <matt@nmatt.com>
>>>
>>> Acked-by: Serge Hallyn <serge@hallyn.com>
>>
>> This Ack didn't end up in the v5, but I think it stands, yes?
>>
>> Greg, is the v5 okay to pull for you or would a v6 with Acks/Reviews
>> included be preferred?
>
> v6 would be great, and we are dropping patch 2 from the series, right?
> I was expecting this to be resent.  I'll start looking at new patches
> like this after 4.12-rc1 is out.
>

I will create a v6 with the Acks/Reviews. I'd like to keep patch 2 in
since that got acked by at least Serge. (Kees also? or just patch 1?)

> thanks,
>
> greg k-h
>

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

#1635258

On Wed, May 3, 2017 at 1:02 PM, Matt Brown <matt@nmatt.com> wrote:
> On 05/03/2017 03:45 PM, Greg KH wrote:
>>
>> On Wed, May 03, 2017 at 12:32:07PM -0700, Kees Cook wrote:
>>>
>>> On Mon, Apr 24, 2017 at 6:57 AM, Serge E. Hallyn <serge@hallyn.com>
>>> wrote:
>>>>
>>>> Quoting Matt Brown (matt@nmatt.com):
>>>>>
>>>>> This patch adds struct user_namespace *owner_user_ns to the tty_struct.
>>>>> Then it is set to current_user_ns() in the alloc_tty_struct function.
>>>>>
>>>>> This is done to facilitate capability checks against the original user
>>>>> namespace that allocated the tty.
>>>>>
>>>>> E.g. ns_capable(tty->owner_user_ns,CAP_SYS_ADMIN)
>>>>>
>>>>> This combined with the use of user namespace's will allow hardening
>>>>> protections to be built to mitigate container escapes that utilize TTY
>>>>> ioctls such as TIOCSTI.
>>>>>
>>>>> See: https://bugzilla.redhat.com/show_bug.cgi?id=1411256
>>>>>
>>>>> Signed-off-by: Matt Brown <matt@nmatt.com>
>>>>
>>>>
>>>> Acked-by: Serge Hallyn <serge@hallyn.com>
>>>
>>>
>>> This Ack didn't end up in the v5, but I think it stands, yes?
>>>
>>> Greg, is the v5 okay to pull for you or would a v6 with Acks/Reviews
>>> included be preferred?
>>
>>
>> v6 would be great, and we are dropping patch 2 from the series, right?
>> I was expecting this to be resent.  I'll start looking at new patches
>> like this after 4.12-rc1 is out.
>>
>
> I will create a v6 with the Acks/Reviews. I'd like to keep patch 2 in
> since that got acked by at least Serge. (Kees also? or just patch 1?)

Sorry, I meant that patch 2's ack from serge got dropped accidentally.
i.e. he Acked v4, but it wasn't in v5.

Serge, just to double-check, does your Ack stand?

-Kees

-- 
Kees Cook
Pixel Security

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

#1635409 — Re: [kernel-hardening] Re: [PATCH v4 1/2] tiocsti-restrict : Add owner user namespace to tty_struct

On Wed, May 03, 2017 at 01:19:41PM -0700, Kees Cook wrote:
> On Wed, May 3, 2017 at 1:02 PM, Matt Brown <matt@nmatt.com> wrote:
> > On 05/03/2017 03:45 PM, Greg KH wrote:
> >>
> >> On Wed, May 03, 2017 at 12:32:07PM -0700, Kees Cook wrote:
> >>>
> >>> On Mon, Apr 24, 2017 at 6:57 AM, Serge E. Hallyn <serge@hallyn.com>
> >>> wrote:
> >>>>
> >>>> Quoting Matt Brown (matt@nmatt.com):
> >>>>>
> >>>>> This patch adds struct user_namespace *owner_user_ns to the tty_struct.
> >>>>> Then it is set to current_user_ns() in the alloc_tty_struct function.
> >>>>>
> >>>>> This is done to facilitate capability checks against the original user
> >>>>> namespace that allocated the tty.
> >>>>>
> >>>>> E.g. ns_capable(tty->owner_user_ns,CAP_SYS_ADMIN)
> >>>>>
> >>>>> This combined with the use of user namespace's will allow hardening
> >>>>> protections to be built to mitigate container escapes that utilize TTY
> >>>>> ioctls such as TIOCSTI.
> >>>>>
> >>>>> See: https://bugzilla.redhat.com/show_bug.cgi?id=1411256
> >>>>>
> >>>>> Signed-off-by: Matt Brown <matt@nmatt.com>
> >>>>
> >>>>
> >>>> Acked-by: Serge Hallyn <serge@hallyn.com>
> >>>
> >>>
> >>> This Ack didn't end up in the v5, but I think it stands, yes?
> >>>
> >>> Greg, is the v5 okay to pull for you or would a v6 with Acks/Reviews
> >>> included be preferred?
> >>
> >>
> >> v6 would be great, and we are dropping patch 2 from the series, right?
> >> I was expecting this to be resent.  I'll start looking at new patches
> >> like this after 4.12-rc1 is out.
> >>
> >
> > I will create a v6 with the Acks/Reviews. I'd like to keep patch 2 in
> > since that got acked by at least Serge. (Kees also? or just patch 1?)
> 
> Sorry, I meant that patch 2's ack from serge got dropped accidentally.
> i.e. he Acked v4, but it wasn't in v5.
> 
> Serge, just to double-check, does your Ack stand?

Yes.

thanks,
-serge

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

#1630578 — Re: [PATCH v4 1/2] tiocsti-restrict : Add owner user namespace to tty_struct

On Mon, 24 Apr 2017 01:15:11 -0400
Matt Brown <matt@nmatt.com> wrote:

> This patch adds struct user_namespace *owner_user_ns to the tty_struct.
> Then it is set to current_user_ns() in the alloc_tty_struct function.
> 
> This is done to facilitate capability checks against the original user
> namespace that allocated the tty.
> 
> E.g. ns_capable(tty->owner_user_ns,CAP_SYS_ADMIN)
> 
> This combined with the use of user namespace's will allow hardening
> protections to be built to mitigate container escapes that utilize TTY
> ioctls such as TIOCSTI.

Regardles of the TIOCSTI usefulness this makes complete sense.

Alan

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web