On 1/9/19 5:03 PM, Casey Schaufler wrote:
On 1/9/2019 12:37 PM, Stephen Smalley wrote:
On 1/9/19 12:19 PM, Casey Schaufler wrote:
On 1/9/2019 8:28 AM, Ondrej Mosnacek wrote:
Changes in v2:
- add docstring for the new hook in union security_list_options
- initialize *ctx to NULL and *ctxlen to 0 in case the hook is not
implemented
v1: https://lore.kernel.org/selinux/20190109091028.24485-1-omosnace@xxxxxxxxxx/T/
This series adds a new security hook that allows to initialize the security
context of kernfs properly, taking into account the parent context. Kernfs
nodes require special handling here, since they are not bound to specific
inodes/superblocks, but instead represent the backing tree structure that
is used to build the VFS tree when the kernfs tree is mounted.
The kernfs nodes initially do not store any security context and rely on
the LSM to assign some default context to inodes created over them.
This seems like a bug in kernfs. Why doesn't kernfs adhere to the usual
and expected filesystem behavior?
sysfs / kernfs didn't support xattrs at all when we first added support for setting security contexts to it, so originally all sysfs / kernfs inodes had a single security context, and we only required separate storage for the inodes that were explicitly labeled by userspace.
Later kernfs grew support for trusted.* xattrs using simple_xattrs but the existing security.* support was left mostly unchanged.
OK, so as I said, this seems like a bug in kernfs.
Kernfs
inodes, however, allow setting an explicit context via the *setxattr(2)
syscalls, in which case the context is stored inside the kernfs node's
metadata.
SELinux (and possibly other LSMs) initialize the context of newly created
FS objects based on the parent object's context (usually the child inherits
the parent's context, unless the policy dictates otherwise).
An LSM might use information about the parent other than the "context".
Smack, for example, uses an attribute SMACK64TRANSMUTE from the parent
to determine whether the Smack label of the new object should be taken
from the parent or the process. Passing the "context" of the parent is
insufficient for Smack.
IIUC, this would involve switching the handling of security.* xattrs in kernfs over to use simple_xattrs too (so that we can store multiple such attributes), and then pass the entire simple_xattrs list or at least anything with a security.* prefix when initializing a new node or refreshing an existing inode. Then the security module could extract any security.* attributes of interest for use in determining the label of new inodes and in refreshing the label of an inode.
Right. But I'll point out that there is nothing to prevent an
LSM from using inode information outside of the xattrs (e.g. uids)
to determine the security state it wants to give a new object.
If that's a real concern, the hook could pass the ia_iattr structure in
addition to the simple_xattrs list and the security module could use any
inode attributes it likes in making the decision. Effectively it would
be passing the entire kernfs_iattrs structure, but probably not directly
since that definition is presently private to kernfs.
I suggest that the better solution would be for kernfs to
use inodes like a real filesystem. Every special case like this
results in special cases like this special hook. It's hard
enough to keep track of the general case in the Linux kernel.
Feel free to propose an implementation if you like, but doing a complete
rewrite of kernfs internals seems a bit out of scope.
This is done
by hooking the creation of the new inode corresponding to the newly created
file/directory via security_inode_init_security() (most filesystems always
create a fresh inode when a new FS object is created). However, kernfs nodes
can be created "behind the scenes" while the filesystem is not mounted
anywhere and thus no inodes exist.
Therefore, to allow maintaining similar behavior for kernfs nodes, a new LSM
hook is needed, which would allow initializing the kernfs node's security
context based on the context stored in the parent's node (if any).
The main motivation for this change is that the userspace users of cgroupfs
(which is built on kernfs) expect the usual security context inheritance
to work under SELinux (see [1] and [2]). This functionality is required for
better confinement of containers under SELinux.
The first patch adds the new LSM hook; the second patch implements the hook
in SELinux; and the third patch modifies kernfs to use the new hook to
initialize the security context of kernfs nodes whenever its parent node
has a non-default context set.
Note: the patches are based on current selinux/next [3], but they seem to
apply cleanly on top of v5.0-rc1 as well.
Testing:
- passed SELinux testsuite on Fedora 29 (x86_64) when applied on top of
current Rawhide kernel (5.0.0-0.rc1.git0.1) [4]
- passed the reproducer from the last patch
[1] https://github.com/SELinuxProject/selinux-kernel/issues/39
[2] https://bugzilla.redhat.com/show_bug.cgi?id=1553803
[3] https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/selinux.git/log/?h=selinux-pr-20181224
[4] https://copr.fedorainfracloud.org/coprs/omos/kernel-testing/build/842855/
Ondrej Mosnacek (3):
LSM: Add new hook for generic node initialization
selinux: Implement the object_init_security hook
kernfs: Initialize security of newly created nodes
fs/kernfs/dir.c | 49 ++++++++++++++++++++++++++++++++++---
fs/kernfs/inode.c | 9 +++----
fs/kernfs/kernfs-internal.h | 4 +++
include/linux/lsm_hooks.h | 30 +++++++++++++++++++++++
include/linux/security.h | 14 +++++++++++
security/security.c | 10 ++++++++
security/selinux/hooks.c | 41 +++++++++++++++++++++++++++++++
7 files changed, 149 insertions(+), 8 deletions(-)
