Project

General

Profile

Bug #8554

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

Added by Steven Brown over 1 year ago. Updated 8 months ago.

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

100%

Estimated time:
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)) {

Associated revisions

Revision 5142c80a (diff)
Added by Jim Pingle 11 months ago

Rewrite /etc/rc.kill_states to use pfSense module state functions. Fixes #8554

Eliminates inaccurate shell exec/grep/preg_match syntax issues.

Revision 0edf0420 (diff)
Added by Jim Pingle 11 months ago

Rewrite /etc/rc.kill_states to use pfSense module state functions. Fixes #8554

Eliminates inaccurate shell exec/grep/preg_match syntax issues.

(cherry picked from commit 5142c80abbaa7b2dd219c03edd60c4f675d2fb62)

History

#1 Updated by Steven Brown over 1 year 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)) {

#2 Updated by Luke Hamburg 11 months ago

Did you ever submit a PR for this?

#3 Updated by Jim Pingle 11 months 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 set to All

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

#4 Updated by Jim Pingle 11 months ago

  • Status changed from New to Feedback
  • % Done changed from 0 to 100

#5 Updated by Renato Botelho 10 months ago

  • Status changed from Feedback to Resolved

Works

#6 Updated by Srdjan Jankovic 8 months 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?

Also available in: Atom PDF