PSA: careful when setting TCP Flags=Any + Protocol=Any on a firewall rule

[luckman212]

For weeks, I've been trying to figure out why connections from my IoT VLAN (from specific "trusted hosts" contained in an IP alias) were getting blocked, when I specifically had a rule to allow that traffic through.

I thought at first it was a timing issue where I was just coincidentally reloading the filter exactly when this traffic was hitting the firewall, but after weeks it was happening too often to be a coincidence. The logs showed it happening at 4am too.

Today, I finally think I figured it out- turns out it's a sneaky bug that was reported (see redmine #16563) almost a year ago. The redmine has more details, but in a nutshell, this combo silently fails to set the tcp flags on the generated rule (proto=Any + Flags=any):

I did some further testing and came up with a patch that so far seems to have solved this. Just wanted to bring some awareness since I doubt people peruse old redmines much the way I do.

[q54e3w]

@luckman212 thanks for the heads up and patch. OOC, what’s the use case for these TCP flags?

[johnpoz]

@q54e3w said in PSA: careful when setting TCP Flags=Any + Protocol=Any on a firewall rule:

what’s the use case for these TCP flags?

there is no use case in protocol any - there would only be use case for tcp rules.

There is something wrong that pfsense would even let you try and set tcp flags on a any protocol rule.

[luckman212]

@q54e3w said in PSA: careful when setting TCP Flags=Any + Protocol=Any on a firewall rule:

@luckman212 what’s the use case for these TCP flags?

I'll share my use case: I have a rule on my IoT VLAN to catch and allow traffic from a few trusted hosts (airplay devices: AppleTV, wireless speakers) that was not always getting hit, once in a while packets (some sort of Apple airplay voodoo) were falling through to the explicit block rule right below it. After some packet tracing, I discovered that there were packets being sent "out of state" so to speak without the S/SA flags and thus they were not matching the rule, since it was being generated as e.g.:

pass  in  quick  on $VLAN101_IOT inet from $h_trusted_devices to $n_LANS_subnets ridentifier 1787426400 flags S/SA keep state (pflow) label "id=1787426400" label "tags=user_rule" label "descr=allow trusted"

This could have been achieved with 2 separate rules, one "Any" and another "TCP + flags" but imo the patch provided is minimal, makes managing the ruleset simpler, and is not more ambiguous than the current undefined behavior. In the redmine, the person who opened it commented that the more correct fix would be for the GUI to disallow an Any rule that has flags set. I do agree with that, but that would make the patch much larger, as it would then involve new input validation, changes to config upgrade scripts, GUI changes, etc etc. I opted for this quicker fix.

[johnpoz]

@luckman212 said in PSA: careful when setting TCP Flags=Any + Protocol=Any on a firewall rule:

makes managing the ruleset simpler

And confusing - If I saw a any protocol rule, I would never think there would be specific tcp set in the rule.

out of state traffic should be blocked, and you should fix what is sending out of state - or just let it get blocked and it will figure out that hey I should open a new session and send syn.

You would think the fix would be simple in the gui, if you set any as the protocol you should never be allowed to try and set states for tcp, it should be greyed out and no sort of tcp states should should be attempted to be set on the rule. Only syn should be allowed to open an state.

I would never in a million years think that a any protocol rule would allow all states of tcp - the only thing in that sort of configuration would be tcp syn to open a state.

[luckman212]

@johnpoz well be my guest if you want to try to tackle that PR. The current code as-is I would argue is just as incorrect... it allows you to create a rule with flags that are just silently discarded. I shared my findings and my patch in case it was useful to someone else. I sure wish someone had done the same for me & saved me about 2 weeks of head scratching. as for "you should fix what is sending out of state"... uh sure I'll rewrite the IP stack on my AppleTV... that screwdriver's around here somewhere… 🫠

[johnpoz]

@luckman212 I would never run into such issue since I would never set any flags on a any protocol rule. I have 2 apple tvs, and have never had any issues.. So not exactly sure what you are seeing from them. I don't log default deny on that network - but have never had any problem with them connecting to anything,

Guess I could turn on logging from them. But if an out of state is block, as it should be - they should just open a new session. I have never had an issue with them connecting to any service or my local plex server that sits on another vlan.

The fix should be if you set any as your protocol you shouldn't be trying to set tcp state flags in that rule - imho.

[luckman212]

@johnpoz I do log & block on my IoT network because I like to see all the Chinese IPs that this crap phones home to. So yeah I guess as a side effect my AppleTV got caught in the same web. Not sure what it was trying to do either, but it was annoying enough to see in my morning log review that I set out to fix it. I suppose I could set up a whole separate "A_Little_More_Trusted_IoT VLAN" and put the ATV in there, but my firewall rules are already more complicated than they probably should be, and I have diminishing amounts of energy after my day job is done to fiddle.

[Patch]

@luckman212 Sorry you had to waste time rediscovering a known bug. Clearly you are not the only one who has been caught by this bug.

Thank you for generating a PR to fix it.

I guess it's up to Netgate to decide if they use it to allow pfsense to implement a filter base on currently permitted data entry or update input validation to prevent similar data entry.

IMO either would be better than the current situation.