From a62b7c752261d6378607d4cd9e842fddfefe365b Mon Sep 17 00:00:00 2001 From: Sunil Mohan Adapa Date: Fri, 26 Aug 2022 16:37:42 -0700 Subject: [PATCH] firewall: Use privileged decorator, drop showing running status - If a daemon is not-running, we already show an error message to the user. Use that mechanism instead of the custom one. Tests: - Functional tests work. - Initial setup for firewall on first boot works. - Default zone of the firewalld is set to external in /etc/firewalld.conf - Status of various apps is shown properly in the app page - If firewalld is not running, the app page is still displayed properly and message that firewalld is not running is shown. Signed-off-by: Sunil Mohan Adapa Reviewed-by: James Valleroy --- plinth/modules/firewall/__init__.py | 36 +---- .../modules/firewall/privileged.py | 48 +----- .../modules/firewall/templates/firewall.html | 149 ++++++++---------- plinth/modules/firewall/views.py | 23 +-- 4 files changed, 88 insertions(+), 168 deletions(-) rename actions/firewall => plinth/modules/firewall/privileged.py (66%) mode change 100755 => 100644 diff --git a/plinth/modules/firewall/__init__.py b/plinth/modules/firewall/__init__.py index 8b4dc11f2..9603b8f06 100644 --- a/plinth/modules/firewall/__init__.py +++ b/plinth/modules/firewall/__init__.py @@ -1,14 +1,11 @@ # SPDX-License-Identifier: AGPL-3.0-or-later -""" -FreedomBox app to configure a firewall. -""" +"""FreedomBox app to configure a firewall.""" import contextlib import logging from django.utils.translation import gettext_lazy as _ -from plinth import actions from plinth import app as app_module from plinth import cfg, menu from plinth.daemon import Daemon @@ -16,7 +13,7 @@ from plinth.modules.backups.components import BackupRestore from plinth.package import Packages, install from plinth.utils import Version, format_lazy, import_from_gi -from . import manifest +from . import manifest, privileged gio = import_from_gi('Gio', '2.0') glib = import_from_gi('GLib', '2.0') @@ -98,7 +95,7 @@ class FirewallApp(app_module.App): def _run_setup(): """Run firewalld setup.""" - _run(['setup'], superuser=True) + privileged.setup() add_service('http', 'external') add_service('http', 'internal') add_service('https', 'external') @@ -171,17 +168,8 @@ def try_with_reload(operation): operation() -def get_enabled_status(): - """Return whether firewall is enabled""" - output = _run(['get-status'], superuser=True) - if not output: - return False - else: - return output.split()[0] == 'running' - - def get_enabled_services(zone): - """Return the status of various services currently enabled""" + """Return the status of various services currently enabled.""" with ignore_dbus_error(dbus_error='ServiceUnknown'): zone_proxy = _get_dbus_proxy(_FIREWALLD_OBJECT, _ZONE_INTERFACE) return zone_proxy.getServices('(s)', zone) @@ -190,7 +178,7 @@ def get_enabled_services(zone): def get_port_details(service_port): - """Return the port types and numbers for a service port""" + """Return the port types and numbers for a service port.""" try: return _port_details[service_port] except KeyError: @@ -215,7 +203,7 @@ def get_interfaces(zone): def add_service(port, zone): - """Enable a service in firewall""" + """Enable a service in firewall.""" with ignore_dbus_error(dbus_error='ServiceUnknown'): zone_proxy = _get_dbus_proxy(_FIREWALLD_OBJECT, _ZONE_INTERFACE) with ignore_dbus_error(service_error='ALREADY_ENABLED'): @@ -229,7 +217,7 @@ def add_service(port, zone): def remove_service(port, zone): - """Remove a service in firewall""" + """Remove a service in firewall.""" with ignore_dbus_error(dbus_error='ServiceUnknown'): zone_proxy = _get_dbus_proxy(_FIREWALLD_OBJECT, _ZONE_INTERFACE) with ignore_dbus_error(service_error='NOT_ENABLED'): @@ -240,13 +228,3 @@ def remove_service(port, zone): config_zone = _get_dbus_proxy(zone_path, _CONFIG_ZONE_INTERFACE) with ignore_dbus_error(service_error='NOT_ENABLED'): config_zone.removeService('(s)', port) - - -def _run(arguments, superuser=False): - """Run an given command and raise exception if there was an error""" - command = 'firewall' - - if superuser: - return actions.superuser_run(command, arguments) - else: - return actions.run(command, arguments) diff --git a/actions/firewall b/plinth/modules/firewall/privileged.py old mode 100755 new mode 100644 similarity index 66% rename from actions/firewall rename to plinth/modules/firewall/privileged.py index 68263087c..56592cdc1 --- a/actions/firewall +++ b/plinth/modules/firewall/privileged.py @@ -1,31 +1,12 @@ -#!/usr/bin/python3 # SPDX-License-Identifier: AGPL-3.0-or-later -""" -Configuration helper for FreedomBox firewall interface. -""" +"""Configuration helper for FreedomBox firewall interface.""" -import argparse import subprocess import augeas from plinth import action_utils - - -def parse_arguments(): - """Return parsed command line arguments as dictionary""" - parser = argparse.ArgumentParser() - subparsers = parser.add_subparsers(dest='subcommand', help='Sub command') - - # Setup - subparsers.add_parser('setup', help='Perform basic firewall setup') - - # Get status - subparsers.add_parser('get-status', - help='Get whether firewalld is running') - - subparsers.required = True - return parser.parse_args() +from plinth.actions import privileged def _flush_iptables_rules(): @@ -81,26 +62,11 @@ def set_firewall_backend(backend): action_utils.service_restart('firewalld') -def subcommand_setup(_): +@privileged +def setup(): """Perform basic firewalld setup.""" action_utils.service_enable('firewalld') - subprocess.call(['firewall-cmd', '--set-default-zone=external']) + subprocess.run(['firewall-cmd', '--set-default-zone=external'], + stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, + check=True) set_firewall_backend('nftables') - - -def subcommand_get_status(_): - """Print status of the firewalld service""" - subprocess.call(['firewall-cmd', '--state']) - - -def main(): - """Parse arguments and perform all duties""" - arguments = parse_arguments() - - subcommand = arguments.subcommand.replace('-', '_') - subcommand_method = globals()['subcommand_' + subcommand] - subcommand_method(arguments) - - -if __name__ == "__main__": - main() diff --git a/plinth/modules/firewall/templates/firewall.html b/plinth/modules/firewall/templates/firewall.html index e87d4618c..a9dc0eb9d 100644 --- a/plinth/modules/firewall/templates/firewall.html +++ b/plinth/modules/firewall/templates/firewall.html @@ -15,97 +15,82 @@

{% trans "Status" %}

- {% if firewall_status == 'not_running' %} +
+ + + + + -

- {% blocktrans trimmed %} - Firewall daemon is not running. Please run it. Firewall comes - enabled by default on {{ box_name }}. On any Debian based - system (such as {{ box_name }}) you may run it using the - command 'service firewalld start' or in case of a system with - systemd 'systemctl start firewalld'. - {% endblocktrans %} -

- - {% else %} - -
-
{% trans "Service/Port" %}{% trans "Status" %}
- - - - - - - {% for component in components|dictsort:"name" %} - {% if component.ports %} - - + {% for component in components|dictsort:"name" %} + {% if component.ports %} + + + + + {% for port in component.ports_details %} + + - - {% for port in component.ports_details %} - - - - - {% endfor %} - {% endif %} - {% endfor %} - -
{% trans "Service/Port" %}{% trans "Status" %}
- +
+ + + {% if component.is_enabled %} + + {% trans "Enabled" %} + {% else %} + + {% trans "Disabled" %} + {% endif %} +
+ {{ port.name }}: + {% for port_number, protocol in port.details %} + {{ port_number }}/{{ protocol }} + {% endfor %} - {% if component.is_enabled %} + + {% if port.name in internal_enabled_ports and port.name in external_enabled_ports %} - {% trans "Enabled" %} - {% else %} + {% trans "Permitted" %} + {% elif port.name in internal_enabled_ports %} - {% trans "Disabled" %} + {% trans "Permitted (internal only)" %} + {% elif port.name in external_enabled_ports %} + + {% trans "Permitted (external only)" %} + {% else %} + + {% trans "Blocked" %} {% endif %}
- {{ port.name }}: - {% for port_number, protocol in port.details %} - {{ port_number }}/{{ protocol }} - {% endfor %} - - {% if port.name in internal_enabled_ports and port.name in external_enabled_ports %} - - {% trans "Permitted" %} - {% elif port.name in internal_enabled_ports %} - - {% trans "Permitted (internal only)" %} - {% elif port.name in external_enabled_ports %} - - {% trans "Permitted (external only)" %} - {% else %} - - {% trans "Blocked" %} - {% endif %} -
-
+ {% endfor %} + {% endif %} + {% endfor %} + + + -

- - {% blocktrans trimmed %} - The operation of the firewall is automatic. When you enable - a service it is also permitted in the firewall and when you - disable a service it is also disabled in the firewall. - {% endblocktrans %} - -

- -

{%trans "Advanced" %}

-

+

+ {% blocktrans trimmed %} - Advanced firewall operations such as opening custom ports are provided - by the Cockpit app. + The operation of the firewall is automatic. When you enable + a service it is also permitted in the firewall and when you + disable a service it is also disabled in the firewall. {% endblocktrans %} -

- {% endif %} + +

+ +

{%trans "Advanced" %}

+

+ {% blocktrans trimmed %} + Advanced firewall operations such as opening custom ports are provided + by the Cockpit app. + {% endblocktrans %} +

{% endblock %} diff --git a/plinth/modules/firewall/views.py b/plinth/modules/firewall/views.py index c517a231a..69fc1d366 100644 --- a/plinth/modules/firewall/views.py +++ b/plinth/modules/firewall/views.py @@ -1,7 +1,5 @@ # SPDX-License-Identifier: AGPL-3.0-or-later -""" -FreedomBox app to configure a firewall. -""" +"""FreedomBox app to configure a firewall.""" from plinth import views from plinth.modules import firewall @@ -11,23 +9,16 @@ from . import components class FirewallAppView(views.AppView): """Serve firewall index page.""" + app_id = 'firewall' template_name = 'firewall.html' def get_context_data(self, *args, **kwargs): """Add additional context data for the template.""" context = super().get_context_data(*args, **kwargs) - - status = 'running' if firewall.get_enabled_status() else 'not_running' - context['firewall_status'] = status - - if status == 'running': - context['components'] = components.Firewall.list() - internal_enabled_ports = firewall.get_enabled_services( - zone='internal') - external_enabled_ports = firewall.get_enabled_services( - zone='external') - context['internal_enabled_ports'] = internal_enabled_ports - context['external_enabled_ports'] = external_enabled_ports - + context['components'] = components.Firewall.list() + internal_enabled_ports = firewall.get_enabled_services(zone='internal') + external_enabled_ports = firewall.get_enabled_services(zone='external') + context['internal_enabled_ports'] = internal_enabled_ports + context['external_enabled_ports'] = external_enabled_ports return context