[Quantal,CVE] userns: Changing any namespace id mappings should require privileges

From: Andy Lutomirski <luto@amacapital.net>

CVE-2013-1979

commit 41c21e351e79004dbb4efa4bc14a53a7e0af38c5 upstream.

Changing uid/gid/projid mappings doesn't change your id within the
namespace; it reconfigures the namespace.  Unprivileged programs should
*not* be able to write these files.  (We're also checking the privileges
on the wrong task.)

Given the write-once nature of these files and the other security
checks, this is likely impossible to usefully exploit.

Signed-off-by: Andy Lutomirski <luto@amacapital.net>
Signed-off-by: Steve Conklin <sconklin@canonical.com>
---
 kernel/user_namespace.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

On 06/27/2013 12:44 PM, Steve Conklin wrote:
> From: Andy Lutomirski <luto@amacapital.net>
> 
> CVE-2013-1979
> 
> commit 41c21e351e79004dbb4efa4bc14a53a7e0af38c5 upstream.
> 
> Changing uid/gid/projid mappings doesn't change your id within the
> namespace; it reconfigures the namespace.  Unprivileged programs should
> *not* be able to write these files.  (We're also checking the privileges
> on the wrong task.)
> 
> Given the write-once nature of these files and the other security
> checks, this is likely impossible to usefully exploit.
> 
> Signed-off-by: Andy Lutomirski <luto@amacapital.net>
> Signed-off-by: Steve Conklin <sconklin@canonical.com>
> ---
>  kernel/user_namespace.c |    6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
> index 8660231..34e91b3 100644
> --- a/kernel/user_namespace.c
> +++ b/kernel/user_namespace.c
> @@ -431,10 +431,10 @@ static ssize_t map_write(struct file *file, const char __user *buf,
>  	if (map->nr_extents != 0)
>  		goto out;
>  
> -	/* Require the appropriate privilege CAP_SETUID or CAP_SETGID
> -	 * over the user namespace in order to set the id mapping.
> +	/*
> +	 * Adjusting namespace settings requires capabilities on the target.
>  	 */
> -	if (!ns_capable(ns, cap_setid))
> +	if (!file_ns_capable(file, ns, CAP_SYS_ADMIN))
>  		goto out;
>  
>  	/* Get a buffer */
>

Steve Conklin <sconklin@canonical.com> writes:

> From: Andy Lutomirski <luto@amacapital.net>
>
> CVE-2013-1979
>
> commit 41c21e351e79004dbb4efa4bc14a53a7e0af38c5 upstream.
>
> Changing uid/gid/projid mappings doesn't change your id within the
> namespace; it reconfigures the namespace.  Unprivileged programs should
> *not* be able to write these files.  (We're also checking the privileges
> on the wrong task.)
>
> Given the write-once nature of these files and the other security
> checks, this is likely impossible to usefully exploit.
>
> Signed-off-by: Andy Lutomirski <luto@amacapital.net>
> Signed-off-by: Steve Conklin <sconklin@canonical.com>
> ---
>  kernel/user_namespace.c |    6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
> index 8660231..34e91b3 100644
> --- a/kernel/user_namespace.c
> +++ b/kernel/user_namespace.c
> @@ -431,10 +431,10 @@ static ssize_t map_write(struct file *file, const char __user *buf,
>  	if (map->nr_extents != 0)
>  		goto out;
>  
> -	/* Require the appropriate privilege CAP_SETUID or CAP_SETGID
> -	 * over the user namespace in order to set the id mapping.
> +	/*
> +	 * Adjusting namespace settings requires capabilities on the target.
>  	 */
> -	if (!ns_capable(ns, cap_setid))
> +	if (!file_ns_capable(file, ns, CAP_SYS_ADMIN))
>  		goto out;
>  
>  	/* Get a buffer */

Although this is an additional fix suggested by sarnold (the actual
CVE is already fixed in Quantal), it provides additional hardening to
the kernel.  The backport seems correct to me.

Note however that the Q kernel doesn't seem to activate this code
(USER_NS isn't set).

Also, the buglink is missing in the commit text:

 BugLink: http://bugs.launchpad.net/bugs/1174827

Cheers,

Um, file_ns_capable() does not exist in Quantal

On 06/28/2013 06:54 AM, Tim Gardner wrote:
> Um, file_ns_capable() does not exist in Quantal
> 

A little further investigation indicates that USER_NS depends on
UIDGID_CONVERTED. In order to satisfy _that_ dependency we'd have to
disable most of the useful features in the kernel.

This CVE patch is dependent on 935d8aabd4331f47a89c3e1daa5779d23cf244ee
(which _does_ apply and compile), but I'm questioning whether this CVE
even impacts Quantal in the first place. It certainly does not seem to
given or set of configs.

rtg

Quoting Tim Gardner (tim.gardner@canonical.com):
> Um, file_ns_capable() does not exist in Quantal

Yeah, that is only executed when you write to /proc/self/maps,
which only exists when CONFIG_USERNS=y, which can't be the
case in our kernels yet - not even saucy right now.

So our first CONFIG_USERNS=y kernel should get this fix
automatically from upstream, and no earlier kernels should
need this.

-serge