diff --git a/build_num b/build_num index 5510faa7c..97d41567c 100644 --- a/build_num +++ b/build_num @@ -1 +1 @@ -#define BUILD_NUM 2502 +#define BUILD_NUM 2503 diff --git a/doc/ChangeLog b/doc/ChangeLog index bd1e45643..65edbd540 100644 --- a/doc/ChangeLog +++ b/doc/ChangeLog @@ -1,5 +1,13 @@ 2010-02-06 vadim + * src/pflib/PolicyCompiler_pf_writers.cpp (PrintRule::processNext): + fixes #1210 "syntax error in PF rule - "modulate state" is + required". Per bug reported in the mailing list (and according to + the pf.conf manual), pf.conf requires "keep state", "modulate + state" or "synproxy"if any of the stateful tracking options are + used in the rule. These include "max", "no-sync", "pflow", + "sloppy", "source-track" and others. + * src/pflib/PolicyCompiler_pf_writers.cpp (PrintRule::processNext): fixes #1209 "incorrect syntax in PF rules when only "Activate source tracking" option is on". Compiler sometimes generated empty diff --git a/src/pflib/PolicyCompiler_pf_writers.cpp b/src/pflib/PolicyCompiler_pf_writers.cpp index 40381d223..4302061d5 100644 --- a/src/pflib/PolicyCompiler_pf_writers.cpp +++ b/src/pflib/PolicyCompiler_pf_writers.cpp @@ -998,20 +998,31 @@ bool PolicyCompiler_pf::PrintRule::processNext() } } - // in PF "modulate state", "synproxy state", "keep state" are mutually - // exclusive - // "keep state" can be used with any protocol, while "modulate state" - // and "synproxy state" can only be used with tcp. + /* + * in PF "modulate state", "synproxy state", "keep state" are + * mutually exclusive "keep state" can be used with any + * protocol, while "modulate state" and "synproxy state" can + * only be used with tcp. + */ + bool have_state_option = false; + + /* + * First, set explicit state tracking parameter, then add + * stateful tracking options. + */ if (compiler->getCachedFwOpt()->getBool("pf_synproxy") && tcpsrv!=NULL) + { compiler->output << "synproxy state "; - else + have_state_option = true; + } else { if ((ruleopt->getBool("pf_modulate_state") || compiler->getCachedFwOpt()->getBool("pf_modulate_state")) && tcpsrv!=NULL) { compiler->output << "modulate state "; + have_state_option = true; } else { /* @@ -1035,43 +1046,63 @@ bool PolicyCompiler_pf::PrintRule::processNext() * explicitly" to cope with this. */ if (XMLTools::version_compare(version, "4.0") < 0 || - //if ( version != "4.x" || compiler->getCachedFwOpt()->getBool("pf_keep_state")) + { compiler->output << "keep state "; + have_state_option = true; + } } } + /* + * Stateful tracking options. According to the pf.conf manual, + * one of keep state, modulate state, or synproxy state must + * be specified explicitly to apply these options to a rule. + * Using flags need_state_option and have_state_option for that. + */ + QStringList options; + bool need_state_option = false; if (ruleopt->getInt("pf_rule_max_state")>0) { options.push_back(QString("max %1").arg(ruleopt->getInt("pf_rule_max_state"))); + need_state_option = true; } if (ruleopt->getBool("pf_sloppy_tracker")) { options.push_back("sloppy"); + need_state_option = true; } if (ruleopt->getBool("pf_no_sync")) { options.push_back("no-sync"); + need_state_option = true; } if (ruleopt->getBool("pf_pflow")) { options.push_back("pflow"); + need_state_option = true; } if (ruleopt->getBool("pf_source_tracking")) { if (ruleopt->getInt("pf_max_src_nodes") > 0) + { options.push_back(QString("max-src-nodes %1").arg( ruleopt->getInt("pf_max_src_nodes"))); + need_state_option = true; + } if (ruleopt->getInt("pf_max_src_states")>0) + { options.push_back(QString("max-src-states %1").arg( ruleopt->getInt("pf_max_src_states"))); + need_state_option = true; + } } bool check_overload_opts = false; @@ -1080,6 +1111,7 @@ bool PolicyCompiler_pf::PrintRule::processNext() options.push_back(QString("max-src-conn %1").arg( ruleopt->getInt("pf_max_src_conn"))); check_overload_opts = true; + need_state_option = true; } if (ruleopt->getInt("pf_max_src_conn_rate_num")>0 && @@ -1089,6 +1121,7 @@ bool PolicyCompiler_pf::PrintRule::processNext() .arg(ruleopt->getInt("pf_max_src_conn_rate_num")) .arg(ruleopt->getInt("pf_max_src_conn_rate_seconds"))); check_overload_opts = true; + need_state_option = true; } if (check_overload_opts) @@ -1106,6 +1139,11 @@ bool PolicyCompiler_pf::PrintRule::processNext() options.push_back(overload_opts.join(" ")); } + if (need_state_option && !have_state_option) + { + compiler->output << "keep state "; + } + // looks like pf.conf syntax requires '(' ')' even if there is // only one option if (options.size() > 0) compiler->output << "( "; diff --git a/test/pf/objects-for-regression-tests.fwb b/test/pf/objects-for-regression-tests.fwb index e927a9a92..c31c13035 100644 --- a/test/pf/objects-for-regression-tests.fwb +++ b/test/pf/objects-for-regression-tests.fwb @@ -17433,7 +17433,7 @@ - + @@ -17549,7 +17549,7 @@ - + @@ -17586,7 +17586,81 @@ - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + @@ -17623,7 +17697,7 @@ - +