Bug #12529
closed
Interface group name starting with a digit creates invalid XML for rule separators
100%
Description
Tested on: 2.5.2 as well as plus-25.01
As per the definition of Interface groups, group names may have digits in it and even start with one (for sorting purposes):
Only letters (A-Z), digits (0-9) and '_' are allowed. The group name cannot end with a digit.
Creating groups like that is fine, however if you create a firewall rule on such a group, it gets an immediate rollback error, the rule is discarded and the last config reinstated.
Could this be fixed please as numbers seem the only good way to structure interface groups in case of "sorting" them so they follow a specific order (as my last observation was, that groups are written to pf.conf in a sorted order and not the order they are created in - which is good!). If you need mulitple groups and want to create a specific flow that you can rely on (group 1_test, 2_foo, 3_bar) and to have rules in the right order, it's a great helper.
Cheers
\jens
Files
Updated by Jim Pingle about 3 years ago
- Status changed from New to Rejected
I can't replicate this as stated and there isn't nearly enough detail to guess what might be happening in your environment. I can use groups starting with a digit without error. If the config rolls back that's typically from a non-XML-safe character being used (like an international accented character, unicode, etc) in a field that doesn't get CDATA escaped, not from a leading digit.
This site is not for support or diagnostic discussion.
For assistance in solving problems, please post on the Netgate Forum and include the full error message along with relevant portions of your configuration.
See Reporting Issues with pfSense Software for more information.
Updated by Jens Groh about 3 years ago
I didn't think it would be hard to reproduce. Nice if it works for you, Jim, but no it is nothing about special characters.
I just did test it again on different systems - same problem again:
1) Interface Assignments, Interface Groups
2) Create Group with name "0Test", added 3 Vlans to it
3) Got to Firewall/Rules
4) Add+ Rule on 0Test Tab
5) add simplest rule (pass from 1.2.3.4 port any to any - simple)
6) save (https://i.imgur.com/x5ri5hS.png)
7) you get to the 0Rule 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
When you check the rules.debug file in /tmp, you'll find no trace of an Alias or IF Group named "0Test". If you go to the Interface Group and rename it to e.g. "ATest" and do the steps again -> all working. Rule gets saved. If you rename the Group back to "0Test" the rule is still there, but if you try to save you get an error again. If you check the /tmp/rules.debug the pf.conf still has the last working Group name (ATest) in it, 0Test wasn't even correctly saved and written to rules.debug or pf.conf.
So if that works for you, great but I'm very much interested in how that is a problem with special character or umlauts or something along those lines if everything we did while testing was use the exact allowed character set as quoted from the group interface page. :)
Cheers
Updated by Jim Pingle about 3 years ago
Maybe it's already been fixed on 22.01 then. I get a rule in the GUI tab and in /tmp/rules.debug.
0blah = "{ 0blah }"
pass in quick on $0blah inet proto tcp from any to any tracker 1637262430 flags S/SA keep state label "USER_RULE: blah test"
Updated by Jens Groh about 3 years ago
Please have a look at https://forum.netgate.com/topic/167988/pot-bug-s-with-interface-groups-firewall-rules
I did a further check and I can reproduce the problems on either 21.05.x and 2.5.2. Starting a group name with a digit initially like your 0blah throws errors and problems if you want to add the first rule. If you manage to make a save/apply that actually writes the filter config, then it seems the group name gets written down (what seems to be missing initially) and rules can be created without problems. But a groupname starting with a digit that gets freshly created seems to miss some pointers as it somehow writes the groups tag on the interfaces correctly but seems to have problems with the first rule being written to rules.debug and the filter config.
Updated by Jim Pingle about 3 years ago
- Subject changed from creation of groups numbers allowed but rules create errors and rollbacks to Interface group name starting with a digit creates invalid XML for rule separators
- Status changed from Rejected to Confirmed
- Priority changed from Normal to Very Low
That isn't quite right either, see my reply on your forum thread. The problem is actually separators being in the configuration:
<separator>
<wan></wan>
<openvpn></openvpn>
<wireguard></wireguard>
<0test></0test>
</separator>
The XML tag name can't start with a digit, so it is invalid XML. Whatever you have done with edits/renames probably only prolonged the issue until it happens to update the separators again.
: xmllint /conf/config.xml.bad
/conf/config.xml.bad:965: parser error : StartTag: invalid element name
<0test></0test>
^
/conf/config.xml.bad:965: parser error : expected '>'
<0test></0test>
^
/conf/config.xml.bad:965: parser error : Opening and ending tag mismatch: separator line 961 and unparseable
<0test></0test>
^
[...]
There may not be a good way around that other than preventing groups from starting with a digit. Seems like changing the way separators are handled may be too drastic to accommodate them, but that's not code I've worked with so I'll defer to others there.
Updated by Jens Groh about 3 years ago
I'd agree that simply disallowing them to start and end with a digit would be easier. Even if that means that a great way of adding some sorting to groups would be unavailable then.
Sorry about not supplying enough data in the first place. Should have known better myself as that's what I always tell my customers, too. Not my best day it seems. No offence meant!
Updated by Viktor Gurov about 3 years ago
input validation improvements:
https://gitlab.netgate.com/pfSense/pfSense/-/merge_requests/468
Updated by Jim Pingle almost 3 years ago
- Status changed from Confirmed to Pull Request Review
- Assignee set to Viktor Gurov
- Target version set to 2.6.0
- Plus Target Version set to 22.01
Updated by Viktor Gurov almost 3 years ago
- Status changed from Pull Request Review to Feedback
- % Done changed from 0 to 100
Applied in changeset b58cb30a0881a221c9c5ff1eb5752ac0660271b9.
Updated by Danilo Zrenjanin almost 3 years ago
- Status changed from Feedback to Resolved
Tested against:
2.6.0-DEVELOPMENT (amd64) built on Tue Nov 23 06:23:34 UTC 2021 FreeBSD 12.3-PRERELEASE
The input validation works as expected. Ticket resolved.