Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752785AbcDOQtn (ORCPT ); Fri, 15 Apr 2016 12:49:43 -0400 Received: from mail-ob0-f179.google.com ([209.85.214.179]:36429 "EHLO mail-ob0-f179.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752456AbcDOQtl (ORCPT ); Fri, 15 Apr 2016 12:49:41 -0400 MIME-Version: 1.0 In-Reply-To: <877ffyzy1j.fsf_-_@x220.int.ebiederm.org> References: <878u0s3orx.fsf_-_@x220.int.ebiederm.org> <1459819769-30387-1-git-send-email-ebiederm@xmission.com> <87twjcorwg.fsf@x220.int.ebiederm.org> <20160409140909.42315e6d@lxorguk.ukuu.org.uk> <83FE8CD2-C0A2-4ADB-AEBD-8DD89AD4F88A@zytor.com> <87bn5ij0x1.fsf@x220.int.ebiederm.org> <78205895-E11D-417F-91DC-4BCA0B61A122@zytor.com> <570D4781.3070600@zytor.com> <877ffyzy1j.fsf_-_@x220.int.ebiederm.org> From: Andy Lutomirski Date: Fri, 15 Apr 2016 09:49:21 -0700 Message-ID: Subject: Re: [PATCH 01/16] devpts: Attempting to get it right To: "Eric W. Biederman" Cc: Linus Torvalds , "H. Peter Anvin" , security@debian.org, "security@kernel.org" , Al Viro , "security@ubuntu.com >> security" , Peter Hurley , Serge Hallyn , Willy Tarreau , Aurelien Jarno , One Thousand Gnomes , Jann Horn , Greg KH , Linux Kernel Mailing List , Jiri Slaby , Florian Weimer Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2824 Lines: 59 On Fri, Apr 15, 2016 at 8:34 AM, Eric W. Biederman wrote: > > To recap the situation for those who have not been following closely. > > There are programs such as xen-create-image that run as root and setup > a chroot environment with: > "mknod dev/ptmx c 5 2" > "mkdir dev/pts" > "mount -t devpts none dev/pts" > > Which mostly works but stomps the mount options of the system /dev/pts. > In particular the options of "gid=5,mode=620" are lost resulting in a > situation where creating a new pty by opening /dev/ptmx results in > that pty having the wrong permissions. > > Some distributions have been working around this problem by continuing > to install a setuid root pt_chown binary that will be called by glibc > to fix the permissions. > > Maintaining a setuid root pt_chown binary is not too scary until > multiple instances of devpts are considered at which point it becomes > possible to trick the setuid root pt_chown binary into operating on the > wrong files and directories. Leading to all of the things one might > fear when a setuid root binary goes wrong. > > The following patchset digs us out of that hole by carefully devpts and > /dev/ptmx in a way that does not introduce any userspace regressions, > while making each mount of devpts distinct (so pt_chown is unnecessary) > and arranging things so that enough information is available so > that a secure pt_chown binary is possible to write if that is ever > needed. > > The approach I have chosen to take is to first enhance the /dev/ptmx > device node to automount /dev/pts/ptmx on top of it. This leads to a > simple high performance solution that allows applications such as > xen-create-image (that call "mknod ptmx c 5 2" and mount devpts) > to continue to run as before even when they are given a non-system > instance of devpts. > > Using automountic bind mounts of /dev/pts/ptmx results in no new > security cases to consider as this can already be done, and actually > results in a simplification of the analysis of the code. As now all > opens of ptmx are of /dev/pts/ptmx. /dev/ptmx is now just a magic > mountpoint that does the right thing. And what happens when someone tries to rm /dev/ptmx or unmount their pts instance or similar? What happens if /dev/ptmx is in a mount that is set to propagate elsewhere but /dev/pts was replaced by an unprivileged user? (Can this happen? I'm not sure.) This seems much weirder than the previous approach. I think I'm starting to come over to Linus' view -- the magic lookup was fine, and I still can't think of a case where the permissions matter. If we care, we can cause the /dev/ptmx magic lookup to fail if the devpts it finds was created with newinstance. (After all, devpts instances created with newinstance *never* worked via /dev/ptmx magic.) --Andy