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

Groups > linux.kernel > #1712436 > unrolled thread

Re: [PATCH] ioctl_tty.2: add TIOCGPTPEER documentation

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

  Re: [PATCH] ioctl_tty.2: add TIOCGPTPEER documentation "Michael Kerrisk (man-pages)" <mtk.manpages@gmail.com> - 2017-08-15 21:30 +0200
    Re: [PATCH] ioctl_tty.2: add TIOCGPTPEER documentation Aleksa Sarai <asarai@suse.de> - 2017-08-16 06:50 +0200
    Re: [PATCH] ioctl_tty.2: add TIOCGPTPEER documentation ebiederm@xmission.com (Eric W. Biederman) - 2017-08-16 18:50 +0200
      Re: [PATCH] ioctl_tty.2: add TIOCGPTPEER documentation Aleksa Sarai <asarai@suse.de> - 2017-08-16 19:00 +0200
        Re: [PATCH] ioctl_tty.2: add TIOCGPTPEER documentation ebiederm@xmission.com (Eric W. Biederman) - 2017-08-16 19:20 +0200

#1712436 — Re: [PATCH] ioctl_tty.2: add TIOCGPTPEER documentation

On 06/09/2017 07:01 PM, Aleksa Sarai wrote:
> The feature this patch references has currently only been accepted into
> tty-testing, but Greg told me to kick this down to man-pages. As a
> result, I can't reference upstream commit id's because the code isn't in
> Linus' tree yet -- should I resend this once it lands in tty-next or
> Linus' tree?
> 
> Also obviously the release version is a bit of a lie.

Hello Aleksa,

I've applied this patch, and then tweaked the wording a little. Could
you please check the following text:

       TIOCGPTPEER    int flags
              (since Linux 4.13) Given  a  file  descriptor  in  fd  that
              refers  to  a  pseudoterminal  master, open (with the given
              open(2)-style flags) and return a new file descriptor  that
              refers to the peer pseudoterminal slave device.  This oper‐
              ation can be performed regardless of whether  the  pathname
              of  the  slave  device  is  accessible  through the calling
              process's mount namespaces.

              Security-conscious programs interacting with namespaces may
              wish  to  use  this  operation rather than open(2) with the
              pathname returned by ptsname(3), and similar library  func‐
              tions that have insecure APIs.

I also have a question on the last sentence: what are the "similar library
functions that have insecure APIs"? It's not clear to me what you are 
referring to here.

Cheers,

Michael

> 
> 8<-----------------------------------------------------------------------
> 
> This is an ioctl(2) recently added by myself, to allow for container
> runtimes and other programs that interact with (potentially hostile)
> Linux namespaces to safely create {master,slave} pseudoterminal pairs
> without needing to open potentially unsafe /dev/pts/... filenames that
> may be malicious mountpoints or similar in an untrusted namespace
> (avoiding the endless issues with ptsname(3) and similar approaches).
> 
> Cc: <containers@lists.linux-foundation.org>
> Signed-off-by: Aleksa Sarai <asarai@suse.de>
> ---
>  man2/ioctl_tty.2 | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/man2/ioctl_tty.2 b/man2/ioctl_tty.2
> index d280beacf..61e147d99 100644
> --- a/man2/ioctl_tty.2
> +++ b/man2/ioctl_tty.2
> @@ -380,6 +380,21 @@ Place the current lock state of the pseudoterminal slave device
>  in the location pointed to by
>  .IR argp
>  (since Linux 3.8).
> +.TP
> +.BI "TIOCGPTPEER	int " flags
> +Opens and returns a new file handle to the pseudoterminal slave
> +device with the given
> +.BR open (2)-style
> +.IR flags ,
> +regardless of whether the path is accessible through the calling process's
> +mount namespaces.
> +
> +Security-conscious programs interacting with namespaces may wish to use this
> +over
> +.BR open (2)
> +with the path provided by
> +.BR ptsname (3),
> +and similar library methods that have insecure APIs (since Linux 4.13).
>  .PP
>  The BSD ioctls
>  .BR TIOCSTOP ,
> 


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

[toc] | [next] | [standalone]

#1712659

> I've applied this patch, and then tweaked the wording a little. Could
> you please check the following text:
> 
>         TIOCGPTPEER    int flags
>                (since Linux 4.13) Given  a  file  descriptor  in  fd  that
>                refers  to  a  pseudoterminal  master, open (with the given
>                open(2)-style flags) and return a new file descriptor  that
>                refers to the peer pseudoterminal slave device.  This oper‐
>                ation can be performed regardless of whether  the  pathname
>                of  the  slave  device  is  accessible  through the calling
>                process's mount namespaces.
> 
>                Security-conscious programs interacting with namespaces may
>                wish  to  use  this  operation rather than open(2) with the
>                pathname returned by ptsname(3), and similar library  func‐
>                tions that have insecure APIs.

Yup, that sounds good.

> I also have a question on the last sentence: what are the "similar library
> functions that have insecure APIs"? It's not clear to me what you are
> referring to here.

There are a few posix_-style functions provided by glibc that are just 
wrappers around the open+ptsname combo that I mention earlier in the 
sentence (and thus are vulnerable to the same issue). But if you feel 
it's confusing you can feel free to drop it.

Thanks.

-- 
Aleksa Sarai
Software Engineer (Containers)
SUSE Linux GmbH
https://www.cyphar.com/

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

#1713113

"Michael Kerrisk (man-pages)" <mtk.manpages@gmail.com> writes:

> On 06/09/2017 07:01 PM, Aleksa Sarai wrote:
>> The feature this patch references has currently only been accepted into
>> tty-testing, but Greg told me to kick this down to man-pages. As a
>> result, I can't reference upstream commit id's because the code isn't in
>> Linus' tree yet -- should I resend this once it lands in tty-next or
>> Linus' tree?
>> 
>> Also obviously the release version is a bit of a lie.
>
> Hello Aleksa,
>
> I've applied this patch, and then tweaked the wording a little. Could
> you please check the following text:
>
>        TIOCGPTPEER    int flags
>               (since Linux 4.13) Given  a  file  descriptor  in  fd  that
>               refers  to  a  pseudoterminal  master, open (with the given
>               open(2)-style flags) and return a new file descriptor  that
>               refers to the peer pseudoterminal slave device.  This oper‐
>               ation can be performed regardless of whether  the  pathname
>               of  the  slave  device  is  accessible  through the calling
>               process's mount namespaces.
>
>               Security-conscious programs interacting with namespaces may
>               wish  to  use  this  operation rather than open(2) with the
>               pathname returned by ptsname(3), and similar library  func‐
>               tions that have insecure APIs.
>
> I also have a question on the last sentence: what are the "similar library
> functions that have insecure APIs"? It's not clear to me what you are 
> referring to here.

A couple of things to note on the bigger picture.

The glibc library on all distributions has been changed to not have a
setuid binary pt_chown, that uses ptsname.  This was the primary fix
for the security issue.

The behavior of opening /dev/ptmx has been changed to perform a path
lookup relative to the location of /dev/ptmx of  ./pts/ptmx and open
it it is a devpts filesystem and to fail otherwise.    This further
makes it hard to confuse userspace this way as /dev/ptmx always
corresponds to /dev/pts/ptmx.  Even in chroots and in other mount
namespaces.

Both of these changes largely makes glibc's use of these features
secure.  /dev/ptmx always corresponds to /dev/pts and there no readily
available suid root applications too fool.

That makes TIOCGPTPEER a very nice addition, but not something people
have to scramble to use to ensure their system is secure.  As a hostile
environment now has to work very hard to confuse the existing mechanisms.

>> This is an ioctl(2) recently added by myself, to allow for container
>> runtimes and other programs that interact with (potentially hostile)
>> Linux namespaces to safely create {master,slave} pseudoterminal pairs
>> without needing to open potentially unsafe /dev/pts/... filenames that
>> may be malicious mountpoints or similar in an untrusted namespace
>> (avoiding the endless issues with ptsname(3) and similar approaches).
>> 
>> Cc: <containers@lists.linux-foundation.org>
>> Signed-off-by: Aleksa Sarai <asarai@suse.de>

Eric

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

#1713123

> A couple of things to note on the bigger picture.
> 
> The glibc library on all distributions has been changed to not have a
> setuid binary pt_chown, that uses ptsname.  This was the primary fix
> for the security issue.
> 
> The behavior of opening /dev/ptmx has been changed to perform a path
> lookup relative to the location of /dev/ptmx of  ./pts/ptmx and open
> it it is a devpts filesystem and to fail otherwise.    This further
> makes it hard to confuse userspace this way as /dev/ptmx always
> corresponds to /dev/pts/ptmx.  Even in chroots and in other mount
> namespaces.

I have a feeling that there might be a way to trick glibc if you use 
FUSE, but I haven't actually tried to create a PoC for it. Fair point 
though.

> That makes TIOCGPTPEER a very nice addition, but not something people
> have to scramble to use to ensure their system is secure.  As a hostile
> environment now has to work very hard to confuse the existing mechanisms.

There are usecases where you simply need TIOCGPTPEER, and no other 
userspace alternative will do, but maybe if we modified the paragraph to 
read (as suggested):

   Security-conscious programs interacting with namespaces may
   wish  to  use  this  operation rather than open(2) with the
   pathname returned by ptsname(3).

This would clarify that there are usecases where you need this 
particular feature, without saying causing people to panic over 
inaccurate claims of glibc being broken. Does that sound better?

-- 
Aleksa Sarai
Software Engineer (Containers)
SUSE Linux GmbH
https://www.cyphar.com/

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

#1713143

Aleksa Sarai <asarai@suse.de> writes:

>> A couple of things to note on the bigger picture.
>>
>> The glibc library on all distributions has been changed to not have a
>> setuid binary pt_chown, that uses ptsname.  This was the primary fix
>> for the security issue.
>>
>> The behavior of opening /dev/ptmx has been changed to perform a path
>> lookup relative to the location of /dev/ptmx of  ./pts/ptmx and open
>> it it is a devpts filesystem and to fail otherwise.    This further
>> makes it hard to confuse userspace this way as /dev/ptmx always
>> corresponds to /dev/pts/ptmx.  Even in chroots and in other mount
>> namespaces.
>
> I have a feeling that there might be a way to trick glibc if you use
> FUSE, but I haven't actually tried to create a PoC for it. Fair point
> though.

To trick glibc fuse would have to be mounted somewhere on /dev.

>> That makes TIOCGPTPEER a very nice addition, but not something people
>> have to scramble to use to ensure their system is secure.  As a hostile
>> environment now has to work very hard to confuse the existing mechanisms.
>
> There are usecases where you simply need TIOCGPTPEER, and no other
> userspace alternative will do, but maybe if we modified the paragraph
> to read (as suggested):
>
>   Security-conscious programs interacting with namespaces may
>   wish  to  use  this  operation rather than open(2) with the
>   pathname returned by ptsname(3).
>
> This would clarify that there are usecases where you need this
> particular feature, without saying causing people to panic over
> inaccurate claims of glibc being broken. Does that sound better?

I think your original words sounded fine.  I would even go for new
programs may want to use the new ioctl as it fundamentally less racy
and more of what is actually trying to be implemented with the userspace
pieces.

I just wanted to point out that TIOCGPTPEER while being the interface
that it would have been nice had we had since the beginning (and would
have avoided all of the problems) is actually not something we need to
scramble and use it is just a very nice to have.  As the immediate
issues have been fixed in other ways.    It was not clear to me from the
other discussions if you and Michael Kerrisk were aware of the
mitigations that had been made to address the security issue.

The change to the behavior of /dev/ptmx may need to be documented
somewhere.   I am not certain if anything has been documented since
devpts has started allowing multiple mounts.

Eric

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web