Project

General

Profile

Actions

Bug #12529

closed

Interface group name starting with a digit creates invalid XML for rule separators

Added by Jens Groh over 2 years ago. Updated over 2 years ago.

Status:
Resolved
Priority:
Very Low
Assignee:
Viktor Gurov
Category:
Rules / NAT
Target version:
Start date:
Due date:
% Done:

100%

Estimated time:
Plus Target Version:
22.01
Release Notes:
Default
Affected Version:
2.5.2
Affected Architecture:
amd64

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

clipboard-202111181525-fvlzj.png (28.9 KB) clipboard-202111181525-fvlzj.png Jim Pingle, 11/18/2021 02:25 PM
Actions #1

Updated by Jim Pingle over 2 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.

Actions #2

Updated by Jens Groh over 2 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

Actions #3

Updated by Jim Pingle over 2 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" 

Actions #4

Updated by Jens Groh over 2 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.

Actions #5

Updated by Jim Pingle over 2 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.

Actions #6

Updated by Jens Groh over 2 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!

Actions #7

Updated by Viktor Gurov over 2 years ago

Actions #8

Updated by Jim Pingle over 2 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
Actions #9

Updated by Viktor Gurov over 2 years ago

  • Status changed from Pull Request Review to Feedback
  • % Done changed from 0 to 100
Actions #10

Updated by Danilo Zrenjanin over 2 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.

Actions

Also available in: Atom PDF