mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Serge E. Hallyn" <serge@hallyn.com>
To: "Serge E. Hallyn" <serge.hallyn@canonical.com>
Cc: "Eric W. Biederman" <ebiederm@xmission.com>,
	linux-kernel@vger.kernel.org, akpm@linux-foundation.org,
	oleg@redhat.com, richard@nod.at, mikevs@xs4all.net,
	segoon@openwall.com, gregkh@suse.de, dhowells@redhat.com,
	eparis@redhat.com, netdev@vger.kernel.org
Subject: Re: [PATCH 9/9] make net/core/scm.c uid comparisons user namespace aware
Date: Mon, 24 Oct 2011 04:15:29 +0000	[thread overview]
Message-ID: <20111024041529.GA23618@hallyn.com> (raw)
In-Reply-To: <20111020141440.GA6201@sergelap>

Quoting Serge E. Hallyn (serge.hallyn@canonical.com):
> Quoting Eric W. Biederman (ebiederm@xmission.com):
> > "Serge E. Hallyn" <serge@hallyn.com> writes:
> > 
> > > Quoting Eric W. Biederman (ebiederm@xmission.com):
> > >> Serge Hallyn <serge@hallyn.com> writes:
> > >> 
> > >> > From: "Serge E. Hallyn" <serge.hallyn@canonical.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.
> > >
> > > Thanks for looking, Eric.
> > >
> > >> Because we pass the cred and the pid through the socket socket itself
> > >> is just a conduit and should be ignored in this context.
> > >
> > > Ok, that makes sense, but
> > >
> > >> The only interesting test should be are you allowed to impersonate other
> > >> users in your current userk namespace.
> > >
> > > Why in your current user namespace?  Shouldn't it be in the
> > > target user ns?  I understand it could be wrong to tie the
> > > user ns owning the socket to the target userns (though I still
> > > kind of like it), but just because I have CAP_SETUID in my
> > > own user_ns doesn't mean I should be able to pose as another
> > > uid in your user_ns.
> > 
> > First and foremost it is important that you be able if you have the
> > capability to impersonate other users in your current user namespace.
> > That is what the capability actually controls.
> > 
> > None of this allows you to impersonate any user in any other user
> > namespace.  The translation between users prevents that.
> > 
> > > (Now I also see that cred_to_ucred() translates to the current
> > > user_ns, so that should have been a hint to me before about
> > > your intent, but I'm not convinced I agree with your intent).
> > >
> > > And you do the same with the pid.  Why is that a valid assumption?
> > 
> > Yes.  Basically all the code is allow you to impersonate people you
> > would have been able to impersonate before.  If your target is in
> > another namespace you can not fool them.
> > 
> > With pids the logic should be a lot clearer.  Pretend to be a pid you can
> > see in your current pid namespace.  Lookup and convert to struct pid aka
> > the namespace agnostic object.  On output return the pid value that
> 
> No.  That conversion is happending before the user-specified pid is
> set.

Never mind, it all gets a little convoluted, but I see how it works,
and - when the time comes - how to do it right for userns.  :)  Sorry
about that.

thanks,
-serge

  reply	other threads:[~2011-10-24  4:15 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-10-18 21:54 user namespaces: fix some uid/privilege leaks Serge Hallyn
2011-10-18 21:54 ` [PATCH 1/9] pid_ns: ensure pid is not freed during kill_pid_info_as_uid Serge Hallyn
2011-10-18 21:54 ` [PATCH 2/9] user namespace: usb: make usb urbs user namespace aware (v2) Serge Hallyn
2011-10-18 21:54 ` [PATCH 3/9] user namespace: make signal.c respect user namespaces (v4) Serge Hallyn
2011-10-18 21:54 ` [PATCH 4/9] User namespace: don't allow sysctl in non-init user ns (v2) Serge Hallyn
2011-10-18 21:54 ` [PATCH 5/9] user namespace: clamp down users of cap_raised Serge Hallyn
2011-10-19  4:33   ` Andrew G. Morgan
2011-10-20 13:01     ` Serge E. Hallyn
2011-10-19  9:01   ` David Howells
2011-10-20 13:16     ` Serge E. Hallyn
2011-10-24 14:43     ` [PATCH 05/10] " Serge E. Hallyn
2011-10-24 15:47       ` Andrew G. Morgan
2011-10-24 17:28         ` Serge E. Hallyn
2011-10-25  0:43           ` Andrew G. Morgan
2011-10-25  3:03             ` Serge E. Hallyn
2011-10-25 17:33               ` Eric Paris
2011-10-25 20:09                 ` Serge E. Hallyn
2011-10-18 21:54 ` [PATCH 6/9] Add Documentation/namespaces/user_namespace.txt (v3) Serge Hallyn
2011-10-18 21:54 ` [PATCH 7/9] user namespace: make each net (net_ns) belong to a user_ns Serge Hallyn
2011-10-18 21:54 ` [PATCH 8/9] protect cap_netlink_recv from user namespaces Serge Hallyn
2011-10-18 21:54 ` [PATCH 9/9] make net/core/scm.c uid comparisons user namespace aware Serge Hallyn
2011-10-18 22:14   ` Joe Perches
2011-10-18 23:22     ` Serge E. Hallyn
2011-10-19  2:25       ` [PATCH 9/9] make net/core/scm.c uid comparisons user namespace aware (v2) Serge E. Hallyn
2011-10-19 13:52   ` [PATCH 9/9] make net/core/scm.c uid comparisons user namespace aware Eric W. Biederman
2011-10-20 12:58     ` Serge E. Hallyn
2011-10-20 13:35       ` Eric W. Biederman
2011-10-20 14:14         ` Serge E. Hallyn
2011-10-24  4:15           ` Serge E. Hallyn [this message]
2011-10-24  4:27             ` Eric W. Biederman
2011-10-20 14:24         ` Serge E. Hallyn
2011-10-19  9:36 ` [PATCH 6/9] Add Documentation/namespaces/user_namespace.txt (v3) David Howells
2011-10-20 12:58   ` Serge E. Hallyn
2011-10-26 20:33   ` [PATCH 06/10] Add Documentation/namespaces/user_namespace.txt (v4) Serge E. Hallyn

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20111024041529.GA23618@hallyn.com \
    --to=serge@hallyn.com \
    --cc=akpm@linux-foundation.org \
    --cc=dhowells@redhat.com \
    --cc=ebiederm@xmission.com \
    --cc=eparis@redhat.com \
    --cc=gregkh@suse.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mikevs@xs4all.net \
    --cc=netdev@vger.kernel.org \
    --cc=oleg@redhat.com \
    --cc=richard@nod.at \
    --cc=segoon@openwall.com \
    --cc=serge.hallyn@canonical.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome