From 388f69537c5afa940f28c91c393004e40992e8c0 Mon Sep 17 00:00:00 2001 From: Vadim Kurland Date: Sat, 1 May 2010 22:23:01 +0000 Subject: [PATCH] * CompilerDriver_ipt_policy.cpp (CompilerDriver_ipt::processPolicyRuleSet): fixes #1432 "automatic rule with --restore-mark is missing if rules using action Tag are not in the default Policy rule set". --- build_num | 2 +- doc/ChangeLog | 6 + src/compiler_lib/CompilerDriver.cpp | 18 +- src/iptlib/CompilerDriver_ipt.cpp | 21 +- src/iptlib/CompilerDriver_ipt.h | 7 +- src/iptlib/CompilerDriver_ipt_policy.cpp | 87 ++-- src/iptlib/CompilerDriver_ipt_run.cpp | 4 + src/iptlib/MangleTableCompiler_ipt.cpp | 7 +- src/iptlib/PolicyCompiler_ipt.cpp | 6 +- .../linux24/script_body_iptables_restore | 7 +- .../linux24/script_body_single_rule | 3 +- test/ipt/objects-for-regression-tests.fwb | 406 +++++++++++++++++- test/ipt/quick-cmp.sh | 2 +- 13 files changed, 509 insertions(+), 67 deletions(-) diff --git a/build_num b/build_num index 4554b55ff..e58343718 100644 --- a/build_num +++ b/build_num @@ -1 +1 @@ -#define BUILD_NUM 2863 +#define BUILD_NUM 2866 diff --git a/doc/ChangeLog b/doc/ChangeLog index 2ef564839..d127d775e 100644 --- a/doc/ChangeLog +++ b/doc/ChangeLog @@ -1,3 +1,9 @@ +2010-05-01 vadim + + * CompilerDriver_ipt_policy.cpp (CompilerDriver_ipt::processPolicyRuleSet): + fixes #1432 "automatic rule with --restore-mark is missing if + rules using action Tag are not in the default Policy rule set". + 2010-05-01 yalovoy * RuleSetView.cpp: fixes #1431 GUI crash adding rules to rule group diff --git a/src/compiler_lib/CompilerDriver.cpp b/src/compiler_lib/CompilerDriver.cpp index 46bab1a2f..60b69ba34 100644 --- a/src/compiler_lib/CompilerDriver.cpp +++ b/src/compiler_lib/CompilerDriver.cpp @@ -753,15 +753,8 @@ QTextStream& operator<< (QTextStream &text_stream, const string &str) */ string CompilerDriver::indent(int n_spaces, const string &txt) { - ostringstream output; - istringstream str(txt); - char line[65536]; - while (!str.eof()) - { - str.getline(line, sizeof(line)); - output << std::setw(n_spaces) << std::setfill(' ') << " " << line << endl; - } - return output.str(); + QString res = indent(n_spaces, QString(txt.c_str())); + return res.toStdString(); } QString CompilerDriver::indent(int n_spaces, const QString &txt) @@ -770,12 +763,17 @@ QString CompilerDriver::indent(int n_spaces, const QString &txt) return prepend(fill, txt); } +/* + * prepend each line in @txt with @prep, however there is no need to + * prepend empty lines + */ QString CompilerDriver::prepend(const QString &prep, const QString &txt) { QStringList str; foreach (QString line, txt.split("\n")) { - str.append(line.prepend(prep)); + if (line.isEmpty()) str.append(line); + else str.append(line.prepend(prep)); } return str.join("\n"); } diff --git a/src/iptlib/CompilerDriver_ipt.cpp b/src/iptlib/CompilerDriver_ipt.cpp index 9a66b672d..7fc9a1085 100644 --- a/src/iptlib/CompilerDriver_ipt.cpp +++ b/src/iptlib/CompilerDriver_ipt.cpp @@ -52,6 +52,8 @@ using namespace fwcompiler; CompilerDriver_ipt::CompilerDriver_ipt(FWObjectDatabase *db) : CompilerDriver(db) { + have_connmark = false; + have_connmark_in_output = false; } // create a copy of itself, including objdb @@ -127,7 +129,8 @@ void CompilerDriver_ipt::findBranchesInMangleTable(Firewall*, * compile or more if-then-else in configlet code. */ string CompilerDriver_ipt::dumpScript(Firewall *fw, - const string& reset_script, + const string& automatic_rules_script, + const string& automatic_mangle_script, const string& nat_script, const string& mangle_script, const string& filter_script, @@ -138,11 +141,11 @@ string CompilerDriver_ipt::dumpScript(Firewall *fw, string prolog_place = fw->getOptionsObject()->getStr("prolog_place"); Configlet *conf = NULL; - bool have_reset = !reset_script.empty(); + bool have_auto = !automatic_rules_script.empty() || !automatic_mangle_script.empty(); if (single_rule_compile_on) { - have_reset = false; + have_auto = false; conf = new Configlet(fw, "linux24", "script_body_single_rule"); conf->collapseEmptyStrings(true); } else @@ -154,20 +157,22 @@ string CompilerDriver_ipt::dumpScript(Firewall *fw, conf = new Configlet(fw, "linux24", "script_body_single_rule"); } - conf->setVariable("reset", have_reset); - conf->setVariable("reset_script", reset_script.c_str()); + conf->setVariable("auto", have_auto); conf->setVariable("filter", !filter_script.empty()); - conf->setVariable("filter_or_reset", have_reset || !filter_script.empty()); + conf->setVariable("filter_or_auto", have_auto || !filter_script.empty()); + conf->setVariable("filter_auto_script", automatic_rules_script.c_str()); conf->setVariable("filter_script", filter_script.c_str()); conf->setVariable("mangle", !mangle_script.empty()); + conf->setVariable("mangle_or_auto", !mangle_script.empty() || !automatic_mangle_script.empty()); + conf->setVariable("mangle_auto_script", automatic_mangle_script.c_str()); conf->setVariable("mangle_script", mangle_script.c_str()); - + conf->setVariable("nat", !nat_script.empty()); conf->setVariable("nat_script", nat_script.c_str()); - bool have_script = (have_reset || + bool have_script = (have_auto || !filter_script.empty() || !mangle_script.empty() || !nat_script.empty()); diff --git a/src/iptlib/CompilerDriver_ipt.h b/src/iptlib/CompilerDriver_ipt.h index d044de818..52e21dd9c 100644 --- a/src/iptlib/CompilerDriver_ipt.h +++ b/src/iptlib/CompilerDriver_ipt.h @@ -72,6 +72,9 @@ namespace fwcompiler { fwcompiler::OSConfigurator *_oscnf, std::map *m_n_commands_map); + bool have_connmark; + bool have_connmark_in_output; + public: CompilerDriver_ipt(libfwbuilder::FWObjectDatabase *db); @@ -88,7 +91,8 @@ public: std::list &all_policies); std::string dumpScript(libfwbuilder::Firewall *fw, - const std::string& reset_script, + const std::string& automatic_rules_script, + const std::string& automatic_mangle_script, const std::string& nat_script, const std::string& mangle_script, const std::string& filter_script, @@ -101,6 +105,7 @@ public: std::ostringstream &filter_table_stream, std::ostringstream &mangle_table_stream, std::ostringstream &automatic_rules_stream, + std::ostringstream &automatic_mangle_stream, fwcompiler::OSConfigurator_linux24 *oscnf, int policy_af, std::map &minus_n_commands_filter, diff --git a/src/iptlib/CompilerDriver_ipt_policy.cpp b/src/iptlib/CompilerDriver_ipt_policy.cpp index 471a088ff..cd5259029 100644 --- a/src/iptlib/CompilerDriver_ipt_policy.cpp +++ b/src/iptlib/CompilerDriver_ipt_policy.cpp @@ -49,14 +49,16 @@ using namespace std; using namespace libfwbuilder; using namespace fwcompiler; - +// we always first process all non-top rule sets, then all top rule +// sets bool CompilerDriver_ipt::processPolicyRuleSet( Firewall *fw, FWObject *ruleset, const string &single_rule_id, - ostringstream &filter_table_stream, - ostringstream &mangle_table_stream, + ostringstream &filter_rules_stream, + ostringstream &mangle_rules_stream, ostringstream &automatic_rules_stream, + ostringstream &automatic_mangle_stream, OSConfigurator_linux24 *oscnf, int policy_af, std::map &minus_n_commands_filter, @@ -64,8 +66,6 @@ bool CompilerDriver_ipt::processPolicyRuleSet( { int policy_rules_count = 0; int mangle_rules_count = 0; - bool have_connmark = false; - bool have_connmark_in_output = false; bool empty_output = true; string prolog_place = fw->getOptionsObject()->getStr("prolog_place"); string platform = fw->getStr("platform"); @@ -124,28 +124,7 @@ bool CompilerDriver_ipt::processPolicyRuleSet( have_connmark |= mangle_compiler->haveConnMarkRules(); have_connmark_in_output |= mangle_compiler->haveConnMarkRulesInOutput(); - long m_str_pos = mangle_table_stream.tellp(); - - if (policy->isTop()) - { - ostringstream tmp; - - if (flush_and_set_default_policy) - tmp << mangle_compiler->flushAndSetDefaultPolicy(); - - tmp << mangle_compiler->printAutomaticRules(); - - if (tmp.tellp() > 0) - { - if (!single_rule_compile_on) - { - mangle_table_stream << "# ================ Table 'mangle', "; - mangle_table_stream << "automatic rules"; - mangle_table_stream << "\n"; - } - mangle_table_stream << tmp.str(); - } - } + long m_str_pos = mangle_rules_stream.tellp(); if (mangle_compiler->getCompiledScriptLength() > 0) { @@ -157,10 +136,10 @@ bool CompilerDriver_ipt::processPolicyRuleSet( { if (!single_rule_compile_on) { - mangle_table_stream << "# ================ Table 'mangle', "; - mangle_table_stream << "rule set " << branch_name << "\n"; + mangle_rules_stream << "# ================ Table 'mangle', "; + mangle_rules_stream << "rule set " << branch_name << "\n"; } - mangle_table_stream << tmp.str(); + mangle_rules_stream << tmp.str(); } } @@ -169,11 +148,12 @@ bool CompilerDriver_ipt::processPolicyRuleSet( all_errors.push_back(mangle_compiler->getErrors("").c_str()); } - if (m_str_pos!=mangle_table_stream.tellp()) + if (m_str_pos!=mangle_rules_stream.tellp()) { - mangle_table_stream << "\n"; + //mangle_rules_stream << "\n"; empty_output = false; } + } std::auto_ptr policy_compiler = createPolicyCompiler( @@ -207,10 +187,10 @@ bool CompilerDriver_ipt::processPolicyRuleSet( empty_output = false; if (!single_rule_compile_on) { - filter_table_stream << "# ================ Table 'filter', "; - filter_table_stream << "rule set " << branch_name << "\n"; + filter_rules_stream << "# ================ Table 'filter', "; + filter_rules_stream << "rule set " << branch_name << "\n"; } - filter_table_stream << tmp.str(); + filter_rules_stream << tmp.str(); } } @@ -246,6 +226,12 @@ bool CompilerDriver_ipt::processPolicyRuleSet( tmp << policy_compiler->printAutomaticRules(); + // printAutomaticRules() can generate errors and warnings + if (policy_compiler->haveErrorsAndWarnings()) + { + all_errors.push_back(policy_compiler->getErrors("").c_str()); + } + if (tmp.tellp() > 0) { empty_output = false; @@ -258,5 +244,36 @@ bool CompilerDriver_ipt::processPolicyRuleSet( automatic_rules_stream << tmp.str(); } } + + long auto_mangle_stream_position = automatic_mangle_stream.tellp(); + if (policy->isTop() && auto_mangle_stream_position <= 0) + { + // Note that we process non-top rule sets first and then + // deal with the top rule set. By the time we get here the + // have_connmark flags reflect the state of all other rule + // sets and the top one. + + ostringstream tmp_m; + tmp_m << mangle_compiler->printAutomaticRulesForMangleTable( + have_connmark, have_connmark_in_output); + + // printAutomaticRulesForMangleTable() can generate errors and warnings + if (mangle_compiler->haveErrorsAndWarnings()) + { + all_errors.push_back(mangle_compiler->getErrors("").c_str()); + } + + if (tmp_m.tellp() > 0) + { + if (!single_rule_compile_on) + { + automatic_mangle_stream << "# ================ Table 'mangle', "; + automatic_mangle_stream << "automatic rules"; + automatic_mangle_stream << "\n"; + } + automatic_mangle_stream << tmp_m.str(); + } + } + return empty_output; } diff --git a/src/iptlib/CompilerDriver_ipt_run.cpp b/src/iptlib/CompilerDriver_ipt_run.cpp index 28694ec46..34946de34 100644 --- a/src/iptlib/CompilerDriver_ipt_run.cpp +++ b/src/iptlib/CompilerDriver_ipt_run.cpp @@ -288,6 +288,7 @@ QString CompilerDriver_ipt::run(const std::string &cluster_id, } ostringstream automaitc_rules_stream; + ostringstream automaitc_mangle_stream; ostringstream filter_rules_stream; ostringstream mangle_rules_stream; ostringstream nat_rules_stream; @@ -327,6 +328,7 @@ QString CompilerDriver_ipt::run(const std::string &cluster_id, policy_af, minus_n_commands_nat)) empty_output = false; + // first process all non-top rule sets, then all top rule sets for (int all_top = 0; all_top < 2; ++all_top) { for (list::iterator p=all_policies.begin(); @@ -345,6 +347,7 @@ QString CompilerDriver_ipt::run(const std::string &cluster_id, filter_rules_stream, mangle_rules_stream, automaitc_rules_stream, + automaitc_mangle_stream, oscnf.get(), policy_af, minus_n_commands_filter, @@ -370,6 +373,7 @@ QString CompilerDriver_ipt::run(const std::string &cluster_id, generated_script += dumpScript(fw, automaitc_rules_stream.str(), + automaitc_mangle_stream.str(), nat_rules_stream.str(), mangle_rules_stream.str(), filter_rules_stream.str(), diff --git a/src/iptlib/MangleTableCompiler_ipt.cpp b/src/iptlib/MangleTableCompiler_ipt.cpp index ab54f3252..da8e39316 100644 --- a/src/iptlib/MangleTableCompiler_ipt.cpp +++ b/src/iptlib/MangleTableCompiler_ipt.cpp @@ -160,10 +160,13 @@ string MangleTableCompiler_ipt::flushAndSetDefaultPolicy() return ""; } +// mangle table compiler is special, it needs additional parameters to +// generate automatic rules correctly. But virtual function +// printAutomaticRules() has no parameters so we have another one +// that takes parameters: printAutomaticRulesForMangleTable() string MangleTableCompiler_ipt::printAutomaticRules() { - return printAutomaticRulesForMangleTable(have_connmark, - have_connmark_in_output); + return ""; } string MangleTableCompiler_ipt::printAutomaticRulesForMangleTable( diff --git a/src/iptlib/PolicyCompiler_ipt.cpp b/src/iptlib/PolicyCompiler_ipt.cpp index 21f0d5a43..40dbdcbd8 100644 --- a/src/iptlib/PolicyCompiler_ipt.cpp +++ b/src/iptlib/PolicyCompiler_ipt.cpp @@ -1740,10 +1740,10 @@ bool PolicyCompiler_ipt::splitIfTagAndConnmark::processNext() PolicyRule *r, *r1; if (make_terminating) - ruleopt->setBool("already_terminating_target",true); + ruleopt->setBool("already_terminating_target", true); - string this_chain = rule->getStr("ipt_chain"); - string new_chain=ipt_comp->getNewChainName(rule,rule_iface); + string this_chain = rule->getStr("ipt_chain"); + string new_chain = ipt_comp->getNewChainName(rule,rule_iface); r= compiler->dbcopy->createPolicyRule(); compiler->temp_ruleset->add(r); diff --git a/src/res/configlets/linux24/script_body_iptables_restore b/src/res/configlets/linux24/script_body_iptables_restore index 8fe2c11aa..732aa3ae9 100644 --- a/src/res/configlets/linux24/script_body_iptables_restore +++ b/src/res/configlets/linux24/script_body_iptables_restore @@ -14,14 +14,15 @@ ## iptables-restore method, not single rule compile {{if have_script}} ( -{{if filter_or_reset}} +{{if filter_or_auto}} echo '*filter' -{{$reset_script}} +{{$filter_auto_script}} {{$filter_script}} echo COMMIT {{endif}} -{{if mangle}} +{{if mangle_or_auto}} echo '*mangle' +{{$mangle_auto_script}} {{$mangle_script}} echo COMMIT {{endif}} diff --git a/src/res/configlets/linux24/script_body_single_rule b/src/res/configlets/linux24/script_body_single_rule index a0415fd29..68b4b9645 100644 --- a/src/res/configlets/linux24/script_body_single_rule +++ b/src/res/configlets/linux24/script_body_single_rule @@ -14,7 +14,8 @@ ## this template is used for single rule compile, both ## iptables-restore and regular, as well as for the regular ## (not iptables-restore) script -{{if reset}}{{$reset_script}}{{endif}} +{{if auto}}{{$filter_auto_script}} +{{$mangle_auto_script}}{{endif}} {{if nat}}{{$nat_script}}{{endif}} diff --git a/test/ipt/objects-for-regression-tests.fwb b/test/ipt/objects-for-regression-tests.fwb index fb27cedba..57b6589fc 100644 --- a/test/ipt/objects-for-regression-tests.fwb +++ b/test/ipt/objects-for-regression-tests.fwb @@ -1,6 +1,6 @@ - + @@ -50763,7 +50763,7 @@ echo '%FWBPROMPT%'; sh /tmp/%FWSCRIPT% - + @@ -50789,6 +50789,26 @@ echo '%FWBPROMPT%'; sh /tmp/%FWSCRIPT% + + + + + + + + + + + + + + + + + + + + @@ -51145,6 +51165,388 @@ echo '%FWBPROMPT%'; sh /tmp/%FWSCRIPT% + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/test/ipt/quick-cmp.sh b/test/ipt/quick-cmp.sh index 3d4d4c6a3..166a3ccc7 100755 --- a/test/ipt/quick-cmp.sh +++ b/test/ipt/quick-cmp.sh @@ -1,7 +1,7 @@ #!/bin/sh -DIFFCMD="diff -C 5 -c -b -B -w -I \"# Generated\" -I 'Activating ' -I '# Firewall Builder fwb_ipt v' -I 'Can not find file' -I '====' -I 'log '" +DIFFCMD="diff -C 5 -c -b -w -I \"^ *$\" -I \" *# *$\" -I \"# Generated\" -I 'Activating ' -I '# Firewall Builder fwb_ipt v' -I 'Can not find file' -I '====' -I 'log '" for f in $(ls *.fw.orig) do