On Thu, Feb 20, 2020 at 10:11 AM Casey Schaufler [off-list ref] wrote:

On 2/17/2020 4:14 PM, Paul Moore wrote:

quoted

On Thu, Feb 13, 2020 at 2:41 PM Stephen Smalley [off-list ref] wrote:

quoted

Add support for labeling and controlling access to files attached to anon
inodes. Introduce extended interfaces for creating such files to permit
passing a related file as an input to decide how to label the anon
inode. Define a security hook for initializing the anon inode security
attributes. Security attributes are either inherited from a related file
or determined based on some combination of the creating task and policy
(in the case of SELinux, using type_transition rules).  As an
example user of the inheritance support, convert kvm to use the new
interface for passing the related file so that the anon inode can inherit
the security attributes of /dev/kvm and provide consistent access control
for subsequent ioctl operations.  Other users of anon inodes, including
userfaultfd, will default to the transition-based mechanism instead.

Compared to the series in
https://lore.kernel.org/selinux/20200211225547.235083-1-dancol@google.com/ (local),
this approach differs in that it does not require creation of a separate
anonymous inode for each file (instead storing the per-instance security
information in the file security blob), it applies labeling and control
to all users of anonymous inodes rather than requiring opt-in via a new
flag, it supports labeling based on a related inode if provided,
it relies on type transitions to compute the label of the anon inode
when there is no related inode, and it does not require introducing a new
security class for each user of anonymous inodes.

On the other hand, the approach in this patch does expose the name passed
by the creator of the anon inode to the policy (an indirect mapping could
be provided within SELinux if these names aren't considered to be stable),
requires the definition of type_transition rules to distinguish userfaultfd
inodes from proc inodes based on type since they share the same class,
doesn't support denying the creation of anonymous inodes (making the hook
added by this patch return something other than void is problematic due to
it being called after the file is already allocated and error handling in
the callers can't presently account for this scenario and end up calling
release methods multiple times), and may be more expensive
(security_transition_sid overhead on each anon inode allocation).

We are primarily posting this RFC patch now so that the two different
approaches can be concretely compared.  We anticipate a hybrid of the
two approaches being the likely outcome in the end.  In particular
if support for allocating a separate inode for each of these files
is acceptable, then we would favor storing the security information
in the inode security blob and using it instead of the file security
blob.

Bringing this back up in hopes of attracting some attention from the
fs-devel crowd and Al.  As Stephen already mentioned, from a SELinux
perspective we would prefer to attach the security blob to the inode
as opposed to the file struct; does anyone have any objections to
that?

Sorry for the delay - been sick the past few days.

I agree that the inode is a better place than the file for information
about the inode. This is especially true for Smack, which uses
multiple extended attributes in some cases. I don't believe that any
except the access label will be relevant to anonymous inodes, but
I can imagine security modules with policies that would.

I am always an advocate of full xattr support. It goes a long
way in reducing the number and complexity of special case interfaces.

It sounds like we have broad consensus on using the inode to hold
security information, implying that anon_inodes should create new
inodes. Do any of the VFS people want to object?