Messages in this thread Patch in this message | | | Subject | Re: [PATCH] selinux: allow kernfs symlinks to inherit parent directory context | From | Stephen Smalley <> | Date | Fri, 24 Jan 2020 13:53:30 -0500 |
| |
On 1/24/20 1:42 PM, Christian Göttsche wrote: > Currently symlinks on kernel filesystems, like sysfs, are labeled on > creation with the parent fs root sid. > > Allow symlinks to inherit the parent directory context, so fine-grained > kernfs labeling can be applied to symlinks too and checking contexts > doesn't complain about them. > > For backward-compatibility this behavior is contained in a new policy > capability: kernfs_sovereign_symlinks > > Signed-off-by: Christian Göttsche <cgzones@googlemail.com> > --- > security/selinux/hooks.c | 5 ++++- > security/selinux/include/security.h | 8 ++++++++ > security/selinux/ss/services.c | 3 ++- > 3 files changed, 14 insertions(+), 2 deletions(-) > > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > index d9e8b2131..1303bc8c4 100644 > --- a/security/selinux/hooks.c > +++ b/security/selinux/hooks.c > @@ -1475,7 +1475,10 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent > /* Default to the fs superblock SID. */ > sid = sbsec->sid; > > - if ((sbsec->flags & SE_SBGENFS) && !S_ISLNK(inode->i_mode)) { > + if (((sbsec->flags & SE_SBGENFS) && > + (!S_ISLNK(inode->i_mode))) || > + (selinux_policycap_kernfs_sovereign_symlinks() &&
Not fond of the name. 1) kernfs is a kernel implementation detail, shouldn't be exposed to policy; genfs is the policy construct 2) sovereign doesn't seem to fit the meaning of this capability; seclabel would be more appropriate.
> + (sbsec->flags & SE_SBGENFS_XATTR))) {
Why limit this to SE_SBGENFS_XATTR filesystems? Why not just make the test: if ((sbsec->flags & SE_SBGENFS) && (!S_ISLNK(inode->i_mode) || selinux_policycap_genfs_symlinkseclabel())) or similar.
> /* We must have a dentry to determine the label on > * procfs inodes */ > if (opt_dentry) { > diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h > index a39f9565d..cc8217848 100644 > --- a/security/selinux/include/security.h > +++ b/security/selinux/include/security.h > @@ -79,6 +79,7 @@ enum { > POLICYDB_CAPABILITY_ALWAYSNETWORK, > POLICYDB_CAPABILITY_CGROUPSECLABEL, > POLICYDB_CAPABILITY_NNP_NOSUID_TRANSITION, > + POLICYDB_CAPABILITY_KERNFS_SOVEREIGN_SYMLINKS, > __POLICYDB_CAPABILITY_MAX > }; > #define POLICYDB_CAPABILITY_MAX (__POLICYDB_CAPABILITY_MAX - 1) > @@ -209,6 +210,13 @@ static inline bool selinux_policycap_nnp_nosuid_transition(void) > return state->policycap[POLICYDB_CAPABILITY_NNP_NOSUID_TRANSITION]; > } > > +static inline bool selinux_policycap_kernfs_sovereign_symlinks(void) > +{ > + struct selinux_state *state = &selinux_state; > + > + return state->policycap[POLICYDB_CAPABILITY_KERNFS_SOVEREIGN_SYMLINKS]; > +} > + > int security_mls_enabled(struct selinux_state *state); > int security_load_policy(struct selinux_state *state, > void *data, size_t len); > diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c > index 216ce602a..b70380947 100644 > --- a/security/selinux/ss/services.c > +++ b/security/selinux/ss/services.c > @@ -73,7 +73,8 @@ const char *selinux_policycap_names[__POLICYDB_CAPABILITY_MAX] = { > "extended_socket_class", > "always_check_network", > "cgroup_seclabel", > - "nnp_nosuid_transition" > + "nnp_nosuid_transition", > + "kernfs_sovereign_symlinks" > }; > > static struct selinux_ss selinux_ss; >
| |