2004-03-05 01:06:08

by Matthew Strait

[permalink] [raw]
Subject: [PATCH] matching any helper in ipt_helper.c

> > It seems like I'd have to make significantly more invasive changes than
> > are really called for to get it to accept an empty string. What do you
> > think?
>
> You just need to remove the check for empty strings in ipt_helper.c:
>
> /* verify that we actually should match anything */
> if ( strlen(info->name) == 0 )
> return 0;

Silly me, I assumed that the error was generated in user space. Ok. In
that case, let's forget translating "any" to "", because that just makes
the output of "iptables -L" confusing. Sound good?

Kernel patch:

--- ipt_helper.c.old 2004-03-03 21:34:05.000000000 -0600
+++ ipt_helper.c 2004-03-04 18:38:32.234521176 -0600
@@ -68,8 +68,11 @@
DEBUGP("master's name = %s , info->name = %s\n",
exp->expectant->helper->name, info->name);

- ret = !strncmp(exp->expectant->helper->name, info->name,
- strlen(exp->expectant->helper->name)) ^ info->invert;
+ if(info->name[0] == '\0') /* special case meaning "any" */
+ ret = !info->invert;
+ else
+ ret = !strncmp(exp->expectant->helper->name, info->name,
+ strlen(exp->expectant->helper->name)) ^ info->invert;
out_unlock:
READ_UNLOCK(&ip_conntrack_lock);
return ret;
@@ -89,10 +92,6 @@
if (matchsize != IPT_ALIGN(sizeof(struct ipt_helper_info)))
return 0;

- /* verify that we actually should match anything */
- if ( strlen(info->name) == 0 )
- return 0;
-
return 1;
}




And documentational changes in iptables:

--- libipt_helper.c.old 2004-03-03 21:39:07.000000000 -0600
+++ libipt_helper.c 2004-03-04 18:31:54.156038304 -0600
@@ -15,6 +15,7 @@
printf(
"helper match v%s options:\n"
"[!] --helper string Match helper identified by string\n"
+" (or any helper if string is \"\")"
"\n",
IPTABLES_VERSION);
}


--- iptables.8.old 2004-03-04 18:35:11.994962216 -0600
+++ iptables.8 2004-03-04 18:34:38.263090240 -0600
@@ -458,6 +458,8 @@
For other ports append -portnr to the value, ie. "ftp-2121".
.PP
Same rules apply for other conntrack-helpers.
+.PP
+If string is "", it will match any packet related to a conntrack-helper.
.RE
.SS icmp
This extension is loaded if `--protocol icmp' is specified. It


2004-03-05 02:18:55

by Patrick McHardy

[permalink] [raw]
Subject: Re: [PATCH] matching any helper in ipt_helper.c

Matthew Strait wrote:
> Silly me, I assumed that the error was generated in user space. Ok. In
> that case, let's forget translating "any" to "", because that just makes
> the output of "iptables -L" confusing. Sound good?
>

I actually meant translate in both direction. But no problem, I'm going
to make a patch for iptables myself, if Martin is fine with it we can
remove the childlevel match.

Thanks.

Patrick

2004-03-05 09:23:40

by Martin Josefsson

[permalink] [raw]
Subject: Re: [PATCH] matching any helper in ipt_helper.c

On Fri, 5 Mar 2004, Patrick McHardy wrote:

> Matthew Strait wrote:
> > Silly me, I assumed that the error was generated in user space. Ok. In
> > that case, let's forget translating "any" to "", because that just makes
> > the output of "iptables -L" confusing. Sound good?
> >
>
> I actually meant translate in both direction. But no problem, I'm going
> to make a patch for iptables myself, if Martin is fine with it we can
> remove the childlevel match.

I'm fine with making ipt_helper able to match any helper if so desired.

/Martin