[Devel] [PATCH vz8 v5 2/5] trusted/ve/fs/exec: Don't allow a privileged user to execute untrusted files

[Valeriy Vdovin]

On Tue, Jun 08, 2021 at 07:31:27PM +0300, Konstantin Khorenko wrote:
> From: Pavel Tikhomirov <ptikhomirov at virtuozzo.com>
> 
> If we run some binary (exploit) from CT on host, it can easily give a
> user in these CT an ability to do anything on host sending commands
> through unix socket to the exploit. Such an exploit can mimic to bash,
> ip, systemd, ping or some other "trusted" utility.
> 
> I've tested with these patch that we don't call from VE0 any binaries
> from CT-fs on start, stop, enter, suspend, resume or migration. But to
> be on the safe side, so that in future we don't become affected, lets
> prohibit running any binary from ploop disks and from CT mounts if the
> caller is from VE0.
> 
> Also we protect admins of our customer from unintentionally calling such
> an exploit:
> 
> [root at kuchy ~]# strace -e trace=execve /vz/root/58a2c524-b486-42c8-849\
>   b-c659bf165a91/bin/ls
> execve("/vz/root/58a2c524-b486-42c8-849b-c659bf165a91/bin/ls",\
>   ["/vz/root/58a2c524-b486-42c8-849b"...], [/* 27 vars */]) = -1\
>   EACCES (Permission denied)
> strace: exec: Permission denied
> +++ exited with 1 +++
> 
> We need same check in sys_uselib as process from host can also try to
> load shared library from the file in CT's ploop, which cannot be trusted
> too.
> 
> https://jira.sw.ru/browse/PSBM-98094
> 
> Signed-off-by: Pavel Tikhomirov <ptikhomirov at virtuozzo.com>
> Acked-by: Konstantin Khorenko <khorenko at virtuozzo.com>
> 
> https://jira.sw.ru/browse/PSBM-129741
> 
> Based on vz7 commit 29154b5e5af9 ("ve/fs/exec: don't allow a privileged
> user to execute untrusted files")
> 
> Signed-off-by: Valeriy Vdovin <valeriy.vdovin at virtuozzo.com>
> Reviewed-by: Pavel Tikhomirov <ptikhomirov at virtuozzo.com>
> Reviewed-by: Konstantin Khorenko <khorenko at virtuozzo.com>
> ---
>  fs/exec.c          | 16 ++++++++++++++--
>  include/linux/ve.h |  1 +
>  kernel/ve/ve.c     | 47 ++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 62 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/exec.c b/fs/exec.c
> index c036db0323e0..0f4c741e19db 100644
> --- a/fs/exec.c
> +++ b/fs/exec.c
> @@ -62,6 +62,7 @@
>  #include <linux/oom.h>
>  #include <linux/compat.h>
>  #include <linux/vmalloc.h>
> +#include <linux/ve.h>
>  
>  #include <linux/uaccess.h>
>  #include <asm/mmu_context.h>
> @@ -134,10 +135,9 @@ SYSCALL_DEFINE1(uselib, const char __user *, library)
>  		goto out;
>  
>  	file = do_filp_open(AT_FDCWD, tmp, &uselib_flags);
> -	putname(tmp);
>  	error = PTR_ERR(file);
>  	if (IS_ERR(file))
> -		goto out;
> +		goto put;
>  
>  	error = -EINVAL;
>  	if (!S_ISREG(file_inode(file)->i_mode))
> @@ -147,6 +147,12 @@ SYSCALL_DEFINE1(uselib, const char __user *, library)
>  	if (path_noexec(&file->f_path))
>  		goto exit;
>  
> +	if (!ve_check_trusted_exec(file, tmp))
> +		goto exit;
> +
> +	putname(tmp);
> +	tmp = NULL;
> +
>  	fsnotify_open(file);
>  
>  	error = -ENOEXEC;
> @@ -167,6 +173,9 @@ SYSCALL_DEFINE1(uselib, const char __user *, library)
>  	read_unlock(&binfmt_lock);
>  exit:
>  	fput(file);
> +put:
> +	if (tmp)
> +		putname(tmp);
>  out:
>    	return error;
>  }
> @@ -861,6 +870,9 @@ static struct file *do_open_execat(int fd, struct filename *name, int flags)
>  	if (path_noexec(&file->f_path))
>  		goto exit;
>  
> +	if (!ve_check_trusted_exec(file, name))
> +		goto exit;
> +
>  	err = deny_write_access(file);
>  	if (err)
>  		goto exit;
> diff --git a/include/linux/ve.h b/include/linux/ve.h
> index 9c553ac96072..edf4e95d97e7 100644
> --- a/include/linux/ve.h
> +++ b/include/linux/ve.h
> @@ -162,6 +162,7 @@ extern void put_ve(struct ve_struct *ve);
>  
>  void ve_stop_ns(struct pid_namespace *ns);
>  void ve_exit_ns(struct pid_namespace *ns);
> +bool ve_check_trusted_exec(struct file *file, struct filename *name);
>  
>  #ifdef CONFIG_TTY
>  #define MAX_NR_VTTY_CONSOLES	(12)
> diff --git a/kernel/ve/ve.c b/kernel/ve/ve.c
> index 12e91e6ee1a1..6594772a10dd 100644
> --- a/kernel/ve/ve.c
> +++ b/kernel/ve/ve.c
> @@ -28,10 +28,12 @@
>  #include <linux/task_work.h>
>  #include <linux/ctype.h>
>  #include <linux/tty.h>
> +#include <linux/genhd.h>
>  
>  #include <uapi/linux/vzcalluser.h>
>  #include <net/rtnetlink.h>
>  
> +#include "../fs/mount.h"
>  #include "../cgroup/cgroup-internal.h" /* For cgroup_task_count() */
>  
>  struct per_cgroot_data {
> @@ -1781,6 +1783,51 @@ static int __init ve_subsys_init(void)
>  }
>  late_initcall(ve_subsys_init);
>  
> +static bool ve_check_trusted_file(struct file *file)
> +{
> +	struct block_device *bdev;
> +	bool exec_from_ct;
> +	bool file_on_host_mount;
> +
> +	/* The current process does not belong to ve0. */
> +	exec_from_ct = !ve_is_super(get_exec_env());
> +	if (exec_from_ct)
> +		return true;
> +
> +	/* The current process belongs to ve0. */
> +	bdev = file->f_inode->i_sb->s_bdev;
> +	if (bdev) {
> +		/* The file to execute is stored on trusted block device. */
> +		if (bdev->bd_disk->vz_trusted_exec)
> +			return true;
> +	} else {
> +		/*
> +		 * bdev can be NULL if the file is on tmpfs, for example.
> +		 * If this is a host's tmpfs - execution is allowed.
> +		 */
> +		file_on_host_mount = ve_is_super(
> +				     real_mount(file->f_path.mnt)->ve_owner);
> +		if (file_on_host_mount)
> +			return true;
> +	}
> +
> +	return false;
> +}

I've tested this version of ve_check_trusted_file, it works great.
Lets commit it that way.
> +
> +/*
> + * We don't want a VE0-privileged user intentionally or by mistake
> + * to execute files of container, these files are untrusted.
> + */
> +bool ve_check_trusted_exec(struct file *file, struct filename *name)
> +{
> +	if (ve_check_trusted_file(file))
> +		return true;
> +
> +	WARN_ONCE(1, "VE0's %s tried to execute untrusted file %s from VEX\n",
> +		     current->comm, name->name);
> +	return false;
> +}
> +
>  #ifdef CONFIG_CGROUP_SCHED
>  int cpu_cgroup_proc_stat(struct cgroup_subsys_state *cpu_css,
>  			 struct cgroup_subsys_state *cpuacct_css,
> -- 
> 2.28.0
>