[isf-wifidog] Bug in fw_iptables.c

[Philippe April]

Hi!

It's been a while now since I've checked code into Wifidog, but I  
think I found a bug, which I'd like others to confirm.

Revision 1241 introduced the bug. The line:

             rc = iptables_do_command("-t mangle -D "  
TABLE_WIFIDOG_INCOMING " -d %s -j DROP", ip);

should read

             rc = iptables_do_command("-t mangle -D "  
TABLE_WIFIDOG_INCOMING " -d %s -j ACCEPT", ip);

It's in the case statement "FW_ACCESS_DENY", which is run when we  
deny the connection (called from wifidog, or wdctl reset).

The line in mangle, with destination "ip of client" with target  
ACCEPT is just there to count the traffic, it's established in  
FW_ACCESS_ALLOW:

             rc = iptables_do_command("-t mangle -A "  
TABLE_WIFIDOG_INCOMING " -d %s -j ACCEPT", ip);

So basically the -D in FW_ACCESS_DENY just "deletes" the "ACCEPT line".

I would personally suggest releasing a 1.1.4... It leaves entries in  
mangle until wifidog is restarted (it clears all chains it created).  
More importantly, it messes up the stats for a user who logs back in  
during the same wifidog process. To reproduce:

Log in
do something, wait until the stats get sent
Kick yourself out (wdctl reset <your_ip>)
Log back in
check your stats, the "incoming" (download) will be at the same  
amount as before.

Why is that? It's because wifidog gets the counters from running  
"iptables", and I assume it stops on the first match for your IP  
address from iptables -v -t mangle -L. Because you now have 2  
entries, the first line (the old connection) matches first.

Benoit, should I just commit the code once you confirm?

Philippe April