linux-kernel - Re: [PATCH 2/5] kernel: add a helper to get an owning user namespace for a namespace

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20160714190742.GA24913@odin.tremily.us>
Date:	Thu, 14 Jul 2016 12:07:42 -0700
From:	"W. Trevor King" <wking@...mily.us>
To:	Andrey Vagin <avagin@...nvz.org>
Cc:	linux-kernel@...r.kernel.org, criu@...nvz.org,
	linux-api@...r.kernel.org, containers@...ts.linux-foundation.org,
	linux-fsdevel@...r.kernel.org
Subject: Re: [PATCH 2/5] kernel: add a helper to get an owning user namespace
 for a namespace

On Thu, Jul 14, 2016 at 11:20:16AM -0700, Andrey Vagin wrote:
> +struct ns_common *ns_get_owner(struct ns_common *ns)
> +{
> +	const struct cred *cred = current_cred();
> +	struct user_namespace *user_ns, *p;
> +
> +	user_ns = p = ns->user_ns;
> +	if (user_ns == NULL) { /* ns is init_user_ns */
> +		/* Unprivileged user should not know that it's init_user_ns. */
> +		if (capable(CAP_SYS_ADMIN))
> +			return ERR_PTR(-ENOENT);
> +		return ERR_PTR(-EPERM);
> +	}
> +
> +	for (;;) {
> +		if (p == cred->user_ns)
> +			break;
> +		if (p == &init_user_ns)
> +			return ERR_PTR(-EPERM);
> +		p = p->parent;
> +	}
> +
> +	return &get_user_ns(user_ns)->ns;
> +}

I'm still not sure we need the CAP_SYS_ADMIN check [1].  Maybe “you
have an open file descriptor for the namespace” means you've already
been authorized to access the parent information (e.g. via POSIX
permissions on /proc/<pid>/ns/… or the bind-mounted namespace).
Whether you can get the parent information probably depends whether
you can use setns to join the parent namespace (I haven't looked up
the backing code for that).

But whichever way we go there, I think we do want to be consistent
between init_user_ns and other namespaces.  So we should have a
CAP_SYS_ADMIN check for init_user_ns if and only if we also have a
CAP_SYS_ADMIN check for the returned parent in the non-init_user_ns
case as well:

  user_ns = p = ns->user_ns;
  if (user_ns == NULL) { /* ns is init_user_ns */
    /* Unprivileged user should not know that it's init_user_ns. */
    if (capable(CAP_SYS_ADMIN))
      return ERR_PTR(-ENOENT);
     return ERR_PTR(-EPERM);
  } else if (! capable_in(user_ns, CAP_SYS_ADMIN)) {
    /* Unprivileged user should not know about the owning user ns. */
    return ERR_PTR(-ENOENT);
  }

Although I'm not sure what the real name for capable_in is, or even if
it exists.

Cheers,
Trevor

[1]: https://github.com/avagin/linux-task-diag/commit/2663bc803d324785e328261f3c07a0fef37d2088#commitcomment-18223327

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

Download attachment "signature.asc" of type "application/pgp-signature" (820 bytes)