linux-kernel - Re: [PATCH v1 2/2] Show /proc/self/net only for CAP_NET

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87blk0ncpb.fsf@x220.int.ebiederm.org>
Date:   Mon, 27 Jul 2020 11:29:36 -0500
From:   ebiederm@...ssion.com (Eric W. Biederman)
To:     Alexey Gladkov <gladkov.alexey@...il.com>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        Linux FS Devel <linux-fsdevel@...r.kernel.org>,
        Alexander Viro <viro@...iv.linux.org.uk>,
        Alexey Gladkov <legion@...nel.org>,
        Kees Cook <keescook@...omium.org>
Subject: Re: [PATCH v1 2/2] Show /proc/self/net only for CAP_NET_ADMIN

Alexey Gladkov <gladkov.alexey@...il.com> writes:

> Show /proc/self/net only for CAP_NET_ADMIN if procfs is mounted with
> subset=pid option in user namespace. This is done to avoid possible
> information leakage.
>
> Signed-off-by: Alexey Gladkov <gladkov.alexey@...il.com>
> ---
>  fs/proc/proc_net.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/fs/proc/proc_net.c b/fs/proc/proc_net.c
> index dba63b2429f0..11fa2c4b3529 100644
> --- a/fs/proc/proc_net.c
> +++ b/fs/proc/proc_net.c
> @@ -275,6 +275,12 @@ static struct net *get_proc_task_net(struct inode *dir)
>  	struct task_struct *task;
>  	struct nsproxy *ns;
>  	struct net *net = NULL;
> +	struct proc_fs_info *fs_info = proc_sb_info(dir->i_sb);
> +
> +	if ((fs_info->pidonly == PROC_PIDONLY_ON) &&
> +	    (current_user_ns() != &init_user_ns) &&
> +	    !capable(CAP_NET_ADMIN))
> +		return net;
>
>  	rcu_read_lock();
>  	task = pid_task(proc_pid(dir), PIDTYPE_PID);

Hmm.

I see 3 options going forward.

1) We just make PROC_PIDONLY_ON mean the net directory does not exist.
   No permission checks just always fail.

2) Move the permission checks into opendir/readdir and whichever
   is the appropriate method there and always allow the dentries
   to be cached.

3) Simply cache the mounters credentials and make access to the
   net directories contingent of the permisions of the mounter of
   proc.  Something like the code below.

static struct net *get_proc_task_net(struct inode *dir)
{
	struct task_struct *task;
	struct nsproxy *ns;
	struct net *net = NULL;

	rcu_read_lock();
	task = pid_task(proc_pid(dir), PIDTYPE_PID);
	if (task != NULL) {
		task_lock(task);
		ns = task->nsproxy;
		if (ns != NULL)
			net = get_net(ns->net_ns);
		task_unlock(task);
	}
	rcu_read_unlock();
	if ((fs_info->pidonly == PROC_PIDONLY_ON) &&
            !security_capable(fs_info->mounter_cred,
			      net->user_ns, CAP_SYS_ADMIN,
			      CAP_OPT_NONE)) {
		put_net(net);
		net = NULL;
	}
	return net;
}

Eric