On Fri, Feb 25, 2022 at 1:34 AM Paul Moore <paul@xxxxxxxxxxxxxx> wrote: > On Thu, Feb 24, 2022 at 5:24 AM Richard Haines > <richard_c_haines@xxxxxxxxxxxxxx> wrote: > > > > These ioctls are equivalent to fcntl(fd, F_SETFD, flags), which SELinux > > always allows too. Furthermore, a failed FIOCLEX could result in a file > > descriptor being leaked to a process that should not have access to it. > > > > As this patch removes access controls, a policy capability needs to be > > enabled in policy to always allow these ioctls. > > > > Based-on-patch-by: Demi Marie Obenour <demiobenour@xxxxxxxxx> > > Signed-off-by: Richard Haines <richard_c_haines@xxxxxxxxxxxxxx> > > --- > > V2 Change: Control via a policy capability. > > V3 Change: Update switch check. > > > > security/selinux/hooks.c | 6 ++++++ > > security/selinux/include/policycap.h | 1 + > > security/selinux/include/policycap_names.h | 3 ++- > > security/selinux/include/security.h | 7 +++++++ > > 4 files changed, 16 insertions(+), 1 deletion(-) > > This looks good to me, but before I merge this are the SELinux > userspace folks okay with the policy capability's name and enum value? Since you mention it... I would suggest naming the enum POLICYDB_CAPABILITY_IOCTL_SKIP_CLOEXEC to match the display name. Yes, it becomes awkwardly long, but e.g. POLICYDB_CAPABILITY_GENFS_SECLABEL_SYMLINKS is already longer than that, so I'd prefer more descriptiveness over brevity. (IMHO the POLICYDB_CAPABILITY_ prefix is ridiculously long for no reason and we should simply shorten it (just POLCAP_ would be perfectly fine, IMHO) instead of trying to abbreviate the rest. Of course, this doesn't have to be done now - I'm taking a note to myself to splice in such rename next time I add a new capability, if not earlier.) -- Ondrej Mosnacek Software Engineer, Linux Security - SELinux kernel Red Hat, Inc.
