Bug 1477771 - FIrewalld in RHEL 7 doesn't have any option to check the rule set syntax of .xml file contents.
Summary: FIrewalld in RHEL 7 doesn't have any option to check the rule set syntax of ....
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Red Hat Enterprise Linux 7
Classification: Red Hat
Component: firewalld
Version: 7.3
Hardware: Unspecified
OS: Unspecified
high
high
Target Milestone: rc
: ---
Assignee: Eric Garver
QA Contact: Jiri Peska
Mirek Jahoda
URL:
Whiteboard:
Depends On: 1628278
Blocks:
TreeView+ depends on / blocked
 
Reported: 2017-08-02 21:41 UTC by Akhil John
Modified: 2019-08-14 17:11 UTC (History)
7 users (show)

Fixed In Version: firewalld-0.5.3-2.el7
Doc Type: Enhancement
Doc Text:
"firewalld-cmd --check-config" now checks the validity of XML configuration files This update introduces the "--check-config" option for the "firewall-cmd" and "firewall-offline-cmd" commands. The new option checks a user configuration of the *firewalld* daemon in XML files. The verification script reports syntax errors in custom rule definitions if any.
Clone Of:
: 1628278 (view as bug list)
Environment:
Last Closed: 2018-10-30 10:11:40 UTC


Attachments (Terms of Use)


Links
System ID Priority Status Summary Last Updated
Red Hat Product Errata RHBA-2018:3120 None None None 2018-10-30 10:12:19 UTC

Description Akhil John 2017-08-02 21:41:26 UTC
Description of problem:
No option in firewalld to check the rule set syntax of .xml files added. 

Version-Release number of selected component (if applicable):
RHEL 7
FirewallD

How reproducible:
Everytime

Steps to Reproduce:
1.Add or Modify any .xml file in /etc/firewalld/ direcorty

For example, if I add the following line to public.xml
<service name="someservice"/>

then a reload of the rules succeeds.
> firewall-cmd --reload
success

This success message is misleading. journalctl -t firewalld --since=-5m` will show that there were warning or errors. Luckily, most of the time, firewalld will just ignore the invalid rules and add the ones which work. Still, this behaviour is not very useful.

We need to reload the firewalld using #firewall-cmd --reload to fail if there are errors.



If at all we had some option like:

# firewall-cmd --checkconf

which will check the syntax and the configuration in .xml files.

Comment 2 Tomas Dolezal 2018-03-21 16:59:38 UTC
John, isn't xml schema check enough?

# pwd; /usr/lib/firewalld/xmlschema/check.sh 
/etc/firewalld
Checking zones
Checking services
Checking icmptypes
Checking ipsets

Comment 3 Akhil John 2018-03-22 03:17:57 UTC
Hello Tomas Dolezal,

I was wondering will it be possible to include this schema check integrated into a firewall-cmd command, something like:

# firewall-cmd --checkconf

Comment 4 Tomas Dolezal 2018-03-22 12:52:35 UTC
Thanks, I got it. I just wanted to make sure that the schema check is not sufficient for this report.

Comment 5 Eric Garver 2018-05-25 13:49:14 UTC
(In reply to Akhil John from comment #3)
> Hello Tomas Dolezal,
> 
> I was wondering will it be possible to include this schema check integrated
> into a firewall-cmd command, something like:
> 
> # firewall-cmd --checkconf

I've implemented this for firewall-offline-cmd. However, some configuration issues are treated only as warnings; e.g. invalid entry to an ipset. These scenarios would not pass an error back to firewall-cmd (if --checkconf implemented there as well), but would instead print a warning to the firewalld log. Hard errors (invalid XML, etc) would still propagate a real error to firewall-cmd.

Do you think implementing this for firewall-offline-cmd is enough?

Adding support to firewall-cmd may require finding all these "warning" scenarios and translating them into hard errors. I'm hesitant to do that because in the past the reverse happened - they were changed _from_ hard errors to warnings.

Thoughts?

Comment 6 Akhil John 2018-05-28 04:58:02 UTC
(In reply to Eric Garver from comment #5)
 
> I've implemented this for firewall-offline-cmd. However, some configuration
> issues are treated only as warnings; e.g. invalid entry to an ipset. These
> scenarios would not pass an error back to firewall-cmd (if --checkconf
> implemented there as well), but would instead print a warning to the
> firewalld log. Hard errors (invalid XML, etc) would still propagate a real
> error to firewall-cmd.
> 
> Do you think implementing this for firewall-offline-cmd is enough?
> 
> Adding support to firewall-cmd may require finding all these "warning"
> scenarios and translating them into hard errors. I'm hesitant to do that
> because in the past the reverse happened - they were changed _from_ hard
> errors to warnings.
> 
> Thoughts?

The main intention of this Bugzilla was to find the Hard errors like you have mentioned invalid XML, as an initial face what if we can make use of this feature only to check the invalid XML format and later we can work on the complete scenario like "warning" etc. which will give an extra hand to whoever so is using firewalld, which I think will make firewalld more easy to use and transparent in troubleshooting.

And it would be really helpful if we can include this feature with firewall-cmd.

Comment 7 Tomas Dolezal 2018-05-30 16:00:11 UTC
(In reply to Eric Garver from comment #5)
> (In reply to Akhil John from comment #3)
> > Hello Tomas Dolezal,
> > 
> > I was wondering will it be possible to include this schema check integrated
> > into a firewall-cmd command, something like:
> > 
> > # firewall-cmd --checkconf
> 
> I've implemented this for firewall-offline-cmd. However, some configuration
> issues are treated only as warnings; e.g. invalid entry to an ipset. These
> scenarios would not pass an error back to firewall-cmd (if --checkconf
> implemented there as well), but would instead print a warning to the
> firewalld log. Hard errors (invalid XML, etc) would still propagate a real
> error to firewall-cmd.
> 
> Do you think implementing this for firewall-offline-cmd is enough?
For me the offline tool seems very reasonable. Is there something important in runtime to check against so firewall-cmd would be required? For the latter afaik only permissions differ - firewall-offline-cmd can be run only as a root.

> Adding support to firewall-cmd may require finding all these "warning"
> scenarios and translating them into hard errors. I'm hesitant to do that
> because in the past the reverse happened - they were changed _from_ hard
> errors to warnings.
> 
> Thoughts?
Is the required functionality worth changing warning levels and their propagation across the code to satisfy runtime-aware (above) check? It seems unnecessarily risky to me.

John, are there functional or other important requirements of "--checkconf" to be part of runtime-aware management tool?

Comment 8 Eric Garver 2018-05-31 17:28:36 UTC
(In reply to Tomas Dolezal from comment #7)
> (In reply to Eric Garver from comment #5)
> > (In reply to Akhil John from comment #3)
> > > Hello Tomas Dolezal,
> > > 
> > > I was wondering will it be possible to include this schema check integrated
> > > into a firewall-cmd command, something like:
> > > 
> > > # firewall-cmd --checkconf
> > 
> > I've implemented this for firewall-offline-cmd. However, some configuration
> > issues are treated only as warnings; e.g. invalid entry to an ipset. These
> > scenarios would not pass an error back to firewall-cmd (if --checkconf
> > implemented there as well), but would instead print a warning to the
> > firewalld log. Hard errors (invalid XML, etc) would still propagate a real
> > error to firewall-cmd.
> > 
> > Do you think implementing this for firewall-offline-cmd is enough?
> For me the offline tool seems very reasonable. Is there something important
> in runtime to check against so firewall-cmd would be required? For the

Nothing major that I'm away of.

> latter afaik only permissions differ - firewall-offline-cmd can be run only
> as a root.
> 
> > Adding support to firewall-cmd may require finding all these "warning"
> > scenarios and translating them into hard errors. I'm hesitant to do that
> > because in the past the reverse happened - they were changed _from_ hard
> > errors to warnings.
> > 
> > Thoughts?
> Is the required functionality worth changing warning levels and their
> propagation across the code to satisfy runtime-aware (above) check? It seems
> unnecessarily risky to me.

I'll give it a try. Worst case we can move forward with just firewall-offline-cmd.

Comment 9 Eric Garver 2018-06-01 18:40:28 UTC
Support for both firewall-cmd firewall-offline-cmd are upstream. Implemented as the new "--check-config" option.

I believe this is backportable to RHEL-7.6, so granting devel_ack as well.

c2bd43e71018 ("tests/firewall-cmd: exercise --check-config")
b071536beb7e ("firewall-cmd: add --check-config option")
749e64b74cff ("firewall-offline-cmd: add --check-config option")
4164148b88f1 ("firewall/core/io/functions: add check_config()")
ebe0cb93c3f3 ("ipset: check type when parsing ipset definition")

Comment 10 Akhil John 2018-06-04 01:22:16 UTC
(In reply to Tomas Dolezal from comment #7)

> 
> John, are there functional or other important requirements of "--checkconf"
> to be part of runtime-aware management tool?

The main functionality that we are expecting to be added is the firewalld configuration(xml) syntax checking. Like mentioned in the bug description, including the schema check to firewall-cmd.

In addition to the above, adding the features of checking the ERROR, WARNING will provide more clarity for the users. Like a one-stop for checking the config related issue.

Comment 11 Eric Garver 2018-06-04 19:21:57 UTC
Tomas, should you or Akhil qa_ack this?

Comment 12 Eric Garver 2018-06-08 20:20:57 UTC
This has been backported. I'm just waiting on qa_ack+ and pm_ack+ to push.

Comment 19 errata-xmlrpc 2018-10-30 10:11:40 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHBA-2018:3120


Note You need to log in before you can comment on or make changes to this bug.