diff mbox

[v3,01/11] user_ns: 3 new LSM hooks for user namespace operations

Message ID 1437732285-11524-2-git-send-email-l.pawelczyk@samsung.com (mailing list archive)
State New, archived
Headers show

Commit Message

Lukasz Pawelczyk July 24, 2015, 10:04 a.m. UTC
This commit implements 3 new LSM hooks that provide the means for LSMs
to embed their own security context within user namespace, effectively
creating some sort of a user_ns related security namespace.

The first one to take advantage of this mechanism is Smack.

The hooks has been documented in the in the security.h below.

Signed-off-by: Lukasz Pawelczyk <l.pawelczyk@samsung.com>
Reviewed-by: Casey Schaufler <casey@schaufler-ca.com>
---
 include/linux/lsm_hooks.h      | 28 ++++++++++++++++++++++++++++
 include/linux/security.h       | 23 +++++++++++++++++++++++
 include/linux/user_namespace.h |  4 ++++
 kernel/user.c                  |  3 +++
 kernel/user_namespace.c        | 18 ++++++++++++++++++
 security/security.c            | 28 ++++++++++++++++++++++++++++
 6 files changed, 104 insertions(+)

Comments

Serge E. Hallyn July 30, 2015, 9:30 p.m. UTC | #1
On Fri, Jul 24, 2015 at 12:04:35PM +0200, Lukasz Pawelczyk wrote:
> This commit implements 3 new LSM hooks that provide the means for LSMs
> to embed their own security context within user namespace, effectively
> creating some sort of a user_ns related security namespace.
> 
> The first one to take advantage of this mechanism is Smack.
> 
> The hooks has been documented in the in the security.h below.
> 
> Signed-off-by: Lukasz Pawelczyk <l.pawelczyk@samsung.com>
> Reviewed-by: Casey Schaufler <casey@schaufler-ca.com>
> ---
>  include/linux/lsm_hooks.h      | 28 ++++++++++++++++++++++++++++
>  include/linux/security.h       | 23 +++++++++++++++++++++++
>  include/linux/user_namespace.h |  4 ++++
>  kernel/user.c                  |  3 +++
>  kernel/user_namespace.c        | 18 ++++++++++++++++++
>  security/security.c            | 28 ++++++++++++++++++++++++++++
>  6 files changed, 104 insertions(+)
> 
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 9429f05..228558c 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -1261,6 +1261,23 @@
>   *	audit_rule_init.
>   *	@rule contains the allocated rule
>   *
> + * @userns_create:
> + *	Allocates and fills the security part of a new user namespace.
> + *	@ns points to a newly created user namespace.
> + *	Returns 0 or an error code.
> + *
> + * @userns_free:
> + *	Deallocates the security part of a user namespace.
> + *	@ns points to a user namespace about to be destroyed.
> + *
> + * @userns_setns:
> + *	Run during a setns syscall to add a process to an already existing
> + *	user namespace. Returning failure here will block the operation
> + *	requested from userspace (setns() with CLONE_NEWUSER).
> + *	@nsproxy contains nsproxy to which the user namespace will be assigned.
> + *	@ns contains user namespace that is to be incorporated to the nsproxy.
> + *	Returns 0 or an error code.
> + *
>   * @inode_notifysecctx:
>   *	Notify the security module of what the security context of an inode
>   *	should be.  Initializes the incore security context managed by the
> @@ -1613,6 +1630,12 @@ union security_list_options {
>  				struct audit_context *actx);
>  	void (*audit_rule_free)(void *lsmrule);
>  #endif /* CONFIG_AUDIT */
> +
> +#ifdef CONFIG_USER_NS
> +	int (*userns_create)(struct user_namespace *ns);
> +	void (*userns_free)(struct user_namespace *ns);
> +	int (*userns_setns)(struct nsproxy *nsproxy, struct user_namespace *ns);
> +#endif /* CONFIG_USER_NS */
>  };
>  
>  struct security_hook_heads {
> @@ -1824,6 +1847,11 @@ struct security_hook_heads {
>  	struct list_head audit_rule_match;
>  	struct list_head audit_rule_free;
>  #endif /* CONFIG_AUDIT */
> +#ifdef CONFIG_USER_NS
> +	struct list_head userns_create;
> +	struct list_head userns_free;
> +	struct list_head userns_setns;
> +#endif /* CONFIG_USER_NS */
>  };
>  
>  /*
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 79d85dd..1b0eccc 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -1584,6 +1584,29 @@ static inline void security_audit_rule_free(void *lsmrule)
>  #endif /* CONFIG_SECURITY */
>  #endif /* CONFIG_AUDIT */
>  
> +#ifdef CONFIG_USER_NS
> +int security_userns_create(struct user_namespace *ns);
> +void security_userns_free(struct user_namespace *ns);
> +int security_userns_setns(struct nsproxy *nsproxy, struct user_namespace *ns);
> +
> +#else
> +
> +static inline int security_userns_create(struct user_namespace *ns)
> +{
> +	return 0;
> +}
> +
> +static inline void security_userns_free(struct user_namespace *ns)
> +{ }
> +
> +static inline int security_userns_setns(struct nsproxy *nsproxy,
> +					struct user_namespace *ns)
> +{
> +	return 0;
> +}
> +
> +#endif /* CONFIG_USER_NS */
> +
>  #ifdef CONFIG_SECURITYFS
>  
>  extern struct dentry *securityfs_create_file(const char *name, umode_t mode,
> diff --git a/include/linux/user_namespace.h b/include/linux/user_namespace.h
> index 8297e5b..a9400cc 100644
> --- a/include/linux/user_namespace.h
> +++ b/include/linux/user_namespace.h
> @@ -39,6 +39,10 @@ struct user_namespace {
>  	struct key		*persistent_keyring_register;
>  	struct rw_semaphore	persistent_keyring_register_sem;
>  #endif
> +
> +#ifdef CONFIG_SECURITY
> +	void *security;
> +#endif
>  };
>  
>  extern struct user_namespace init_user_ns;
> diff --git a/kernel/user.c b/kernel/user.c
> index b069ccb..ce5419e 100644
> --- a/kernel/user.c
> +++ b/kernel/user.c
> @@ -59,6 +59,9 @@ struct user_namespace init_user_ns = {
>  	.persistent_keyring_register_sem =
>  	__RWSEM_INITIALIZER(init_user_ns.persistent_keyring_register_sem),
>  #endif
> +#ifdef CONFIG_SECURITY
> +	.security = NULL,
> +#endif
>  };
>  EXPORT_SYMBOL_GPL(init_user_ns);
>  
> diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
> index 4109f83..cadffb6 100644
> --- a/kernel/user_namespace.c
> +++ b/kernel/user_namespace.c
> @@ -22,6 +22,7 @@
>  #include <linux/ctype.h>
>  #include <linux/projid.h>
>  #include <linux/fs_struct.h>
> +#include <linux/security.h>
>  
>  static struct kmem_cache *user_ns_cachep __read_mostly;
>  static DEFINE_MUTEX(userns_state_mutex);
> @@ -108,6 +109,15 @@ int create_user_ns(struct cred *new)
>  
>  	set_cred_user_ns(new, ns);
>  
> +#ifdef CONFIG_SECURITY
> +	ret = security_userns_create(ns);
> +	if (ret) {
> +		ns_free_inum(&ns->ns);
> +		kmem_cache_free(user_ns_cachep, ns);
> +		return ret;
> +	}
> +#endif
> +
>  #ifdef CONFIG_PERSISTENT_KEYRINGS
>  	init_rwsem(&ns->persistent_keyring_register_sem);
>  #endif
> @@ -143,6 +153,9 @@ void free_user_ns(struct user_namespace *ns)
>  #ifdef CONFIG_PERSISTENT_KEYRINGS
>  		key_put(ns->persistent_keyring_register);
>  #endif
> +#ifdef CONFIG_SECURITY
> +		security_userns_free(ns);
> +#endif
>  		ns_free_inum(&ns->ns);
>  		kmem_cache_free(user_ns_cachep, ns);
>  		ns = parent;
> @@ -969,6 +982,7 @@ static int userns_install(struct nsproxy *nsproxy, struct ns_common *ns)
>  {
>  	struct user_namespace *user_ns = to_user_ns(ns);
>  	struct cred *cred;
> +	int err;
>  
>  	/* Don't allow gaining capabilities by reentering
>  	 * the same user namespace.
> @@ -986,6 +1000,10 @@ static int userns_install(struct nsproxy *nsproxy, struct ns_common *ns)
>  	if (!ns_capable(user_ns, CAP_SYS_ADMIN))
>  		return -EPERM;
>  
> +	err = security_userns_setns(nsproxy, user_ns);
> +	if (err)
> +		return err;

So at this point the LSM thinks current is in the new ns.  If
prepare_creds() fails below, should it be informed of that?
(Or am I over-thinking this?)

> +
>  	cred = prepare_creds();
>  	if (!cred)
>  		return -ENOMEM;
> diff --git a/security/security.c b/security/security.c
> index 595fffa..5e66388 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -25,6 +25,7 @@
>  #include <linux/mount.h>
>  #include <linux/personality.h>
>  #include <linux/backing-dev.h>
> +#include <linux/user_namespace.h>
>  #include <net/flow.h>
>  
>  #define MAX_LSM_EVM_XATTR	2
> @@ -1542,6 +1543,25 @@ int security_audit_rule_match(u32 secid, u32 field, u32 op, void *lsmrule,
>  }
>  #endif /* CONFIG_AUDIT */
>  
> +#ifdef CONFIG_USER_NS
> +
> +int security_userns_create(struct user_namespace *ns)
> +{
> +	return call_int_hook(userns_create, 0, ns);
> +}
> +
> +void security_userns_free(struct user_namespace *ns)
> +{
> +	call_void_hook(userns_free, ns);
> +}
> +
> +int security_userns_setns(struct nsproxy *nsproxy, struct user_namespace *ns)
> +{
> +	return call_int_hook(userns_setns, 0, nsproxy, ns);
> +}
> +
> +#endif /* CONFIG_USER_NS */
> +
>  struct security_hook_heads security_hook_heads = {
>  	.binder_set_context_mgr =
>  		LIST_HEAD_INIT(security_hook_heads.binder_set_context_mgr),
> @@ -1886,4 +1906,12 @@ struct security_hook_heads security_hook_heads = {
>  	.audit_rule_free =
>  		LIST_HEAD_INIT(security_hook_heads.audit_rule_free),
>  #endif /* CONFIG_AUDIT */
> +#ifdef CONFIG_USER_NS
> +	.userns_create =
> +		LIST_HEAD_INIT(security_hook_heads.userns_create),
> +	.userns_free =
> +		LIST_HEAD_INIT(security_hook_heads.userns_free),
> +	.userns_setns =
> +		LIST_HEAD_INIT(security_hook_heads.userns_setns),
> +#endif /* CONFIG_USER_NS */
>  };
> -- 
> 2.4.3
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Lukasz Pawelczyk July 31, 2015, 9:28 a.m. UTC | #2
On czw, 2015-07-30 at 16:30 -0500, Serge E. Hallyn wrote:
> On Fri, Jul 24, 2015 at 12:04:35PM +0200, Lukasz Pawelczyk wrote:
> > @@ -969,6 +982,7 @@ static int userns_install(struct nsproxy 
> > *nsproxy, struct ns_common *ns)
> >  {
> >  	struct user_namespace *user_ns = to_user_ns(ns);
> >  	struct cred *cred;
> > +	int err;
> >  
> >  	/* Don't allow gaining capabilities by reentering
> >  	 * the same user namespace.
> > @@ -986,6 +1000,10 @@ static int userns_install(struct nsproxy 
> > *nsproxy, struct ns_common *ns)
> >  	if (!ns_capable(user_ns, CAP_SYS_ADMIN))
> >  		return -EPERM;
> >  
> > +	err = security_userns_setns(nsproxy, user_ns);
> > +	if (err)
> > +		return err;
> 
> So at this point the LSM thinks current is in the new ns.  If
> prepare_creds() fails below, should it be informed of that?
> (Or am I over-thinking this?)
> 
> > +
> >  	cred = prepare_creds();
> >  	if (!cred)
> >  		return -ENOMEM;

Hmm, the use case for this hook I had in mind was just to allow or
disallow the operation based on the information passed in arguments.
Not to register the current in any way so LSM can think it is or isn't
in the new namespace.

I think that any other LSM check that would like to know in what
namespace the current is, would just check that from current's creds.
Not use some stale and duplicated information the above hook could have
registered.

I see no reason for this hook to change the LSM state, only to answer
the question: allowed/disallowed (eventually return an error cause it
is unable to give an answer which falls into the disallow category).
Serge E. Hallyn Aug. 1, 2015, 3:48 a.m. UTC | #3
On Fri, Jul 31, 2015 at 11:28:56AM +0200, Lukasz Pawelczyk wrote:
> On czw, 2015-07-30 at 16:30 -0500, Serge E. Hallyn wrote:
> > On Fri, Jul 24, 2015 at 12:04:35PM +0200, Lukasz Pawelczyk wrote:
> > > @@ -969,6 +982,7 @@ static int userns_install(struct nsproxy 
> > > *nsproxy, struct ns_common *ns)
> > >  {
> > >  	struct user_namespace *user_ns = to_user_ns(ns);
> > >  	struct cred *cred;
> > > +	int err;
> > >  
> > >  	/* Don't allow gaining capabilities by reentering
> > >  	 * the same user namespace.
> > > @@ -986,6 +1000,10 @@ static int userns_install(struct nsproxy 
> > > *nsproxy, struct ns_common *ns)
> > >  	if (!ns_capable(user_ns, CAP_SYS_ADMIN))
> > >  		return -EPERM;
> > >  
> > > +	err = security_userns_setns(nsproxy, user_ns);
> > > +	if (err)
> > > +		return err;
> > 
> > So at this point the LSM thinks current is in the new ns.  If
> > prepare_creds() fails below, should it be informed of that?
> > (Or am I over-thinking this?)
> > 
> > > +
> > >  	cred = prepare_creds();
> > >  	if (!cred)
> > >  		return -ENOMEM;
> 
> Hmm, the use case for this hook I had in mind was just to allow or
> disallow the operation based on the information passed in arguments.
> Not to register the current in any way so LSM can think it is or isn't
> in the new namespace.
> 
> I think that any other LSM check that would like to know in what
> namespace the current is, would just check that from current's creds.
> Not use some stale and duplicated information the above hook could have
> registered.
> 
> I see no reason for this hook to change the LSM state, only to answer
> the question: allowed/disallowed (eventually return an error cause it
> is unable to give an answer which falls into the disallow category).

How about renaming it "security_userns_may_setns()" for clarity?
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Lukasz Pawelczyk Aug. 3, 2015, 11:34 a.m. UTC | #4
On pi?, 2015-07-31 at 22:48 -0500, Serge E. Hallyn wrote:
> On Fri, Jul 31, 2015 at 11:28:56AM +0200, Lukasz Pawelczyk wrote:
> > On czw, 2015-07-30 at 16:30 -0500, Serge E. Hallyn wrote:
> > > On Fri, Jul 24, 2015 at 12:04:35PM +0200, Lukasz Pawelczyk wrote:
> > > > @@ -969,6 +982,7 @@ static int userns_install(struct nsproxy 
> > > > *nsproxy, struct ns_common *ns)
> > > >  {
> > > >  	struct user_namespace *user_ns = to_user_ns(ns);
> > > >  	struct cred *cred;
> > > > +	int err;
> > > >  
> > > >  	/* Don't allow gaining capabilities by reentering
> > > >  	 * the same user namespace.
> > > > @@ -986,6 +1000,10 @@ static int userns_install(struct nsproxy 
> > > > *nsproxy, struct ns_common *ns)
> > > >  	if (!ns_capable(user_ns, CAP_SYS_ADMIN))
> > > >  		return -EPERM;
> > > >  
> > > > +	err = security_userns_setns(nsproxy, user_ns);
> > > > +	if (err)
> > > > +		return err;
> > > 
> > > So at this point the LSM thinks current is in the new ns.  If
> > > prepare_creds() fails below, should it be informed of that?
> > > (Or am I over-thinking this?)
> > > 
> > > > +
> > > >  	cred = prepare_creds();
> > > >  	if (!cred)
> > > >  		return -ENOMEM;
> > 
> > Hmm, the use case for this hook I had in mind was just to allow or
> > disallow the operation based on the information passed in 
> > arguments.
> > Not to register the current in any way so LSM can think it is or 
> > isn't
> > in the new namespace.
> > 
> > I think that any other LSM check that would like to know in what
> > namespace the current is, would just check that from current's 
> > creds.
> > Not use some stale and duplicated information the above hook could 
> > have
> > registered.
> > 
> > I see no reason for this hook to change the LSM state, only to 
> > answer
> > the question: allowed/disallowed (eventually return an error cause 
> > it
> > is unable to give an answer which falls into the disallow 
> > category).
> 
> How about renaming it "security_userns_may_setns()" for clarity?

I personally have nothing against it. However looking at already
existing hooks only one of them has "may" in the name (unix_may_send)
while a lot clearly have exactly this purpose (e.g. most of inode_*
family, some from file_* and task_*). So it seems the trend is against
it.

What do you think? Anyone else has an opinion?
Kees Cook Aug. 4, 2015, 1:38 a.m. UTC | #5
On Mon, Aug 3, 2015 at 4:34 AM, Lukasz Pawelczyk
<l.pawelczyk@samsung.com> wrote:
> On pi?, 2015-07-31 at 22:48 -0500, Serge E. Hallyn wrote:
>> On Fri, Jul 31, 2015 at 11:28:56AM +0200, Lukasz Pawelczyk wrote:
>> > On czw, 2015-07-30 at 16:30 -0500, Serge E. Hallyn wrote:
>> > > On Fri, Jul 24, 2015 at 12:04:35PM +0200, Lukasz Pawelczyk wrote:
>> > > > @@ -969,6 +982,7 @@ static int userns_install(struct nsproxy
>> > > > *nsproxy, struct ns_common *ns)
>> > > >  {
>> > > >         struct user_namespace *user_ns = to_user_ns(ns);
>> > > >         struct cred *cred;
>> > > > +       int err;
>> > > >
>> > > >         /* Don't allow gaining capabilities by reentering
>> > > >          * the same user namespace.
>> > > > @@ -986,6 +1000,10 @@ static int userns_install(struct nsproxy
>> > > > *nsproxy, struct ns_common *ns)
>> > > >         if (!ns_capable(user_ns, CAP_SYS_ADMIN))
>> > > >                 return -EPERM;
>> > > >
>> > > > +       err = security_userns_setns(nsproxy, user_ns);
>> > > > +       if (err)
>> > > > +               return err;
>> > >
>> > > So at this point the LSM thinks current is in the new ns.  If
>> > > prepare_creds() fails below, should it be informed of that?
>> > > (Or am I over-thinking this?)
>> > >
>> > > > +
>> > > >         cred = prepare_creds();
>> > > >         if (!cred)
>> > > >                 return -ENOMEM;
>> >
>> > Hmm, the use case for this hook I had in mind was just to allow or
>> > disallow the operation based on the information passed in
>> > arguments.
>> > Not to register the current in any way so LSM can think it is or
>> > isn't
>> > in the new namespace.
>> >
>> > I think that any other LSM check that would like to know in what
>> > namespace the current is, would just check that from current's
>> > creds.
>> > Not use some stale and duplicated information the above hook could
>> > have
>> > registered.
>> >
>> > I see no reason for this hook to change the LSM state, only to
>> > answer
>> > the question: allowed/disallowed (eventually return an error cause
>> > it
>> > is unable to give an answer which falls into the disallow
>> > category).
>>
>> How about renaming it "security_userns_may_setns()" for clarity?
>
> I personally have nothing against it. However looking at already
> existing hooks only one of them has "may" in the name (unix_may_send)
> while a lot clearly have exactly this purpose (e.g. most of inode_*
> family, some from file_* and task_*). So it seems the trend is against
> it.
>
> What do you think? Anyone else has an opinion?

Personally, I prefer that hooks be named as closely to their caller,
or calling context, as possible. In this case, it seems like "may" is
implied. It's an LSM like all the others, so it can fail, which would
cause the caller to fail too, so "may" tends to be implicit. I would
leave it as-is, but I could be convinced otherwise.

-Kees
Paul Moore Aug. 21, 2015, 5:04 a.m. UTC | #6
On Mon, Aug 3, 2015 at 9:38 PM, Kees Cook <keescook@chromium.org> wrote:
> On Mon, Aug 3, 2015 at 4:34 AM, Lukasz Pawelczyk
> <l.pawelczyk@samsung.com> wrote:
>> On pi?, 2015-07-31 at 22:48 -0500, Serge E. Hallyn wrote:
>>> On Fri, Jul 31, 2015 at 11:28:56AM +0200, Lukasz Pawelczyk wrote:
>>> > On czw, 2015-07-30 at 16:30 -0500, Serge E. Hallyn wrote:
>>> > > On Fri, Jul 24, 2015 at 12:04:35PM +0200, Lukasz Pawelczyk wrote:
>>> > > > @@ -969,6 +982,7 @@ static int userns_install(struct nsproxy
>>> > > > *nsproxy, struct ns_common *ns)
>>> > > >  {
>>> > > >         struct user_namespace *user_ns = to_user_ns(ns);
>>> > > >         struct cred *cred;
>>> > > > +       int err;
>>> > > >
>>> > > >         /* Don't allow gaining capabilities by reentering
>>> > > >          * the same user namespace.
>>> > > > @@ -986,6 +1000,10 @@ static int userns_install(struct nsproxy
>>> > > > *nsproxy, struct ns_common *ns)
>>> > > >         if (!ns_capable(user_ns, CAP_SYS_ADMIN))
>>> > > >                 return -EPERM;
>>> > > >
>>> > > > +       err = security_userns_setns(nsproxy, user_ns);
>>> > > > +       if (err)
>>> > > > +               return err;
>>> > >
>>> > > So at this point the LSM thinks current is in the new ns.  If
>>> > > prepare_creds() fails below, should it be informed of that?
>>> > > (Or am I over-thinking this?)
>>> > >
>>> > > > +
>>> > > >         cred = prepare_creds();
>>> > > >         if (!cred)
>>> > > >                 return -ENOMEM;
>>> >
>>> > Hmm, the use case for this hook I had in mind was just to allow or
>>> > disallow the operation based on the information passed in
>>> > arguments.
>>> > Not to register the current in any way so LSM can think it is or
>>> > isn't
>>> > in the new namespace.
>>> >
>>> > I think that any other LSM check that would like to know in what
>>> > namespace the current is, would just check that from current's
>>> > creds.
>>> > Not use some stale and duplicated information the above hook could
>>> > have
>>> > registered.
>>> >
>>> > I see no reason for this hook to change the LSM state, only to
>>> > answer
>>> > the question: allowed/disallowed (eventually return an error cause
>>> > it
>>> > is unable to give an answer which falls into the disallow
>>> > category).
>>>
>>> How about renaming it "security_userns_may_setns()" for clarity?
>>
>> I personally have nothing against it. However looking at already
>> existing hooks only one of them has "may" in the name (unix_may_send)
>> while a lot clearly have exactly this purpose (e.g. most of inode_*
>> family, some from file_* and task_*). So it seems the trend is against
>> it.
>>
>> What do you think? Anyone else has an opinion?
>
> Personally, I prefer that hooks be named as closely to their caller,
> or calling context, as possible. In this case, it seems like "may" is
> implied. It's an LSM like all the others, so it can fail, which would
> cause the caller to fail too, so "may" tends to be implicit. I would
> leave it as-is, but I could be convinced otherwise.

I agree with Kees, sticking as close as possible to the general format
of "security_<caller>" for LSM hooks makes it easier when
reading/reviewing code.
Paul Moore Aug. 21, 2015, 3:56 p.m. UTC | #7
On Fri, Jul 24, 2015 at 6:04 AM, Lukasz Pawelczyk
<l.pawelczyk@samsung.com> wrote:
> This commit implements 3 new LSM hooks that provide the means for LSMs
> to embed their own security context within user namespace, effectively
> creating some sort of a user_ns related security namespace.
>
> The first one to take advantage of this mechanism is Smack.
>
> The hooks has been documented in the in the security.h below.
>
> Signed-off-by: Lukasz Pawelczyk <l.pawelczyk@samsung.com>
> Reviewed-by: Casey Schaufler <casey@schaufler-ca.com>
> ---
>  include/linux/lsm_hooks.h      | 28 ++++++++++++++++++++++++++++
>  include/linux/security.h       | 23 +++++++++++++++++++++++
>  include/linux/user_namespace.h |  4 ++++
>  kernel/user.c                  |  3 +++
>  kernel/user_namespace.c        | 18 ++++++++++++++++++
>  security/security.c            | 28 ++++++++++++++++++++++++++++
>  6 files changed, 104 insertions(+)

I'm not sure at this point in time we know what we want to do with
SELinux and these hooks, if anything, but they look reasonable to me.

Acked-by: Paul Moore <paul@paul-moore.com>

> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 9429f05..228558c 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -1261,6 +1261,23 @@
>   *     audit_rule_init.
>   *     @rule contains the allocated rule
>   *
> + * @userns_create:
> + *     Allocates and fills the security part of a new user namespace.
> + *     @ns points to a newly created user namespace.
> + *     Returns 0 or an error code.
> + *
> + * @userns_free:
> + *     Deallocates the security part of a user namespace.
> + *     @ns points to a user namespace about to be destroyed.
> + *
> + * @userns_setns:
> + *     Run during a setns syscall to add a process to an already existing
> + *     user namespace. Returning failure here will block the operation
> + *     requested from userspace (setns() with CLONE_NEWUSER).
> + *     @nsproxy contains nsproxy to which the user namespace will be assigned.
> + *     @ns contains user namespace that is to be incorporated to the nsproxy.
> + *     Returns 0 or an error code.
> + *
>   * @inode_notifysecctx:
>   *     Notify the security module of what the security context of an inode
>   *     should be.  Initializes the incore security context managed by the
> @@ -1613,6 +1630,12 @@ union security_list_options {
>                                 struct audit_context *actx);
>         void (*audit_rule_free)(void *lsmrule);
>  #endif /* CONFIG_AUDIT */
> +
> +#ifdef CONFIG_USER_NS
> +       int (*userns_create)(struct user_namespace *ns);
> +       void (*userns_free)(struct user_namespace *ns);
> +       int (*userns_setns)(struct nsproxy *nsproxy, struct user_namespace *ns);
> +#endif /* CONFIG_USER_NS */
>  };
>
>  struct security_hook_heads {
> @@ -1824,6 +1847,11 @@ struct security_hook_heads {
>         struct list_head audit_rule_match;
>         struct list_head audit_rule_free;
>  #endif /* CONFIG_AUDIT */
> +#ifdef CONFIG_USER_NS
> +       struct list_head userns_create;
> +       struct list_head userns_free;
> +       struct list_head userns_setns;
> +#endif /* CONFIG_USER_NS */
>  };
>
>  /*
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 79d85dd..1b0eccc 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -1584,6 +1584,29 @@ static inline void security_audit_rule_free(void *lsmrule)
>  #endif /* CONFIG_SECURITY */
>  #endif /* CONFIG_AUDIT */
>
> +#ifdef CONFIG_USER_NS
> +int security_userns_create(struct user_namespace *ns);
> +void security_userns_free(struct user_namespace *ns);
> +int security_userns_setns(struct nsproxy *nsproxy, struct user_namespace *ns);
> +
> +#else
> +
> +static inline int security_userns_create(struct user_namespace *ns)
> +{
> +       return 0;
> +}
> +
> +static inline void security_userns_free(struct user_namespace *ns)
> +{ }
> +
> +static inline int security_userns_setns(struct nsproxy *nsproxy,
> +                                       struct user_namespace *ns)
> +{
> +       return 0;
> +}
> +
> +#endif /* CONFIG_USER_NS */
> +
>  #ifdef CONFIG_SECURITYFS
>
>  extern struct dentry *securityfs_create_file(const char *name, umode_t mode,
> diff --git a/include/linux/user_namespace.h b/include/linux/user_namespace.h
> index 8297e5b..a9400cc 100644
> --- a/include/linux/user_namespace.h
> +++ b/include/linux/user_namespace.h
> @@ -39,6 +39,10 @@ struct user_namespace {
>         struct key              *persistent_keyring_register;
>         struct rw_semaphore     persistent_keyring_register_sem;
>  #endif
> +
> +#ifdef CONFIG_SECURITY
> +       void *security;
> +#endif
>  };
>
>  extern struct user_namespace init_user_ns;
> diff --git a/kernel/user.c b/kernel/user.c
> index b069ccb..ce5419e 100644
> --- a/kernel/user.c
> +++ b/kernel/user.c
> @@ -59,6 +59,9 @@ struct user_namespace init_user_ns = {
>         .persistent_keyring_register_sem =
>         __RWSEM_INITIALIZER(init_user_ns.persistent_keyring_register_sem),
>  #endif
> +#ifdef CONFIG_SECURITY
> +       .security = NULL,
> +#endif
>  };
>  EXPORT_SYMBOL_GPL(init_user_ns);
>
> diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
> index 4109f83..cadffb6 100644
> --- a/kernel/user_namespace.c
> +++ b/kernel/user_namespace.c
> @@ -22,6 +22,7 @@
>  #include <linux/ctype.h>
>  #include <linux/projid.h>
>  #include <linux/fs_struct.h>
> +#include <linux/security.h>
>
>  static struct kmem_cache *user_ns_cachep __read_mostly;
>  static DEFINE_MUTEX(userns_state_mutex);
> @@ -108,6 +109,15 @@ int create_user_ns(struct cred *new)
>
>         set_cred_user_ns(new, ns);
>
> +#ifdef CONFIG_SECURITY
> +       ret = security_userns_create(ns);
> +       if (ret) {
> +               ns_free_inum(&ns->ns);
> +               kmem_cache_free(user_ns_cachep, ns);
> +               return ret;
> +       }
> +#endif
> +
>  #ifdef CONFIG_PERSISTENT_KEYRINGS
>         init_rwsem(&ns->persistent_keyring_register_sem);
>  #endif
> @@ -143,6 +153,9 @@ void free_user_ns(struct user_namespace *ns)
>  #ifdef CONFIG_PERSISTENT_KEYRINGS
>                 key_put(ns->persistent_keyring_register);
>  #endif
> +#ifdef CONFIG_SECURITY
> +               security_userns_free(ns);
> +#endif
>                 ns_free_inum(&ns->ns);
>                 kmem_cache_free(user_ns_cachep, ns);
>                 ns = parent;
> @@ -969,6 +982,7 @@ static int userns_install(struct nsproxy *nsproxy, struct ns_common *ns)
>  {
>         struct user_namespace *user_ns = to_user_ns(ns);
>         struct cred *cred;
> +       int err;
>
>         /* Don't allow gaining capabilities by reentering
>          * the same user namespace.
> @@ -986,6 +1000,10 @@ static int userns_install(struct nsproxy *nsproxy, struct ns_common *ns)
>         if (!ns_capable(user_ns, CAP_SYS_ADMIN))
>                 return -EPERM;
>
> +       err = security_userns_setns(nsproxy, user_ns);
> +       if (err)
> +               return err;
> +
>         cred = prepare_creds();
>         if (!cred)
>                 return -ENOMEM;
> diff --git a/security/security.c b/security/security.c
> index 595fffa..5e66388 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -25,6 +25,7 @@
>  #include <linux/mount.h>
>  #include <linux/personality.h>
>  #include <linux/backing-dev.h>
> +#include <linux/user_namespace.h>
>  #include <net/flow.h>
>
>  #define MAX_LSM_EVM_XATTR      2
> @@ -1542,6 +1543,25 @@ int security_audit_rule_match(u32 secid, u32 field, u32 op, void *lsmrule,
>  }
>  #endif /* CONFIG_AUDIT */
>
> +#ifdef CONFIG_USER_NS
> +
> +int security_userns_create(struct user_namespace *ns)
> +{
> +       return call_int_hook(userns_create, 0, ns);
> +}
> +
> +void security_userns_free(struct user_namespace *ns)
> +{
> +       call_void_hook(userns_free, ns);
> +}
> +
> +int security_userns_setns(struct nsproxy *nsproxy, struct user_namespace *ns)
> +{
> +       return call_int_hook(userns_setns, 0, nsproxy, ns);
> +}
> +
> +#endif /* CONFIG_USER_NS */
> +
>  struct security_hook_heads security_hook_heads = {
>         .binder_set_context_mgr =
>                 LIST_HEAD_INIT(security_hook_heads.binder_set_context_mgr),
> @@ -1886,4 +1906,12 @@ struct security_hook_heads security_hook_heads = {
>         .audit_rule_free =
>                 LIST_HEAD_INIT(security_hook_heads.audit_rule_free),
>  #endif /* CONFIG_AUDIT */
> +#ifdef CONFIG_USER_NS
> +       .userns_create =
> +               LIST_HEAD_INIT(security_hook_heads.userns_create),
> +       .userns_free =
> +               LIST_HEAD_INIT(security_hook_heads.userns_free),
> +       .userns_setns =
> +               LIST_HEAD_INIT(security_hook_heads.userns_setns),
> +#endif /* CONFIG_USER_NS */
>  };
> --
> 2.4.3
>
diff mbox

Patch

diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 9429f05..228558c 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -1261,6 +1261,23 @@ 
  *	audit_rule_init.
  *	@rule contains the allocated rule
  *
+ * @userns_create:
+ *	Allocates and fills the security part of a new user namespace.
+ *	@ns points to a newly created user namespace.
+ *	Returns 0 or an error code.
+ *
+ * @userns_free:
+ *	Deallocates the security part of a user namespace.
+ *	@ns points to a user namespace about to be destroyed.
+ *
+ * @userns_setns:
+ *	Run during a setns syscall to add a process to an already existing
+ *	user namespace. Returning failure here will block the operation
+ *	requested from userspace (setns() with CLONE_NEWUSER).
+ *	@nsproxy contains nsproxy to which the user namespace will be assigned.
+ *	@ns contains user namespace that is to be incorporated to the nsproxy.
+ *	Returns 0 or an error code.
+ *
  * @inode_notifysecctx:
  *	Notify the security module of what the security context of an inode
  *	should be.  Initializes the incore security context managed by the
@@ -1613,6 +1630,12 @@  union security_list_options {
 				struct audit_context *actx);
 	void (*audit_rule_free)(void *lsmrule);
 #endif /* CONFIG_AUDIT */
+
+#ifdef CONFIG_USER_NS
+	int (*userns_create)(struct user_namespace *ns);
+	void (*userns_free)(struct user_namespace *ns);
+	int (*userns_setns)(struct nsproxy *nsproxy, struct user_namespace *ns);
+#endif /* CONFIG_USER_NS */
 };
 
 struct security_hook_heads {
@@ -1824,6 +1847,11 @@  struct security_hook_heads {
 	struct list_head audit_rule_match;
 	struct list_head audit_rule_free;
 #endif /* CONFIG_AUDIT */
+#ifdef CONFIG_USER_NS
+	struct list_head userns_create;
+	struct list_head userns_free;
+	struct list_head userns_setns;
+#endif /* CONFIG_USER_NS */
 };
 
 /*
diff --git a/include/linux/security.h b/include/linux/security.h
index 79d85dd..1b0eccc 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -1584,6 +1584,29 @@  static inline void security_audit_rule_free(void *lsmrule)
 #endif /* CONFIG_SECURITY */
 #endif /* CONFIG_AUDIT */
 
+#ifdef CONFIG_USER_NS
+int security_userns_create(struct user_namespace *ns);
+void security_userns_free(struct user_namespace *ns);
+int security_userns_setns(struct nsproxy *nsproxy, struct user_namespace *ns);
+
+#else
+
+static inline int security_userns_create(struct user_namespace *ns)
+{
+	return 0;
+}
+
+static inline void security_userns_free(struct user_namespace *ns)
+{ }
+
+static inline int security_userns_setns(struct nsproxy *nsproxy,
+					struct user_namespace *ns)
+{
+	return 0;
+}
+
+#endif /* CONFIG_USER_NS */
+
 #ifdef CONFIG_SECURITYFS
 
 extern struct dentry *securityfs_create_file(const char *name, umode_t mode,
diff --git a/include/linux/user_namespace.h b/include/linux/user_namespace.h
index 8297e5b..a9400cc 100644
--- a/include/linux/user_namespace.h
+++ b/include/linux/user_namespace.h
@@ -39,6 +39,10 @@  struct user_namespace {
 	struct key		*persistent_keyring_register;
 	struct rw_semaphore	persistent_keyring_register_sem;
 #endif
+
+#ifdef CONFIG_SECURITY
+	void *security;
+#endif
 };
 
 extern struct user_namespace init_user_ns;
diff --git a/kernel/user.c b/kernel/user.c
index b069ccb..ce5419e 100644
--- a/kernel/user.c
+++ b/kernel/user.c
@@ -59,6 +59,9 @@  struct user_namespace init_user_ns = {
 	.persistent_keyring_register_sem =
 	__RWSEM_INITIALIZER(init_user_ns.persistent_keyring_register_sem),
 #endif
+#ifdef CONFIG_SECURITY
+	.security = NULL,
+#endif
 };
 EXPORT_SYMBOL_GPL(init_user_ns);
 
diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
index 4109f83..cadffb6 100644
--- a/kernel/user_namespace.c
+++ b/kernel/user_namespace.c
@@ -22,6 +22,7 @@ 
 #include <linux/ctype.h>
 #include <linux/projid.h>
 #include <linux/fs_struct.h>
+#include <linux/security.h>
 
 static struct kmem_cache *user_ns_cachep __read_mostly;
 static DEFINE_MUTEX(userns_state_mutex);
@@ -108,6 +109,15 @@  int create_user_ns(struct cred *new)
 
 	set_cred_user_ns(new, ns);
 
+#ifdef CONFIG_SECURITY
+	ret = security_userns_create(ns);
+	if (ret) {
+		ns_free_inum(&ns->ns);
+		kmem_cache_free(user_ns_cachep, ns);
+		return ret;
+	}
+#endif
+
 #ifdef CONFIG_PERSISTENT_KEYRINGS
 	init_rwsem(&ns->persistent_keyring_register_sem);
 #endif
@@ -143,6 +153,9 @@  void free_user_ns(struct user_namespace *ns)
 #ifdef CONFIG_PERSISTENT_KEYRINGS
 		key_put(ns->persistent_keyring_register);
 #endif
+#ifdef CONFIG_SECURITY
+		security_userns_free(ns);
+#endif
 		ns_free_inum(&ns->ns);
 		kmem_cache_free(user_ns_cachep, ns);
 		ns = parent;
@@ -969,6 +982,7 @@  static int userns_install(struct nsproxy *nsproxy, struct ns_common *ns)
 {
 	struct user_namespace *user_ns = to_user_ns(ns);
 	struct cred *cred;
+	int err;
 
 	/* Don't allow gaining capabilities by reentering
 	 * the same user namespace.
@@ -986,6 +1000,10 @@  static int userns_install(struct nsproxy *nsproxy, struct ns_common *ns)
 	if (!ns_capable(user_ns, CAP_SYS_ADMIN))
 		return -EPERM;
 
+	err = security_userns_setns(nsproxy, user_ns);
+	if (err)
+		return err;
+
 	cred = prepare_creds();
 	if (!cred)
 		return -ENOMEM;
diff --git a/security/security.c b/security/security.c
index 595fffa..5e66388 100644
--- a/security/security.c
+++ b/security/security.c
@@ -25,6 +25,7 @@ 
 #include <linux/mount.h>
 #include <linux/personality.h>
 #include <linux/backing-dev.h>
+#include <linux/user_namespace.h>
 #include <net/flow.h>
 
 #define MAX_LSM_EVM_XATTR	2
@@ -1542,6 +1543,25 @@  int security_audit_rule_match(u32 secid, u32 field, u32 op, void *lsmrule,
 }
 #endif /* CONFIG_AUDIT */
 
+#ifdef CONFIG_USER_NS
+
+int security_userns_create(struct user_namespace *ns)
+{
+	return call_int_hook(userns_create, 0, ns);
+}
+
+void security_userns_free(struct user_namespace *ns)
+{
+	call_void_hook(userns_free, ns);
+}
+
+int security_userns_setns(struct nsproxy *nsproxy, struct user_namespace *ns)
+{
+	return call_int_hook(userns_setns, 0, nsproxy, ns);
+}
+
+#endif /* CONFIG_USER_NS */
+
 struct security_hook_heads security_hook_heads = {
 	.binder_set_context_mgr =
 		LIST_HEAD_INIT(security_hook_heads.binder_set_context_mgr),
@@ -1886,4 +1906,12 @@  struct security_hook_heads security_hook_heads = {
 	.audit_rule_free =
 		LIST_HEAD_INIT(security_hook_heads.audit_rule_free),
 #endif /* CONFIG_AUDIT */
+#ifdef CONFIG_USER_NS
+	.userns_create =
+		LIST_HEAD_INIT(security_hook_heads.userns_create),
+	.userns_free =
+		LIST_HEAD_INIT(security_hook_heads.userns_free),
+	.userns_setns =
+		LIST_HEAD_INIT(security_hook_heads.userns_setns),
+#endif /* CONFIG_USER_NS */
 };