Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1759319AbaLLBal (ORCPT ); Thu, 11 Dec 2014 20:30:41 -0500 Received: from mail-lb0-f169.google.com ([209.85.217.169]:33481 "EHLO mail-lb0-f169.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756961AbaLLBah convert rfc822-to-8bit (ORCPT ); Thu, 11 Dec 2014 20:30:37 -0500 MIME-Version: 1.0 In-Reply-To: <87iohh3c9c.fsf@x220.int.ebiederm.org> References: <52e0643bd47b1e5c65921d6e00aea1f724bb510a.1417281801.git.luto@amacapital.net> <87fvcxyf28.fsf_-_@x220.int.ebiederm.org> <874mtdyexp.fsf_-_@x220.int.ebiederm.org> <87a935u3nj.fsf@x220.int.ebiederm.org> <87388xodlj.fsf@x220.int.ebiederm.org> <87h9x5re41.fsf_-_@x220.int.ebiederm.org> <87mw6xpzb0.fsf_-_@x220.int.ebiederm.org> <87ppbtn4mv.fsf@x220.int.ebiederm.org> <87a92xn2io.fsf@x220.int.ebiederm.org> <87r3w8liw4.fsf@x220.int.ebiederm.org> <87iohklfvj.fsf_-_@x220.int.ebiederm.org> <87fvcok11h.fsf_-_@x220.int.ebiederm.org> <971ad3f6-90fd-4e3f-916c-8988af3c826d@email.android.com> <87wq5zf83t.fsf@x220.int.ebiederm.org> <87iohh3c9c.fsf@x220.int.ebiederm.org> From: Andy Lutomirski Date: Thu, 11 Dec 2014 17:30:15 -0800 Message-ID: Subject: Re: [CFT][PATCH 7/8] userns: Add a knob to disable setgroups on a per user namespace basis To: "Eric W. Biederman" Cc: Linux Containers , Josh Triplett , Andrew Morton , Kees Cook , Michael Kerrisk-manpages , Linux API , linux-man , "linux-kernel@vger.kernel.org" , LSM , Casey Schaufler , "Serge E. Hallyn" , Richard Weinberger , Kenton Varda , stable Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Dec 11, 2014 at 5:09 PM, Eric W. Biederman wrote: > ebiederm@xmission.com (Eric W. Biederman) writes: > >> Andy Lutomirski writes: >> >>> On Tue, Dec 9, 2014 at 4:04 PM, Eric W.Biederman wrote: >>>> >>>> >>>> On December 9, 2014 4:28:38 PM CST, Andy Lutomirski wrote: >>>>>On Tue, Dec 9, 2014 at 12:42 PM, Eric W. Biederman >>>>> wrote: >>>>>> >>>>>> - Expose the knob to user space through a proc file >>>>>/proc//setgroups >>>>>> >>>>>> A value of "deny" means the setgroups system call is disabled in >>>>>the >>>>>> current processes user namespace and can not be enabled in the >>>>>> future in this user namespace. >>>>>> >>>>>> A value of "allow" means the segtoups system call is enabled. >>>>>> >>>>>> - Descendant user namespaces inherit the value of setgroups from >>>>>> their parents. >>>>>> >>>>>> - A proc file is used (instead of a sysctl) as sysctls >>>>>> currently do not pass in a struct file so file_ns_capable >>>>>> is unusable. >>>>> >>>>>Reviewed-by: Andy Lutomirski >>>>> >>>>>But I still don't like the name "setgroups". People may look at that >>>>>and have no clue what the scope of the setting is. And anyone who, as >>>>>root, writes "deny" to /proc/self/setgroups, thinking that it acts on >>>>>self, will be in for a surprise. >>>> >>>> True setgroups isn't perfect. Documenting it in a manpage may have to be enough. The only real improvement I can think of would be to make the setting a sysctl. But I think pursuing that approaches the point where perfection is the enemy of getting this problem fixed. >>>> >>> >>> Would "userns_setgroups" be okay? >> >> Maybe. >> >> I just played with this and this is a much bigger booby trap than I had >> realized. Disabling setgroups disables the possibility of logging in the >> future and since it is a one way switch the only way out is to reboot. >> >> Hooray our software checks the returns of setgroups. Booh. This is a >> really nasty knob to have anywhere. >> >> I need to think about this a little bit. Giving root the power to shoot >> himself in the foot is one thing. Giving root a loaded gun pointed at >> his foot with the hammer pulled back, and a sign that says I dare you to >> pull the trigger, seems like a bad idea. >> >> I think I need to reduce when that knob can be used. Grr. >> Back to the drawing board! > > I tried out a bunch of things and finally found a simple rule. Don't > allow setgroups to be disabled after setgroups has been enabled in a > user namespace. Or in practical terms don't allow setgroups to be > disabled after the gid_map has been set. > > Which in practice pretty nearly means that we are only allowing writes > to setgroups when it is a single process and it's eventual children that > can be affected. > > At which point I don't think a name change would make things any > clearer. The name change still helps the user to does: $ ls /proc/self "setgroups? what's that?" > > I have also updated the code to move the permission checks to open > where they belong (doh!). Patch follows. Will review and test. > > Eric > > > > > > -- Andy Lutomirski AMA Capital Management, LLC -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/