On Fri, Dec 22, 2017 at 03:32:30PM +0100, Dongsu Park wrote:
> From: Seth Forshee <seth.fors...@canonical.com>
>
> A privileged user in s_user_ns will generally have the ability to
> manipulate the backing store and insert security.* xattrs into
> the filesystem directly. Therefore the kernel must be prepared to
> handle these xattrs from unprivileged mounts, and it makes little
> sense for commoncap to prevent writing these xattrs to the
> filesystem. The capability and LSM code have already been updated
> to appropriately handle xattrs from unprivileged mounts, so it
> is safe to loosen this restriction on setting xattrs.
>
> The exception to this logic is that writing xattrs to a mounted
> filesystem may also cause the LSM inode_post_setxattr or
> inode_setsecurity callbacks to be invoked. SELinux will deny the
> xattr update by virtue of applying mountpoint labeling to
> unprivileged userns mounts, and Smack will deny the writes for
> any user without global CAP_MAC_ADMIN, so loosening the
> capability check in commoncap is safe in this respect as well.
>
> Patch v4 is available: https://patchwork.kernel.org/patch/8944641/
>
> Cc: linux-security-mod...@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: James Morris <james.l.mor...@oracle.com>
> Cc: Serge Hallyn <se...@hallyn.com>
Reviewed-by: Serge Hallyn <se...@hallyn.com>
> Signed-off-by: Seth Forshee <seth.fors...@canonical.com>
> Signed-off-by: Dongsu Park <don...@kinvolk.io>
> ---
> security/commoncap.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/security/commoncap.c b/security/commoncap.c
> index 4f8e0934..dd0afef9 100644
> --- a/security/commoncap.c
> +++ b/security/commoncap.c
> @@ -920,6 +920,8 @@ int cap_bprm_set_creds(struct linux_binprm *bprm)
> int cap_inode_setxattr(struct dentry *dentry, const char *name,
> const void *value, size_t size, int flags)
> {
> + struct user_namespace *user_ns = dentry->d_sb->s_user_ns;
> +
> /* Ignore non-security xattrs */
> if (strncmp(name, XATTR_SECURITY_PREFIX,
> sizeof(XATTR_SECURITY_PREFIX) - 1) != 0)
> @@ -932,7 +934,7 @@ int cap_inode_setxattr(struct dentry *dentry, const char
> *name,
> if (strcmp(name, XATTR_NAME_CAPS) == 0)
> return 0;
>
> - if (!capable(CAP_SYS_ADMIN))
> + if (!ns_capable(user_ns, CAP_SYS_ADMIN))
> return -EPERM;
> return 0;
> }
> @@ -950,6 +952,8 @@ int cap_inode_setxattr(struct dentry *dentry, const char
> *name,
> */
> int cap_inode_removexattr(struct dentry *dentry, const char *name)
> {
> + struct user_namespace *user_ns = dentry->d_sb->s_user_ns;
> +
> /* Ignore non-security xattrs */
> if (strncmp(name, XATTR_SECURITY_PREFIX,
> sizeof(XATTR_SECURITY_PREFIX) - 1) != 0)
> @@ -965,7 +969,7 @@ int cap_inode_removexattr(struct dentry *dentry, const
> char *name)
> return 0;
> }
>
> - if (!capable(CAP_SYS_ADMIN))
> + if (!ns_capable(user_ns, CAP_SYS_ADMIN))
> return -EPERM;
> return 0;
> }
> --
> 2.13.6
