Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   ebiederm@xmission.com (Eric W. Biederman)
To:     Alexey Gladkov <gladkov.alexey@gmail.com>
Cc:     LKML <linux-kernel@vger.kernel.org>,
        Linux FS Devel <linux-fsdevel@vger.kernel.org>,
        Alexander Viro <viro@zeniv.linux.org.uk>,
        Alexey Gladkov <legion@kernel.org>,
        Kees Cook <keescook@chromium.org>
References: <20200727141411.203770-1-gladkov.alexey@gmail.com>
        <20200727141411.203770-3-gladkov.alexey@gmail.com>
Date:   Mon, 27 Jul 2020 11:29:36 -0500
In-Reply-To: <20200727141411.203770-3-gladkov.alexey@gmail.com> (Alexey
        Gladkov's message of "Mon, 27 Jul 2020 16:14:11 +0200")
Message-ID: <87blk0ncpb.fsf@x220.int.ebiederm.org>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain
Subject: Re: [PATCH v1 2/2] Show /proc/self/net only for CAP_NET_ADMIN
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Alexey Gladkov <gladkov.alexey@gmail.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@gmail.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