Netgate Discussion Forum
    • Categories
    • Recent
    • Tags
    • Popular
    • Users
    • Search
    • Register
    • Login

    Advanced Options - Multiple State / Connection Controls Not Working

    Scheduled Pinned Locked Moved
    2.1 Snapshot Feedback and Problems - RETIRED
    4
    31
    10.4k
    Loading More Posts
    • Oldest to Newest
    • Newest to Oldest
    • Most Votes
    Reply
    • Reply as topic
    Log in to reply
    This topic has been deleted. Only users with topic management privileges can see it.
    • K
      kejianshi
      last edited by

      Have you done a pfctl -sa dump into a file then apply a rule and pfctl -sa into a seperate file and compare the files?
      See if there are differences?

      1 Reply Last reply Reply Quote 0
      • K
        ky41083 Banned
        last edited by

        The failure is the script that reads / parses the saved options and turns them into PHP commands.

        Yes, I could find it. No, I couldn't fix it without about a days worth of Googling for the correct way to parse / implement it on the command line and massaging it all together.

        That is why I would just submit a bug in the issue tracker, the devs know far better than me off hand, what script specifically processes or is supposed to process these options, and the proper way to code in the commands to implement the options properly again.

        For me, it's 2:35am and time to relax until tomorrow ;-)

        1 Reply Last reply Reply Quote 0
        • K
          kejianshi
          last edited by

          I'll check and see what differences I see after another beer (-:  Then rest.

          1 Reply Last reply Reply Quote 0
          • K
            ky41083 Banned
            last edited by

            According to rules.debug, after applying a Maximum new connections / per second(s) value of 704 per 5 seconds it, nor any of the other settings under the "Advanced Options" section appear, the rule remains as follows:

            pass  in  quick  on $LAN inet from 192.168.1.0/24 to ! $PrivateIPv4  label "USER_RULE: Session Limit Internet Traffic"

            Update: Running - pfctl -sa | grep "704" - at the command line returns absolutely nothing.

            1 Reply Last reply Reply Quote 0
            • K
              kejianshi
              last edited by

              That is weird.  Must be broken.

              1 Reply Last reply Reply Quote 0
              • P
                phil.davis
                last edited by

                filter.inc

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

                If the state type (another advanced rule option) is set to "none" then the "Advanced Options" settings on the rule do not apply. That is the only obvious way I see that the code can bypass putting max-src-states and similar into the pf rule.

                As the Greek philosopher Isosceles used to say, "There are 3 sides to every triangle."
                If I helped you, then help someone else - buy someone a gift from the INF catalog http://secure.inf.org/gifts/usd/

                1 Reply Last reply Reply Quote 0
                • K
                  kejianshi
                  last edited by

                  I've tried this last night with beer and today without and got the same results.

                  So, this time I'm only applying 853 connections / 1 second then doing a filter reload.
                  Checked to make  sure its still "keep state"

                  pfctl -sa | grep "853"
                  pass in quick on nfe0 reply-to (nfe0 78.22.113.1) inet proto tcp from any to 192.168.13.1 port = 49132 flags S/SA keep state (source-track rule, max-src-conn-rate 853/1, overload <virusprot>flush global, src.track 1) label "USER_RULE: NAT 49132 Stunnel to Foscam"

                  It is making changes to PF on my version.  If same thing in 2.1 doesn't end up in PF rules it must be broken.
                  Mine is 2.03</virusprot>

                  1 Reply Last reply Reply Quote 0
                  • K
                    ky41083 Banned
                    last edited by

                    Ok, so just for the record, you're on 2.0.3-RELEASE? and it's working (you're getting something anyways).

                    I'm on 2.1-RC0 (i386) built on Thu Jul 18 21:59:09 EDT 2013 (pfSense-2.1-RC0-2g-i386-nanobsd_vga-upgrade-20130718-2159.img.gz)

                    @phil.davis:

                    filter.inc

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

                    If the state type (another advanced rule option) is set to "none" then the "Advanced Options" settings on the rule do not apply. That is the only obvious way I see that the code can bypass putting max-src-states and similar into the pf rule.

                    The state type on all of my rules in question (and the rest to be honest) are all set to the default value of "keep state", so by that function all of the advanced options I have set should be applied on the backend, and they are not :P

                    So, bug submittal time ;)

                    1 Reply Last reply Reply Quote 0
                    • P
                      phil.davis
                      last edited by

                      I looked further up the code block:

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

                      So this is only actioned for rules that are for protocol TCP.
                      If protocol is "all" it won't put the advanced settings in.
                      I guess that is reasonable, states are only TCP things, but for protocol "all" it could first write a TCP rule with the advanced settings then another generic rule for everything else (UDP, ICMP…). Or at the GUI could give an error message when saving, telling you that these advanced settings are only for protocol TCP.

                      As the Greek philosopher Isosceles used to say, "There are 3 sides to every triangle."
                      If I helped you, then help someone else - buy someone a gift from the INF catalog http://secure.inf.org/gifts/usd/

                      1 Reply Last reply Reply Quote 0
                      • K
                        ky41083 Banned
                        last edited by

                        Submitted to Redmine, can follow up at:

                        https://redmine.pfsense.org/issues/3098

                        1 Reply Last reply Reply Quote 0
                        • K
                          ky41083 Banned
                          last edited by

                          @phil.davis:

                          I looked further up the code block:

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

                          So this is only actioned for rules that are for protocol TCP.
                          If protocol is "all" it won't put the advanced settings in.
                          I guess that is reasonable, states are only TCP things, but for protocol "all" it could first write a TCP rule with the advanced settings then another generic rule for everything else (UDP, ICMP…). Or at the GUI could give an error message when saving, telling you that these advanced settings are only for protocol TCP.

                          Ahh that's starting to make sense ;D Sweet, and I agree, it should at LEAST be notated that they will ONLY apply to TCP rules. Going to test now…

                          Update: These options should be applied on the backend for UDP also at the very least, and honestly for any protocol that can make a state table entry. Outgoing UDP can cripple a router, in the session table sense, in a matter of seconds…. we need more control here, and I believe pf is more than capable of this.

                          1 Reply Last reply Reply Quote 0
                          • K
                            ky41083 Banned
                            last edited by

                            Yeup, that's the kicker. My outgoing rule is protocol "any", and my BitTorrent rule was TCP/UDP. Separated the BitTorrent rule out into single TCP and UDP rules, and this is what's coming back in rules.debug:

                            pass  in  quick  on $WAN reply-to ( xl0 1.2.3.6 ) inet proto tcp  from any to $BitTorrent_Host_IP port $BitTorrent_Port flags S/SA keep state ( max 64 max-src-nodes 64 max-src-conn 64 max-src-states 64 max-src-conn-rate 64 /5, overload <virusprot>flush global  )  label "USER_RULE: NAT BitTorrent Client - TCP"

                            pass  in  quick  on $WAN reply-to ( xl0 1.2.3.6 ) inet proto udp  from any to $BitTorrent_Host_IP port $BitTorrent_Port  label "USER_RULE: NAT BitTorrent Client - UDP"

                            Both rules have exactly the same settings under advanced options in the GUI, and only the TCP rule is getting any of them applied too it.</virusprot>

                            1 Reply Last reply Reply Quote 0
                            • P
                              phil.davis
                              last edited by

                              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 <number>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 <number>/ <seconds>Limit the rate of new connections over a time interval.  The con-
                                nection rate is an approximation calculated as a moving average.</seconds></number></number>

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

                              As the Greek philosopher Isosceles used to say, "There are 3 sides to every triangle."
                              If I helped you, then help someone else - buy someone a gift from the INF catalog http://secure.inf.org/gifts/usd/

                              1 Reply Last reply Reply Quote 0
                              • K
                                ky41083 Banned
                                last edited by

                                @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 <number>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 <number>/ <seconds>Limit the rate of new connections over a time interval.  The con-
                                  nection rate is an approximation calculated as a moving average.</seconds></number></number>

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

                                Sweet, will update bug report! So yes, we need notations and code additions because right now absolutely none of the options, even valid ones, are applied to anything but TCP rules.

                                1 Reply Last reply Reply Quote 0
                                • K
                                  ky41083 Banned
                                  last edited by

                                  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 ;D

                                  filter.inc.patch.txt

                                  1 Reply Last reply Reply Quote 0
                                  • K
                                    ky41083 Banned
                                    last edited by

                                    Edit: Corrected commit request for 2.1 branch: https://github.com/ky41083/pfsense/commit/a3ff93398a5cbec0e18960be36fd2fdc24cd5aa9

                                    Note: Yes, it's better (cleaner) than the previously attached patch, but they both do exactly the same thing in the end.

                                    This: https://github.com/pfsense/pfsense/pull/721 - needs to be reverted or preferably just fixed (again) to match the newly supported protocols. Currently it will not let you apply advanced options to rules that now support them, except TCP only rules.

                                    1 Reply Last reply Reply Quote 0
                                    • P
                                      phil.davis
                                      last edited by

                                      I have made a single pull request that has the code for the GUI input validation and the pf rule generation: https://github.com/pfsense/pfsense/pull/729
                                      Review, test and comment…

                                      As the Greek philosopher Isosceles used to say, "There are 3 sides to every triangle."
                                      If I helped you, then help someone else - buy someone a gift from the INF catalog http://secure.inf.org/gifts/usd/

                                      1 Reply Last reply Reply Quote 0
                                      • K
                                        ky41083 Banned
                                        last edited by

                                        Commit https://github.com/pfsense/pfsense/commit/08597fcc811eaa8299610b1e797b16abe3c7235d Line 485, "if (!empty($_POST['max']))" is overriding a preexisting function below on line 494. From (http://www.freebsd.org/cgi/man.cgi?query=pf.conf&apropos=0&sektion=5&manpath=FreeBSD+8.3-RELEASE&arch=default&format=html#STATEFUL_TRACKING_OPTIONS) that we referred too, there is no traffic prerequisite for using any stateful tracking options except "max-src-conn" and "max-src-conn-rate" which are TCP only, as you have them currently. With the newer change to the filter.inc function we were working on (https://github.com/pfsense/pfsense/commit/dde3cae3dcbd7b64757c66acc4b56f1183831ede) that brings light to this fact, shouldn't some of these validation rules also be changed to reflect this? And while we're here, remove the double occurrence of "if (!empty($_POST['max'])". The function that states the rule must simply be a pass type rule to be accepted being more correct.

                                        I think the basis for rule validation, rather than protocol, should be state type, as described at the very top of the linked section above: "A number of options related to stateful tracking can be applied on a per-rule basis. keep state, modulate state and synproxy state support these options, and keep state must be specified explicitly to apply options to a rule." So any protocol of any state type (basically any pass rule) can use all state options, while keep state must be specified to use the two TCP only "max-src-conn" and "max-src-conn-rate" state options.

                                        Am I making sense here?

                                        1 Reply Last reply Reply Quote 0
                                        • First post
                                          Last post