From df8f14eec61f050d5c03e384ce52958098185bb1 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Thu, 22 Dec 2022 20:04:37 +0100 Subject: [PATCH] app resources: implement logic for port 'exposed' and 'fixed' options --- src/firewall.py | 20 ++++++++++++------ src/tests/test_app_resources.py | 36 +++++++++++++++++++-------------- src/utils/resources.py | 33 ++++++++++++++++++++++-------- 3 files changed, 60 insertions(+), 29 deletions(-) diff --git a/src/firewall.py b/src/firewall.py index 8e0e70e99..eb3c9b009 100644 --- a/src/firewall.py +++ b/src/firewall.py @@ -32,7 +32,7 @@ logger = getActionLogger("yunohost.firewall") def firewall_allow( - protocol, port, ipv4_only=False, ipv6_only=False, no_upnp=False, no_reload=False + protocol, port, ipv4_only=False, ipv6_only=False, no_upnp=False, no_reload=False, reload_only_if_change=False ): """ Allow connections on a port @@ -70,14 +70,18 @@ def firewall_allow( "ipv6", ] + changed = False + for p in protocols: # Iterate over IP versions to add port for i in ipvs: if port not in firewall[i][p]: firewall[i][p].append(port) + changed = True else: ipv = "IPv%s" % i[3] - logger.warning(m18n.n("port_already_opened", port=port, ip_version=ipv)) + if not reload_only_if_change: + logger.warning(m18n.n("port_already_opened", port=port, ip_version=ipv)) # Add port forwarding with UPnP if not no_upnp and port not in firewall["uPnP"][p]: firewall["uPnP"][p].append(port) @@ -89,12 +93,12 @@ def firewall_allow( # Update and reload firewall _update_firewall_file(firewall) - if not no_reload: + if not no_reload or (reload_only_if_change and changed): return firewall_reload() def firewall_disallow( - protocol, port, ipv4_only=False, ipv6_only=False, upnp_only=False, no_reload=False + protocol, port, ipv4_only=False, ipv6_only=False, upnp_only=False, no_reload=False, reload_only_if_change=False ): """ Disallow connections on a port @@ -139,14 +143,18 @@ def firewall_disallow( elif upnp_only: ipvs = [] + changed = False + for p in protocols: # Iterate over IP versions to remove port for i in ipvs: if port in firewall[i][p]: firewall[i][p].remove(port) + changed = True else: ipv = "IPv%s" % i[3] - logger.warning(m18n.n("port_already_closed", port=port, ip_version=ipv)) + if not reload_only_if_change: + logger.warning(m18n.n("port_already_closed", port=port, ip_version=ipv)) # Remove port forwarding with UPnP if upnp and port in firewall["uPnP"][p]: firewall["uPnP"][p].remove(port) @@ -156,7 +164,7 @@ def firewall_disallow( # Update and reload firewall _update_firewall_file(firewall) - if not no_reload: + if not no_reload or (reload_only_if_change and changed): return firewall_reload() diff --git a/src/tests/test_app_resources.py b/src/tests/test_app_resources.py index 779a5f7a2..879f6e29a 100644 --- a/src/tests/test_app_resources.py +++ b/src/tests/test_app_resources.py @@ -11,6 +11,7 @@ from yunohost.utils.resources import ( AppResourceClassesByType, ) from yunohost.permission import user_permission_list, permission_delete +from yunohost.firewall import firewall_list dummyfile = "/tmp/dummyappresource-testapp" @@ -120,21 +121,6 @@ def test_update_dummy(): assert open(dummyfile).read().strip() == "bar" -def test_update_dummy_fail(): - - current = {"resources": {"dummy": {}}} - wanted = {"resources": {"dummy": {"content": "forbiddenvalue"}}} - - open(dummyfile, "w").write("foo") - - assert open(dummyfile).read().strip() == "foo" - with pytest.raises(Exception): - AppResourceManager("testapp", current=current, wanted=wanted).apply( - rollback_and_raise_exception_if_failure=False - ) - assert open(dummyfile).read().strip() == "forbiddenvalue" - - def test_update_dummy_failwithrollback(): current = {"resources": {"dummy": {}}} @@ -276,6 +262,26 @@ def test_resource_ports_several(): assert not app_setting("testapp", "port_foobar") +def test_resource_ports_firewall(): + + r = AppResourceClassesByType["ports"] + conf = {"main": {"default": 12345}} + + r(conf, "testapp").provision_or_update() + + assert 12345 not in firewall_list()["opened_ports"] + + conf = {"main": {"default": 12345, "exposed": "TCP"}} + + r(conf, "testapp").provision_or_update() + + assert 12345 in firewall_list()["opened_ports"] + + r(conf, "testapp").deprovision() + + assert 12345 not in firewall_list()["opened_ports"] + + def test_resource_database(): r = AppResourceClassesByType["database"] diff --git a/src/utils/resources.py b/src/utils/resources.py index 813bf9979..4efefc576 100644 --- a/src/utils/resources.py +++ b/src/utils/resources.py @@ -778,16 +778,16 @@ class PortsResource(AppResource): ##### Properties (for every port name): - `default`: The prefered value for the port. If this port is already being used by another process right now, or is booked in another app's setting, the code will increment the value until it finds a free port and store that value as the setting. If no value is specified, a random value between 10000 and 60000 is used. - - `exposed`: (default: `false`) Wether this port should be opened on the firewall and be publicly reachable. This should be kept to `false` for the majority of apps than only need a port for internal reverse-proxying! Possible values: `false`, `true`(=`Both`), `Both`, `TCP`, `UDP`. This will result in the port being opened on the firewall, and the diagnosis checking that a program answers on that port. (FIXME: this is not implemented yet) - - `fixed`: (default: `false`) Tells that the app absolutely needs the specific value provided in `default`, typically because it's needed for a specific protocol (FIXME: this is not implemented yet) + - `exposed`: (default: `false`) Wether this port should be opened on the firewall and be publicly reachable. This should be kept to `false` for the majority of apps than only need a port for internal reverse-proxying! Possible values: `false`, `true`(=`Both`), `Both`, `TCP`, `UDP`. This will result in the port being opened on the firewall, and the diagnosis checking that a program answers on that port. + - `fixed`: (default: `false`) Tells that the app absolutely needs the specific value provided in `default`, typically because it's needed for a specific protocol ##### Provision/Update (for every port name): - If not already booked, look for a free port, starting with the `default` value (or a random value between 10000 and 60000 if no `default` set) - - (FIXME) If `exposed` is not `false`, open the port in the firewall accordingly - otherwise make sure it's closed. + - If `exposed` is not `false`, open the port in the firewall accordingly - otherwise make sure it's closed. - The value of the port is stored in the `$port` setting for the `main` port, or `$port_NAME` for other `NAME`s ##### Deprovision: - - (FIXME) Close the ports on the firewall + - Close the ports on the firewall if relevant - Deletes all the port settings ##### Legacy management: @@ -806,8 +806,8 @@ class PortsResource(AppResource): default_port_properties = { "default": None, - "exposed": False, # or True(="Both"), "TCP", "UDP" # FIXME : implement logic for exposed port (allow/disallow in firewall ?) - "fixed": False, # FIXME: implement logic. Corresponding to wether or not the port is "fixed" or any random port is ok + "exposed": False, # or True(="Both"), "TCP", "UDP" + "fixed": False, } ports: Dict[str, Dict[str, Any]] @@ -839,6 +839,8 @@ class PortsResource(AppResource): def provision_or_update(self, context: Dict = {}): + from yunohost.firewall import firewall_allow, firewall_disallow + for name, infos in self.ports.items(): setting_name = f"port_{name}" if name != "main" else "port" @@ -854,16 +856,31 @@ class PortsResource(AppResource): if not port_value: port_value = infos["default"] - while self._port_is_used(port_value): - port_value += 1 + + if infos["fixed"]: + if self._port_is_used(port_value): + raise ValidationError(f"Port {port_value} is already used by another process or app.") + else: + while self._port_is_used(port_value): + port_value += 1 self.set_setting(setting_name, port_value) + if infos["exposed"]: + firewall_allow(infos["exposed"], port_value, reload_only_if_change=True) + else: + firewall_disallow(infos["exposed"], port_value, reload_only_if_change=True) + def deprovision(self, context: Dict = {}): + from yunohost.firewall import firewall_disallow + for name, infos in self.ports.items(): setting_name = f"port_{name}" if name != "main" else "port" + value = self.get_setting(setting_name) self.delete_setting(setting_name) + if value and str(value).strip(): + firewall_disallow(infos["exposed"], int(value), reload_only_if_change=True) class DatabaseAppResource(AppResource):