iptables and iptables-restore syntaxical testing

iptables and iptables-restore syntaxical testing

[Herve Eychenne]

 Hi,

I just discovered iptables-restore -t today. It is exactly what I was
looking for, a mean to validate the iptables-save file format (possibly
generated by a tool like wallfire) without having to commit any changes
to the kernel.
If I only discovered it today, it's because I looked at the code: this
option was not documented in the manpage.
However, I think that the letter choosed for this option (-t) is not very
accurate:
- I'm also looking for a way to restore only a particular table, and I
  cannot think of an other option than
  # iptables-restore -t table
  to do that
- This kind of test option should (I need it) be transposed to iptables
  command as well (it's quite useful to test the syntax and the proper
  loading/availability of matches without applying real changes).
  And of course, iptables -t switch is already taken... It would be
  better for homogeneity if both commands had the same option letter.

What I would suggest is:
- implementing this test mechanism at a higher level (in iptc library) by
  adding a nocommit variable to the structure, and get iptables and
  iptables-restore to take advantage of it (I guess it's the only
  proper way to do it for iptables as libiptc itself already calls
  iptc_commit internally at several places, so preventing iptables.c to
  call iptc_commit would not be enough).
- adding a common switch to these two commands (iptables and
  iptables-restore), one that is not already taken by iptables, of
  course. Why not -S, --simulate ?

As far as the backward compability with iptables-restore is concerned,
I don't think turning -t/--test into -S/--simulate and adding -t/--table
would be very harmful, as I suspect the number of people using this
undocumented feature can be counted on the fingers of my hand.

Conclusion: if no one stands up and shouts against this proposal
within the next two days, expect a patch very soon.

 Herve

-- 
 _
(°=  Hervé Eychenne
//)
v_/_ WallFire project:  http://www.wallfire.org/

Re: iptables and iptables-restore syntaxical testing

[Henrik Nordstrom]

On Fri, 20 Aug 2004, Herve Eychenne wrote:

> I just discovered iptables-restore -t today. It is exactly what I was
> looking for, a mean to validate the iptables-save file format (possibly
> generated by a tool like wallfire) without having to commit any changes
> to the kernel.

Please note that this is not 100% true. It will still trigger loading of 
the specified table modules if not already loaded.

Meaning that if you run "iptables-restore -t" on a ruleset including a 
*nat table then iptable_nat will be loaded if it was not before.

> - I'm also looking for a way to restore only a particular table, and I
>  cannot think of an other option than
>  # iptables-restore -t table
>  to do that

Just limit your input to the table you want to test.

> - This kind of test option should (I need it) be transposed to iptables
>  command as well (it's quite useful to test the syntax and the proper
>  loading/availability of matches without applying real changes).
>  And of course, iptables -t switch is already taken... It would be
>  better for homogeneity if both commands had the same option letter.

Agreed.

Regards
Henrik

Re: iptables and iptables-restore syntaxical testing

[Herve Eychenne]

On Sat, Aug 21, 2004 at 02:09:55AM +0200, Henrik Nordstrom wrote:

> On Fri, 20 Aug 2004, Herve Eychenne wrote:

> >I just discovered iptables-restore -t today. It is exactly what I was
> >looking for, a mean to validate the iptables-save file format (possibly
> >generated by a tool like wallfire) without having to commit any changes
> >to the kernel.

> Please note that this is not 100% true. It will still trigger loading of 
> the specified table modules if not already loaded.

> Meaning that if you run "iptables-restore -t" on a ruleset including a 
> *nat table then iptable_nat will be loaded if it was not before.

I had already thought about this, and considered it not very harmful,
if documented.
But I had forgotten conntrack, which can be annoying, because of performance
penalty. You're absolutely right.
So the solution might be to track each kernel module insertion and unload
modules that were not inserted before on exit. A little heavy, but I
see no other way to test line validity as much as possible.
And we could even add an option that would test without inserting
kernel modules, if lighter testing is desired.

> >- I'm also looking for a way to restore only a particular table, and I
> > cannot think of an other option than
> > # iptables-restore -t table
> > to do that

> Just limit your input to the table you want to test.

Yes, but when you have a complete ruleset as a base, that requires
sed/awk/perl preprocessing. I agree that is not much of a problem by
itself, but if we can avoid that by adding only a very few lines of
code to iptables-restore, why not doing it?

 Herve

-- 
 _
(°=  Hervé Eychenne
//)
v_/_ WallFire project:  http://www.wallfire.org/

Re: iptables and iptables-restore syntaxical testing

[Martin Josefsson]

On Fri, 20 Aug 2004, Herve Eychenne wrote:

>  Hi,
>
> I just discovered iptables-restore -t today. It is exactly what I was
> looking for, a mean to validate the iptables-save file format (possibly
> generated by a tool like wallfire) without having to commit any changes
> to the kernel.

I also wanted it some time ago and saw that it was easy to implement in
iptables-restore so I did. I must have been asleep when deciding on the
switch... Worse... no one complained until now :)

http://cvs.netfilter.org/cgi-bin/viewcvs.cgi/iptables/iptables-restore.c?r2=1.33&r1=1.32&diff_format=u

> If I only discovered it today, it's because I looked at the code: this
> option was not documented in the manpage.

It's at least partially documented... in the usage help, -h, that
iptables-restore displays. I'm hopeless when it comes to updating
documentation :(

> However, I think that the letter choosed for this option (-t) is not very
> accurate:
> - I'm also looking for a way to restore only a particular table, and I
>   cannot think of an other option than
>   # iptables-restore -t table
>   to do that

This could be nice if you don't want to split the rules up into
several files before restoring.

> - This kind of test option should (I need it) be transposed to iptables
>   command as well (it's quite useful to test the syntax and the proper
>   loading/availability of matches without applying real changes).
>   And of course, iptables -t switch is already taken... It would be
>   better for homogeneity if both commands had the same option letter.

Yes iptables should have the same switch with the same meaning.

> What I would suggest is:
> - implementing this test mechanism at a higher level (in iptc library) by
>   adding a nocommit variable to the structure, and get iptables and
>   iptables-restore to take advantage of it (I guess it's the only
>   proper way to do it for iptables as libiptc itself already calls
>   iptc_commit internally at several places, so preventing iptables.c to
>   call iptc_commit would not be enough).
> - adding a common switch to these two commands (iptables and
>   iptables-restore), one that is not already taken by iptables, of
>   course. Why not -S, --simulate ?
>
> As far as the backward compability with iptables-restore is concerned,
> I don't think turning -t/--test into -S/--simulate and adding -t/--table
> would be very harmful, as I suspect the number of people using this
> undocumented feature can be counted on the fingers of my hand.

I don't think there's too many users of this feature, and the users that
might be using it are probably clever enough to find out that the switch
was badly named and has changed. So go ahead and change it, I think -S
sounds nice.

Not exactly all checking of parameters is done in libiptc + libipt_*
some things are checked in the ipt_* modules, mostly checks to see if the
values supplied by userspace are in the allowed range. (usually userspace
also checks this when parsing the options but I wouldn't rely on it)
Probably not very important but maybe it should be mentioned somewhere.

> Conclusion: if no one stands up and shouts against this proposal
> within the next two days, expect a patch very soon.

/Martin is waiting for the patch :)