diff mbox series

[v2,4/4] selinux: Implement create_user_ns hook

Message ID 20220707223228.1940249-5-fred@cloudflare.com (mailing list archive)
State Superseded
Delegated to: Paul Moore
Headers show
Series Introduce security_create_user_ns() | expand

Commit Message

Frederick Lawler July 7, 2022, 10:32 p.m. UTC
Unprivileged user namespace creation is an intended feature to enable
sandboxing, however this feature is often used to as an initial step to
perform a privilege escalation attack.

This patch implements a new namespace { userns_create } access control
permission to restrict which domains allow or deny user namespace
creation. This is necessary for system administrators to quickly protect
their systems while waiting for vulnerability patches to be applied.

This permission can be used in the following way:

        allow domA_t domB_t : namespace { userns_create };

Signed-off-by: Frederick Lawler <fred@cloudflare.com>

---
Changes since v1:
- Introduce this patch
---
 security/selinux/hooks.c            | 9 +++++++++
 security/selinux/include/classmap.h | 2 ++
 2 files changed, 11 insertions(+)

Comments

Paul Moore July 20, 2022, 1:32 a.m. UTC | #1
On Thu, Jul 7, 2022 at 6:32 PM Frederick Lawler <fred@cloudflare.com> wrote:
>
> Unprivileged user namespace creation is an intended feature to enable
> sandboxing, however this feature is often used to as an initial step to
> perform a privilege escalation attack.
>
> This patch implements a new namespace { userns_create } access control
> permission to restrict which domains allow or deny user namespace
> creation. This is necessary for system administrators to quickly protect
> their systems while waiting for vulnerability patches to be applied.
>
> This permission can be used in the following way:
>
>         allow domA_t domB_t : namespace { userns_create };
>
> Signed-off-by: Frederick Lawler <fred@cloudflare.com>
>
> ---
> Changes since v1:
> - Introduce this patch
> ---
>  security/selinux/hooks.c            | 9 +++++++++
>  security/selinux/include/classmap.h | 2 ++
>  2 files changed, 11 insertions(+)
>
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index beceb89f68d9..73fbcb434fe0 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -4227,6 +4227,14 @@ static void selinux_task_to_inode(struct task_struct *p,
>         spin_unlock(&isec->lock);
>  }
>
> +static int selinux_userns_create(const struct cred *cred)
> +{
> +       u32 sid = current_sid();
> +
> +       return avc_has_perm(&selinux_state, sid, sid, SECCLASS_NAMESPACE,
> +                                               NAMESPACE__USERNS_CREATE, NULL);
> +}

As we continue to discuss this, I'm beginning to think that having a
dedicated object class for the userns might be a good idea.  I believe
I was the one who gave you these code snippets, so feel free to blame
me for the respin ;)

This is what I'm thinking:

  static int selinux_userns_create(const struct cred *cred)
  {
    u32 sid = current_sid();

    return avc_has_perm(&selinux_state, sid, sid,
                        SECCLASS_USER_NAMESPACE,
                        USER_NAMESPACE__CREATE, NULL);
  }

> diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h
> index ff757ae5f253..9943e85c6b3e 100644
> --- a/security/selinux/include/classmap.h
> +++ b/security/selinux/include/classmap.h
> @@ -254,6 +254,8 @@ const struct security_class_mapping secclass_map[] = {
>           { COMMON_FILE_PERMS, NULL } },
>         { "io_uring",
>           { "override_creds", "sqpoll", NULL } },
> +       { "namespace",
> +         { "userns_create", NULL } },

The above would need to change to:

  { "user_namespace",
    { "create", NULL } }
Paul Moore July 20, 2022, 2:52 p.m. UTC | #2
On Tue, Jul 19, 2022 at 10:42 PM Karl MacMillan
<karl@bigbadwolfsecurity.com> wrote:
> On Thu, Jul 7, 2022 at 6:34 PM Frederick Lawler <fred@cloudflare.com> wrote:
>>
>> Unprivileged user namespace creation is an intended feature to enable
>> sandboxing, however this feature is often used to as an initial step to
>> perform a privilege escalation attack.
>>
>> This patch implements a new namespace { userns_create } access control
>> permission to restrict which domains allow or deny user namespace
>> creation. This is necessary for system administrators to quickly protect
>> their systems while waiting for vulnerability patches to be applied.
>>
>> This permission can be used in the following way:
>>
>>         allow domA_t domB_t : namespace { userns_create };
>
>
> Isn’t this actually domA_t domA_t : namespace . . .
>
> I got confused reading this initially trying to figure out what the second domain type would be, but looking at the code cleared that up.

Ah, good catch, thanks Karl!
Frederick Lawler July 20, 2022, 2:57 p.m. UTC | #3
On 7/19/22 8:32 PM, Paul Moore wrote:
> On Thu, Jul 7, 2022 at 6:32 PM Frederick Lawler <fred@cloudflare.com> wrote:
>>
>> Unprivileged user namespace creation is an intended feature to enable
>> sandboxing, however this feature is often used to as an initial step to
>> perform a privilege escalation attack.
>>
>> This patch implements a new namespace { userns_create } access control
>> permission to restrict which domains allow or deny user namespace
>> creation. This is necessary for system administrators to quickly protect
>> their systems while waiting for vulnerability patches to be applied.
>>
>> This permission can be used in the following way:
>>
>>          allow domA_t domB_t : namespace { userns_create };
>>
>> Signed-off-by: Frederick Lawler <fred@cloudflare.com>
>>
>> ---
>> Changes since v1:
>> - Introduce this patch
>> ---
>>   security/selinux/hooks.c            | 9 +++++++++
>>   security/selinux/include/classmap.h | 2 ++
>>   2 files changed, 11 insertions(+)
>>
>> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
>> index beceb89f68d9..73fbcb434fe0 100644
>> --- a/security/selinux/hooks.c
>> +++ b/security/selinux/hooks.c
>> @@ -4227,6 +4227,14 @@ static void selinux_task_to_inode(struct task_struct *p,
>>          spin_unlock(&isec->lock);
>>   }
>>
>> +static int selinux_userns_create(const struct cred *cred)
>> +{
>> +       u32 sid = current_sid();
>> +
>> +       return avc_has_perm(&selinux_state, sid, sid, SECCLASS_NAMESPACE,
>> +                                               NAMESPACE__USERNS_CREATE, NULL);
>> +}
> 
> As we continue to discuss this, I'm beginning to think that having a
> dedicated object class for the userns might be a good idea.  I believe
> I was the one who gave you these code snippets, so feel free to blame
> me for the respin ;)
> 

No worries, I'll make this change for v3.

> This is what I'm thinking:
> 
>    static int selinux_userns_create(const struct cred *cred)
>    {
>      u32 sid = current_sid();
> 
>      return avc_has_perm(&selinux_state, sid, sid,
>                          SECCLASS_USER_NAMESPACE,
>                          USER_NAMESPACE__CREATE, NULL);
>    }
> 
>> diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h
>> index ff757ae5f253..9943e85c6b3e 100644
>> --- a/security/selinux/include/classmap.h
>> +++ b/security/selinux/include/classmap.h
>> @@ -254,6 +254,8 @@ const struct security_class_mapping secclass_map[] = {
>>            { COMMON_FILE_PERMS, NULL } },
>>          { "io_uring",
>>            { "override_creds", "sqpoll", NULL } },
>> +       { "namespace",
>> +         { "userns_create", NULL } },
> 
> The above would need to change to:
> 
>    { "user_namespace",
>      { "create", NULL } }
>
diff mbox series

Patch

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index beceb89f68d9..73fbcb434fe0 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -4227,6 +4227,14 @@  static void selinux_task_to_inode(struct task_struct *p,
 	spin_unlock(&isec->lock);
 }
 
+static int selinux_userns_create(const struct cred *cred)
+{
+	u32 sid = current_sid();
+
+	return avc_has_perm(&selinux_state, sid, sid, SECCLASS_NAMESPACE,
+						NAMESPACE__USERNS_CREATE, NULL);
+}
+
 /* Returns error only if unable to parse addresses */
 static int selinux_parse_skb_ipv4(struct sk_buff *skb,
 			struct common_audit_data *ad, u8 *proto)
@@ -7117,6 +7125,7 @@  static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
 	LSM_HOOK_INIT(task_movememory, selinux_task_movememory),
 	LSM_HOOK_INIT(task_kill, selinux_task_kill),
 	LSM_HOOK_INIT(task_to_inode, selinux_task_to_inode),
+	LSM_HOOK_INIT(create_user_ns, selinux_userns_create),
 
 	LSM_HOOK_INIT(ipc_permission, selinux_ipc_permission),
 	LSM_HOOK_INIT(ipc_getsecid, selinux_ipc_getsecid),
diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h
index ff757ae5f253..9943e85c6b3e 100644
--- a/security/selinux/include/classmap.h
+++ b/security/selinux/include/classmap.h
@@ -254,6 +254,8 @@  const struct security_class_mapping secclass_map[] = {
 	  { COMMON_FILE_PERMS, NULL } },
 	{ "io_uring",
 	  { "override_creds", "sqpoll", NULL } },
+	{ "namespace",
+	  { "userns_create", NULL } },
 	{ NULL }
   };