Project

General

Profile

Actions

Bug #8554

closed

/etc/rc.kill_states code not correctly parsing pfctl output

Added by Steven Brown over 3 years ago. Updated almost 3 years ago.

Status:
Resolved
Priority:
Normal
Assignee:
Category:
Rules / NAT
Target version:
Start date:
06/06/2018
Due date:
% Done:

100%

Estimated time:
Plus Target Version:
Release Notes:
Affected Version:
2.4.x
Affected Architecture:
All

Description

The patches added in Bug #2887 no longer works as expected because the output of pfctl -ss no longer matches the format expected.

The previous output was like:

pppoe0 tcp lan_subnet_ip:port -> pppoe_IP:port -> target_ip:port
lan_if tcp lan_subnet_ip:port <- target_ip:port

Where the new output (from at least pfsense 2.3) looks like:

pppoe0 tcp pppoe_IP:port (lan_subnet_ip:port) -> target_ip:port       ESTABLISHED:ESTABLISHED
lan_if tcp target_ip:port <- lan_subnet_ip:port       ESTABLISHED:ESTABLISHED

I believe this patch fixes the code:

@@ -63,11 +63,11 @@

         $cleared_states = array();
         foreach (explode("\n", $nat_states) as $nat_state) {
-            if (preg_match_all('/([\d\.]+):[\d]+[\s->]+/i', $nat_state, $matches, PREG_SET_ORDER) != 3) {
+            if (preg_match_all('/([\d\.]+):[\d]+[\s->)]+/i', $nat_state, $matches, PREG_SET_ORDER) != 3) {
                 continue;
             }

-            $src = $matches[0][1];
+            $src = $matches[1][1];
             $dst = $matches[2][1];

             if (empty($src) || empty($dst) || in_array("{$src},{$dst}", $cleared_states)) {

Actions #1

Updated by Steven Brown over 3 years ago

Sorry, I believe the patch should be:

@@ -59,15 +59,15 @@
     if (!empty($local_ip)) {
         log_error("rc.kill_states: Removing states for IP {$local_ip}/{$subnet_bits}");
         $nat_states = exec_command("/sbin/pfctl -i {$interface} -ss | " .
-            "/usr/bin/egrep '\-> +{$local_ip}:[0-9]+ +\->'");
+            "/usr/bin/egrep '{$local_ip}:[0-9]+ ('");

         $cleared_states = array();
         foreach (explode("\n", $nat_states) as $nat_state) {
-            if (preg_match_all('/([\d\.]+):[\d]+[\s->]+/i', $nat_state, $matches, PREG_SET_ORDER) != 3) {
+            if (preg_match_all('/([\d\.]+):[\d]+[\s->)]+/i', $nat_state, $matches, PREG_SET_ORDER) != 3) {
                 continue;
             }

-            $src = $matches[0][1];
+            $src = $matches[1][1];
             $dst = $matches[2][1];

             if (empty($src) || empty($dst) || in_array("{$src},{$dst}", $cleared_states)) {

Actions #2

Updated by → luckman212 about 3 years ago

Did you ever submit a PR for this?

Actions #3

Updated by Jim Pingle about 3 years ago

  • Category set to Rules / NAT
  • Assignee set to Jim Pingle
  • Target version set to 2.4.4-p1
  • Affected Version set to 2.4.x
  • Affected Architecture All added
  • Affected Architecture deleted ()

I'd rather not change one funky regex matching pattern for another. I have a better fix. Push incoming.

Actions #4

Updated by Jim Pingle about 3 years ago

  • Status changed from New to Feedback
  • % Done changed from 0 to 100
Actions #5

Updated by Renato Botelho about 3 years ago

  • Status changed from Feedback to Resolved

Works

Actions #6

Updated by Srdjan Jankovic almost 3 years ago

I'm running 2.4.4_2 and it still seems to be an issue. Are those actions logged somewhere so I can take a look please?

Actions

Also available in: Atom PDF