From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932529AbbFIWGR (ORCPT ); Tue, 9 Jun 2015 18:06:17 -0400 Received: from mail-vn0-f47.google.com ([209.85.216.47]:45922 "EHLO mail-vn0-f47.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753597AbbFIWGI (ORCPT ); Tue, 9 Jun 2015 18:06:08 -0400 MIME-Version: 1.0 In-Reply-To: <20150609215227.GG2045@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> Date: Tue, 9 Jun 2015 15:06:07 -0700 X-Google-Sender-Auth: XtGNDyNiWfN-jcBoupJ4EehEAaM Message-ID: Subject: Re: [PATCH v2] seccomp: add ptrace options for suspend/resume From: Kees Cook To: Tycho Andersen Cc: Oleg Nesterov , Andy Lutomirski , LKML , Linux API , Roland McGrath , Pavel Emelyanov , "Serge E. Hallyn" , Will Drewry Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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? -Kees -- Kees Cook Chrome OS Security