From 905f7eb62dd31a58b86fbfa191b2ce2482361b0b Mon Sep 17 00:00:00 2001
From: Eric Garver <eric@garver.life>
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 @@
</listitem>
</varlistentry>
<varlistentry id="FirewallD1.Methods.getServiceSettings">
- <term><methodname>getServiceSettings</methodname>(s: <parameter>service</parameter>) → (sssa(ss)asa{ss}asa(ss)as)</term>
+ <term><methodname>getServiceSettings</methodname>(s: <parameter>service</parameter>) → (sssa(ss)asa{ss}asa(ss))</term>
<listitem>
<para>
Return runtime settings of given <replaceable>service</replaceable>.
For getting permanent settings see <link linkend="FirewallD1.config.service.Methods.getSettings">org.fedoraproject.FirewallD1.config.service.Methods.getSettings</link>.
- Settings are in format: <parameter>version</parameter>, <parameter>name</parameter>, <parameter>description</parameter>, array of <parameter>ports</parameter> (port, protocol), array of <parameter>module names</parameter>, dictionary of <parameter>destinations</parameter>, array of <parameter>protocols</parameter>, array of <parameter>source-ports</parameter> (port, protocol) and array of service <parameter>includes</parameter>.
+ Settings are in format: <parameter>version</parameter>, <parameter>name</parameter>, <parameter>description</parameter>, array of <parameter>ports</parameter> (port, protocol), array of <parameter>module names</parameter>, dictionary of <parameter>destinations</parameter>, array of <parameter>protocols</parameter>, array of <parameter>source-ports</parameter> (port, protocol).
</para>
<para>
<variablelist>
@@ -259,7 +259,6 @@
<varlistentry><term><parameter>destinations (a{ss})</parameter>: dictionary of {IP family : IP address} where 'IP family' key can be either 'ipv4' or 'ipv6'. See <literal>destination</literal> tag in <citerefentry><refentrytitle>firewalld.service</refentrytitle><manvolnum>5</manvolnum></citerefentry>.</term></varlistentry>
<varlistentry><term><parameter>protocols (as)</parameter>: array of protocols, see <literal>protocol</literal> tag in <citerefentry><refentrytitle>firewalld.service</refentrytitle><manvolnum>5</manvolnum></citerefentry>.</term></varlistentry>
<varlistentry><term><parameter>source-ports (a(ss))</parameter>: array of port and protocol pairs. See <literal>source-port</literal> tag in <citerefentry><refentrytitle>firewalld.service</refentrytitle><manvolnum>5</manvolnum></citerefentry>.</term></varlistentry>
- <varlistentry><term><parameter>includes (as)</parameter>: array of service includes, see <literal>include</literal> tag in <citerefentry><refentrytitle>firewalld.service</refentrytitle><manvolnum>5</manvolnum></citerefentry>.</term></varlistentry>
</variablelist>
</para>
<para>
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