linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Vivek Goyal <vgoyal@redhat.com>
To: Andy Lutomirski <luto@amacapital.net>
Cc: Simo Sorce <ssorce@redhat.com>,
	David Miller <davem@davemloft.net>, Tejun Heo <tj@kernel.org>,
	Daniel Walsh <dwalsh@redhat.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	lpoetter@redhat.com, cgroups@vger.kernel.org, kay@redhat.com,
	Network Development <netdev@vger.kernel.org>
Subject: Re: [PATCH 2/2] net: Implement SO_PASSCGROUP to enable passing cgroup path
Date: Wed, 16 Apr 2014 15:39:18 -0400	[thread overview]
Message-ID: <20140416193918.GM31074@redhat.com> (raw)
In-Reply-To: <CALCETrUv56awd+UoO_f8LLL2FVq-Hc6Bd6iBGMqWjVGpgxgTSg@mail.gmail.com>

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@redhat.com> 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@redhat.com> 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

  reply	other threads:[~2014-04-16 19:39 UTC|newest]

Thread overview: 91+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-04-15 21:15 [PATCH 0/2] net: Implement SO_PEERCGROUP and SO_PASSCGROUP socket options Vivek Goyal
2014-04-15 21:15 ` [PATCH 1/2] net: Implement SO_PEERCGROUP Vivek Goyal
2014-04-15 21:54   ` Andy Lutomirski
2014-04-16  0:22     ` Vivek Goyal
2014-04-15 21:15 ` [PATCH 2/2] net: Implement SO_PASSCGROUP to enable passing cgroup path Vivek Goyal
2014-04-15 21:53   ` Andy Lutomirski
2014-04-15 23:09     ` Simo Sorce
2014-04-16  0:20     ` Vivek Goyal
2014-04-16  1:05       ` David Miller
2014-04-16  3:47       ` Andy Lutomirski
2014-04-16 10:17         ` Vivek Goyal
2014-04-16 14:34           ` Andy Lutomirski
2014-04-16 15:10             ` Vivek Goyal
2014-04-16 12:57         ` David Miller
2014-04-16 14:37           ` Andy Lutomirski
2014-04-16 16:13             ` Simo Sorce
2014-04-16 16:21               ` Tejun Heo
2014-04-16 16:54                 ` Simo Sorce
2014-04-16 16:31               ` Andy Lutomirski
2014-04-16 17:02                 ` Simo Sorce
2014-04-16 17:29                   ` Andy Lutomirski
2014-04-16 17:34                     ` Simo Sorce
2014-04-16 17:53                       ` Andy Lutomirski
2014-04-16 18:36                     ` Vivek Goyal
2014-04-16 18:40                       ` Andy Lutomirski
2014-04-16 18:51                         ` Vivek Goyal
2014-04-16 18:59                           ` Andy Lutomirski
2014-04-16 18:06                 ` Vivek Goyal
2014-04-16 18:13                   ` Andy Lutomirski
2014-04-16 18:25                     ` Vivek Goyal
2014-04-16 18:35                       ` Andy Lutomirski
2014-04-16 19:06                         ` Vivek Goyal
2014-04-16 19:13                           ` Andy Lutomirski
2014-04-16 19:39                             ` Vivek Goyal [this message]
2014-04-16 20:24                               ` Andy Lutomirski
2014-04-17 13:41                                 ` Vivek Goyal
2014-04-16 18:59                     ` Vivek Goyal
2014-04-17 15:41                       ` Daniel J Walsh
2014-04-17 16:04                         ` Simo Sorce
2014-04-17 16:11                           ` Andy Lutomirski
2014-04-17 16:24                             ` Simo Sorce
2014-04-17 16:37                               ` Andy Lutomirski
2014-04-17 16:48                                 ` Simo Sorce
2014-04-17 16:55                                   ` Andy Lutomirski
2014-04-17 17:12                                     ` Vivek Goyal
2014-04-17 17:26                                       ` Andy Lutomirski
2014-04-17 17:33                                         ` Simo Sorce
2014-04-17 17:35                                           ` Andy Lutomirski
2014-04-17 17:47                                             ` Simo Sorce
2014-04-17 18:05                                               ` Andy Lutomirski
2014-04-17 18:23                                             ` Simo Sorce
2014-04-17 18:33                                               ` Andy Lutomirski
2014-04-17 18:50                                               ` Vivek Goyal
2014-04-17 18:57                                                 ` Vivek Goyal
2014-04-17 19:06                                                   ` Andy Lutomirski
2014-04-17 19:15                                                     ` Simo Sorce
2014-04-17 19:19                                                       ` Andy Lutomirski
2014-04-17 19:10                                                 ` Simo Sorce
2014-04-17 19:16                                                   ` Vivek Goyal
2014-04-17 19:46                                                     ` Andy Lutomirski
2014-04-21 15:03                                                       ` Vivek Goyal
2014-04-21 15:47                                                         ` Andy Lutomirski
2014-04-23 15:07                                                           ` Vivek Goyal
2014-04-23 15:37                                                             ` Andy Lutomirski
2014-04-23 16:01                                                               ` Vivek Goyal
2014-04-17 19:23                                                 ` Andy Lutomirski
2014-04-17 17:52                                         ` Simo Sorce
2014-04-17 18:04                                           ` Andy Lutomirski
2014-04-17 18:31                                             ` Simo Sorce
2014-04-17 16:38                             ` Vivek Goyal
2014-04-17 16:05                         ` Andy Lutomirski
2014-04-17 16:12                         ` Vivek Goyal
2014-04-23 16:45         ` Vivek Goyal
2014-04-23 17:29           ` David Miller
2014-04-24 20:34             ` Vivek Goyal
2014-04-24 20:48               ` David Miller
2014-04-24 21:04                 ` Vivek Goyal
2014-04-24 21:11                   ` David Miller
2014-04-25  0:29                     ` Simo Sorce
2014-04-22 20:05 ` [PATCH 0/2] net: Implement SO_PEERCGROUP and SO_PASSCGROUP socket options David Miller
2014-04-22 20:08   ` Andy Lutomirski
2014-04-22 20:29     ` David Miller
2014-04-22 20:31       ` Andy Lutomirski
2014-04-22 20:32         ` David Miller
2014-04-23  0:37           ` Andy Lutomirski
2014-04-23 19:05         ` Vivek Goyal
2014-04-23 20:53           ` Daniel J Walsh
2014-04-24 13:01             ` Vivek Goyal
2014-04-23 15:55   ` Vivek Goyal
2014-04-23 16:16     ` Vivek Goyal
2014-04-23 17:21     ` Andy Lutomirski

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=20140416193918.GM31074@redhat.com \
    --to=vgoyal@redhat.com \
    --cc=cgroups@vger.kernel.org \
    --cc=davem@davemloft.net \
    --cc=dwalsh@redhat.com \
    --cc=kay@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lpoetter@redhat.com \
    --cc=luto@amacapital.net \
    --cc=netdev@vger.kernel.org \
    --cc=ssorce@redhat.com \
    --cc=tj@kernel.org \
    /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;
as well as URLs for NNTP newsgroup(s).