Message-ID: <20140416151057.GE31074@redhat.com>
Date:	Wed, 16 Apr 2014 11:10:57 -0400
From:	Vivek Goyal <vgoyal@...hat.com>
To:	Andy Lutomirski <luto@...capital.net>
Cc:	Tejun Heo <tj@...nel.org>, Daniel Walsh <dwalsh@...hat.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	lpoetter@...hat.com, Simo Sorce <ssorce@...hat.com>,
	cgroups@...r.kernel.org, "David S. Miller" <davem@...emloft.net>,
	kay@...hat.com, Network Development <netdev@...r.kernel.org>
Subject: Re: [PATCH 2/2] net: Implement SO_PASSCGROUP to enable passing
 cgroup path

On Wed, Apr 16, 2014 at 07:34:41AM -0700, Andy Lutomirski wrote:
> On Wed, Apr 16, 2014 at 3:17 AM, Vivek Goyal <vgoyal@...hat.com> wrote:
> > On Tue, Apr 15, 2014 at 08:47:54PM -0700, Andy Lutomirski wrote:
> >> On Apr 15, 2014 5:20 PM, "Vivek Goyal" <vgoyal@...hat.com> wrote:
> >> >
> >> > On Tue, Apr 15, 2014 at 02:53:13PM -0700, Andy Lutomirski wrote:
> >> > > On Tue, Apr 15, 2014 at 2:15 PM, Vivek Goyal <vgoyal@...hat.com> wrote:
> >> > > > This patch implements socket option SO_PASSCGROUP along the lines of
> >> > > > SO_PASSCRED.
> >> > > >
> >> > > > If SO_PASSCGROUP is set, then recvmsg() will get a control message
> >> > > > SCM_CGROUP which will contain the cgroup path of sender. This cgroup
> >> > > > belongs to first mounted hierarchy in the sytem.
> >> > > >
> >> > > > SCM_CGROUP control message can only be received and sender can not send
> >> > > > a SCM_CGROUP message. Kernel automatically generates one if receiver
> >> > > > chooses to receive one.
> >> > > >
> >> > > > This works both for unix stream and datagram sockets.
> >> > > >
> >> > > > cgroup information is passed only if either the sender or receiver has
> >> > > > SO_PASSCGROUP option set. This means for existing workloads they should
> >> > > > not see any significant performance impact of this change.
> >> > >
> >> > > This is odd.  Shouldn't an SCM_CGROUP cmsg be generated when the
> >> > > receiver has SO_PASSCGROUP set and the sender passes SCM_CGROUP to
> >> > > sendmsg?
> >> >
> >> > How can receiver trust the cgroup info generated by sender. It needs to
> >> > be generated by kernel so that receiver can trust it.
> >> >
> >> > And if receiver needs to know cgroup of sender, receiver can just set
> >> > SO_PASSCGROUP on socket and receiver should get one SCM_CGROUP message
> >> > with each message received.
> >>
> >> I think the kernel should validate the data.
> >>
> >> Here's an attack against SO_PEERCGROUP: if you create a container with
> >> a super secret name, then every time you connect to any unix socket,
> >> you leak the name.
> >
> > One should be able to do that already today with SO_PASSCRED option and
> > then map pid to cgroup. Or if one is using user namespaces then go
> > through uid mappings and  figure out which container sent message.
> 
> Not if you've locked down proc, perhaps by using hidepid.
> 

I think before we dive into smaller details lets take a step back. Core of
your argument seems to be that exposing cgroup information of sender to
receiver is a security risk. Hence only sender should decide when to
send that information and when not to.

I find several issues here.

- Why exposing cgroup information of sender is a security risk?

- How would a sender decide when to send SCM_CGROUP info and when not to.

- Additional higher level protocols need to be established now between
  sender and receiver so that they agree upon that cgroup information need
  to be sent. This will just make the implementation complicated and 
  this should be done only if benefits outweigh the cons.  

> >
> >>
> >> Here's an attack against SO_PASSCGROUP, as you implemented it: connect
> >> a socket and get someone else to write(2) to it.  This isn't very
> >> hard.  Now you've impersonated.
> >
> > If you can get another process to write to your socket and impersonate,
> > then what will stop from that process to also send SCM_CGROUP message
> > also? So I don't see how SCM_CGROUP from client will solve this problem.
> >
> 
> I can easily get other processed to write to my socket.  Passing that
> socket as stderr to a setuid program is probably the easiest way.
> Finding a service that accepts a socket using SCM_RIGHTS and writes to
> it is another.  It is supposed to be safe to write(2) to an untrusted
> file descriptor, or, at the very least, it is supposed to be a DoS at
> worst.  In this case, it's also either an information leak.

Hold on, so what's the problem here.

- First of all, if you opened the connection SO_PEERCGROUP will still give
  the right information to receiver. Does not matter if later you got some
  priviliged process to write to that socket descriptor.

- Now if you passed fd to a privliged process as stderr, it is not asking
  for any services. And if it does, then there is a design issue on that
  privliged service side. 

I really don't see any issue here.

> 
> It's true that SO_PASSCRED has the same problem.  I consider that to
> be a mistake, and I suspect that there are a large number of
> longstanding security problems caused by it.  Regardless, we shouldn't
> exacerbate this problem.  There is no legacy code using SCM_CGROUP at
> all right now, because the option has never been in a released kernel.
>  So let's get the interface right the first time around.
> 
> If I find some time later today, I can try to write a variant of the
> patch that only sends SCM_CGROUP when the sender requests it.

I don't think implementation is the issue here. You need to provide a
stronger argument that why passing cgroup information is a security
risk.

> 
> On an unrelated note, what happens when there are multiple cgroup hierarchies?
> 

People are moving away from multiple cgroup hierarchiy model. In future a
single hiearchy is planned and multiple hierarchy will remain there only
for backward compatibility.

Passing cgroup of task in every hierarchy becomes too expensive. (4K per
hierarchy). And its  not clear how many hierarchies are present in the
system.

So there is not much point in passing cgroup of task in every hierarchy as
it is slowly being phased out.


> > Kernel cgroup verification will also not help in this case as sender
> > is sending his own cgroup.
> 
> Sure it will -- the sender sticks a string into SCM_CGROUP and the
> kernel checks it.

Kernel will check whether sender is sending right cgroup information or
not. So in your example, if you passed fd to a privliged process and
that process sends an SCM_CGROUP message, then kernel will just let
it go. SCM_CGROUP contains the cgroup of priviliged process and that's
the right information. 

If a priviliged process is asking for a service on a fd passed by 
unpriviliged process, that itself is a risk. And that risk will not
be mitigated by forcing the usage of SCM_CGROUP. 

> 
> >
> >>
> >> I advocate for the following semantics: if sendmsg is passed a
> >> SCM_CGROUP cmsg, and that cmsg has the right cgroup, and the receiver
> >> has SO_PASSCGROUP set, then the receiver gets SCM_CGROUP.  If you try
> >> to lie using SCM_CGROUP, you get -EPERM.  If you set SO_PASSCGROUP,
> >> but your peer doesn't sent SCM_CREDS, you get nothing.
> >>
> >> This is immune to both attacks.  It should be cheaper, too, since
> >> there's no overhead for people who don't use it.
> >
> > I think you seem to be saying that a client's credentials should not be
> > visible to receiver until and unless client himself wants to reveal
> > those. IOW, it kind of looks like an anonymous mode of operation where
> > client connects to a socket but receiver client not want to reveal any of
> > the information about itself to receiver.
> >
> > I am not sure how useful that mode really is. If it is really useful, I
> > think one could implement another socket option on client side to
> > deny passing cgroup information to receiver. Say SO_NOPASSCGROUP.
> 
> This won't help -- an attacker will simply not set that option, and
> the program being attacked is certainly not going to set
> SO_NOPASSCGROUP right before calling write.

This was for the case where you gave example of "super secret container"
which did not want to reveal its identity to receiver. 

For the case of priviliged process asking for service on an fd passed
by unpriviliged process, SCM_CGROUP will not help you either. So it is
a bad idea to begin with.


> 
> >
> > Before we even get there, I will question that what's so secret about
> > cgroup information that one would like to hide it from receiver. We don't
> > hide uid, pid, gid.
> >
> 
> I think we should hide uid, gid, and pid too, but that ship has sailed.
> 
> The bigger issue isn't hiding so much as accidental assertions of
> authority.  If a program accidentally leaks its uid, that's one thing.
>  If a program, by accidentally leaking its uid, causes another program
> to think that the sender wants some action to be taken on behalf of
> its uid, then there's a real security problem.
> 
> > Secondly, how would client know when to send SCM_CGROUP to receiver. For
> > the use case I mentioned that init wants to log cgroup of every message
> > going into journal. How would client know that every message needs to
> > have SCM_CGROUP. By automatically getting client information when receiver
> > needs it, simplifies the things a lot without any client modificaiton.
> 
> I think that the client *should* be modified.  What if there's an
> existing program that runs as a container's root but does not intend
> to sign off on a message with its cgroup?

That goes back to my previous question. Why revealing cgroup of sender
is a problem.

And in case we agree that it is a problem SO_NOPASSCGROUP will solve
that problem.

> 
> In any event, I still think that the journald case has no need for any
> kernel changes at all.  From a very cursory inspection, the journal
> code expects to find a socket in /run/systemd/journal/socket.  It
> should be enough to stick a different socket into that location in
> each container.  This will work on all kernels and may even work
> without modifying any code in the container.

This is a little generic discussion. No containers here. Think of various
services running in their own cgroups and logging messages into journal.

And that's only one use case.

Thanks
Vivek
--
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

Powered by blists - more mailing lists