From 9e4bf24e1e0a5d54398d2220f0a5217eff0704a7 Mon Sep 17 00:00:00 2001
From: Eric Garver <e@erig.me>
Date: Mon, 13 Aug 2018 16:53:46 -0400
Subject: [PATCH 3/4] fw: if failure occurs during startup set state to FAILED
Previously if a failure occurred at startup firewalld would get stuck in
INIT state and the policy would remain as "DROP". This commit changes
that behavior and introduces a new state "FAILED", which means a failure
occurred and we're running in a failed state. Policy is set to "ACCEPT"
so as to hopefully prevent locking out an admin.
(cherry picked from commit f475bd2293b7ba01ad4b56b68bef1b61d01526f0)
---
doc/xml/firewall-cmd.xml.in | 2 +-
doc/xml/firewalld.dbus.xml | 2 +-
src/firewall-cmd | 2 +
src/firewall/core/fw.py | 131 +++++++++++++++-------------
src/firewall/errors.py | 1 +
src/tests/regression/rhbz1498923.at | 8 +-
6 files changed, 83 insertions(+), 63 deletions(-)
diff --git a/doc/xml/firewall-cmd.xml.in b/doc/xml/firewall-cmd.xml.in
index 32c89591db86..c2606553e549 100644
--- a/doc/xml/firewall-cmd.xml.in
+++ b/doc/xml/firewall-cmd.xml.in
@@ -118,7 +118,7 @@
<term><option>--state</option></term>
<listitem>
<para>
- Check whether the firewalld daemon is active (i.e. running). Returns an exit code 0 if it is active, <replaceable>NOT_RUNNING</replaceable> otherwise (see <xref linkend="exit_codes"/>). This will also print the state to <replaceable>STDOUT</replaceable>.
+ Check whether the firewalld daemon is active (i.e. running). Returns an exit code 0 if it is active, <replaceable>RUNNING_BUT_FAILED</replaceable> if failure occurred on startup, <replaceable>NOT_RUNNING</replaceable> otherwise. See <xref linkend="exit_codes"/>. This will also print the state to <replaceable>STDOUT</replaceable>.
</para>
</listitem>
</varlistentry>
diff --git a/doc/xml/firewalld.dbus.xml b/doc/xml/firewalld.dbus.xml
index acdbb5fd6e00..ec82d4cad077 100644
--- a/doc/xml/firewalld.dbus.xml
+++ b/doc/xml/firewalld.dbus.xml
@@ -488,7 +488,7 @@
</varlistentry>
<varlistentry id="FirewallD1.Properties.state">
<term><parameter>state</parameter> - s - (ro)</term>
- <listitem><para>firewalld state. This can be either <literal>INIT</literal> or <literal>RUNNING</literal>. In <literal>INIT</literal> state, firewalld is starting up and initializing.</para></listitem>
+ <listitem><para>firewalld state. This can be either <literal>INIT</literal>, <literal>FAILED</literal>, or <literal>RUNNING</literal>. In <literal>INIT</literal> state, firewalld is starting up and initializing. In <literal>FAILED</literal> state, firewalld completely started but experienced a failure.</para></listitem>
</varlistentry>
<varlistentry id="FirewallD1.Properties.version">
<term><parameter>version</parameter> - s - (ro)</term>
diff --git a/src/firewall-cmd b/src/firewall-cmd
index b80115564e1b..12e18bb88a54 100755
--- a/src/firewall-cmd
+++ b/src/firewall-cmd
@@ -2022,6 +2022,8 @@ elif a.state:
state = fw.get_property("state")
if state == "RUNNING":
cmd.print_and_exit ("running")
+ elif state == "FAILED":
+ cmd.print_and_exit("failed", errors.RUNNING_BUT_FAILED)
else:
cmd.print_and_exit ("not running", errors.NOT_RUNNING)
elif a.get_log_denied:
diff --git a/src/firewall/core/fw.py b/src/firewall/core/fw.py
index 1ff36f18cd99..5b706d6d3e80 100644
--- a/src/firewall/core/fw.py
+++ b/src/firewall/core/fw.py
@@ -451,11 +451,16 @@ class Firewall(object):
tm2 = time.time()
log.debug2("Flushing and applying took %f seconds" % (tm2 - tm1))
- self._state = "RUNNING"
-
def start(self):
- self._start()
- self.set_policy("ACCEPT")
+ try:
+ self._start()
+ except Exception:
+ self._state = "FAILED"
+ self.set_policy("ACCEPT")
+ raise
+ else:
+ self._state = "RUNNING"
+ self.set_policy("ACCEPT")
def _loader(self, path, reader_type, combine=False):
# combine: several zone files are getting combined into one obj
@@ -905,64 +910,70 @@ class Firewall(object):
def reload(self, stop=False):
_panic = self._panic
- # save zone interfaces
- _zone_interfaces = { }
- for zone in self.zone.get_zones():
- _zone_interfaces[zone] = self.zone.get_settings(zone)["interfaces"]
- # save direct config
- _direct_config = self.direct.get_runtime_config()
- _old_dz = self.get_default_zone()
-
- # stop
- self.cleanup()
-
- self.set_policy("DROP")
-
- # start
- self._start(reload=True, complete_reload=stop)
-
- # handle interfaces in the default zone and move them to the new
- # default zone if it changed
- _new_dz = self.get_default_zone()
- if _new_dz != _old_dz:
- # if_new_dz has been introduced with the reload, we need to add it
- # https://github.com/firewalld/firewalld/issues/53
- if _new_dz not in _zone_interfaces:
- _zone_interfaces[_new_dz] = { }
- # default zone changed. Move interfaces from old default zone to
- # the new one.
- for iface, settings in list(_zone_interfaces[_old_dz].items()):
- if settings["__default__"]:
- # move only those that were added to default zone
- # (not those that were added to specific zone same as
- # default)
- _zone_interfaces[_new_dz][iface] = \
- _zone_interfaces[_old_dz][iface]
- del _zone_interfaces[_old_dz][iface]
-
- # add interfaces to zones again
- for zone in self.zone.get_zones():
- if zone in _zone_interfaces:
- self.zone.set_settings(zone, { "interfaces":
- _zone_interfaces[zone] })
- del _zone_interfaces[zone]
+ try:
+ # save zone interfaces
+ _zone_interfaces = { }
+ for zone in self.zone.get_zones():
+ _zone_interfaces[zone] = self.zone.get_settings(zone)["interfaces"]
+ # save direct config
+ _direct_config = self.direct.get_runtime_config()
+ _old_dz = self.get_default_zone()
+
+ # stop
+ self.cleanup()
+
+ self.set_policy("DROP")
+
+ self._start(reload=True, complete_reload=stop)
+
+ # handle interfaces in the default zone and move them to the new
+ # default zone if it changed
+ _new_dz = self.get_default_zone()
+ if _new_dz != _old_dz:
+ # if_new_dz has been introduced with the reload, we need to add it
+ # https://github.com/firewalld/firewalld/issues/53
+ if _new_dz not in _zone_interfaces:
+ _zone_interfaces[_new_dz] = { }
+ # default zone changed. Move interfaces from old default zone to
+ # the new one.
+ for iface, settings in list(_zone_interfaces[_old_dz].items()):
+ if settings["__default__"]:
+ # move only those that were added to default zone
+ # (not those that were added to specific zone same as
+ # default)
+ _zone_interfaces[_new_dz][iface] = \
+ _zone_interfaces[_old_dz][iface]
+ del _zone_interfaces[_old_dz][iface]
+
+ # add interfaces to zones again
+ for zone in self.zone.get_zones():
+ if zone in _zone_interfaces:
+ self.zone.set_settings(zone, { "interfaces":
+ _zone_interfaces[zone] })
+ del _zone_interfaces[zone]
+ else:
+ log.info1("New zone '%s'.", zone)
+ if len(_zone_interfaces) > 0:
+ for zone in list(_zone_interfaces.keys()):
+ log.info1("Lost zone '%s', zone interfaces dropped.", zone)
+ del _zone_interfaces[zone]
+ del _zone_interfaces
+
+ # restore direct config
+ self.direct.set_config(_direct_config)
+
+ # enable panic mode again if it has been enabled before or set policy
+ # to ACCEPT
+ if _panic:
+ self.enable_panic_mode()
else:
- log.info1("New zone '%s'.", zone)
- if len(_zone_interfaces) > 0:
- for zone in list(_zone_interfaces.keys()):
- log.info1("Lost zone '%s', zone interfaces dropped.", zone)
- del _zone_interfaces[zone]
- del _zone_interfaces
-
- # restore direct config
- self.direct.set_config(_direct_config)
-
- # enable panic mode again if it has been enabled before or set policy
- # to ACCEPT
- if _panic:
- self.enable_panic_mode()
- else:
+ self.set_policy("ACCEPT")
+
+ self._state = "RUNNING"
+ except Exception:
+ self._state = "FAILED"
self.set_policy("ACCEPT")
+ raise
# STATE
diff --git a/src/firewall/errors.py b/src/firewall/errors.py
index 1cd604884c99..63d007191ffa 100644
--- a/src/firewall/errors.py
+++ b/src/firewall/errors.py
@@ -97,6 +97,7 @@ MISSING_NAME = 205
MISSING_SETTING = 206
MISSING_FAMILY = 207
+RUNNING_BUT_FAILED = 251
NOT_RUNNING = 252
NOT_AUTHORIZED = 253
UNKNOWN_ERROR = 254
diff --git a/src/tests/regression/rhbz1498923.at b/src/tests/regression/rhbz1498923.at
index 505a523d5cc4..bb0d841db2a7 100644
--- a/src/tests/regression/rhbz1498923.at
+++ b/src/tests/regression/rhbz1498923.at
@@ -1,5 +1,11 @@
FWD_START_TEST([invalid direct rule causes reload error])
FWD_CHECK([-q --permanent --direct --add-rule ipv4 filter INPUT 0 -p tcp --dport 8080 -j ACCEPT])
FWD_CHECK([-q --permanent --direct --add-rule ipv4 filter INPUT 1 --a-bogus-flag])
-FWD_RELOAD(13, [ignore], [ignore])
+FWD_RELOAD(13, [ignore], [ignore], 251)
+FWD_CHECK([--state], 251, [ignore], [failed
+])
+
+dnl now remove the bad rule and reload successfully
+FWD_CHECK([-q --permanent --direct --remove-rule ipv4 filter INPUT 1 --a-bogus-flag])
+FWD_RELOAD
FWD_END_TEST([-e '/.*a-bogus-flag.*/d'])
--
2.18.0