From 905f7eb62dd31a58b86fbfa191b2ce2482361b0b Mon Sep 17 00:00:00 2001 From: Eric Garver Date: Mon, 24 Jun 2019 10:36:40 -0400 Subject: [PATCH 10/20] fix: dbus: fix service API break This fixes a dbus API break that occurred when introducing service includes. The includes were added to the method's tuple, but doing so changed the dbus signature and thus broke the API. This restores the old signature. Move to using key,value based import/export and sanity checking. Previously we were using a tuple with semi-undocumented positions. Fixes: 1fc208bf9317 ("feat: service includes") Fixes: rhbz 1721414 (cherry picked from commit 335a68c1bba5b1b1fbd430505a485a9eb035360c) --- doc/xml/firewalld.dbus.xml | 5 ++- src/firewall/core/fw_config.py | 59 +++++++++++++++++++++++++++++++- src/firewall/core/io/service.py | 35 ++++++++++++++++--- src/firewall/server/firewalld.py | 13 ++++++- 4 files changed, 103 insertions(+), 9 deletions(-) diff --git a/doc/xml/firewalld.dbus.xml b/doc/xml/firewalld.dbus.xml index 64d4d2b9c73b..cb4e1eac0fb9 100644 --- a/doc/xml/firewalld.dbus.xml +++ b/doc/xml/firewalld.dbus.xml @@ -242,12 +242,12 @@ - getServiceSettings(s: service) → (sssa(ss)asa{ss}asa(ss)as) + getServiceSettings(s: service) → (sssa(ss)asa{ss}asa(ss)) Return runtime settings of given service. For getting permanent settings see org.fedoraproject.FirewallD1.config.service.Methods.getSettings. - Settings are in format: version, name, description, array of ports (port, protocol), array of module names, dictionary of destinations, array of protocols, array of source-ports (port, protocol) and array of service includes. + Settings are in format: version, name, description, array of ports (port, protocol), array of module names, dictionary of destinations, array of protocols, array of source-ports (port, protocol). @@ -259,7 +259,6 @@ destinations (a{ss}): dictionary of {IP family : IP address} where 'IP family' key can be either 'ipv4' or 'ipv6'. See destination tag in firewalld.service5. protocols (as): array of protocols, see protocol tag in firewalld.service5. source-ports (a(ss)): array of port and protocol pairs. See source-port tag in firewalld.service5. - includes (as): array of service includes, see include tag in firewalld.service5. diff --git a/src/firewall/core/fw_config.py b/src/firewall/core/fw_config.py index a759cfdf83b3..8f29f0c416d2 100644 --- a/src/firewall/core/fw_config.py +++ b/src/firewall/core/fw_config.py @@ -545,9 +545,43 @@ class FirewallConfig(object): return self._builtin_services[obj.name] def get_service_config(self, obj): + conf_dict = obj.export_config() + conf_list = [] + for i in range(8): # tuple based dbus API has 8 elements + if obj.IMPORT_EXPORT_STRUCTURE[i][0] not in conf_dict: + # old API needs the empty elements as well. Grab it from the + # object otherwise we don't know the type. + conf_list.append(copy.deepcopy(getattr(obj, obj.IMPORT_EXPORT_STRUCTURE[i][0]))) + else: + conf_list.append(conf_dict[obj.IMPORT_EXPORT_STRUCTURE[i][0]]) + return tuple(conf_list) + + def get_service_config_dict(self, obj): return obj.export_config() def set_service_config(self, obj, conf): + conf_dict = {} + for i,value in enumerate(conf): + conf_dict[obj.IMPORT_EXPORT_STRUCTURE[i][0]] = value + + if obj.builtin: + x = copy.copy(obj) + x.cleanup() + x.import_config(conf_dict) + x.path = config.ETC_FIREWALLD_SERVICES + x.builtin = False + if obj.path != x.path: + x.default = False + self.add_service(x) + service_writer(x) + return x + else: + obj.cleanup() + obj.import_config(conf_dict) + service_writer(obj) + return obj + + def set_service_config_dict(self, obj, conf): if obj.builtin: x = copy.copy(obj) x.import_config(conf) @@ -568,6 +602,29 @@ class FirewallConfig(object): raise FirewallError(errors.NAME_CONFLICT, "new_service(): '%s'" % name) + conf_dict = {} + for i,value in enumerate(conf): + conf_dict[Service.IMPORT_EXPORT_STRUCTURE[i][0]] = value + + x = Service() + x.check_name(name) + x.import_config(conf_dict) + x.name = name + x.filename = "%s.xml" % name + x.path = config.ETC_FIREWALLD_SERVICES + # It is not possible to add a new one with a name of a buitin + x.builtin = False + x.default = True + + service_writer(x) + self.add_service(x) + return x + + def new_service_dict(self, name, conf): + if name in self._services or name in self._builtin_services: + raise FirewallError(errors.NAME_CONFLICT, + "new_service(): '%s'" % name) + x = Service() x.check_name(name) x.import_config(conf) @@ -684,7 +741,7 @@ class FirewallConfig(object): return new_service def _copy_service(self, obj, name): - return self.new_service(name, obj.export_config()) + return self.new_service_dict(name, obj.export_config()) # zones diff --git a/src/firewall/core/io/service.py b/src/firewall/core/io/service.py index 3479dab7f175..44dc0ff8a9b0 100644 --- a/src/firewall/core/io/service.py +++ b/src/firewall/core/io/service.py @@ -25,6 +25,8 @@ import xml.sax as sax import os import io import shutil +import copy +from collections import OrderedDict from firewall import config from firewall.functions import u2b_if_py2 @@ -47,7 +49,7 @@ class Service(IO_Object): ( "source_ports", [ ( "", "" ), ], ), # a(ss) ( "includes", [ "" ], ), # as ) - DBUS_SIGNATURE = '(sssa(ss)asa{ss}asa(ss)as)' + DBUS_SIGNATURE = '(sssa(ss)asa{ss}asa(ss))' ADDITIONAL_ALNUM_CHARS = [ "_", "-" ] PARSER_REQUIRED_ELEMENT_ATTRS = { "short": None, @@ -76,6 +78,34 @@ class Service(IO_Object): self.source_ports = [ ] self.includes = [ ] + def import_config(self, conf): + self.check_config(conf) + + for key in conf: + if not hasattr(self, key): + raise FirewallError(errors.UNKNOWN_ERROR, "Internal error. '{}' is not a valid attribute".format(key)) + if isinstance(conf[key], list): + # maintain list order while removing duplicates + setattr(self, key, list(OrderedDict.fromkeys(copy.deepcopy(conf[key])))) + else: + setattr(self, key, copy.deepcopy(conf[key])) + + def export_config(self): + conf = {} + type_formats = dict([(x[0], x[1]) for x in self.IMPORT_EXPORT_STRUCTURE]) + for key in type_formats: + if getattr(self, key): + conf[key] = copy.deepcopy(getattr(self, key)) + return conf + + def check_config(self, conf): + type_formats = dict([(x[0], x[1]) for x in self.IMPORT_EXPORT_STRUCTURE]) + for key in conf: + if key not in [x for (x,y) in self.IMPORT_EXPORT_STRUCTURE]: + raise FirewallError(errors.INVALID_OPTION, "service option '{}' is not valid".format(key)) + self._check_config_structure(conf[key], type_formats[key]) + self._check_config(conf[key], key) + def cleanup(self): self.version = "" self.short = "" @@ -138,9 +168,6 @@ class Service(IO_Object): if len(module) < 2: raise FirewallError(errors.INVALID_MODULE, module) - elif item == "includes": - pass - # PARSER class service_ContentHandler(IO_Object_ContentHandler): diff --git a/src/firewall/server/firewalld.py b/src/firewall/server/firewalld.py index bc04f2d0f4c3..233160b64b18 100644 --- a/src/firewall/server/firewalld.py +++ b/src/firewall/server/firewalld.py @@ -26,6 +26,7 @@ from gi.repository import GLib, GObject import sys sys.modules['gobject'] = GObject +import copy import dbus import dbus.service import slip.dbus @@ -921,7 +922,17 @@ class FirewallD(slip.dbus.service.Object): # returns service settings for service service = dbus_to_python(service, str) log.debug1("getServiceSettings(%s)", service) - return self.fw.service.get_service(service).export_config() + obj = self.fw.service.get_service(service) + conf_dict = obj.export_config() + conf_list = [] + for i in range(8): # tuple based dbus API has 8 elements + if obj.IMPORT_EXPORT_STRUCTURE[i][0] not in conf_dict: + # old API needs the empty elements as well. Grab it from the + # object otherwise we don't know the type. + conf_list.append(copy.deepcopy(getattr(obj, obj.IMPORT_EXPORT_STRUCTURE[i][0]))) + else: + conf_list.append(conf_dict[obj.IMPORT_EXPORT_STRUCTURE[i][0]]) + return tuple(conf_list) @slip.dbus.polkit.require_auth(config.dbus.PK_ACTION_INFO) @dbus_service_method(config.dbus.DBUS_INTERFACE, in_signature='', -- 2.20.1