Subject: [patch] to add device+inode check to ipt_owner.c - HACKED UP

dear kernel people,

this is a first pass at attempting to add per-program firewall rule
checking to iptables.

fireflier makes a complete hash of this in user-space because there
is no way it track state information (therefore it cannot track RST
and FIN and FIN ACK packets)

so, i figured i'd try do this in kernel space instead, where it
should be done.

digitally signing the name of the exe is not okay.
_using_ the name of the exe isn't really okay in my book, either.

inodes are only meaningful on a per-device basis.

therefore i add a device (80 chars as a hack) and an inode number
argument to ipt_owner.h

i blatantly cut/paste the code from fs/proc/base.c into ipt_owner.c
only to find that a compile resulted in warnings about mmput
and mmget (present in kernel/sched.c) not existing.

therefore i blatantly hacked fs/proc/base.c to expose the functionality
i required, which is, "given a task struct, give me the dentry and
vfsmount structs associated with its executable". i called it
proc_task_dentry_lookup().

from there, i can grab the inode number and the device name.

i had to add an EXPORT_SYMBOL(proc_task_dentry_lookup).
i realise this is not ideal (it shouldn't be in fs/proc/base.c,
ipt_owner.c shouldn't be dependent on CONFIG_PROC_FS, etc. at
this stage i don't care)

i feel certain that the functionality required in this function
is not only required elsewhere but also available elsewhere
(in which case, why is it being duplicated in fs/proc/base.c)
so clearly i must be missing something.


from previous messages:

- default rules should be "DENY ALL" and invididual "ALLOW"s this patch
is NOT good to use with "DENY" rules because users can always copy the
program (even on selinux systems)



anyone reading this far be warned of the following:

- if you don't like what i have done, i so don't care: i HAVE to
get this to work and no amount of "i don't like" is going to
take that away. don't like it? save yourself some effort:
delete this message and your mindset.

- if you do like what i have done, and you are not an experienced
kernel hacker, please be aware that i am still experimenting,
i am very much in the dark on this, and your input and
assistance would be greatly appreciated, but you NEED to take
precautions to ensure it doesn't do your system any damage.

- you'll also need a hacked version of userspace iptables.
i'm working on it.

with that in mind, comments and assistance much appreciated because
i feel certain that i cannot be the only person looking for this
sort of functionality.

l.

--
--
Truth, honesty and respect are rare commodities that all spring from
the same well: Love. If you love yourself and everyone and everything
around you, funnily and coincidentally enough, life gets a lot better.
--
<a href="http://lkcl.net"> lkcl.net </a> <br />
<a href="mailto:[email protected]"> [email protected] </a> <br />


2004-09-08 10:14:58

by Arjan van de Ven

[permalink] [raw]
Subject: Re: [patch] to add device+inode check to ipt_owner.c - HACKED UP

On Wed, 2004-09-08 at 12:09, Luke Kenneth Casson Leighton wrote:
> dear kernel people,
>
> this is a first pass at attempting to add per-program firewall rule
> checking to iptables.

question: any reason you didn't use something like selinux-like contexts
instead of dentry/device pairs ?


Attachments:
signature.asc (189.00 B)
This is a digitally signed message part
Subject: Re: [patch] to add device+inode check to ipt_owner.c - HACKED UP

... did i sent a patch?

did i send a patch?? i don't _think_ so *lol* :)

On Wed, Sep 08, 2004 at 11:09:47AM +0100, Luke Kenneth Casson Leighton wrote:
> dear kernel people,
>
> this is a first pass at attempting to add per-program firewall rule
> checking to iptables.


Attachments:
(No filename) (273.00 B)
ipt_owner.patch (6.34 kB)
Download all attachments

2004-09-08 10:47:41

by Al Viro

[permalink] [raw]
Subject: Re: [patch] to add device+inode check to ipt_owner.c - HACKED UP

On Wed, Sep 08, 2004 at 11:39:22AM +0100, Luke Kenneth Casson Leighton wrote:
> +static int
> +match_inode(const struct sk_buff *skb, const char *devname, unsigned long i_num)
> +{
> + struct task_struct *g, *p;
> + struct files_struct *files;
> + /*
> + struct inode *inode;
> + struct super_block *sb;
> + struct block_device *bd;
> + */
> + int i;
> + read_lock(&tasklist_lock);
> +
> + /* lkcl: these are fairly obvious (just obtuse): hunt for the
> + * filesystem device, then its superblock, then the inode is
> + * relevant to that superblock, _then_ we can find the inode.
> + bd = bdget(dev);


... the hell? Where does that "dev" come from?

Subject: Re: [patch] to add device+inode check to ipt_owner.c - HACKED UP

On Wed, Sep 08, 2004 at 12:14:50PM +0200, Arjan van de Ven wrote:
> On Wed, 2004-09-08 at 12:09, Luke Kenneth Casson Leighton wrote:
> > dear kernel people,
> >
> > this is a first pass at attempting to add per-program firewall rule
> > checking to iptables.
>
> question: any reason you didn't use something like selinux-like contexts
> instead of dentry/device pairs ?

a very good question: stephen smalley described an approach in which
exactly what you suggest can be done.

please bear with me whilst i explain, then i will answer.

the issue is that FireFlier is an on-demand (user-driven) popup firewall
program [and there literally ISN'T any firewall program available for
linux that even remotely comes close to the same capabilities as
fireflier]

so rules are queued (ipt_queue) and the popup thrown at the user until
they select "yes, no, create-a-firewall-rule".

to parallel the same functionality i would need to place a hook in
selinux to catch an audit operation (hooks are already there), then
alert the user to it, then create a rule, recompile the policy, and
_then_ let the hook proceed.

i'm not sure if this would work!!!

so, i didn't want to use selinux contexts because it involves
dynamically creating selinux policy rules.

fireflier is NOT a "create-it-once-then-apply-it-suck-it-and-see"
firewall program.

it's an on-demand "popup" firewall program where the default is
"block by virtue of the packet being in the ip_queue, awaiting
user approval or disapproval".


unless... *shudder* ... you mean ... why didn't i consider getting
FireFlier to _create_ selinux contexts, blatting them into the policy
directly? (which i know is possible, there do exist binary policy
editing-and-writing tools).

well... if this approach turns out to be a total nightmare, then
your question is really appreciated because it makes me think of
other possibilities.

l.


--
--
Truth, honesty and respect are rare commodities that all spring from
the same well: Love. If you love yourself and everyone and everything
around you, funnily and coincidentally enough, life gets a lot better.
--
<a href="http://lkcl.net"> lkcl.net </a> <br />
<a href="mailto:[email protected]"> [email protected] </a> <br />

Subject: Re: [patch] to add device+inode check to ipt_owner.c - HACKED UP

hello, hello, thank you v. much for responding.

On Wed, Sep 08, 2004 at 11:47:39AM +0100, [email protected] wrote:

> On Wed, Sep 08, 2004 at 11:39:22AM +0100, Luke Kenneth Casson Leighton wrote:
> > +static int
> > +match_inode(const struct sk_buff *skb, const char *devname, unsigned long i_num)
> > +{
> > + struct task_struct *g, *p;
> > + struct files_struct *files;
> > + /*
> > + struct inode *inode;
> > + struct super_block *sb;
> > + struct block_device *bd;
> > + */
> > + int i;
> > + read_lock(&tasklist_lock);
> > +
> > + /* lkcl: these are fairly obvious (just obtuse): hunt for the
> > + * filesystem device, then its superblock, then the inode is
> > + * relevant to that superblock, _then_ we can find the inode.
> > + bd = bdget(dev);
>
>
> ... the hell? Where does that "dev" come from?

it's code commented out that i put in there when i _really_ wasn't
sure what i was doing.

it might come in handy so i haven't deleted it yet.

basically in an earlier experiment, i put the device (dev_t)
major/minor number into ipt_owner.h.

then i discovered that fs/proc/base.c could look up a vfsmount
struct, which according to struct vfsmount contains the _name_
of the device.

i'm counting on that actually working.

if it doesn't work, then it's back to the drawing board and uncommenting
the stuff that you have noticed is all commented out, above.

l.


--
--
Truth, honesty and respect are rare commodities that all spring from
the same well: Love. If you love yourself and everyone and everything
around you, funnily and coincidentally enough, life gets a lot better.
--
<a href="http://lkcl.net"> lkcl.net </a> <br />
<a href="mailto:[email protected]"> [email protected] </a> <br />

2004-09-10 07:49:53

by Gianni Tedesco

[permalink] [raw]
Subject: Re: [patch] to add device+inode check to ipt_owner.c - HACKED UP

On Wed, 2004-09-08 at 11:39 +0100, Luke Kenneth Casson Leighton wrote:
> ... did i sent a patch?
>
> did i send a patch?? i don't _think_ so *lol* :)

heh :)

IMO the number of constraints involed here make using this patch fairly
involved (for something security related at least) in that, as you said,
you have to:

- be careful to use ACCEPT rules only
- be careful that you do:
1. remove fw rules
2. upgrade software
3. replace rules

plus the fastpath code looks very hairy with at least 3 locks taken and
O(num_tasks * max_fds) unpreemptable in softirq...

There has to be a simpler approach, perhaps passing in a path, looking
up the dentry in current namespace and setting an unused flag in
d_vfs_flags? That way you could just match on skb->sk->sk_socket->file-
>f_dentry.

I don't know enough about VFS to know if that's really possible. I mean
would you need vfsmnt too to make it accurate across namespaces? and if
so, could dcookie infrastructure be used?

--
// Gianni Tedesco (gianni at scaramanga dot co dot uk)
lynx --source http://www.scaramanga.co.uk/scaramanga.asc | gpg --import
8646BE7D: 6D9F 2287 870E A2C9 8F60 3A3C 91B5 7669 8646 BE7D

Subject: Re: [patch] to add device+inode check to ipt_owner.c - HACKED UP

On Fri, Sep 10, 2004 at 08:49:28AM +0100, Gianni Tedesco wrote:
> On Wed, 2004-09-08 at 11:39 +0100, Luke Kenneth Casson Leighton wrote:
> > ... did i sent a patch?
> >
> > did i send a patch?? i don't _think_ so *lol* :)
>
> heh :)
>
> IMO the number of constraints involed here make using this patch fairly
> involved (for something security related at least) in that, as you said,
> you have to:
>
> - be careful to use ACCEPT rules only
> - be careful that you do:
> 1. remove fw rules
> 2. upgrade software
> 3. replace rules

i've since replaced the code with use of d_path() which stephen
smalley kindly pointed out, negating the need for doing a
commit-replace-thing.

> plus the fastpath code looks very hairy with at least 3 locks taken and
> O(num_tasks * max_fds) unpreemptable in softirq...

*shrug*. that's the way ipt_owner works...

> There has to be a simpler approach, perhaps passing in a path, looking
> up the dentry in current namespace and setting an unused flag in
> d_vfs_flags? That way you could just match on skb->sk->sk_socket->file-
> >f_dentry.

ooo...

> I don't know enough about VFS to know if that's really possible. I mean
> would you need vfsmnt too to make it accurate across namespaces?


well you need vfsmnt in order to trackdown the full path name.

l.

--
--
Truth, honesty and respect are rare commodities that all spring from
the same well: Love. If you love yourself and everyone and everything
around you, funnily and coincidentally enough, life gets a lot better.
--
<a href="http://lkcl.net"> lkcl.net </a> <br />
<a href="mailto:[email protected]"> [email protected] </a> <br />

Subject: Re: [patch] to add device+inode check to ipt_owner.c - HACKED UP

On Fri, Sep 10, 2004 at 08:49:28AM +0100, Gianni Tedesco wrote:
> On Wed, 2004-09-08 at 11:39 +0100, Luke Kenneth Casson Leighton wrote:
> > ... did i sent a patch?
> >
> > did i send a patch?? i don't _think_ so *lol* :)
>
> heh :)
>
> IMO the number of constraints involed here make using this patch fairly
> involved (for something security related at least) in that, as you said,
> you have to:
>
> - be careful to use ACCEPT rules only
> - be careful that you do:
> 1. remove fw rules
> 2. upgrade software
> 3. replace rules
>
> plus the fastpath code looks very hairy with at least 3 locks taken and
> O(num_tasks * max_fds) unpreemptable in softirq...

it's no worse than the present fireflier solution, which on a
per-packet basis in userspace will go hunting through /proc
looking for the socket _that_ way *gibber*.

fireflier reads /proc/NNNN/exe, then also hunts through the fds for
that process on /proc looking for things beginning with "socket:".

[actually it used not to bother with the qualification for "socket:"
resulting in a complete nightmare time for creating appropriate
selinux policy]

l.