On Thu, Apr 22, 2021 at 3:21 PM Stephen Smalley
[off-list ref] wrote:

On Wed, Apr 21, 2021 at 1:14 PM Ondrej Mosnacek [off-list ref] wrote:

quoted

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 <omosnace@redhat.com>

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.

Hmm, so are you implying that anon inodes should be thought of the
same as control /dev nodes? I.e. that even though there may be many
one-time actual inodes created by different processes, they should be
thought of as a single "static interface" to the respective kernel
functionality? That would justify having a common type/label for all
of them, but I'm not sure if it doesn't open some gap due to the
possibility to pass the associated file descriptors between processes
(as AFAIK, these can hold some context)...

I thought this was supposed to resemble more the way BPF, perf_event,
etc. support was implemented - the BPF and perf_event fds are also
anon inodes under the hood, BTW - where each file descriptor is
considered a separate object that inherits the label of its creator
and there is some class separation (e.g. bpf vs. perf_event).

--
Ondrej Mosnacek
Software Engineer, Linux Security - SELinux kernel
Red Hat, Inc.