From 9e4bf24e1e0a5d54398d2220f0a5217eff0704a7 Mon Sep 17 00:00:00 2001 From: Eric Garver 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 @@ - Check whether the firewalld daemon is active (i.e. running). Returns an exit code 0 if it is active, NOT_RUNNING otherwise (see ). This will also print the state to STDOUT. + Check whether the firewalld daemon is active (i.e. running). Returns an exit code 0 if it is active, RUNNING_BUT_FAILED if failure occurred on startup, NOT_RUNNING otherwise. See . This will also print the state to STDOUT. 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 @@ state - s - (ro) - firewalld state. This can be either INIT or RUNNING. In INIT state, firewalld is starting up and initializing. + firewalld state. This can be either INIT, FAILED, or RUNNING. In INIT state, firewalld is starting up and initializing. In FAILED state, firewalld completely started but experienced a failure. version - s - (ro) 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