iptables-save counters on builtin chains not restored?

iptables-save counters on builtin chains not restored?

[Herve Eychenne]

 Hi,

Seems like I'm studying iptables-save too much tonight...

When fed with the result of iptables-save -c, iptables-restore -c
does not seem to restore counters on chains (I'm not talking about
rules), as I simply cannot find any parsing code for that.

Note that it would make sense only on builtin chains, but not
user-chains, because only builtin chains have a policy, and the
counters are about packets that hit the policy.

Anyway, it doesn't seem to be restored at all, and I suspect an
omission, so... a bug. Can someone confirm?

 Herve

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

Re: iptables-save counters on builtin chains not restored?

[Harald Welte]

[-- Attachment #1: Type: text/plain, Size: 1107 bytes --]

On Tue, Aug 17, 2004 at 11:18:21PM +0200, Herve Eychenne wrote:
 
> When fed with the result of iptables-save -c, iptables-restore -c
> does not seem to restore counters on chains (I'm not talking about
> rules), as I simply cannot find any parsing code for that.
> 
> Note that it would make sense only on builtin chains, but not
> user-chains, because only builtin chains have a policy, and the
> counters are about packets that hit the policy.
> 
> Anyway, it doesn't seem to be restored at all, and I suspect an
> omission, so... a bug. Can someone confirm?

Yes, now that you say it, I don't remember having written that code ;)

Please put it in bugzilla... and patches are obviously always welcome.

>  Herve

-- 
- Harald Welte <laforge@netfilter.org>             http://www.netfilter.org/
============================================================================
  "Fragmentation is like classful addressing -- an interesting early
   architectural error that shows how much experimentation was going
   on while IP was being designed."                    -- Paul Vixie

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: iptables-save counters on builtin chains not restored?

[Herve Eychenne]

On Thu, Aug 19, 2004 at 12:13:14PM +0200, Harald Welte wrote:

> On Tue, Aug 17, 2004 at 11:18:21PM +0200, Herve Eychenne wrote:

> > When fed with the result of iptables-save -c, iptables-restore -c
> > does not seem to restore counters on chains (I'm not talking about
> > rules), as I simply cannot find any parsing code for that.
> > 
> > Note that it would make sense only on builtin chains, but not
> > user-chains, because only builtin chains have a policy, and the
> > counters are about packets that hit the policy.
> > 
> > Anyway, it doesn't seem to be restored at all, and I suspect an
> > omission, so... a bug. Can someone confirm?

> Yes, now that you say it, I don't remember having written that code ;)

Did you ask your pet as well? ;-)

> Please put it in bugzilla... and patches are obviously always welcome.

I'm currently writing it, at least partly:
- for now iptables-save (with or without -c) used to dump counters for
  builtin-chains, which is wrong (useless when not called with -c).
  I'll fix that.
- iptables-save (also with or without -c) used to dump dummy counters
  (always [0:0]) for user-chains, which is also wrong (never needed,
  as it makes no sense for user-chains, right?). I'll fix that too.

The side effect of this change will be that dump files created by new
iptables-save command (without -c) won't be restorable with old
iptables-restore (without -c). But i think it's acceptable, as:
- people should not want to do that, as they should use
  iptables-restore.new, then
- if people really have to use iptables-restore.old, they can use
  iptables-save.new dumps, but with -c
- a very simple sed line fixes that

One thing that puzzles me is that old iptables-restore -c used to
restore old iptables-save (without -c) dumps without any complaints
about missing counters (for rules, as counters for builtin-chains were
dumped anyway).
So I guess new iptables-restore -c should act likewise, that is
restore new iptables-save dumps (without -c) without error, but shouldn't
it at least issue a warning about the lack of the expected counters?

Thanks for commenting everything above.

 Herve

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

Re: iptables-save counters on builtin chains not restored?

[Herve Eychenne]

On Fri, Aug 20, 2004 at 04:36:17PM +0200, Herve Eychenne wrote:

> On Thu, Aug 19, 2004 at 12:13:14PM +0200, Harald Welte wrote:

> > Please put it in bugzilla... and patches are obviously always welcome.

> I'm currently writing it, at least partly:
> - for now iptables-save (with or without -c) used to dump counters for
>   builtin-chains, which is wrong (useless when not called with -c).
>   I'll fix that.
> - iptables-save (also with or without -c) used to dump dummy counters
>   (always [0:0]) for user-chains, which is also wrong (never needed,
>   as it makes no sense for user-chains, right?). I'll fix that too.

> The side effect of this change will be that dump files created by new
> iptables-save command (without -c) won't be restorable with old
> iptables-restore (without -c).

Sorry... you should have read:
dump files created by new iptables-save command (without -c) won't be
restorable with old iptables-restore -c
So, 
# iptables-save.new | iptables-restore.old
works well. That's even less harmful.

> But i think it's acceptable, as:
> - people should not want to do that, as they should use
>   iptables-restore.new, then
> - if people really have to use iptables-restore.old, they can use
>   iptables-save.new dumps, but with -c
> - a very simple sed line fixes that

> One thing that puzzles me is that old iptables-restore -c used to
> restore old iptables-save (without -c) dumps without any complaints
> about missing counters (for rules, as counters for builtin-chains were
> dumped anyway).
> So I guess new iptables-restore -c should act likewise, that is
> restore new iptables-save dumps (without -c) without error, but shouldn't
> it at least issue a warning about the lack of the expected counters?

> Thanks for commenting everything above.

 Herve

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

Re: iptables-save counters on builtin chains not restored?

[Henrik Nordstrom]

On Fri, 20 Aug 2004, Herve Eychenne wrote:

> The side effect of this change will be that dump files created by new
> iptables-save command (without -c) won't be restorable with old
> iptables-restore (without -c).

Are you sure? From what I can see this should work. I never specify 
counters in my iptables-restore input from what I can tell.. not on 
policies, not in rules. And it works just fine.

iptables-restore -c without having counters specified in policies or 
custom chains complains, but I never use this so I don't care, and what 
would the point be?

What would break is reading in output from new iptables-save (with or 
without -c) using old iptables-restore -c.

> One thing that puzzles me is that old iptables-restore -c used to
> restore old iptables-save (without -c) dumps without any complaints
> about missing counters (for rules, as counters for builtin-chains were
> dumped anyway).

This is due to the parser nor caring if -c is specified or not when 
parsing the rules. It always parse the counters if there is any (in_table 
block), it then discards the result if -c is not specified.

> So I guess new iptables-restore -c should act likewise, that is
> restore new iptables-save dumps (without -c) without error, but shouldn't
> it at least issue a warning about the lack of the expected counters?

Why? Is there a problem with reading a missing counter as an implicit 
zero?

Regards
Henrik

Re: iptables-save counters on builtin chains not restored?

[Herve Eychenne]

On Sat, Aug 21, 2004 at 02:01:44AM +0200, Henrik Nordstrom wrote:

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

> >The side effect of this change will be that dump files created by new
> >iptables-save command (without -c) won't be restorable with old
> >iptables-restore (without -c).

> Are you sure? From what I can see this should work. I never specify 
> counters in my iptables-restore input from what I can tell.. not on 
> policies, not in rules. And it works just fine.

Probably you missed my rectification (sorry again) :
dump files created by new iptables-save command (without -c) won't be
restorable with old iptables-restore -c
                                    ^^^^
So there is no problem for the case you describe, right.

> iptables-restore -c without having counters specified in policies or 
> custom chains complains,

According to me, it does not only complain, it crashes, because we
did not check the return of strtok and tried to parse counters whether
they are actually present or not. I fixed it already in my tree
(patch will be sent soon).

> but I never use this so I don't care, and what would the point be?

> What would break is reading in output from new iptables-save (with or 
> without -c) using old iptables-restore -c.

Yes.

> >One thing that puzzles me is that old iptables-restore -c used to
> >restore old iptables-save (without -c) dumps without any complaints
> >about missing counters (for rules, as counters for builtin-chains were
> >dumped anyway).

> This is due to the parser nor caring if -c is specified or not when 
> parsing the rules. It always parse the counters if there is any (in_table 
> block), it then discards the result if -c is not specified.

I'm talking about chain counters (not rules), and IIRC these counters
were only parsed if -c was specified (memset of the struct to 0 otherwise).
Like I said, the issue was more that counters were tried to be parsed
even if they were lacking (->coredump).
But there is not much point in discussing this very longer... :-)

> >So I guess new iptables-restore -c should act likewise, that is
> >restore new iptables-save dumps (without -c) without error, but shouldn't
> >it at least issue a warning about the lack of the expected counters?

> Why? Is there a problem with reading a missing counter as an implicit 
> zero?

It depends on your personnal policies. When I specify something like -c,
it is hardly by chance, and I tend to consider that trying (on purpose)
to restore something that actually does not exist is at least strange.
So I would personally go for a warning message (median solution),
but that's only a matter of how rigorous you expect the tools to be.

 Herve

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