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:     Christian Brauner <christian.brauner@ubuntu.com>
Cc:     linux-kernel@vger.kernel.org,
        Alexander Viro <viro@zeniv.linux.org.uk>,
        =?utf-8?Q?St=C3=A9phane?= Graber <stgraber@ubuntu.com>,
        Linux Containers <containers@lists.linux-foundation.org>,
        Serge Hallyn <serge@hallyn.com>, Jann Horn <jannh@google.com>,
        Michael Kerrisk <mtk.manpages@gmail.com>,
        Aleksa Sarai <cyphar@cyphar.com>, linux-api@vger.kernel.org
References: <20200430165717.1001605-1-christian.brauner@ubuntu.com>
Date:   Thu, 30 Apr 2020 13:09:30 -0500
In-Reply-To: <20200430165717.1001605-1-christian.brauner@ubuntu.com>
        (Christian Brauner's message of "Thu, 30 Apr 2020 18:57:14 +0200")
Message-ID: <87y2qczvz9.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 v2 1/4] capability: add ns_capable_cred()
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Christian Brauner <christian.brauner@ubuntu.com> writes:

> Add a simple capability helper which makes it possible to determine
> whether a set of creds is ns capable wrt to the passed in credentials.
> This is not something exciting it's just a more pleasant wrapper around
> security_capable() by allowing ns_capable_common() to ake a const struct
> cred argument. In ptrace_has_cap() for example, we're using
> security_capable() directly. ns_capable_cred() will be used in the next
> patch to check against the target credentials the caller is going to
> switch to.

Given that this is to suppot setns.  I don't understand the
justification for this.

Is it your intention to use the reduced permissions that you get
when you install a user namespace?

Why do you want to use the reduced permissions when installing multiple
namespaces at once?

Eric


> Cc: Eric W. Biederman <ebiederm@xmission.com>
> Cc: Serge Hallyn <serge@hallyn.com>
> Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
> ---
> /* v2 */
> patch introduced
> ---
>  include/linux/capability.h |  3 +++
>  kernel/capability.c        | 17 +++++++++++------
>  2 files changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/include/linux/capability.h b/include/linux/capability.h
> index ecce0f43c73a..743a08d936fb 100644
> --- a/include/linux/capability.h
> +++ b/include/linux/capability.h
> @@ -40,6 +40,7 @@ struct cpu_vfs_cap_data {
>  struct file;
>  struct inode;
>  struct dentry;
> +struct cred;
>  struct task_struct;
>  struct user_namespace;
>  
> @@ -209,6 +210,8 @@ extern bool has_ns_capability_noaudit(struct task_struct *t,
>  				      struct user_namespace *ns, int cap);
>  extern bool capable(int cap);
>  extern bool ns_capable(struct user_namespace *ns, int cap);
> +extern bool ns_capable_cred(const struct cred *cred,
> +			    struct user_namespace *ns, int cap);
>  extern bool ns_capable_noaudit(struct user_namespace *ns, int cap);
>  extern bool ns_capable_setid(struct user_namespace *ns, int cap);
>  #else
> diff --git a/kernel/capability.c b/kernel/capability.c
> index 1444f3954d75..84425781917e 100644
> --- a/kernel/capability.c
> +++ b/kernel/capability.c
> @@ -361,8 +361,8 @@ bool has_capability_noaudit(struct task_struct *t, int cap)
>  	return has_ns_capability_noaudit(t, &init_user_ns, cap);
>  }
>  
> -static bool ns_capable_common(struct user_namespace *ns,
> -			      int cap,
> +static bool ns_capable_common(const struct cred *cred,
> +			      struct user_namespace *ns, int cap,
>  			      unsigned int opts)
>  {
>  	int capable;
> @@ -372,7 +372,7 @@ static bool ns_capable_common(struct user_namespace *ns,
>  		BUG();
>  	}
>  
> -	capable = security_capable(current_cred(), ns, cap, opts);
> +	capable = security_capable(cred, ns, cap, opts);
>  	if (capable == 0) {
>  		current->flags |= PF_SUPERPRIV;
>  		return true;
> @@ -393,10 +393,15 @@ static bool ns_capable_common(struct user_namespace *ns,
>   */
>  bool ns_capable(struct user_namespace *ns, int cap)
>  {
> -	return ns_capable_common(ns, cap, CAP_OPT_NONE);
> +	return ns_capable_common(current_cred(), ns, cap, CAP_OPT_NONE);
>  }
>  EXPORT_SYMBOL(ns_capable);
>  
> +bool ns_capable_cred(const struct cred *cred, struct user_namespace *ns, int cap)
> +{
> +	return ns_capable_common(cred, ns, cap, CAP_OPT_NONE);
> +}
> +
>  /**
>   * ns_capable_noaudit - Determine if the current task has a superior capability
>   * (unaudited) in effect
> @@ -411,7 +416,7 @@ EXPORT_SYMBOL(ns_capable);
>   */
>  bool ns_capable_noaudit(struct user_namespace *ns, int cap)
>  {
> -	return ns_capable_common(ns, cap, CAP_OPT_NOAUDIT);
> +	return ns_capable_common(current_cred(), ns, cap, CAP_OPT_NOAUDIT);
>  }
>  EXPORT_SYMBOL(ns_capable_noaudit);
>  
> @@ -430,7 +435,7 @@ EXPORT_SYMBOL(ns_capable_noaudit);
>   */
>  bool ns_capable_setid(struct user_namespace *ns, int cap)
>  {
> -	return ns_capable_common(ns, cap, CAP_OPT_INSETID);
> +	return ns_capable_common(current_cred(), ns, cap, CAP_OPT_INSETID);
>  }
>  EXPORT_SYMBOL(ns_capable_setid);
>  
>
> base-commit: ae83d0b416db002fe95601e7f97f64b59514d936