[enh] Review and add features to firewall category

* allow/disallow a port for each IP version and add ipv4 and ipv6 only arguments
* permit to allow/disable a range of ports
* return combined IPv4 and IPv6 - TCP and UDP - ports in firewall_list and allow to sort results
* review firewall_reload with new moulinette utils and log output of failed commands
* add --no-reload argument to prevent firewall reloading in firewall_allow/disallow
* some other small changes, e.g. add logging

actionsmap/yunohost.yml

@@ -864,6 +864,14 @@ firewall:
                     full: --raw
                     help: Return the complete YAML dict
                     action: store_true
+                -i:
+                    full: --by-ip-version
+                    help: List rules by IP version
+                    action: store_true
+                -f:
+                    full: --list-forwarded
+                    help: List forwarded ports with UPnP
+                    action: store_true
 
         ### firewall_reload()
         reload:
@@ -872,47 +880,69 @@ firewall:
 
         ### firewall_allow()
         allow:
-            action_help: Allow connection port/protocol
+            action_help: Allow connections on a port
             api: POST /firewall/port
             arguments:
                 port:
-                    help: Port to open
+                    help: Port or range of ports to open
                     extra:
-                        pattern: *pattern_port
+                        pattern: &pattern_port_or_range
-                protocol:
+                            - !!str ((^|(?!\A):)([0-9]{1,4}|[1-5][0-9]{4}|6[0-4][0-9]{3}|65[0-4][0-9]{2}|655[0-2][0-9]|6553[0-5])){1,2}?$
-                    help: Protocol associated with port
+                            - "pattern_port_or_range"
+                -p:
+                    full: --protocol
+                    help: "Protocol type to allow (default: TCP)"
                     choices:
-                        - UDP
                         - TCP
+                        - UDP
                         - Both
-                        - []
-                    nargs: "*"
                     default: TCP
-                --ipv6:
+                -4:
+                    full: --ipv4-only
+                    help: Only add a rule for IPv4 connections
+                    action: store_true
+                -6:
+                    full: --ipv6-only
+                    help: Only add a rule for IPv6 connections
                     action: store_true
                 --no-upnp:
+                    help: Do not add forwarding of this port with UPnP
+                    action: store_true
+                --no-reload:
+                    help: Do not reload firewall rules
                     action: store_true
 
 
         ### firewall_disallow()
         disallow:
-            action_help: Disallow connection
+            action_help: Disallow connections on a port
             api: DELETE /firewall/port
             arguments:
                 port:
-                    help: Port to close
+                    help: Port or range of ports to close
                     extra:
-                        pattern: *pattern_port
+                        pattern: *pattern_port_or_range
-                protocol:
+                -p:
-                    help: Protocol associated with port
+                    full: --protocol
+                    help: "Protocol type to allow (default: TCP)"
                     choices:
-                        - UDP
                         - TCP
+                        - UDP
                         - Both
-                        - []
-                    nargs: "*"
                     default: TCP
-                --ipv6:
+                -4:
+                    full: --ipv4-only
+                    help: Only remove the rule for IPv4 connections
+                    action: store_true
+                -6:
+                    full: --ipv6-only
+                    help: Only remove the rule for IPv6 connections
+                    action: store_true
+                --upnp-only:
+                    help: Only remove forwarding of this port with UPnP
+                    action: store_true
+                --no-reload:
+                    help: Do not reload firewall rules
                     action: store_true
 
 

firewall.py

@@ -34,7 +34,9 @@ except ImportError:
     sys.exit(1)
 
 from moulinette.core import MoulinetteError
+from moulinette.utils import process
 from moulinette.utils.log import getActionLogger
+from moulinette.utils.text import prependlines
 
 firewall_file = '/etc/yunohost/firewall.yml'
 upnp_cron_job = '/etc/cron.d/yunohost-firewall-upnp'
@@ -42,104 +44,148 @@ upnp_cron_job = '/etc/cron.d/yunohost-firewall-upnp'
 logger = getActionLogger('yunohost.firewall')
 
 
-def firewall_allow(port=None, protocol=['TCP'], ipv6=False, no_upnp=False):
+def firewall_allow(port, protocol='TCP', ipv4_only=False, ipv6_only=False,
+                   no_upnp=False, no_reload=False):
     """
-    Allow connection port/protocol
+    Allow connections on a port
 
-    Keyword argument:
+    Keyword arguments:
-        port -- Port to open
+        port -- Port or range of ports to open
-        protocol -- Protocol associated with port
+        protocol -- Protocol type to allow (default: TCP)
-        ipv6 -- ipv6
+        ipv4_only -- Only add a rule for IPv4 connections
-        no_upnp -- Do not request for uPnP
+        ipv6_only -- Only add a rule for IPv6 connections
+        no_upnp -- Do not add forwarding of this port with UPnP
+        no_reload -- Do not reload firewall rules
 
     """
-    port = int(port)
-    ipv  = "ipv4"
-    if isinstance(protocol, list):
-        protocols = protocol
-    else:
-        protocols = [protocol]
-    protocol  = protocols[0]
-
     firewall = firewall_list(raw=True)
 
-    upnp = not no_upnp and firewall['uPnP']['enabled']
+    # Validate port
+    if not isinstance(port, int) and ':' not in port:
+        port = int(port)
 
-    if ipv6:
+    # Validate protocols
-        ipv = "ipv6"
+    protocols = ['TCP', 'UDP']
+    if protocol != 'Both' and protocol in protocols:
+        protocols = [protocol,]
 
-    if protocol == "Both":
+    # Validate IP versions
-        protocols = ['UDP', 'TCP']
+    ipvs = ['ipv4', 'ipv6']
+    if ipv4_only and not ipv6_only:
+        ipvs = ['ipv4',]
+    elif ipv6_only and not ipv4_only:
+        ipvs = ['ipv6',]
 
-    for protocol in protocols:
+    for p in protocols:
-        if upnp and port not in firewall['uPnP'][protocol]:
+        # Iterate over IP versions to add port
-            firewall['uPnP'][protocol].append(port)
+        for i in ipvs:
-        if port not in firewall[ipv][protocol]:
+            if port not in firewall[i][p]:
-            firewall[ipv][protocol].append(port)
+                firewall[i][p].append(port)
-        else:
+            else:
-            msignals.display(m18n.n('port_already_opened', port), 'warning')
+                ipv = "IPv%s" % i[3]
+                msignals.display(m18n.n('port_already_opened', port, ipv),
+                                 'warning')
+        # Add port forwarding with UPnP
+        if not no_upnp and port not in firewall['uPnP'][p]:
+            firewall['uPnP'][p].append(port)
 
-    with open(firewall_file, 'w') as f:
+    # Update and reload firewall
-        yaml.safe_dump(firewall, f, default_flow_style=False)
+    _update_firewall_file(firewall)
-
+    if not no_reload:
-    return firewall_reload()
+        return firewall_reload()
 
 
-def firewall_disallow(port=None, protocol=['TCP'], ipv6=False):
+def firewall_disallow(port, protocol='TCP', ipv4_only=False, ipv6_only=False,
+                      upnp_only=False, no_reload=False):
     """
-    Allow connection port/protocol
+    Disallow connections on a port
 
-    Keyword argument:
+    Keyword arguments:
-        port -- Port to open
+        port -- Port or range of ports to close
-        protocol -- Protocol associated with port
+        protocol -- Protocol type to disallow (default: TCP)
-        ipv6 -- ipv6
+        ipv4_only -- Only remove the rule for IPv4 connections
+        ipv6_only -- Only remove the rule for IPv6 connections
+        upnp_only -- Only remove forwarding of this port with UPnP
+        no_reload -- Do not reload firewall rules
 
     """
-    port = int(port)
-    ipv  = "ipv4"
-    if isinstance(protocol, list):
-        protocols = protocol
-    else:
-        protocols = [protocol]
-    protocol  = protocols[0]
-
     firewall = firewall_list(raw=True)
 
-    if ipv6:
+    # Validate port
-        ipv = "ipv6"
+    if ':' not in port:
+        port = int(port)
 
-    if protocol == "Both":
+    # Validate protocols
-        protocols = ['UDP', 'TCP']
+    protocols = ['TCP', 'UDP']
+    if protocol != 'Both' and protocol in protocols:
+        protocols = [protocol,]
 
-    for protocol in protocols:
+    # Validate IP versions and UPnP
-        if port in firewall['uPnP'][protocol]:
+    ipvs = ['ipv4', 'ipv6']
-            firewall['uPnP'][protocol].remove(port)
+    upnp = True
-        if port in firewall[ipv][protocol]:
+    if ipv4_only and ipv6_only:
-            firewall[ipv][protocol].remove(port)
+        upnp = True # automatically disallow UPnP
-        else:
+    elif ipv4_only:
-            msignals.display(m18n.n('port_already_closed', port), 'warning')
+        ipvs = ['ipv4',]
+        upnp = upnp_only
+    elif ipv6_only:
+        ipvs = ['ipv6',]
+        upnp = upnp_only
+    elif upnp_only:
+        ipvs = []
 
-    with open(firewall_file, 'w') as f:
+    for p in protocols:
-        yaml.safe_dump(firewall, f, default_flow_style=False)
+        # Iterate over IP versions to remove port
+        for i in ipvs:
+            if port in firewall[i][p]:
+                firewall[i][p].remove(port)
+            else:
+                ipv = "IPv%s" % i[3]
+                msignals.display(m18n.n('port_already_closed', port, ipv),
+                                 'warning')
+        # Remove port forwarding with UPnP
+        if upnp and port in firewall['uPnP'][p]:
+            firewall['uPnP'][p].remove(port)
 
-    return firewall_reload()
+    # Update and reload firewall
+    _update_firewall_file(firewall)
+    if not no_reload:
+        return firewall_reload()
 
 
-def firewall_list(raw=False):
+def firewall_list(raw=False, by_ip_version=False, list_forwarded=False):
     """
     List all firewall rules
 
-    Keyword argument:
+    Keyword arguments:
         raw -- Return the complete YAML dict
+        by_ip_version -- List rules by IP version
+        list_forwarded -- List forwarded ports with UPnP
 
     """
     with open(firewall_file) as f:
         firewall = yaml.load(f)
-
     if raw:
         return firewall
-    else:
+
-        return { "openned_ports": firewall['ipv4']['TCP'] }
+    # Retrieve all ports for IPv4 and IPv6
+    ports = {}
+    for i in ['ipv4', 'ipv6']:
+        f = firewall[i]
+        # Combine TCP and UDP ports
+        ports[i] = sorted(set(f['TCP']) | set(f['UDP']))
+
+    if not by_ip_version:
+        # Combine IPv4 and IPv6 ports
+        ports = sorted(set(ports['ipv4']) | set(ports['ipv6']))
+
+    # Format returned dict
+    ret = { "opened_ports": ports }
+    if list_forwarded:
+        # Combine TCP and UDP forwarded ports
+        ret['forwarded_ports'] = sorted(
+            set(firewall['uPnP']['TCP']) | set(firewall['uPnP']['UDP']))
+    return ret
 
 
 def firewall_reload():
@@ -150,57 +196,93 @@ def firewall_reload():
     """
     from yunohost.hook import hook_callback
 
-    firewall = firewall_list(raw=True)
+    reloaded = False
-    upnp = firewall['uPnP']['enabled']
+    errors = False
+
+    # Check if SSH port is allowed
     ssh_port = _get_ssh_port()
+    if ssh_port not in firewall_list()['opened_ports']:
+        firewall_allow(ssh_port, no_reload=True)
+
+    # Retrieve firewall rules and UPnP status
+    firewall = firewall_list(raw=True)
+    upnp = firewall_upnp()['enabled']
 
     # IPv4
-    if os.system("iptables -P INPUT ACCEPT") != 0:
+    try:
-        raise MoulinetteError(errno.ESRCH, m18n.n('iptables_unavailable'))
+        process.check_output("iptables -L")
-    if upnp:
+    except process.CalledProcessError as e:
-        firewall_upnp(no_refresh=False)
+        logger.info('iptables seems to be not available, it outputs:\n%s',
+                    prependlines(e.output.rstrip(), '> '))
+        msignals.display(m18n.n('iptables_unavailable'), 'info')
+    else:
+        rules = [
+            "iptables -F",
+            "iptables -X",
+            "iptables -A INPUT -m state --state RELATED,ESTABLISHED -j ACCEPT",
+        ]
+        # Iterate over ports and add rule
+        for protocol in ['TCP', 'UDP']:
+            for port in firewall['ipv4'][protocol]:
+                rules.append("iptables -A INPUT -p %s --dport %s -j ACCEPT" \
+                                 % (protocol, process.quote(str(port))))
+        rules += [
+            "iptables -A INPUT -i lo -j ACCEPT",
+            "iptables -A INPUT -p icmp -j ACCEPT",
+            "iptables -P INPUT DROP",
+        ]
 
-    os.system("iptables -F")
+        # Execute each rule
-    os.system("iptables -X")
+        if process.check_commands(rules, callback=_on_rule_command_error):
-    os.system("iptables -A INPUT -m state --state RELATED,ESTABLISHED -j ACCEPT")
+            errors = True
+        reloaded = True
 
-    if ssh_port not in firewall['ipv4']['TCP']:
+    # IPv6
-        firewall_allow(ssh_port)
+    try:
+        process.check_output("ip6tables -L")
+    except process.CalledProcessError as e:
+        logger.info('ip6tables seems to be not available, it outputs:\n%s',
+                    prependlines(e.output.rstrip(), '> '))
+        msignals.display(m18n.n('ip6tables_unavailable'), 'info')
+    else:
+        rules = [
+            "ip6tables -F",
+            "ip6tables -X",
+            "ip6tables -A INPUT -m state --state RELATED,ESTABLISHED -j ACCEPT",
+        ]
+        # Iterate over ports and add rule
+        for protocol in ['TCP', 'UDP']:
+            for port in firewall['ipv6'][protocol]:
+                rules.append("ip6tables -A INPUT -p %s --dport %s -j ACCEPT" \
+                                 % (protocol, process.quote(str(port))))
+        rules += [
+            "ip6tables -A INPUT -i lo -j ACCEPT",
+            "ip6tables -A INPUT -p icmpv6 -j ACCEPT",
+            "ip6tables -P INPUT DROP",
+        ]
 
-    # Loop
+        # Execute each rule
-    for protocol in ['TCP', 'UDP']:
+        if process.check_commands(rules, callback=_on_rule_command_error):
-        for port in firewall['ipv4'][protocol]:
+            errors = True
-            os.system("iptables -A INPUT -p %s --dport %d -j ACCEPT" % (protocol, port))
+        reloaded = True
+
+    if not reloaded:
+        raise MoulinetteError(errno.ESRCH, m18n.n('firewall_reload_failed'))
 
     hook_callback('post_iptable_rules',
                   args=[upnp, os.path.exists("/proc/net/if_inet6")])
 
-    os.system("iptables -A INPUT -i lo -j ACCEPT")
+    if upnp:
-    os.system("iptables -A INPUT -p icmp -j ACCEPT")
+        # Refresh port forwarding with UPnP
-    os.system("iptables -P INPUT DROP")
+        firewall_upnp(no_refresh=False)
-
-    # IPv6
-    if os.path.exists("/proc/net/if_inet6"):
-        os.system("ip6tables -P INPUT ACCEPT")
-        os.system("ip6tables -F")
-        os.system("ip6tables -X")
-        os.system("ip6tables -A INPUT -m state --state RELATED,ESTABLISHED -j ACCEPT")
-
-        if ssh_port not in firewall['ipv6']['TCP']:
-            firewall_allow(ssh_port, ipv6=True)
-
-        # Loop v6
-        for protocol in ['TCP', 'UDP']:
-            for port in firewall['ipv6'][protocol]:
-                os.system("ip6tables -A INPUT -p %s --dport %d -j ACCEPT" % (protocol, port))
-
-        os.system("ip6tables -A INPUT -i lo -j ACCEPT")
-        os.system("ip6tables -A INPUT -p icmpv6 -j ACCEPT")
-        os.system("ip6tables -P INPUT DROP")
 
+    # TODO: Use service_restart
     os.system("service fail2ban restart")
-    msignals.display(m18n.n('firewall_reloaded'), 'success')
 
+    if errors:
+        msignals.display(m18n.n('firewall_rules_cmd_failed'), 'warning')
+    else:
+        msignals.display(m18n.n('firewall_reloaded'), 'success')
     return firewall_list()
 
 
@@ -349,3 +431,16 @@ def _get_ssh_port(default=22):
     except:
         pass
     return default
+
+def _update_firewall_file(rules):
+    """Make a backup and write new rules to firewall file"""
+    os.system("cp {0} {0}.old".format(firewall_file))
+    with open(firewall_file, 'w') as f:
+        yaml.safe_dump(rules, f, default_flow_style=False)
+
+def _on_rule_command_error(returncode, cmd, output):
+    """Callback for rules commands error"""
+    # Log error and continue commands execution
+    logger.error('"%s" returned non-zero exit status %d:\n%s',
+                 cmd, returncode, prependlines(output.rstrip(), '> '))
+    return True

locales/en.json

@@ -68,15 +68,18 @@
     "dyndns_cron_remove_failed" : "Unable to remove DynDNS cron job",
     "dyndns_cron_removed" : "DynDNS cron job successfully removed",
 
-    "port_available" : "Port {:d} is available",
+    "port_available" : "Port {} is available",
-    "port_unavailable" : "Port {:d} is not available",
+    "port_unavailable" : "Port {} is not available",
-    "port_already_opened" : "Port {:d} is already opened",
+    "port_already_opened" : "Port {} is already opened for {:s} connections",
-    "port_already_closed" : "Port {:d} is already closed",
+    "port_already_closed" : "Port {} is already closed for {:s} connections",
     "iptables_unavailable" : "You cannot play with iptables here. You are either in a container or your kernel does not support it.",
+    "ip6tables_unavailable" : "You cannot play with ip6tables here. You are either in a container or your kernel does not support it.",
     "upnp_dev_not_found" : "No UPnP device found",
     "upnp_port_open_failed" : "Unable to open UPnP ports",
     "upnp_enabled" : "UPnP successfully enabled",
     "upnp_disabled" : "UPnP successfully disabled",
+    "firewall_rules_cmd_failed" : "Some firewall rules commands have failed. For more information, see the log.",
+    "firewall_reload_failed" : "Unable to reload firewall",
     "firewall_reloaded" : "Firewall successfully reloaded",
 
     "hook_list_by_invalid" : "Invalid property to list hook by",
@@ -181,6 +184,7 @@
     "pattern_domain" : "Must be a valid domain name (e.g. my-domain.org)",
     "pattern_listname" : "Must be alphanumeric and underscore characters only",
     "pattern_port" : "Must be a valid port number (i.e. 0-65535)",
+    "pattern_port_or_range" : "Must be a valid port number (i.e. 0-65535) or range of ports (e.g. 100:200)",
     "pattern_backup_archive_name" : "Must be a valid filename with alphanumeric and -_. characters only",
 
     "format_datetime_short" : "%m/%d/%Y %I:%M %p"

locales/fr.json

@@ -68,15 +68,18 @@
     "dyndns_cron_remove_failed" : "Impossible d'enlever la tâche cron pour DynDNS",
     "dyndns_cron_removed" : "Tâche cron pour DynDNS enlevée avec succès",
 
-    "port_available" : "Le port {:d} est disponible",
+    "port_available" : "Le port {} est disponible",
-    "port_unavailable" : "Le port {:d} est indisponible",
+    "port_unavailable" : "Le port {} est indisponible",
-    "port_already_opened" : "Le port {:d} est déjà ouvert",
+    "port_already_opened" : "Le port {} est déjà ouvert pour l'{:s}",
-    "port_already_closed" : "Le port {:d} est déjà fermé",
+    "port_already_closed" : "Le port {} est déjà fermé pour l'{:s}",
     "iptables_unavailable" : "Vous ne pouvez pas faire joujou avec iptables ici. Vous êtes sûrement dans un conteneur, autrement votre noyau ne le supporte pas.",
+    "ip6tables_unavailable" : "Vous ne pouvez pas faire joujou avec ip6tables ici. Vous êtes sûrement dans un conteneur, autrement votre noyau ne le supporte pas.",
     "upnp_dev_not_found" : "Aucun périphérique compatible UPnP trouvé",
     "upnp_port_open_failed" : "Impossible d'ouvrir les ports avec UPnP",
     "upnp_enabled" : "UPnP activé avec succès",
     "upnp_disabled" : "UPnP désactivé avec succès",
+    "firewall_rules_cmd_failed" : "Des commandes de règles du pare-feu ont échoués. Plus d'informations sont disponibles dans le journal d'erreurs.",
+    "firewall_reload_failed" : "Impossible de recharger le pare-feu",
     "firewall_reloaded" : "Pare-feu rechargé avec succès",
 
     "hook_list_by_invalid" : "Propriété pour lister les scripts incorrecte",
@@ -181,6 +184,7 @@
     "pattern_domain" : "Doit être un nom de domaine valide (ex : mon-domaine.org)",
     "pattern_listname" : "Doit être composé uniquement de caractères alphanumérique et de tiret bas",
     "pattern_port" : "Doit être un numéro de port valide (0-65535)",
+    "pattern_port_or_range" : "Doit être un numéro de port valide (0-65535) ou une gamme de ports (ex : 100:200)",
     "pattern_backup_archive_name" : "Doit être un nom de fichier valide composé de caractères alphanumérique et -_. uniquement",
 
     "format_datetime_short" : "%d/%m/%Y %H:%M"