Project

General

Profile

Actions
Copy link

Bug #3098

closed

Advanced Options - Multiple State / Connection Controls Not Working

Added by ky41083 - over 11 years ago. Updated over 11 years ago.

Status:
Resolved
Priority:
Normal
Assignee:
-
Category:
Rules / NAT
Target version:
2.1
Start date:
07/20/2013
Due date:
% Done:

0%

Estimated time:
Plus Target Version:
Release Notes:
Affected Version:
2.1
Affected Architecture:

Description

Issue: Set "Maximum state entries this rule can create" on a rule (I set this on my BitTorrent rule and my default outbound rule, see attached image) and the value is not respected.

Expected behavior: Any traffic matching this rule would be limited to the number of states (basically sessions, at least one state per session) you set this value too. Even if traffic is finding a way around the BitTorrent rule, the default outbound rule should still catch it.

Actual behavior: States / sessions are not limited at all, even on the default rule, and allow lan side hosts to quickly exceed the session limit on any upstream devices (like currently deployed AT&T VDSL routers that have a 1024 session limit, or a similarly extremely low global limit).

Additional Info: Every option under "Advanced Options" have no effect what so ever, according to /tmp/rules.debug, regardless of the value of "State Type". The following function in filter.inc, pointed out by phil.davis, does not apply any advanced options when set to "none":

$noadvoptions = false;
if(isset($rule['statetype']) && $rule['statetype'] <> "") {
switch($rule['statetype']) {
case "none":
$noadvoptions = true;
$aline['flags'] .= " no state ";
break;

Even when using the default value of "keep state" they are not applied what so ever, not even in the wrong place.

Advanced options are reported working and in /tmp/rules.debug by kejianshi on 2.0.3-RELEASE.

Attached is a screen shot of my rules as well as a copy of my /tmp/rules.debug with IP's masked and checked for consistency.

Absolutely none of the advanced options are being applied to any rules on any interface regardless of the state type set.

Files

Download all files
rules.debug.txt (18 KB) rules.debug.txt /tmp/rules.debug ky41083 -, 07/20/2013 10:04 PM
pfSense LAN Rules.jpg (166 KB) pfSense LAN Rules.jpg Screen Shot of LAN Interface Rules ky41083 -, 07/20/2013 10:04 PM
filter.inc.patch (2.67 KB) filter.inc.patch ky41083 -, 07/23/2013 03:25 PM
Actions
Copy link
 #1

Updated by ky41083 - over 11 years ago

The issue appears to be that these setting are only applied to TCP rules. This needs notation in the GUI.

Also, pf is capable of applying some of these options to UDP and ICMP traffic as well. So, functions as well as notations need to be added for rules of traffic type: TCP/UDP | UDP | ICMP

Currently, the following function is handling this, also found by phil.davis in filter.inc:

if (($rule['protocol'] "tcp") && ($type "pass")) {
... do advanced sate-related settings

Direct research on the issue, also from phil.davis:
From http://www.freebsd.org/cgi/man.cgi?query=pf.conf&sektion=5

pf(4) will also create state for other protocols which are effectively stateless by nature. UDP packets are matched to states using only host addresses and ports, and other protocols are matched to states using only the host addresses.

So at least some of the state limiting parameters can be used for UDP/ICMP also. But these ones only apply to TCP:

For stateful TCP connections, limits on established connections (connec-
     tions which have completed the TCP 3-way handshake) can also be enforced
     per source IP.
max-src-conn &lt;number&gt;
      Limits the maximum number of simultaneous TCP connections which
      have completed the 3-way handshake that a single host can make.
     max-src-conn-rate &lt;number&gt; / &lt;seconds&gt;
      Limit the rate of new connections over a time interval.  The con-
      nection rate is an approximation calculated as a moving average.

So it needs a bit of extra explanation and validation in the GUI to allow only the valid combinations of parameters to be entered.

Actions
Copy link
 #2

Updated by ky41083 - over 11 years ago

Or better yet, since we already have elements that become visible or hide depending on the traffic type selected on the edit rule page, make the correct advanced options visible or hidden according to whether TCP/UDP, UDP, or ICMP is selected.

Then add code & option validation on the backend functions in filter.inc since that is where it is currently, unless it should be moved out of filter.inc all together?

Actions
Copy link
 #3

Updated by ky41083 - over 11 years ago

If someone can update the ticket to "all versions" and "urgent" as I can't imagine this isn't also the case with every release up to this point, that would be awesome.

Actions
Copy link
 #4

Updated by ky41083 - over 11 years ago

Keith Hough wrote:

Or better yet, since we already have elements that become visible or hide depending on the traffic type selected on the edit rule page, make the correct advanced options visible or hidden according to whether TCP/UDP, UDP, or ICMP is selected.

Then add code & option validation on the backend functions in filter.inc since that is where it is currently, unless it should be moved out of filter.inc all together?

Wups, make that "according to whether TCP/UDP, TCP, UDP, or ICMP is selected."

Actions
Copy link
 #5

Updated by Phillip Davis over 11 years ago

I have added pull requests https://github.com/pfsense/pfsense/pull/720 (Main) and https://github.com/pfsense/pfsense/pull/721 (2.1 branch) to validate the rule parameters on the GUI using the same checks that are applied in filter.inc when actually implementing the rules. That will make the current system self-validating without changing the actual behaviour.
Now there can be a discussion of which of these criteria to relax (e.g. allow TCP and UDP protocols...), in what version of pfSense to put the changes (Main for 2.2, or get it into 2.1 or?) and some testing to see that these state-based parameters actually work sensibly for some non-TCP protocols.

Actions
Copy link
 #6

Updated by Renato Botelho over 11 years ago

  • Status changed from New to Feedback
  • Priority changed from High to Normal

Both pull requests were merged.

Actions
Copy link
 #7

Updated by ky41083 - over 11 years ago

I was looking more for something like this........ (patch attached). However, with your changes on the rule edit page and these changes to filter.inc (as long as they pass the bug test and check out) I think we might have something ;-)

Actions
Copy link
 #9

Updated by ky41083 - over 11 years ago

Actions
Copy link
 #10

Updated by Phillip Davis over 11 years ago

The extra flexibility allowing relevant state-related advanced options to be used on UDP and ICMP, in addition to TCP, is committed to 2.1 in:
https://github.com/pfsense/pfsense/commit/98f4043e939ab9e31215a0d9da252f06bca28a4b
https://github.com/pfsense/pfsense/commit/ee1577575981efbd36f1dffe251734f6aa12b621
There are corresponding commits to Master also.
Testers, please confirm that this is now "a good thing".

Actions
Copy link
 #11

Updated by Chris Buechler over 11 years ago

  • Status changed from Feedback to Resolved

Confirmed this is all good now.

Actions
Copy link

Also available in: Atom PDF