From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753011AbbFIWNh (ORCPT ); Tue, 9 Jun 2015 18:13:37 -0400 Received: from mail-ig0-f182.google.com ([209.85.213.182]:37687 "EHLO mail-ig0-f182.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751917AbbFIWN2 (ORCPT ); Tue, 9 Jun 2015 18:13:28 -0400 Date: Tue, 9 Jun 2015 16:13:26 -0600 From: Tycho Andersen To: Kees Cook Cc: Oleg Nesterov , Andy Lutomirski , LKML , Linux API , Roland McGrath , Pavel Emelyanov , "Serge E. Hallyn" , Will Drewry Subject: Re: [PATCH v2] seccomp: add ptrace options for suspend/resume Message-ID: <20150609221326.GH2045@hopstrocity> References: <1433369396-13360-1-git-send-email-tycho.andersen@canonical.com> <20150604183149.GA560@redhat.com> <20150604210529.GJ3160@smitten> <20150605211650.GA25718@redhat.com> <20150609212222.GE2045@hopstrocity> <20150609215227.GG2045@hopstrocity> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Jun 09, 2015 at 03:06:07PM -0700, Kees Cook wrote: > On Tue, Jun 9, 2015 at 2:52 PM, Tycho Andersen > wrote: > > On Tue, Jun 09, 2015 at 02:45:49PM -0700, Kees Cook wrote: > >> On Tue, Jun 9, 2015 at 2:22 PM, Tycho Andersen > >> wrote: > >> > Hi Kees, Andy, > >> > > >> > On Fri, Jun 05, 2015 at 11:16:50PM +0200, Oleg Nesterov wrote: > >> >> Hi Tycho, > >> >> > >> >> On 06/04, Tycho Andersen wrote: > >> >> > > > +#ifdef CONFIG_CHECKPOINT_RESTORE > >> >> > > > +bool may_suspend_seccomp(void) > >> >> > > > +{ > >> >> > > > + if (!capable(CAP_SYS_ADMIN)) > >> >> > > > + return false; > >> >> > > > + > >> >> > > > + if (current->seccomp.mode != SECCOMP_MODE_DISABLED) > >> >> > > > + return false; > >> >> > > > >> >> > > Heh. OK, I won't argue with the new check too ;) > >> >> > > >> >> > Actually now that I think about it I agree with you, these checks > >> >> > don't seem necessary. Even inside a user namespace, if you can ptrace > >> >> > a process you can make it do whatever you want irrespective of > >> >> > seccomp, as long as it has the necessary capabilities. Once the > >> >> > seccomp checks are run after ptrace, they'll be enforced so you > >> >> > couldn't have it call whatever you want in the first place. > >> >> > >> >> Good ;) > >> >> > >> >> > Still, perhaps I'm missing something... > >> >> > >> >> Kees, Andy? > >> > > >> > Any thoughts on removing may_suspend_seccomp() all together? > >> > >> As in, just open-code the check? That would be fine by me. > > > > Sorry, I meant getting rid of any checks entirely. Using my argument > > above I've managed to convince myself they don't add any value. You > > guys know a lot more about this than I do, though. > > Well, as things stand currently, yes, that check would be redundant. > The fact that ptrace can be used to bypass seccomp is kind of an > accident, though. The design for ptrace-based seccomp managers was > that the manager would do the work, rather than rewriting the syscall > on behalf of the child. I don't think anything actually uses this > effect. It's something we've wanted to fix, though a clean solution > isn't obvious. As a result, I'm cautious to add this behavior in such > a wide open fashion. For now, I'd like to limit the scope of this to > CAP_SYS_ADMIN. > > I do think dropping the seccomp.mode check is fine -- this would mean > you could set this flag before the child even added seccomp filters. > So, instead of the function call, maybe just add the capable() call? Ok, sounds good; I'll make the change and re-send. Thanks! Tycho