All of lore.kernel.org
 help / color / mirror / Atom feed
From: Luciano Coelho <luciano.coelho@nokia.com>
To: ext Jan Engelhardt <jengelh@medozas.de>
Cc: ext Changli Gao <xiaosuo@gmail.com>,
	"kaber@trash.net" <kaber@trash.net>,
	"netfilter-devel@vger.kernel.org"
	<netfilter-devel@vger.kernel.org>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	James Morris <jmorris@namei.org>,
	"linux-security-module@vger.kernel.org"
	<linux-security-module@vger.kernel.org>
Subject: Re: [PATCH] netfilter: xt_condition: add security capability support
Date: Wed, 25 Aug 2010 10:09:58 +0300	[thread overview]
Message-ID: <1282720198.18016.72.camel@powerslave> (raw)
In-Reply-To: <alpine.LNX.2.01.1008240921230.24349@obet.zrqbmnf.qr>

Hi Jan,

Thanks for your reply.


On Tue, 2010-08-24 at 09:32 +0200, ext Jan Engelhardt wrote:
> On Tuesday 2010-08-24 09:00, Luciano Coelho wrote:
> >>
> >>It is strange that you check this capability from a module focused
> >>on packet handling. For lack of a better example, it's as if you
> >>tried to check the uid of the file, the latter of which is better
> >>left to the routines in fs/.
> >
> >What I don't understand is that I see lots of components, which have
> >nothing to do with security, making this kind of checks. Most of
> >them (if not all) are checking for input from userspace where they
> >provide their own interfaces (eg. ioctl calls, netlink messages).
> >[..] Now, with the xt_condition, we're opening a new route from
> >userspace to the kernel and I think it might be a good idea to
> >protect that too.
> 
> Indeed so. But you did not invent any new interface. You are reusing
> files, which can be protected by DAC modes, or LSMs doing
> funky-stuff. xt_{condition,recent,..} already implement file modes,
> but does it check for it? Well no, because fs/namei.c does it for
> them. As for LSMs, well, I hope they do cater for testing for
> capability bits.

Thanks, I'll investigate all this a bit more and contact our security
people again.

I dug deeper into the code and I can see that /sys/net has capability
checks (implemented in netdev_store() in net-sysfs.c) and nobody without
CAP_NET_ADMIN will be able to write to the files there.  But in procfs I
couldn't see anything similar and anyone with file write permissions can
modify the files in /proc/net/*.


> >It's kind of useless to protect someone without CAP_NET_ADMIN from
> >creating a condition rule if it is possible to change the condition from
> >userspace without any capability protection.
> 
> Certainly not. An administrator may create a condition rule and thus
> procfs entry, but the rule may be sufficiently safe and/or integrated
> that a subordinate may be given permission to control things in a
> limited fashion on a per-need basis. One I use personally is
> 
> 	-A FORWARD -m condition --condition windows -s
> 	192.168.100.0/25 -i eth1 -o eth0 -j ACCEPT
> 	-P FORWARD DROP
> 
> 	chown jengelh /proc/net/nf_condition/windows;
> 
> The presence of such rule would indicate that the administrator
> generally allows Windows machines out; but personal paranoia defaults
> to rejecting that unless explicitly enabled. (Gives the reassurance
> that they won't succeed talking home unless Internet connectivity is
> explicitly needed by the user.)

I see, this is a good example. :) I know that his works (with simple DAC
control), but I think we're looking into a bit more security than that.
I have to discuss this with our platform security people.

Again, thanks for your explanations.


-- 
Cheers,
Luca.


  reply	other threads:[~2010-08-25  7:09 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-08-23 12:50 [PATCH] netfilter: xt_condition: add security capability support luciano.coelho
2010-08-23 13:37 ` Changli Gao
2010-08-23 13:42   ` Luciano Coelho
2010-08-23 14:34     ` Jan Engelhardt
2010-08-23 18:45       ` Luciano Coelho
2010-08-23 18:58         ` Jan Engelhardt
2010-08-24  7:00           ` Luciano Coelho
2010-08-24  7:32             ` Jan Engelhardt
2010-08-25  7:09               ` Luciano Coelho [this message]
2010-08-25  9:04                 ` Jan Engelhardt
2010-08-27  7:55                   ` Luciano Coelho
2010-09-15 20:14                     ` Patrick McHardy
2010-09-21 19:56                       ` Luciano Coelho

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=1282720198.18016.72.camel@powerslave \
    --to=luciano.coelho@nokia.com \
    --cc=jengelh@medozas.de \
    --cc=jmorris@namei.org \
    --cc=kaber@trash.net \
    --cc=linux-security-module@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=netfilter-devel@vger.kernel.org \
    --cc=xiaosuo@gmail.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.