From 98ad28e29092359b3e9a8f4e4b0f6f0d8871616a Mon Sep 17 00:00:00 2001 From: David Buscher Date: Sat, 3 Apr 2021 16:39:20 +0100 Subject: [PATCH] Fixed several long-standing issues with UPnP port forwarding Fixes local firewall rules to allow discovery of SSDP servers. No longer disables UPnP forwarding when refreshing fails. No longer disables UPnP forwarding on system reboot. Cron job runs every 10 minutes to refresh the router tables more promptly after the system or router reboots. Removes the deprecated UPnP "reload" option. --- data/actionsmap/yunohost.yml | 1 - src/yunohost/firewall.py | 99 ++++++++++++++---------------------- 2 files changed, 37 insertions(+), 63 deletions(-) diff --git a/data/actionsmap/yunohost.yml b/data/actionsmap/yunohost.yml index 290952aa3..36116dcf3 100644 --- a/data/actionsmap/yunohost.yml +++ b/data/actionsmap/yunohost.yml @@ -1292,7 +1292,6 @@ firewall: - enable - disable - status - - reload nargs: "?" default: status --no-refresh: diff --git a/src/yunohost/firewall.py b/src/yunohost/firewall.py index bc21f1948..e9d05b7e0 100644 --- a/src/yunohost/firewall.py +++ b/src/yunohost/firewall.py @@ -35,7 +35,11 @@ from moulinette.utils.text import prependlines FIREWALL_FILE = "/etc/yunohost/firewall.yml" UPNP_CRON_JOB = "/etc/cron.d/yunohost-firewall-upnp" - +# A UDP port to use for the SSDP discovery phase of UPNP. +# Assigned by IANA to "Fujitsu ICL Terminal Emulator Program A", so no-one else is +# likely to use it (unlike port 1900 which is used by SSDP servers such +# as miniupnpd) +SSDP_CLIENT_PORT = 1901 logger = getActionLogger("yunohost.firewall") @@ -327,66 +331,57 @@ def firewall_upnp(action="status", no_refresh=False): """ firewall = firewall_list(raw=True) - enabled = firewall["uPnP"]["enabled"] - # Compatibility with previous version - if action == "reload": - logger.debug("'reload' action is deprecated and will be removed") - try: - # Remove old cron job - os.remove("/etc/cron.d/yunohost-firewall") - except Exception: - pass - action = "status" - no_refresh = False - - if action == "status" and no_refresh: - # Only return current state - return {"enabled": enabled} - elif action == "enable" or (enabled and action == "status"): + if action == "status": + enabled = firewall["uPnP"]["enabled"] + elif action == "enable": # Add cron job with open(UPNP_CRON_JOB, "w+") as f: f.write( - "*/50 * * * * root " + "*/10 * * * * root " "/usr/bin/yunohost firewall upnp status >>/dev/null\n" ) - # Open port 1900 to receive discovery message - if 1900 not in firewall["ipv4"]["UDP"]: - firewall_allow("UDP", 1900, no_upnp=True, no_reload=True) - if not enabled: - firewall_reload(skip_upnp=True) enabled = True - elif action == "disable" or (not enabled and action == "status"): + elif action == "disable": try: # Remove cron job os.remove(UPNP_CRON_JOB) except Exception: pass enabled = False - if action == "status": - no_refresh = True else: raise YunohostValidationError("action_invalid", action=action) - # Refresh port mapping using UPnP + # Refresh port mapping + refresh_success = True if not no_refresh: - upnpc = miniupnpc.UPnP(localport=1) + # Open port to receive discovery message + process.run_commands( + ["iptables -w -A INPUT -p udp --dport %d -j ACCEPT" % SSDP_CLIENT_PORT], + callback=_on_rule_command_error, + ) + upnpc = miniupnpc.UPnP(localport=SSDP_CLIENT_PORT) upnpc.discoverdelay = 3000 - # Discover UPnP device(s) logger.debug("discovering UPnP devices...") nb_dev = upnpc.discover() logger.debug("found %d UPnP device(s)", int(nb_dev)) + # Close discovery port + process.run_commands( + ["iptables -w -D INPUT -p udp --dport %d -j ACCEPT" % SSDP_CLIENT_PORT], + callback=_on_rule_command_error, + ) + if nb_dev < 1: logger.error(m18n.n("upnp_dev_not_found")) - enabled = False + refresh_success = False else: try: # Select UPnP device upnpc.selectigd() except Exception: logger.debug("unable to select UPnP device", exc_info=1) - enabled = False + refresh_success = False else: # Iterate over ports for protocol in ["TCP", "UDP"]: @@ -398,7 +393,7 @@ def firewall_upnp(action="status", no_refresh=False): upnpc.deleteportmapping(port, protocol) except Exception: pass - firewall["uPnP"][protocol + "_TO_CLOSE"] = [] + del firewall["uPnP"][protocol + "_TO_CLOSE"] for port in firewall["uPnP"][protocol]: # Clean the mapping of this port @@ -423,34 +418,17 @@ def firewall_upnp(action="status", no_refresh=False): logger.debug( "unable to add port %d using UPnP", port, exc_info=1 ) - enabled = False + refresh_success = False + if refresh_success: + logger.debug("UPnP port refresh successful") + if action == "enable": + logger.success(m18n.n("upnp_enabled")) + elif action == "disable": + logger.success(m18n.n("upnp_disabled")) - _update_firewall_file(firewall) - - if enabled != firewall["uPnP"]["enabled"]: - firewall = firewall_list(raw=True) - firewall["uPnP"]["enabled"] = enabled - - _update_firewall_file(firewall) - - if not no_refresh: - # Display success message if needed - if action == "enable" and enabled: - logger.success(m18n.n("upnp_enabled")) - elif action == "disable" and not enabled: - logger.success(m18n.n("upnp_disabled")) - # Make sure to disable UPnP - elif action != "disable" and not enabled: - firewall_upnp("disable", no_refresh=True) - - if not enabled and (action == "enable" or 1900 in firewall["ipv4"]["UDP"]): - # Close unused port 1900 - firewall_disallow("UDP", 1900, no_reload=True) - if not no_refresh: - firewall_reload(skip_upnp=True) - - if action == "enable" and not enabled: - raise YunohostError("upnp_port_open_failed") + # Save state always (note that refreshing can change the "TO_CLOSE" states) + firewall["uPnP"]["enabled"] = enabled + _update_firewall_file(firewall) return {"enabled": enabled} @@ -472,9 +450,6 @@ def firewall_stop(): os.system("ip6tables -F") os.system("ip6tables -X") - if os.path.exists(UPNP_CRON_JOB): - firewall_upnp("disable") - def _get_ssh_port(default=22): """Return the SSH port to use