Bug #511
closedpkg_generate_rules
0%
Description
HI Ermal
Found a bug with the way the discover_pkg_rules() function handles $pkg_generate_rules functions within each package that is installed.
If you have squid and HAVP installed, only HAVP's generate_rules function is executed, leaving any other packages rules not been applied.
I found by moving 'return $rules' to outside the foreach fixes the problem and allows for the discover_pkg_rules function to move onto the next package in the foreach loop.
Also having ' return "" ' for a package that has incorrect PF syntax penalizes the other packages from been setup with PF correctly - which i don't think is correct or the intended behaviour(?).
Example of the change is below:
function discover_pkg_rules($ruletype) { global $config, $g; if(!is_dir("/usr/local/pkg")) return ""; $files = split("\n", trim(`ls /usr/local/pkg/*.inc`)); foreach($files as $pkg_inc) { if($pkg_inc == "ls: No match.") continue; update_filter_reload_status("Checking for {$ruletype} PF hooks in package {$pkg_inc}"); require_once($pkg_inc); $pkg = basename($pkg_inc, ".inc"); $pkg_generate_rules = "{$pkg}_generate_rules"; if(function_exists($pkg_generate_rules)) { update_filter_reload_status("Processing early {$ruletype} rules for package {$pkg_inc}"); log_error("Processing early {$ruletype} rules for package {$pkg_inc}"); $rules .= $pkg_generate_rules("$ruletype"); file_put_contents("{$g['tmp_path']}/rules.packages", $rules); $status = mwexec("/sbin/pfctl -nf {$g['tmp_path']}/rules.packages"); if ($status <> 0) { $errorrules = "There was an error while parsing the package filter rules for {$pkg_inc}.\n"; log_error($errorrules); file_put_contents("{$g['tmp_path']}/rules.packages.{$pkg_inc}", "#{$errorrules}\n"); return ""; } } } return $rules; }
Thx
Warren
Updated by Chris Buechler over 14 years ago
- Affected Version set to 2.0
Warren: can you confirm if this is now fixed for the scenario you could replicate?
Updated by Warren Baker over 14 years ago
Hey Chris
Chris Buechler wrote:
Warren: can you confirm if this is now fixed for the scenario you could replicate?
Yeah it is perfect. Although I have one recommendation, in the current setup if one package has errors in it's filter rules then it causes the rest of the packages, who have filter rules, to be penalized. See below for the 'return "";'. Should that return statement simply be removed and allow the rest of the packages to continue to load? The package error is been logged.
if ($status <> 0) { $errorrules = "There was an error while parsing the package filter rules for {$pkg_inc}.\n"; log_error($errorrules); file_put_contents("{$g['tmp_path']}/rules.packages.{$pkg_inc}", "#{$errorrules}\n"); return ""; }
Thanks,
Warren
Updated by Warren Baker over 14 years ago
Hey Chris
Chris Buechler wrote:
Warren: can you confirm if this is now fixed for the scenario you could replicate?
Yeah it is perfect. Although I have one recommendation, in the current setup if one package has errors in it's filter rules then it causes the rest of the packages, who have filter rules, to be penalized. See below for the 'return "";'. Should that return statement simply be removed and allow the rest of the packages to continue to load? The package error is been logged so a user would be notified by the system logs.
if ($status <> 0) { $errorrules = "There was an error while parsing the package filter rules for {$pkg_inc}.\n"; log_error($errorrules); file_put_contents("{$g['tmp_path']}/rules.packages.{$pkg_inc}", "#{$errorrules}\n"); return ""; }
Thanks,
Warren
Updated by Ermal Luçi over 14 years ago
I merged fixes yesterday to not penalize the other packages.
Updated by Warren Baker over 14 years ago
Ermal Luçi wrote:
I merged fixes yesterday to not penalize the other packages.
Thanks Ermal - that is perfect.
.warren
Updated by Chris Buechler over 14 years ago
- Status changed from Feedback to Resolved