2006-12-16 17:56:13

by Jan Engelhardt

[permalink] [raw]
Subject: [PATCH] xt_request_find_match

Hi,



Reusing code is a good idea, and I would like to do so from my
match modules. netfilter already provides a xt_request_find_target() but
an xt_request_find_match() does not yet exist. This patch adds it.

Objections welcome :)

---

Signed-off-by: Jan Engelhardt <[email protected]>

Index: linux-2.6.20-rc1/net/netfilter/x_tables.c
===================================================================
--- linux-2.6.20-rc1.orig/net/netfilter/x_tables.c
+++ linux-2.6.20-rc1/net/netfilter/x_tables.c
@@ -206,6 +206,19 @@ struct xt_match *xt_find_match(int af, c
}
EXPORT_SYMBOL(xt_find_match);

+struct xt_match *xt_request_find_match(int af, const char *name, u8 revision)
+{
+ struct xt_match *match;
+
+ match = try_then_request_module(xt_find_match(af, name, revision),
+ "%st_%s", xt_prefix[af], name);
+ if(IS_ERR(match) || match == NULL)
+ return NULL;
+
+ return match;
+}
+EXPORT_SYMBOL_GPL(xt_request_find_match);
+
/* Find target, grabs ref. Returns ERR_PTR() on error. */
struct xt_target *xt_find_target(int af, const char *name, u8 revision)
{
#<EOF>

-`J'
--


2006-12-19 12:10:19

by Patrick McHardy

[permalink] [raw]
Subject: Re: [PATCH] xt_request_find_match

Jan Engelhardt wrote:
> Reusing code is a good idea, and I would like to do so from my
> match modules. netfilter already provides a xt_request_find_target() but
> an xt_request_find_match() does not yet exist. This patch adds it.

Why does your match module needs to lookup other matches?

2006-12-19 13:08:10

by Jan Engelhardt

[permalink] [raw]
Subject: Re: [PATCH] xt_request_find_match


On Dec 19 2006 12:51, Patrick McHardy wrote:
>> Reusing code is a good idea, and I would like to do so from my
>> match modules. netfilter already provides a xt_request_find_target() but
>> an xt_request_find_match() does not yet exist. This patch adds it.
>
>Why does your match module needs to lookup other matches?

To use them?

I did not want to write


some_xt_target() {
if(skb->nh.iph->protocol == IPPROTO_TCP)
do_this();
else
do_that();
}

since the xt_tcpudp module provides far more checks than just the protocol
(TCP/UDP), like

/* To quote Alan:

Don't allow a fragment of TCP 8 bytes in. Nobody normal
causes this. Its a cracker trying to break in by doing a
flag overwrite to pass the direction checks.
*/

(see xt_tcpudp.c)



-`J'
--

2006-12-19 13:29:03

by Patrick McHardy

[permalink] [raw]
Subject: Re: [PATCH] xt_request_find_match

Jan Engelhardt wrote:
> On Dec 19 2006 12:51, Patrick McHardy wrote:
>
>>>Reusing code is a good idea, and I would like to do so from my
>>>match modules. netfilter already provides a xt_request_find_target() but
>>>an xt_request_find_match() does not yet exist. This patch adds it.
>>
>>Why does your match module needs to lookup other matches?
>
>
> To use them?
>
> I did not want to write
>
>
> some_xt_target() {
> if(skb->nh.iph->protocol == IPPROTO_TCP)
> do_this();
> else
> do_that();
> }

I don't think

xt_request_find_match(match->family, "tcp", 0)->match(lots of arguments)

is better than a simple comparison. Besides that the tcp match itself
expects that the protocol match already checked for IPPROTO_TCP, so
you'd still have to do it.

> since the xt_tcpudp module provides far more checks than just the protocol
> (TCP/UDP), like
>
> /* To quote Alan:
>
> Don't allow a fragment of TCP 8 bytes in. Nobody normal
> causes this. Its a cracker trying to break in by doing a
> flag overwrite to pass the direction checks.
> */

This check makes sure the flags are not overwritten _after you
matched on them_. It doesn't matter at all if you're only
interested in the protocol since the user didn't tell you to care.

2006-12-19 15:28:29

by Jan Engelhardt

[permalink] [raw]
Subject: Re: [PATCH] xt_request_find_match


>>>>Reusing code is a good idea, and I would like to do so from my
>>>>match modules. netfilter already provides a xt_request_find_target() but
>>>>an xt_request_find_match() does not yet exist. This patch adds it.
>>>
>>>Why does your match module needs to lookup other matches?
>>
>> To use them?
>>
>> I did not want to write
>>
>> some_xt_target() {
>> if(skb->nh.iph->protocol == IPPROTO_TCP)
>> do_this();
>> else
>> do_that();
>> }
>
>I don't think
>
>xt_request_find_match(match->family, "tcp", 0)->match(lots of arguments)
>
>is better than a simple comparison. Besides that the tcp match itself
>expects that the protocol match already checked for IPPROTO_TCP, so
>you'd still have to do it.
>> /* To quote Alan:
>>
>> Don't allow a fragment of TCP 8 bytes in. Nobody normal
>> causes this. Its a cracker trying to break in by doing a
>> flag overwrite to pass the direction checks.
>> */
>
>This check makes sure the flags are not overwritten _after you
>matched on them_. It doesn't matter at all if you're only
>interested in the protocol since the user didn't tell you to care.

Ok, but let's say I wanted to use a bigger match module (layer7, anyone?)
Then it's just not if(protocol == IPPROTO_TCP). What's the preferred solution
then?


-`J'
--

2006-12-20 08:16:57

by Patrick McHardy

[permalink] [raw]
Subject: Re: [PATCH] xt_request_find_match

Jan Engelhardt wrote:
> [...]
>
> Ok, but let's say I wanted to use a bigger match module (layer7, anyone?)
> Then it's just not if(protocol == IPPROTO_TCP). What's the preferred solution
> then?

Make sure the user specifies the match on the command line before
your match. Look at the TCPMSS or REJECT targets for examples for
this.

2006-12-20 09:12:20

by Jan Engelhardt

[permalink] [raw]
Subject: Re: [PATCH] xt_request_find_match


>Jan Engelhardt wrote:
>> [...]
>>
>> Ok, but let's say I wanted to use a bigger match module (layer7, anyone?)
>> Then it's just not if(protocol == IPPROTO_TCP). What's the preferred solution
>> then?
>
>Make sure the user specifies the match on the command line before
>your match. Look at the TCPMSS or REJECT targets for examples for
>this.

That would mean I'd have to

-p tcp -m multiport --dport 1,2,3,4 -m time --time sundays -m
lotsofothers -j TARGET
-p udp -m multiport --dport 1,2,3,4 -m time --time sundays -m
lotsofothers -j TARGET

which can become quite computationally expensive - which I wanted to
avoid.


-`J'
--

2006-12-20 09:17:11

by Patrick McHardy

[permalink] [raw]
Subject: Re: [PATCH] xt_request_find_match

Jan Engelhardt wrote:
>>Make sure the user specifies the match on the command line before
>>your match. Look at the TCPMSS or REJECT targets for examples for
>>this.
>
>
> That would mean I'd have to
>
> -p tcp -m multiport --dport 1,2,3,4 -m time --time sundays -m
> lotsofothers -j TARGET
> -p udp -m multiport --dport 1,2,3,4 -m time --time sundays -m
> lotsofothers -j TARGET

I don't see any match that would depend on an other match in
your example. How about your start explaining what you would
like to do, ideally with some code.

2006-12-23 21:57:56

by Jan Engelhardt

[permalink] [raw]
Subject: Re: xt_request_find_match


On Dec 20 2006 10:17, Patrick McHardy wrote:
>Jan Engelhardt wrote:
>>>Make sure the user specifies the match on the command line before
>>>your match. Look at the TCPMSS or REJECT targets for examples for
>>>this.
>>
>> That would mean I'd have to
>>
>> -p tcp -m multiport --dport 1,2,3,4 -m time --time sundays -m
>> lotsofothers -j TARGET
>> -p udp -m multiport --dport 1,2,3,4 -m time --time sundays -m
>> lotsofothers -j TARGET
>
>I don't see any match that would depend on an other match in
>your example. How about your start explaining what you would
>like to do, ideally with some code.

Yup, on the spot!
http://jengelh.hopto.org/f/chaostables/chaostables-0.1.tar.bz2
(Contains a target, but still something that could use
xt_request_find_module.)


-`J'
--