Re: [PATCH 2/2] net: Implement SO_PASSCGROUP to enable passing cgroup path

[Vivek Goyal <vgoyal-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]

On Wed, Apr 16, 2014 at 12:13:21PM -0700, Andy Lutomirski wrote:
> On Wed, Apr 16, 2014 at 12:06 PM, Vivek Goyal <vgoyal-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> > On Wed, Apr 16, 2014 at 11:35:13AM -0700, Andy Lutomirski wrote:
> >> On Wed, Apr 16, 2014 at 11:25 AM, Vivek Goyal <vgoyal-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> >> > On Wed, Apr 16, 2014 at 11:13:31AM -0700, Andy Lutomirski wrote:
> >> >
> >> > [..]
> >> >> > Ok, so passing cgroup information is not necessarily a problem as long
> >> >> > as it is not used for authentication. So say somebody is just logging
> >> >> > all the client request and which cgroup client was in, that should not
> >> >> > be a problem.
> >> >>
> >> >> Do you consider correct attribution of logging messages to be
> >> >> important?  If so, then this is a kind of authentication, albeit one
> >> >> where the impact of screwing it up is a bit lower.
> >> >
> >> > So not passing cgroup information makes attribution more correct. Just
> >> > logging of information is authentication how? Both kernel and user space
> >> > log message into /var/log/messages and kernel messages are prefixed with
> >> > "kernel". So this somehow becomes are sort of authentication. I don't
> >> > get it.
> >>
> >> I did a bad job of explaining what I meant.
> >>
> >> I think that, currently, log lines can be correctly attributed to the
> >> kernel or to userspace, but determining where in userspace a log line
> >> came from is a bit flaky.  One of the goals of these patches is to
> >> make log attribution less flaky.  But if you want log attribution to
> >> be completely correct, even in the presence of malicious programs,
> >> then I think that the current patches aren't quite there.
> >
> > Why do you think that current patches are not there yet in the presence of
> > malicious program. In your example, one program opened the socket and
> > passed it to malicious program. And all the future messages are coming
> > from malicious program. As long as receiver checks for SO_PASSCGROUP,
> > it covers your example.
> 
> This is backwards.  The malicious program opens the socket and passes
> it to an unwitting non-malicious program.  That non-malicious program
> sends messages, and the logging daemon things that the non-malicious
> program actually intended for these messages to end up in the system
> log.

Either way you look at it, I can't see the problem. Even without cgroup
info, in your example, a non-malicious programs error message will show
up at the receiver (Because malicious program passed that fd as stdout
to non-malicious program).

Are you complainig about this?

Or are you complaing that non-malicious program's cgroup info will show
up at the receiver. What's the problem with that. Receiver can use
SO_PASSCGROUP and get non-malicious programs cgroup and log it (along
with error message). I don't see where is the problem in that.

> 
> >
> >>
> >> Is the reason that you don't want to modify the senders because you
> >> want users of syslog(3) to get the new behavior?  If so, I think it
> >> would be nice to update glibc to fix that, but maybe the kernel should
> >> cooperate, and maybe SO_PEERCGROUP is a decent way to handle this.
> >
> > Modifying every user of unix sockets to start passing cgroup information
> > will make sense only if it was deemed that passing cgroup information
> > is risky inherently and should be done only in selected cases.
> >
> > I think so far our understanding is that we can't find anything
> > inherently wrong with passing cgroup information, though there might
> > be some corner cases we can run into and which are not obivious right
> > now.
> 
> I think I've explained why causing write(2) to start transmitting the
> caller's cgroup is problematic. Your SO_PASSCGROUP patch does that.
> Your SO_PEERCGROUP patch does not.

That's a generic statement. You have not given an specific example, where
it will be a problem. Two easy ways to mitigate this will be.

- Depending on use case, just look at SO_PEERCGROUP info. This will
  avoid all the use cases you mentioned about tricking a priviliged
  program to write something on a fd.

- Also depending on use case one could compare both SO_PEERCGROUP and
  SO_PASSCGROUP info and make sure cgroup remains the same otherwise
  deny the service.

> 
> I'm still nervous about SO_PEERCGROUP and about a variant of
> SO_PASSCGROUP that used the cgroup as of the time of connect(2), but
> I'm much less convinced that there's an actual problem.  My
> nervousness here is more that I don't like APIs that aren't clearly
> safe.

CVE you pointed to talks about SCM_CREDENTIAL vulnerability. That was
a bug and now it has been fixed. So what's unsafe about SCM_CREDENTIAL
now.

Thanks
Vivek