From 3cfd19c79b7df05bbffe31e11f31fe36451b8118 Mon Sep 17 00:00:00 2001 From: Vadim Kurland Date: Tue, 27 Apr 2010 16:58:05 +0000 Subject: [PATCH] * PolicyCompiler_ipt.cpp (PolicyCompiler_ipt::checkForShadowingPlatformSpecific): see #1417 (SF bug 2992177) rule with greater limit module rate value shadows rule with lower rate value. Comments in the code explain why. --- build_num | 2 +- doc/ChangeLog | 7 + src/iptlib/PolicyCompiler_ipt.cpp | 35 ++-- test/ipt/objects-for-regression-tests.fwb | 196 +++++++++++----------- 4 files changed, 127 insertions(+), 113 deletions(-) diff --git a/build_num b/build_num index 87b4cd631..0622f0669 100644 --- a/build_num +++ b/build_num @@ -1 +1 @@ -#define BUILD_NUM 2847 +#define BUILD_NUM 2848 diff --git a/doc/ChangeLog b/doc/ChangeLog index 935d1eb64..496831c70 100644 --- a/doc/ChangeLog +++ b/doc/ChangeLog @@ -1,3 +1,10 @@ +2010-04-27 vadim + + * PolicyCompiler_ipt.cpp (PolicyCompiler_ipt::checkForShadowingPlatformSpecific): + see #1417 (SF bug 2992177) rule with greater limit module rate + value shadows rule with lower rate value. Comments in the code explain + why. + 2010-04-26 vadim * PolicyCompiler_ipt.cpp (PolicyCompiler_ipt::checkForShadowingPlatformSpecific): diff --git a/src/iptlib/PolicyCompiler_ipt.cpp b/src/iptlib/PolicyCompiler_ipt.cpp index 192a78562..067e6fffb 100644 --- a/src/iptlib/PolicyCompiler_ipt.cpp +++ b/src/iptlib/PolicyCompiler_ipt.cpp @@ -5151,16 +5151,17 @@ list PolicyCompiler_ipt::getUsedChains() * rule with rate "-1" (i.e. no rate limiting at all) shadows rule with * rate > 0 * OR - * rule with lower rate shadows rule with greater rate + * rule with greater rate shadows rule with lower rate * - * consider for example two rules: rule 1 that matches 30 pkts/sec and - * rule 2 that matches 50 pkts/sec + * From man iptables: "A rule using this extension will match until + * this limit is reached " * - * In this case neither rule matches when packet flow is at <30 - * pkts/sec and rule 1 matches if packet flow is greater than 30 - * pkts/sec . Even when packet flow is greater than 50 pkts/sec, it is - * still rule 1 that matches it. So rule 2 will never match at all, - * and rule with lower rate shadows rule with greater rate. + * consider for example two rules: rule 1 that matches 50 pkts/sec and + * rule 2 that matches 30 pkts/sec + * + * rule 1 matches rates between 0 and 49 and rule 2 rates between 0 + * and 29. This means rule 2 will never match any rate and rule with + * greater limit value shadows the one with lower limit value * * we should return true if candidate_rule_2 shadows candidate_rule_1 */ @@ -5172,8 +5173,10 @@ bool PolicyCompiler_ipt::checkForShadowingPlatformSpecific(PolicyRule *candidate if (opt_1->getInt("limit_value")>0 || opt_2->getInt("limit_value")>0) { - if (opt_1->getInt("limit_value") < opt_2->getInt("limit_value")) - return false; + int rate_1 = opt_1->getInt("limit_value"); if (rate_1 == -1) rate_1 = INT_MAX; + int rate_2 = opt_2->getInt("limit_value"); if (rate_2 == -1) rate_2 = INT_MAX; + if (rate_1 > rate_2) return false; + if (opt_1->getStr("limit_value_not") != opt_2->getStr("limit_value_not")) return false; if (opt_1->getStr("limit_suffix") != opt_2->getStr("limit_suffix")) @@ -5182,8 +5185,10 @@ bool PolicyCompiler_ipt::checkForShadowingPlatformSpecific(PolicyRule *candidate if (opt_1->getInt("connlimit_value")>0 || opt_2->getInt("connlimit_value")>0) { - if (opt_1->getInt("connlimit_value") < opt_2->getInt("connlimit_value")) - return false; + int rate_1 = opt_1->getInt("connlimit_value"); if (rate_1 == -1) rate_1 = INT_MAX; + int rate_2 = opt_2->getInt("connlimit_value"); if (rate_2 == -1) rate_2 = INT_MAX; + if (rate_1 > rate_2) return false; + if (opt_1->getStr("connlimit_value_not") != opt_2->getStr("connlimit_value_not")) return false; if (opt_1->getStr("connlimit_suffix") != opt_2->getStr("connlimit_suffix")) @@ -5192,8 +5197,10 @@ bool PolicyCompiler_ipt::checkForShadowingPlatformSpecific(PolicyRule *candidate if (opt_1->getInt("hashlimit_value")>0 || opt_2->getInt("hashlimit_value")>0) { - if (opt_1->getInt("hashlimit_value") < opt_2->getInt("hashlimit_value")) - return false; + int rate_1 = opt_1->getInt("hashlimit_value"); if (rate_1 == -1) rate_1 = INT_MAX; + int rate_2 = opt_2->getInt("hashlimit_value"); if (rate_2 == -1) rate_2 = INT_MAX; + if (rate_1 > rate_2) return false; + if (opt_1->getStr("hashlimit_suffix") != opt_2->getStr("hashlimit_suffix")) return false; if (opt_1->getStr("hashlimit_mode") != opt_2->getStr("hashlimit_mode")) diff --git a/test/ipt/objects-for-regression-tests.fwb b/test/ipt/objects-for-regression-tests.fwb index e512bb0dd..c63a3df24 100644 --- a/test/ipt/objects-for-regression-tests.fwb +++ b/test/ipt/objects-for-regression-tests.fwb @@ -49959,12 +49959,12 @@ echo '%FWBPROMPT%'; sh /tmp/%FWSCRIPT% - + - + @@ -50031,7 +50031,7 @@ echo '%FWBPROMPT%'; sh /tmp/%FWSCRIPT% - + @@ -50098,7 +50098,7 @@ echo '%FWBPROMPT%'; sh /tmp/%FWSCRIPT% - + @@ -50182,7 +50182,7 @@ echo '%FWBPROMPT%'; sh /tmp/%FWSCRIPT% - + @@ -50227,7 +50227,7 @@ echo '%FWBPROMPT%'; sh /tmp/%FWSCRIPT% - + @@ -50258,52 +50258,7 @@ echo '%FWBPROMPT%'; sh /tmp/%FWSCRIPT% - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + @@ -50348,6 +50303,51 @@ echo '%FWBPROMPT%'; sh /tmp/%FWSCRIPT% + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + @@ -50441,52 +50441,7 @@ echo '%FWBPROMPT%'; sh /tmp/%FWSCRIPT% - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + @@ -50531,6 +50486,51 @@ echo '%FWBPROMPT%'; sh /tmp/%FWSCRIPT% + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +