Skip to content

Commit

Permalink
Take user namespaces into account in policy checks
Browse files Browse the repository at this point in the history
Change file related checks to use user namespaces and make
sure involved uids/gids are mappable in the current
namespace.

Note that checks without file ownership information will
still not take user namespaces into account, as some of
these should be handled via 'zfs allow' (otherwise root in a
user namespace could issue commands such as `zpool export`).

This also adds an initial user namespace regression test
for the setgid bit loss, with a user_ns_exec helper usable
in further tests.

Additionally, configure checks for the required user
namespace related features are added for:
  * ns_capable
  * kuid/kgid_has_mapping()
  * user_ns in cred_t

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
Closes openzfs#6800 
Closes openzfs#7270
  • Loading branch information
Blub authored and tonyhutter committed Mar 8, 2018
1 parent 301e9e0 commit 9bdbf51
Show file tree
Hide file tree
Showing 17 changed files with 521 additions and 7 deletions.
67 changes: 67 additions & 0 deletions config/kernel-userns-capabilities.m4
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
dnl #
dnl # 2.6.38 API change
dnl # ns_capable() was introduced
dnl #
AC_DEFUN([ZFS_AC_KERNEL_NS_CAPABLE], [
AC_MSG_CHECKING([whether ns_capable exists])
ZFS_LINUX_TRY_COMPILE([
#include <linux/capability.h>
],[
ns_capable((struct user_namespace *)NULL, CAP_SYS_ADMIN);
],[
AC_MSG_RESULT(yes)
AC_DEFINE(HAVE_NS_CAPABLE, 1,
[ns_capable exists])
],[
AC_MSG_RESULT(no)
])
])

dnl #
dnl # 2.6.39 API change
dnl # struct user_namespace was added to struct cred_t as
dnl # cred->user_ns member
dnl # Note that current_user_ns() was added in 2.6.28.
dnl #
AC_DEFUN([ZFS_AC_KERNEL_CRED_USER_NS], [
AC_MSG_CHECKING([whether cred_t->user_ns exists])
ZFS_LINUX_TRY_COMPILE([
#include <linux/cred.h>
],[
struct cred cr;
cr.user_ns = (struct user_namespace *)NULL;
],[
AC_MSG_RESULT(yes)
AC_DEFINE(HAVE_CRED_USER_NS, 1,
[cred_t->user_ns exists])
],[
AC_MSG_RESULT(no)
])
])

dnl #
dnl # 3.4 API change
dnl # kuid_has_mapping() and kgid_has_mapping() were added to distinguish
dnl # between internal kernel uids/gids and user namespace uids/gids.
dnl #
AC_DEFUN([ZFS_AC_KERNEL_KUID_HAS_MAPPING], [
AC_MSG_CHECKING([whether kuid_has_mapping/kgid_has_mapping exist])
ZFS_LINUX_TRY_COMPILE([
#include <linux/uidgid.h>
],[
kuid_has_mapping((struct user_namespace *)NULL, KUIDT_INIT(0));
kgid_has_mapping((struct user_namespace *)NULL, KGIDT_INIT(0));
],[
AC_MSG_RESULT(yes)
AC_DEFINE(HAVE_KUID_HAS_MAPPING, 1,
[kuid_has_mapping/kgid_has_mapping exist])
],[
AC_MSG_RESULT(no)
])
])

AC_DEFUN([ZFS_AC_KERNEL_USERNS_CAPABILITIES], [
ZFS_AC_KERNEL_NS_CAPABLE
ZFS_AC_KERNEL_CRED_USER_NS
ZFS_AC_KERNEL_KUID_HAS_MAPPING
])
1 change: 1 addition & 0 deletions config/kernel.m4
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ AC_DEFUN([ZFS_AC_CONFIG_KERNEL], [
ZFS_AC_KERNEL_CURRENT_TIME
ZFS_AC_KERNEL_GLOBAL_PAGE_STATE
ZFS_AC_KERNEL_ACL_HAS_REFCOUNT
ZFS_AC_KERNEL_USERNS_CAPABILITIES
AS_IF([test "$LINUX_OBJ" != "$LINUX"], [
KERNELMAKE_PARAMS="$KERNELMAKE_PARAMS O=$LINUX_OBJ"
Expand Down
2 changes: 2 additions & 0 deletions configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ AC_CONFIG_FILES([
tests/zfs-tests/callbacks/Makefile
tests/zfs-tests/cmd/Makefile
tests/zfs-tests/cmd/chg_usr_exec/Makefile
tests/zfs-tests/cmd/user_ns_exec/Makefile
tests/zfs-tests/cmd/devname2devid/Makefile
tests/zfs-tests/cmd/dir_rd_update/Makefile
tests/zfs-tests/cmd/file_check/Makefile
Expand Down Expand Up @@ -284,6 +285,7 @@ AC_CONFIG_FILES([
tests/zfs-tests/tests/functional/threadsappend/Makefile
tests/zfs-tests/tests/functional/tmpfile/Makefile
tests/zfs-tests/tests/functional/truncate/Makefile
tests/zfs-tests/tests/functional/user_namespace/Makefile
tests/zfs-tests/tests/functional/userquota/Makefile
tests/zfs-tests/tests/functional/upgrade/Makefile
tests/zfs-tests/tests/functional/vdev_zaps/Makefile
Expand Down
66 changes: 59 additions & 7 deletions module/zfs/policy.c
Original file line number Diff line number Diff line change
Expand Up @@ -42,19 +42,47 @@
* all other cases this function must fail and return the passed err.
*/
static int
priv_policy(const cred_t *cr, int capability, boolean_t all, int err)
priv_policy_ns(const cred_t *cr, int capability, boolean_t all, int err,
struct user_namespace *ns)
{
ASSERT3S(all, ==, B_FALSE);

if (cr != CRED() && (cr != kcred))
return (err);

#if defined(CONFIG_USER_NS) && defined(HAVE_NS_CAPABLE)
if (!(ns ? ns_capable(ns, capability) : capable(capability)))
#else
if (!capable(capability))
#endif
return (err);

return (0);
}

static int
priv_policy(const cred_t *cr, int capability, boolean_t all, int err)
{
return (priv_policy_ns(cr, capability, all, err, NULL));
}

static int
priv_policy_user(const cred_t *cr, int capability, boolean_t all, int err)
{
/*
* All priv_policy_user checks are preceeded by kuid/kgid_has_mapping()
* checks. If we cannot do them, we shouldn't be using ns_capable()
* since we don't know whether the affected files are valid in our
* namespace. Note that kuid_has_mapping() came after cred->user_ns, so
* we shouldn't need to re-check for HAVE_CRED_USER_NS
*/
#if defined(CONFIG_USER_NS) && defined(HAVE_KUID_HAS_MAPPING)
return (priv_policy_ns(cr, capability, all, err, cr->user_ns));
#else
return (priv_policy_ns(cr, capability, all, err, NULL));
#endif
}

/*
* Checks for operations that are either client-only or are used by
* both clients and servers.
Expand Down Expand Up @@ -102,10 +130,15 @@ secpolicy_vnode_any_access(const cred_t *cr, struct inode *ip, uid_t owner)
if (zpl_inode_owner_or_capable(ip))
return (0);

if (priv_policy(cr, CAP_DAC_OVERRIDE, B_FALSE, EPERM) == 0)
#if defined(CONFIG_USER_NS) && defined(HAVE_KUID_HAS_MAPPING)
if (!kuid_has_mapping(cr->user_ns, SUID_TO_KUID(owner)))
return (EPERM);
#endif

if (priv_policy_user(cr, CAP_DAC_OVERRIDE, B_FALSE, EPERM) == 0)
return (0);

if (priv_policy(cr, CAP_DAC_READ_SEARCH, B_FALSE, EPERM) == 0)
if (priv_policy_user(cr, CAP_DAC_READ_SEARCH, B_FALSE, EPERM) == 0)
return (0);

return (EPERM);
Expand All @@ -120,7 +153,12 @@ secpolicy_vnode_chown(const cred_t *cr, uid_t owner)
if (crgetfsuid(cr) == owner)
return (0);

return (priv_policy(cr, CAP_FOWNER, B_FALSE, EPERM));
#if defined(CONFIG_USER_NS) && defined(HAVE_KUID_HAS_MAPPING)
if (!kuid_has_mapping(cr->user_ns, SUID_TO_KUID(owner)))
return (EPERM);
#endif

return (priv_policy_user(cr, CAP_FOWNER, B_FALSE, EPERM));
}

/*
Expand Down Expand Up @@ -152,7 +190,12 @@ secpolicy_vnode_setdac(const cred_t *cr, uid_t owner)
if (crgetfsuid(cr) == owner)
return (0);

return (priv_policy(cr, CAP_FOWNER, B_FALSE, EPERM));
#if defined(CONFIG_USER_NS) && defined(HAVE_KUID_HAS_MAPPING)
if (!kuid_has_mapping(cr->user_ns, SUID_TO_KUID(owner)))
return (EPERM);
#endif

return (priv_policy_user(cr, CAP_FOWNER, B_FALSE, EPERM));
}

/*
Expand All @@ -175,8 +218,12 @@ secpolicy_vnode_setid_retain(const cred_t *cr, boolean_t issuidroot)
int
secpolicy_vnode_setids_setgids(const cred_t *cr, gid_t gid)
{
#if defined(CONFIG_USER_NS) && defined(HAVE_KUID_HAS_MAPPING)
if (!kgid_has_mapping(cr->user_ns, SGID_TO_KGID(gid)))
return (EPERM);
#endif
if (crgetfsgid(cr) != gid && !groupmember(gid, cr))
return (priv_policy(cr, CAP_FSETID, B_FALSE, EPERM));
return (priv_policy_user(cr, CAP_FSETID, B_FALSE, EPERM));

return (0);
}
Expand Down Expand Up @@ -222,7 +269,12 @@ secpolicy_vnode_setid_modify(const cred_t *cr, uid_t owner)
if (crgetfsuid(cr) == owner)
return (0);

return (priv_policy(cr, CAP_FSETID, B_FALSE, EPERM));
#if defined(CONFIG_USER_NS) && defined(HAVE_KUID_HAS_MAPPING)
if (!kuid_has_mapping(cr->user_ns, SUID_TO_KUID(owner)))
return (EPERM);
#endif

return (priv_policy_user(cr, CAP_FSETID, B_FALSE, EPERM));
}

/*
Expand Down
4 changes: 4 additions & 0 deletions tests/runfiles/linux.run
Original file line number Diff line number Diff line change
Expand Up @@ -648,6 +648,10 @@ tags = ['functional', 'truncate']
tests = [ 'upgrade_userobj_001_pos' ]
tags = ['functional', 'upgrade']

[tests/functional/user_namespace]
tests = ['user_namespace_001']
tags = ['functional', 'user_namespace']

[tests/functional/userquota]
tests = [
'userquota_001_pos', 'userquota_002_pos', 'userquota_003_pos',
Expand Down
1 change: 1 addition & 0 deletions tests/zfs-tests/cmd/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ EXTRA_DIST = file_common.h

SUBDIRS = \
chg_usr_exec \
user_ns_exec \
devname2devid \
dir_rd_update \
file_check \
Expand Down
1 change: 1 addition & 0 deletions tests/zfs-tests/cmd/user_ns_exec/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
/user_ns_exec
6 changes: 6 additions & 0 deletions tests/zfs-tests/cmd/user_ns_exec/Makefile.am
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
include $(top_srcdir)/config/Rules.am

pkgexecdir = $(datadir)/@PACKAGE@/zfs-tests/bin

pkgexec_PROGRAMS = user_ns_exec
user_ns_exec_SOURCES = user_ns_exec.c
Loading

0 comments on commit 9bdbf51

Please sign in to comment.