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=-12.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS 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 F2EA1C43461 for ; Thu, 22 Apr 2021 13:21:22 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 55F2E6144A for ; Thu, 22 Apr 2021 13:21:22 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 55F2E6144A Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 70F366B006E; Thu, 22 Apr 2021 09:21:21 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 6BF9D6B0071; Thu, 22 Apr 2021 09:21:21 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 55FFA8D0002; Thu, 22 Apr 2021 09:21:21 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0099.hostedemail.com [216.40.44.99]) by kanga.kvack.org (Postfix) with ESMTP id 3BA3F6B006E for ; Thu, 22 Apr 2021 09:21:21 -0400 (EDT) Received: from smtpin28.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id DCF2D180ACC31 for ; Thu, 22 Apr 2021 13:21:20 +0000 (UTC) X-FDA: 78060064320.28.9A50019 Received: from mail-lf1-f50.google.com (mail-lf1-f50.google.com [209.85.167.50]) by imf23.hostedemail.com (Postfix) with ESMTP id 7E519A000390 for ; Thu, 22 Apr 2021 13:21:18 +0000 (UTC) Received: by mail-lf1-f50.google.com with SMTP id x20so41818413lfu.6 for ; Thu, 22 Apr 2021 06:21:20 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=PHTEUlxqgA8l0F2eLJhm0lUslDtS68hTg+QYS24mt1A=; b=W7OhzBe3tGUvNLBkODmWurcIqydRnEopd0uCZ7QbiU0h8VJODUAOgr/3XEF92VflWL TXceQfgaNdpDg1UZRmbzPTG/0L4PlAQgTVuPAwopTgPlwj5U+BeXZQoSRaUL5ma+2bCC vsH3TBHRwlPoNzyhQfT99Nn97tBv4CiuPKbmQNBy03lBXBKQm8jTqQmuGDxl9wzOuHOu CHODrbcVzlu+LM35gAf1pN09cFuWMk9cdrJwBiAi/STbb+w6F2eLveY8XqqJLod9m8QX LhegfL4CHs7Cqsan2vBBPlw32MMwx6zv5p5LSqjAhg3Ni6jqTuu1XKMo0Mw/2vgyoUtF q6AQ== 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=PHTEUlxqgA8l0F2eLJhm0lUslDtS68hTg+QYS24mt1A=; b=jW7WcrsOjOxmj8HX191mVaNZoutRTQKGs0FD7mypdRqE7pEmfABfZOy0la9UMeaaMi 1m1Y2AQK8djUTik8atPDev0OfkFjNbmxdubS3ST/aq0RLKALysW0quVkY8RYWEXoUeYE ecrBf3DYOK6IOZRRSS14NhkrIL+slkdRJFVWfTBlIU96DKFHmDT4Ah1PrKAt7KvGm1b0 UUa+U2hNeIm+CAdwLZQQTcAL16bm+rXkrT8W7Y9dLhZAnJ40V1DODc0G7qHz9amOunN4 I/sGPvh2Hiw3aEhRiIgoewX158GZiOrRhATEP8OcKBdH8Eej1YX02Gmik1Du8uclVndd T+ww== X-Gm-Message-State: AOAM530MvajBniFquMTPL/ofqulklNlO65e81XDL3HWHoGJAHIceNE3x MmwM2jwo5ZZWUTRlp6i7ZGa9+t1jQ32J202WTr0= X-Google-Smtp-Source: ABdhPJzchxlr8SAqDCJEYfgZJ8QXE6CDSC4eSAy8LD85f/vmqWZ1RAbHO7YYaqobzfkUvRLmAMfCh19ckF2iWAe3l5w= X-Received: by 2002:ac2:43c5:: with SMTP id u5mr2441430lfl.40.1619097678746; Thu, 22 Apr 2021 06:21:18 -0700 (PDT) MIME-Version: 1.0 References: <20210421171446.785507-1-omosnace@redhat.com> <20210421171446.785507-3-omosnace@redhat.com> In-Reply-To: <20210421171446.785507-3-omosnace@redhat.com> From: Stephen Smalley Date: Thu, 22 Apr 2021 09:21:07 -0400 Message-ID: Subject: Re: [RFC PATCH 2/2] selinux: add capability to map anon inode types to separate classes To: Ondrej Mosnacek Cc: SElinux list , Paul Moore , LSM List , "open list:MEMORY MANAGEMENT" , Linux FS Devel , linux-kernel , Lokesh Gidra Content-Type: text/plain; charset="UTF-8" X-Rspamd-Queue-Id: 7E519A000390 X-Stat-Signature: 11z9xgmsforqaozydhxwj9n4mzzzptcr X-Rspamd-Server: rspam02 Received-SPF: none (gmail.com>: No applicable sender policy available) receiver=imf23; identity=mailfrom; envelope-from=""; helo=mail-lf1-f50.google.com; client-ip=209.85.167.50 X-HE-DKIM-Result: pass/pass X-HE-Tag: 1619097678-874020 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Wed, Apr 21, 2021 at 1:14 PM Ondrej Mosnacek wrote: > > Unfortunately, the approach chosen in commit 29cd6591ab6f ("selinux: > teach SELinux about anonymous inodes") to use a single class for all > anon inodes and let the policy distinguish between them using named > transitions turned out to have a rather unfortunate drawback. > > For example, suppose we have two types of anon inodes, "A" and "B", and > we want to allow a set of domains (represented by an attribute "attr_x") > certain set of permissions on anon inodes of type "A" that were created > by the same domain, but at the same time disallow this set to access > anon inodes of type "B" entirely. Since all inodes share the same class > and we want to distinguish both the inode types and the domains that > created them, we have no choice than to create separate types for the > cartesian product of (domains that belong to attr_x) x ("A", "B") and > add all the necessary allow and transition rules for each domain > individually. > > This makes it very impractical to write sane policies for anon inodes in > the future, as more anon inode types are added. Therefore, this patch > implements an alternative approach that assigns a separate class to each > type of anon inode. This allows the example above to be implemented > without any transition rules and with just a single allow rule: > > allow attr_x self:A { ... }; > > In order to not break possible existing users of the already merged > original approach, this patch also adds a new policy capability > "extended_anon_inode_class" that needs to be set by the policy to enable > the new behavior. > > I decided to keep the named transition mechanism in the new variant, > since there might eventually be some extra information in the anon inode > name that could be used in transitions. > > One minor annoyance is that the kernel still expects the policy to > provide both classes (anon_inode and userfaultfd) regardless of the > capability setting and if one of them is not defined in the policy, the > kernel will print a warning when loading the policy. However, it doesn't > seem worth to work around that in the kernel, as the policy can provide > just the definition of the unused class(es) (and permissions) to avoid > this warning. Keeping the legacy anon_inode class with some fallback > rules may also be desirable to keep the policy compatible with kernels > that only support anon_inode. > > Signed-off-by: Ondrej Mosnacek NAK. We do not want to introduce a new security class for every user of anon inodes - that isn't what security classes are for. For things like kvm device inodes, those should ultimately use the inherited context from the related inode (the /dev/kvm inode itself). That was the original intent of supporting the related inode. > --- > security/selinux/hooks.c | 27 +++++++++++++++++++++- > security/selinux/include/classmap.h | 2 ++ > security/selinux/include/policycap.h | 1 + > security/selinux/include/policycap_names.h | 3 ++- > security/selinux/include/security.h | 7 ++++++ > 5 files changed, 38 insertions(+), 2 deletions(-) > > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > index dc57ba21d8ff..20a8d7d17936 100644 > --- a/security/selinux/hooks.c > +++ b/security/selinux/hooks.c > @@ -3079,7 +3079,32 @@ static int selinux_inode_init_security_anon(struct inode *inode, > isec->sclass = context_isec->sclass; > isec->sid = context_isec->sid; > } else { > - isec->sclass = SECCLASS_ANON_INODE; > + /* > + * If the check below fails: > + * 1. Add the corresponding security class to > + * security/selinux/include/classmap.h > + * 2. Map the new LSM_ANON_INODE_* value to the class in > + * the switch statement below. > + * 3. Update the RHS of the comparison in the BUILD_BUG_ON(). > + * 4. CC selinux@vger.kernel.org and > + * linux-security-module@vger.kernel.org when submitting > + * the patch or in case of any questions. > + */ > + BUILD_BUG_ON(LSM_ANON_INODE_MAX > LSM_ANON_INODE_USERFAULTFD); > + > + if (selinux_policycap_extended_anon_inode()) { > + switch (type) { > + case LSM_ANON_INODE_USERFAULTFD: > + isec->sclass = SECCLASS_USERFAULTFD; > + break; > + default: > + pr_err("SELinux: got invalid anon inode type: %d", > + (int)type); > + return -EINVAL; > + } > + } else { > + isec->sclass = SECCLASS_ANON_INODE; > + } > rc = security_transition_sid( > &selinux_state, tsec->sid, tsec->sid, > isec->sclass, name, &isec->sid); > diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h > index ba2e01a6955c..e4308cad6407 100644 > --- a/security/selinux/include/classmap.h > +++ b/security/selinux/include/classmap.h > @@ -251,6 +251,8 @@ struct security_class_mapping secclass_map[] = { > { "integrity", "confidentiality", NULL } }, > { "anon_inode", > { COMMON_FILE_PERMS, NULL } }, > + { "userfaultfd", > + { COMMON_FILE_PERMS, NULL } }, > { NULL } > }; > > diff --git a/security/selinux/include/policycap.h b/security/selinux/include/policycap.h > index 2ec038efbb03..969804bd6dab 100644 > --- a/security/selinux/include/policycap.h > +++ b/security/selinux/include/policycap.h > @@ -11,6 +11,7 @@ enum { > POLICYDB_CAPABILITY_CGROUPSECLABEL, > POLICYDB_CAPABILITY_NNP_NOSUID_TRANSITION, > POLICYDB_CAPABILITY_GENFS_SECLABEL_SYMLINKS, > + POLICYDB_CAPABILITY_EXTENDED_ANON_INODE_CLASS, > __POLICYDB_CAPABILITY_MAX > }; > #define POLICYDB_CAPABILITY_MAX (__POLICYDB_CAPABILITY_MAX - 1) > diff --git a/security/selinux/include/policycap_names.h b/security/selinux/include/policycap_names.h > index b89289f092c9..78651990425e 100644 > --- a/security/selinux/include/policycap_names.h > +++ b/security/selinux/include/policycap_names.h > @@ -12,7 +12,8 @@ const char *selinux_policycap_names[__POLICYDB_CAPABILITY_MAX] = { > "always_check_network", > "cgroup_seclabel", > "nnp_nosuid_transition", > - "genfs_seclabel_symlinks" > + "genfs_seclabel_symlinks", > + "extended_anon_inode_class", > }; > > #endif /* _SELINUX_POLICYCAP_NAMES_H_ */ > diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h > index 7130c9648ad1..4fb75101aca4 100644 > --- a/security/selinux/include/security.h > +++ b/security/selinux/include/security.h > @@ -219,6 +219,13 @@ static inline bool selinux_policycap_genfs_seclabel_symlinks(void) > return READ_ONCE(state->policycap[POLICYDB_CAPABILITY_GENFS_SECLABEL_SYMLINKS]); > } > > +static inline bool selinux_policycap_extended_anon_inode(void) > +{ > + struct selinux_state *state = &selinux_state; > + > + return READ_ONCE(state->policycap[POLICYDB_CAPABILITY_EXTENDED_ANON_INODE_CLASS]); > +} > + > int security_mls_enabled(struct selinux_state *state); > int security_load_policy(struct selinux_state *state, > void *data, size_t len, > -- > 2.30.2 >