Ian! D. Allen
2004-Jul-15 12:30 UTC
[Shorewall-devel] slight simplification to firewall log_rule_limit code
I think you can change the existing firewall logging code for log_rule_limit (where you have one case for for LOGRULENUMBERS and another almost identical case without) down to this slightly shorter version with no duplication (excerpt): if [ -n "$LOGRULENUMBERS" ]; then eval rulenum=\$${chain}_logrules [ -z "$rulenum" ] && rulenum=1 fi case $level in ULOG) log=ulog LOGTYPE=ULOG loglevel= ;; *) log=log LOGTYPE=LOG loglevel="--log-level $level" ;; esac eval iptables -A $chain $@ $limit -j $LOGTYPE $LOGPARMS \ $loglevel \ --${log}-prefix ''"$(Logprintf "$LOGFORMAT" $chain $rulenum $disposition)"'' if [ $? -ne 0 ] ; then [ -z "$stopping" ] && { stop_firewall; exit 2; } fi if [ -n "$LOGRULENUMBERS" ]; then rulenum=$(($rulenum + 1)) eval ${chain}_logrules=$rulenum fi The $rulenum variable simply expands to nothing if LOGRULENUMBERS is not enabled; no need to remove it explicitly. We eliminate the other duplicate rules by setting three variables depending on ULOG or not. (We are assuming that $chain, $rulenum, and $disposition and most of the other unquoted variables will never contain unexpected blanks or shell GLOB characters...) What was four iptables statements in 48 lines is now one in 33 lines. -- -IAN! Ian! D. Allen Ottawa, Ontario, Canada EMail: idallen@idallen.ca WWW: http://www.idallen.com/ College professor via: http://teaching.idallen.com/ Support free and open public digital rights: http://eff.org/
Tom Eastep
2004-Jul-15 13:00 UTC
[Shorewall-devel] slight simplification to firewall log_rule_limit code
Ian! D. Allen wrote:> I think you can change the existing firewall logging code for > log_rule_limit (where you have one case for for LOGRULENUMBERS and > another almost identical case without) down to this slightly shorter > version with no duplication (excerpt): > > if [ -n "$LOGRULENUMBERS" ]; then > eval rulenum=\$${chain}_logrules > [ -z "$rulenum" ] && rulenum=1 > fi > > case $level in > ULOG) log=ulog LOGTYPE=ULOG loglevel= ;; > *) log=log LOGTYPE=LOG loglevel="--log-level $level" ;; > esac > > eval iptables -A $chain $@ $limit -j $LOGTYPE $LOGPARMS \ > $loglevel \ > --${log}-prefix ''"$(Logprintf "$LOGFORMAT" $chain $rulenum $disposition)"'' > > if [ $? -ne 0 ] ; then > [ -z "$stopping" ] && { stop_firewall; exit 2; } > fi > > if [ -n "$LOGRULENUMBERS" ]; then > rulenum=$(($rulenum + 1)) > eval ${chain}_logrules=$rulenum > fi > > The $rulenum variable simply expands to nothing if LOGRULENUMBERS is > not enabled; no need to remove it explicitly. We eliminate the other > duplicate rules by setting three variables depending on ULOG or not. > (We are assuming that $chain, $rulenum, and $disposition and most of the > other unquoted variables will never contain unexpected blanks or shell > GLOB characters...) > > What was four iptables statements in 48 lines is now one in 33 lines. >Tested patch? (in unified format) -Tom -- Tom Eastep \ Nothing is foolproof to a sufficiently talented fool Shoreline, \ http://shorewall.net Washington USA \ teastep@shorewall.net
Tom Eastep
2004-Jul-15 13:08 UTC
[Shorewall-devel] slight simplification to firewall log_rule_limit code
Tom Eastep wrote:> Ian! D. Allen wrote: > >> I think you can change the existing firewall logging code for >> log_rule_limit (where you have one case for for LOGRULENUMBERS and >> another almost identical case without) down to this slightly shorter >> version with no duplication (excerpt): >> >> if [ -n "$LOGRULENUMBERS" ]; then >> eval rulenum=\$${chain}_logrules >> [ -z "$rulenum" ] && rulenum=1 >> fi >> >> case $level in >> ULOG) log=ulog LOGTYPE=ULOG loglevel= ;; >> *) log=log LOGTYPE=LOG loglevel="--log-level $level" ;; >> esac >> >> eval iptables -A $chain $@ $limit -j $LOGTYPE $LOGPARMS \ >> $loglevel \ >> --${log}-prefix ''"$(Logprintf "$LOGFORMAT" $chain $rulenum >> $disposition)"'' >> >> if [ $? -ne 0 ] ; then >> [ -z "$stopping" ] && { stop_firewall; exit 2; } >> fi >> >> if [ -n "$LOGRULENUMBERS" ]; then >> rulenum=$(($rulenum + 1)) >> eval ${chain}_logrules=$rulenum >> fi >> >> The $rulenum variable simply expands to nothing if LOGRULENUMBERS is >> not enabled; no need to remove it explicitly. We eliminate the other >> duplicate rules by setting three variables depending on ULOG or not. >> (We are assuming that $chain, $rulenum, and $disposition and most of the >> other unquoted variables will never contain unexpected blanks or shell >> GLOB characters...) >> >> What was four iptables statements in 48 lines is now one in 33 lines. >> > > Tested patch? (in unified format)Hmmm -- which version of Shorewall are you looking at? The above code deimplements log tags if applied to Shorewall 2.0.2 or later. Given that 2.0 is the stable release, any changes besides bug fixes need to go into the 2.1 release any way (you can always get a snapshot from CVS). -Tom -- Tom Eastep \ Nothing is foolproof to a sufficiently talented fool Shoreline, \ http://shorewall.net Washington USA \ teastep@shorewall.net
Ian! D. Allen
2004-Jul-15 17:58 UTC
[Shorewall-devel] slight simplification to firewall log_rule_limit code
On Thu, Jul 15, 2004 at 01:08:51PM -0700, Tom Eastep wrote:> Hmmm -- which version of Shorewall are you looking at?Oh my I see you''ve done lots since the 2.0.1 version I got with Mandrake 10. I''ll make sure to get the CVS version in future before making comments on the code - the suggested simplifications I made are largely irrelevant to the 2.1 version (and you also corrected the 29 character prefix problem). -- -IAN! Ian! D. Allen Ottawa, Ontario, Canada EMail: idallen@idallen.ca WWW: http://www.idallen.com/ College professor via: http://teaching.idallen.com/ Support free and open public digital rights: http://eff.org/