pot. Bug(s) with Interface Groups & firewall rules

JeGr · Nov 18, 2021, 8:48 PM

As @jimp closed my ticket about this (https://redmine.pfsense.org/issues/12529) very quickly, here we go again.

Yes, I didn't supply that much information because the problem was/is still very easy to rebuild on a handful of boxes I have available, be it pfSense plus (21.05.1) or CE (2.5.2).

The first bug IMHO was tested like this:

Interface Assignments, Interface Groups
Create Group with name "0Test", added 3 Vlans to it
Got to Firewall/Rules
Add+ rule on 0Test Tab
add a simple rule (pass from 1.2.3.4 port any to any)
save that rule (https://i.imgur.com/x5ri5hS.png)
you get to the 0Test tab with NO rule visible but you get an red error bubble immediatly

Message: pfSense is restoring the configuration /cf/conf/backup/config-1637265144.xml @ 2021-11-18 20:58:19

System log shows errors:

Nov 18 20:58:19    php-fpm    13739    /firewall_rules_edit.php: New alert found: pfSense is restoring the configuration /cf/conf/backup/config-1637265144.xml
Nov 18 20:58:19    php-fpm    13739    /firewall_rules_edit.php: pfSense is restoring the configuration /cf/conf/backup/config-1637265144.xml
Nov 18 20:58:19    php-fpm    13739    /firewall_rules_edit.php: XML error: XML_ERR_NAME_REQUIRED at line 8487 in /conf/config.xml

Same behavior on my testbox with SG2100 on 21.05.1

So after further taking a dive in this, I checked the group assignment. That seems fine, I can see the groups tag on the three interfaces I added to the group:

... 
       groups: 0Test

But after checking the /tmp/rules.debug there's no sign of the Group "0Test " anywhere.
A grep 0Test /tmp/rules.debug shows no results.

OK so perhaps it won't show up until there's a rule for that IF Group. Then if you want to add one, the above happens. No chance to add a rule, it throws errors every time. Not so nice. So we go to the interface group and rename it to "ATest".

Checking interfaces on the shell again - the group tag shows correctly on the interfaces assigned. Good. Then check rules.debug - still no sign of "ATest". Then let's add a rule:

Huh, nice. Working! Let's check the rules.debug:

#> grep ATest /tmp/rules.debug
ATest = "{ ATest }"
pass  in  quick  on $ATest inet proto tcp  from 1.2.3.4 to any tracker 1637267246 flags S/SA keep state  label "USER_RULE"

Aha. There's the group, there's the rule. Works as intended. How about we rename that to 0Testagain?

Rule still shows up on that 0Test tab. But...:

[2.5.2-RELEASE][root@]/: grep 0Test /tmp/rules.debug
[2.5.2-RELEASE][root@]/: grep ATest /tmp/rules.debug
ATest = "{ ATest }"
pass  in  quick  on $ATest inet proto tcp  from 1.2.3.4 to any tracker 1637267246 flags S/SA keep state  label "USER_RULE"

Huh, the rules still show the group named "ATest" and not "0Test" but the interfaces have switched groups to "0Test".

        inet 172.27.1.1 netmask 0xffffff00 broadcast 172.27.1.255
        groups: 0Test
        media: Ethernet autoselect (1000baseT <full-duplex>)
--
        inet 172.27.2.1 netmask 0xffffff00 broadcast 172.27.2.255
        groups: vlan 0Test
        vlan: 272 vlanpcp: 0 parent interface: igb2
--
        inet 172.27.3.1 netmask 0xffffff00 broadcast 172.27.3.255
        groups: vlan 0Test
        vlan: 273 vlanpcp: 0 parent interface: igb2

So perhaps the rules were not saved correctly after renaming the group? So I went to the dummy 1.2.3.4 rule, clicked edit and just saved it again. Oh wonder - it saved without error this time. Checking rules.debug again:

0Test = "{ 0Test }"
pass  in  quick  on $0Test inet proto tcp  from 1.2.3.4 to any tracker 1637267246 flags S/SA keep state  label "USER_RULE"

Huh, now it shows up correct.

So it seems to me, that there's a problem with groups starting with a digit not saved and synced correctly. If you rename it to text only, that seems to work correct, you can add a rule etc. without problems. But if you start with a groupname with a digit that throws the syslog errors and problems described. Also it seems that once you rename it back to a groupname with a starting digit and make a rulechange that saves and applies correctly in the ruleset, the group name gets written correctly and - after that - you can add further rules normally. Adding a second group with a starting digit and trying to create a first rule there fails with the exact same problems while the other group tab still seems to work. So it's something related to group name saving in the filter configuration.

Cheers
\jens

jimp · Nov 18, 2021, 8:54 PM

I tried on several systems and couldn't replicate it at all even with your settings.

But I finally did find one, the problem appears to be separators. They use the interface name as an XML tag name and those cannot start with numbers:

                <separator>
                        <wan></wan>
                        <openvpn></openvpn>
                        <wireguard></wireguard>
                        <0test></0test>
                </separator>

But things like that are why we need so much more information than what you supplied in the Redmine issue, even if you felt it was easy to replicate that does not mean anyone else can. I start with a fresh install (or nearly so) of pfSense when I attempt to replicate things like this.

JeGr · Nov 18, 2021, 9:00 PM

Ah additions:

After checking that I know can after renaming and renaming again add rules to 0Test, I can't remove them. Same error messages, same syslog messages.

Nov 18 21:58:25 mirage php-fpm[93542]: /firewall_rules_edit.php: XML error: XML_ERR_NAME_REQUIRED at line 8584 in /conf/config.xml
Nov 18 21:58:25 mirage php-fpm[93542]:
Nov 18 21:58:26 mirage php-fpm[93542]: /firewall_rules_edit.php: pfSense is restoring the configuration /cf/conf/backup/config-1637269073.xml
Nov 18 21:58:26 mirage php-fpm[93542]: /firewall_rules_edit.php: New alert found: pfSense is restoring the configuration /cf/conf/backup/config-1637269073.xml

as soon as I rename the group name from "0test" or "0blah" (also tested that) to "ablah" it works again, I can create and delete rules without problems. After changing it back to "0blah" I can no longer create them and get the above error with differing line numbers.

JeGr · Nov 18, 2021, 9:09 PM

@jimp said in pot. Bug(s) with Interface Groups & firewall rules:

I tried on several systems and couldn't replicate it at all even with your settings.

But I finally did find one, the problem appears to be separators. They use the interface name as an XML tag name and those cannot start with numbers:
                <separator>
                        <wan></wan>
                        <openvpn></openvpn>
                        <wireguard></wireguard>
                        <0test></0test>
                </separator>
But things like that are why we need so much more information than what you supplied in the Redmine issue, even if you felt it was easy to replicate that does not mean anyone else can. I start with a fresh install (or nearly so) of pfSense when I attempt to replicate things like this.

Separators could be a good guess but I didn't mention them as the interface group doesn't have one. But yes, on the systems I tested with there were separators on other interfaces as we always use them for better rule grouping.

I tested again and could replicate that scenario: it seems that as soon as only one separator is created, the XML file has the separator tag and the interfaces and groupd inside it. So if you don't ever create separators, it seems the problem doesn't exist. As even our lab systems have them, that didn't jump into my mind

But things like that are why we need so much more information than what you supplied in the Redmine issue, even if you felt it was easy to replicate that does not mean anyone else can. I start with a fresh install (or nearly so) of pfSense when I attempt to replicate things like this.

Yeah you're right on that one. As said, our lab systems all have a sort-of lab installation on them (with CA etc.) and a minimal ruleset. And the Test HW (2100) did too, so a full reset would have thrown that problem. And you're also right in wanting more information, I didn't include the errors from syslog in the initial ticket. So shame on me, should have known better indeed.

The error log from system.log shows a line number that's near the separator section so it seems we found the culprit here -.- Could've been easier in the first run :/ So: sorry, got your minimal reply on the wrong foot here but you were right in wanting more intel.

Cheers
\jens

jimp · Nov 18, 2021, 9:04 PM

@jegr said in pot. Bug(s) with Interface Groups & firewall rules:

Separators could be a good guess but I didn't mention them as the interface group doesn't have one. But yes, on the systems I tested with there were separators on other interfaces as we always use them for better rule grouping.

It's not a guess, it's definitive. It's what xmllint flagged as invalid XML which triggered the config rollback.

JeGr · Nov 18, 2021, 9:10 PM

@jimp said in pot. Bug(s) with Interface Groups & firewall rules:

@jegr said in pot. Bug(s) with Interface Groups & firewall rules:

Separators could be a good guess but I didn't mention them as the interface group doesn't have one. But yes, on the systems I tested with there were separators on other interfaces as we always use them for better rule grouping.

It's not a guess, it's definitive. It's what xmllint flagged as invalid XML which triggered the config rollback.

Just edited my post above, sorry.

Seems easiest way would be to limit groups to not only disallow them ending with a digit but also starting with one.