Advanced Options - Multiple State / Connection Controls Not Working

kejianshi

Don't get discouraged. Explain your findings. I'd love to know why this isn't working as expected.

ky41083

Not discouraged, just waiting to see if a dev will notice and chime in. Will most likely create a bug report on the issue tracker if nothing that makes solid repeatable sense comes up first.

Clearly none of the values are being applied at all, not even in the wrong location, according to the /tmp/rules.debug log.

kejianshi

I also had same feeling as you, but I've been too busy to dig into the "why" of it the way you have.
I think that you are correct that its not working as advertised.
Do you have a proposed fix? Can you see where the failure is?

kejianshi

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?

ky41083

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

kejianshi

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

ky41083

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.

kejianshi

That is weird. Must be broken.

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.

kejianshi

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>

ky41083

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

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.

ky41083

Submitted to Redmine, can follow up at:

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

ky41083

@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.

ky41083

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>

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.

ky41083

@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.

ky41083

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

ky41083

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.

phil.davis

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…