netdev - Re: [PATCH 9/9] make net/core/scm.c uid comparisons user namespace aware

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: <m1sjmpytpf.fsf@fess.ebiederm.org>
Date:	Wed, 19 Oct 2011 06:52:28 -0700
From:	ebiederm@...ssion.com (Eric W. Biederman)
To:	Serge Hallyn <serge@...lyn.com>
Cc:	linux-kernel@...r.kernel.org, akpm@...ux-foundation.org,
	oleg@...hat.com, richard@....at, mikevs@...all.net,
	segoon@...nwall.com, gregkh@...e.de, dhowells@...hat.com,
	eparis@...hat.com, "Serge E. Hallyn" <serge.hallyn@...onical.com>,
	netdev@...r.kernel.org
Subject: Re: [PATCH 9/9] make net/core/scm.c uid comparisons user namespace aware

Serge Hallyn <serge@...lyn.com> writes:

> From: "Serge E. Hallyn" <serge.hallyn@...onical.com>
>
> Currently uids are compared without regard for the user namespace.
> Fix that to prevent tasks in a different user namespace from
> wrongly matching on SCM_CREDENTIALS.
>
> In the past, either your uids had to match, or you had to have
> CAP_SETXID.  In a namespaced world, you must either (both be in the
> same user namespace and have your uids match), or you must have
> CAP_SETXID targeted at the other user namespace.  The latter can
> happen for instance if uid 500 created a new user namespace and
> now interacts with uid 0 in it.

Serge this approach is wrong.

Because we pass the cred and the pid through the socket socket itself
is just a conduit and should be ignored in this context.

The only interesting test should be are you allowed to impersonate other
users in your current userk namespace.

So it should be possible to simplify the entire patch to just:
 static __inline__ int scm_check_creds(struct ucred *creds)
 {
 	const struct cred *cred = current_cred();
+	struct user_namespace *ns = cred->user_ns;

-	if ((creds->pid == task_tgid_vnr(current) || capable(CAP_SYS_ADMIN)) &&
-	    ((creds->uid == cred->uid   || creds->uid == cred->euid ||
-	      creds->uid == cred->suid) || capable(CAP_SETUID)) &&
-	    ((creds->gid == cred->gid   || creds->gid == cred->egid ||
-	      creds->gid == cred->sgid) || capable(CAP_SETGID))) {
+	if ((creds->pid == task_tgid_vnr(current) || ns_capable(ns, CAP_SYS_ADMIN)) &&
+	    ((creds->uid == cred->uid   || creds->uid == cred->euid ||
+	      creds->uid == cred->suid) || ns_capable(ns, CAP_SETUID)) &&
+	    ((creds->gid == cred->gid   || creds->gid == cred->egid ||
+	      creds->gid == cred->sgid) || ns_capable(ns, CAP_SETGID))) {
  	       return 0;
  	}
  	return -EPERM;
  }
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html