From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-14.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, MENTIONS_GIT_HOSTING,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2E217C56202 for ; Wed, 11 Nov 2020 02:13:19 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id C203F221F7 for ; Wed, 11 Nov 2020 02:13:18 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=paul-moore-com.20150623.gappssmtp.com header.i=@paul-moore-com.20150623.gappssmtp.com header.b="KTKMv/zO" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1732112AbgKKCNR (ORCPT ); Tue, 10 Nov 2020 21:13:17 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:57704 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726861AbgKKCNQ (ORCPT ); Tue, 10 Nov 2020 21:13:16 -0500 Received: from mail-ej1-x643.google.com (mail-ej1-x643.google.com [IPv6:2a00:1450:4864:20::643]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id F20FCC0613D3 for ; Tue, 10 Nov 2020 18:13:15 -0800 (PST) Received: by mail-ej1-x643.google.com with SMTP id o21so631224ejb.3 for ; Tue, 10 Nov 2020 18:13:15 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=paul-moore-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=41103hI8FWw3JSOY3ZzOk1BVYmgr6In9mKvl3fRAkI8=; b=KTKMv/zOS73xcCukPUb1GuVz+OE9Ty3qAtfJVTZcfPeA5J1bqv+9Va8PAjErGdNHgR 1G7xmF84AhTTwwrjua8RpMOwFtR9ro4zL8YLmhIukEv0R6Z+133UOEdMYnBAPrSKFHvi PuW4eVIDPNHKAdBiXiFsRXFRmFZY8LN5BgQ1AHgdeZ3kKv1IfbXgjcNz0WqPDSLI/u0v 5PVi1F5qwnMIRgTcOR3w/dAzkSlenHtbdkD9zdvAry9c55ph3lFGK3ecQJ0xIphnaxF3 97HeYL7C1OPVMIB3Oc/oTRfFg13kdvzbKeXiMnObS6Z+O3DpYg9NaSQS4OIFzZtFnYx/ ZQ0Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=41103hI8FWw3JSOY3ZzOk1BVYmgr6In9mKvl3fRAkI8=; b=bd5HxHjPDvo8+dnGhda3U/XByDuCHFVRVydJ8orSmXZbcl8BjmatwapBvSKp+1pEn+ r7E2BMLVfNSxCXyMYzv4nxKvE8v93W6Xc7LarwU1BbQU8lCufObje0QQsIpiTyWh68go qJwMKeX5+SX74kkHJSBWgxud5XijOXuHgtpJrUeyl+8JdYmdsnnbYZYhytKUiySWs2aT dYIQjK+MesyDUW9kNfNJcwdMsy7WAZ+TLtEGD61enEYtOrWAb/7S3APszxxayRrjQ3MS OnT07kY1Pyvh61upmT5Hk7vLo29twk/Zs824bBqWk2UKpOngaHj2+zSNFdAVaC4TnEYi BemQ== X-Gm-Message-State: AOAM532sFCaR754L6KmL20Xkgv866+aboWKITPXAqTwGdr5fR3nGmIIf +TDTCO6ptJheBg9qiiejBd516o1SBj5Ye9yjVvh5 X-Google-Smtp-Source: ABdhPJwEZiYZ1o+fxF2LyOo2oZ6n+2uOJ6UBAh5FxYnx0Lq0Y2sWzNLVszxxJ8cdzRbRgUWnfc7V2ucCZgqbu304x1U= X-Received: by 2002:a17:906:180b:: with SMTP id v11mr22849770eje.466.1605060794521; Tue, 10 Nov 2020 18:13:14 -0800 (PST) MIME-Version: 1.0 References: <20201106155626.3395468-1-lokeshgidra@google.com> <20201106155626.3395468-4-lokeshgidra@google.com> In-Reply-To: From: Paul Moore Date: Tue, 10 Nov 2020 21:13:03 -0500 Message-ID: Subject: Re: [PATCH v12 3/4] selinux: teach SELinux about anonymous inodes To: Lokesh Gidra Cc: Andrea Arcangeli , Alexander Viro , James Morris , Stephen Smalley , Casey Schaufler , Eric Biggers , "Serge E. Hallyn" , Eric Paris , Daniel Colascione , Kees Cook , "Eric W. Biederman" , KP Singh , David Howells , Thomas Cedeno , Anders Roxell , Sami Tolvanen , Matthew Garrett , Aaron Goidel , Randy Dunlap , "Joel Fernandes (Google)" , YueHaibing , Christian Brauner , Alexei Starovoitov , Alexey Budankov , Adrian Reber , Aleksa Sarai , Linux FS Devel , linux-kernel , LSM List , SElinux list , Kalesh Singh , Calin Juravle , Suren Baghdasaryan , Nick Kralevich , Jeffrey Vander Stoep , "Cc: Android Kernel" , "open list:MEMORY MANAGEMENT" , Andrew Morton , hch@infradead.org, Daniel Colascione Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Nov 10, 2020 at 1:24 PM Lokesh Gidra wrote: > On Mon, Nov 9, 2020 at 7:12 PM Paul Moore wrote: > > On Fri, Nov 6, 2020 at 10:56 AM Lokesh Gidra wrote: > > > > > > From: Daniel Colascione > > > > > > This change uses the anon_inodes and LSM infrastructure introduced in > > > the previous patches to give SELinux the ability to control > > > anonymous-inode files that are created using the new > > > anon_inode_getfd_secure() function. > > > > > > A SELinux policy author detects and controls these anonymous inodes by > > > adding a name-based type_transition rule that assigns a new security > > > type to anonymous-inode files created in some domain. The name used > > > for the name-based transition is the name associated with the > > > anonymous inode for file listings --- e.g., "[userfaultfd]" or > > > "[perf_event]". > > > > > > Example: > > > > > > type uffd_t; > > > type_transition sysadm_t sysadm_t : anon_inode uffd_t "[userfaultfd]"; > > > allow sysadm_t uffd_t:anon_inode { create }; > > > > > > (The next patch in this series is necessary for making userfaultfd > > > support this new interface. The example above is just > > > for exposition.) > > > > > > Signed-off-by: Daniel Colascione > > > Signed-off-by: Lokesh Gidra > > > --- > > > security/selinux/hooks.c | 53 +++++++++++++++++++++++++++++ > > > security/selinux/include/classmap.h | 2 ++ > > > 2 files changed, 55 insertions(+) > > > > > > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > > > index 6b1826fc3658..1c0adcdce7a8 100644 > > > --- a/security/selinux/hooks.c > > > +++ b/security/selinux/hooks.c > > > @@ -2927,6 +2927,58 @@ static int selinux_inode_init_security(struct inode *inode, struct inode *dir, > > > return 0; > > > } > > > > > > +static int selinux_inode_init_security_anon(struct inode *inode, > > > + const struct qstr *name, > > > + const struct inode *context_inode) > > > +{ > > > + const struct task_security_struct *tsec = selinux_cred(current_cred()); > > > + struct common_audit_data ad; > > > + struct inode_security_struct *isec; > > > + int rc; > > > + > > > + if (unlikely(!selinux_initialized(&selinux_state))) > > > + return 0; > > > + > > > + isec = selinux_inode(inode); > > > + > > > + /* > > > + * We only get here once per ephemeral inode. The inode has > > > + * been initialized via inode_alloc_security but is otherwise > > > + * untouched. > > > + */ > > > + > > > + if (context_inode) { > > > + struct inode_security_struct *context_isec = > > > + selinux_inode(context_inode); > > > + isec->sclass = context_isec->sclass; > > > + isec->sid = context_isec->sid; > > > > I suppose this isn't a major concern given the limited usage at the > > moment, but I wonder if it would be a good idea to make sure the > > context_inode's SELinux label is valid before we assign it to the > > anonymous inode? If it is invalid, what should we do? Do we attempt > > to (re)validate it? Do we simply fallback to the transition approach? > > Frankly, I'm not too familiar with SELinux. Originally this patch > series was developed by Daniel, in consultation with Stephen Smalley. > In my (probably naive) opinion we should fallback to transition > approach. But I'd request you to tell me if this needs to be addressed > now, and if so then what's the right approach. > > If the decision is to address this now, then what's the best way to > check the SELinux label validity? You can check to see if an inode's label is valid by looking at the isec->initialized field; if it is LABEL_INITIALIZED then it is all set, if it is any other value then the label isn't entirely correct. It may have not have ever been fully initialized (and has a default value) or it may have live on a remote filesystem where the host has signaled that the label has changed (and the label is now outdated). This patchset includes support for userfaultfd, which means we don't really have to worry about the remote fs problem, but the never-fully-initialized problem could be real in this case. Normally we would revalidate an inode in SELinux by calling __inode_security_revalidate() which requires either a valid dentry or one that can be found via the inode; does d_find_alias() work on userfaultfd inodes? If all else fails, it seems like the safest approach would be to simply fail the selinux_inode_init_security_anon() call if a context_inode was supplied and the label wasn't valid. If we later decide to change it to falling back to the transition approach we can do that, we can't go the other way (from transition to error). > > > + } else { > > > + isec->sclass = SECCLASS_ANON_INODE; > > > + rc = security_transition_sid( > > > + &selinux_state, tsec->sid, tsec->sid, > > > + isec->sclass, name, &isec->sid); > > > + if (rc) > > > + return rc; > > > + } > > > + > > > + isec->initialized = LABEL_INITIALIZED; > > > + > > > + /* > > > + * Now that we've initialized security, check whether we're > > > + * allowed to actually create this type of anonymous inode. > > > + */ > > > + > > > + ad.type = LSM_AUDIT_DATA_INODE; > > > + ad.u.inode = inode; > > > + > > > + return avc_has_perm(&selinux_state, > > > + tsec->sid, > > > + isec->sid, > > > + isec->sclass, > > > + FILE__CREATE, > > > > I believe you want to use ANON_INODE__CREATE here instead of FILE__CREATE, yes? > > ANON_INODE__CREATE definitely seems more appropriate. I'll change it > in the next revision. > > > This brings up another question, and requirement - what testing are > > you doing for this patchset? We require that new SELinux kernel > > functionality includes additions to the SELinux test suite to help > > verify the functionality. I'm also *strongly* encouraging that new > > contributions come with updates to The SELinux Notebook. If you are > > unsure about what to do for either, let us know and we can help get > > you started. > > > > * https://github.com/SELinuxProject/selinux-testsuite > > * https://github.com/SELinuxProject/selinux-notebook > > > I'd definitely need help with both of these. Kindly guide how to proceed. Well, perhaps the best way to start is to explain how you have been testing this so far and then using that information to draft a test for the testsuite. -- paul moore www.paul-moore.com