From 2f77d998997f540b910d6ebc6f6334b2a3b1f1cf Mon Sep 17 00:00:00 2001 From: fliu <10025-fliu@users.noreply.salsa.debian.org> Date: Tue, 10 Aug 2021 03:32:45 +0000 Subject: [PATCH] email: Code cleanup, address reviews - View: security.html -> email_security.html - Setup: will not install Redis - Setup: put clamav packages on hold - Crash recovery: opening firewall ports becomes the last post action - Crash recovery: group postconf.set_many into small transactions - Crash recovery: safer postconf.set_master_cf_options --- plinth/modules/email_server/__init__.py | 92 +++++++++++-------- plinth/modules/email_server/audit/ldap.py | 5 +- plinth/modules/email_server/postconf.py | 73 ++++++++++++--- .../{security.html => email_security.html} | 0 plinth/modules/email_server/urls.py | 2 +- plinth/modules/email_server/views.py | 4 +- 6 files changed, 119 insertions(+), 57 deletions(-) rename plinth/modules/email_server/templates/{security.html => email_security.html} (100%) diff --git a/plinth/modules/email_server/__init__.py b/plinth/modules/email_server/__init__.py index e9051d224..8a4939021 100644 --- a/plinth/modules/email_server/__init__.py +++ b/plinth/modules/email_server/__init__.py @@ -16,26 +16,51 @@ from . import audit from . import manifest version = 1 -managed_packages = ['postfix-ldap', 'dovecot-pop3d', 'dovecot-imapd', - 'dovecot-ldap', 'dovecot-lmtpd', 'dovecot-managesieved', - 'rspamd', 'clamav', 'clamav-daemon'] -managed_services = ['postfix', 'dovecot', 'rspamd', 'redis', 'clamav-daemon', - 'clamav-freshclam'] + +packages = [ + 'postfix-ldap', 'dovecot-pop3d', 'dovecot-imapd', + 'dovecot-ldap', 'dovecot-lmtpd', 'dovecot-managesieved', +] + +packages_bloat = ['rspamd'] + +clamav_packages = ['clamav', 'clamav-daemon'] +clamav_daemons = ['clamav-daemon', 'clamav-freshclam'] + +port_info = { + 'postfix': ('smtp', 25, 'smtps', 465, 'smtp-submission', 587), + 'dovecot': ('imaps', 993, 'pop3s', 995), +} + +managed_services = ['postfix', 'dovecot', 'rspamd'] + +managed_packages = packages + packages_bloat app = None class EmailServerApp(plinth.app.App): """FreedomBox email server app""" app_id = 'email_server' + app_name = _('Email Server') def __init__(self): """The app's constructor""" super().__init__() + self._add_ui_components() + self._add_daemons() + self._add_firewall_ports() + # /rspamd location + webserver = Webserver('webserver-email', # unique id + 'email-server-freedombox', # config file name + urls=['https://{host}/rspamd']) + self.add(webserver) + + def _add_ui_components(self): info = plinth.app.Info( app_id=self.app_id, version=version, - name=_('Email Server'), + name=self.app_name, short_description=_('Powered by Postfix, Dovecot & Rspamd'), manual_page='EmailServer', clients=manifest.clients, @@ -53,11 +78,6 @@ class EmailServerApp(plinth.app.App): ) self.add(menu_item) - # /rspamd location - webserver = Webserver('webserver-email', 'email-server-freedombox', - urls=['https://{host}/rspamd']) - self.add(webserver) - shortcut = plinth.frontpage.Shortcut( 'shortcut_' + self.app_id, name=info.name, @@ -69,30 +89,28 @@ class EmailServerApp(plinth.app.App): ) self.add(shortcut) - postfix_ports = [] - dovecot_ports = [] - all_firewalld_ports = [] - for port in (25, 465, 587): - postfix_ports.extend([(port, 'tcp4'), (port, 'tcp6')]) - for port in (993, 995): - dovecot_ports.extend([(port, 'tcp4'), (port, 'tcp6')]) - all_firewalld_ports.extend(['smtp', 'smtps', 'smtp-submission']) - all_firewalld_ports.extend(['pop3s', 'imaps']) - - # Manage daemons - postfixd = plinth.daemon.Daemon('daemon-postfix', 'postfix', - listen_ports=postfix_ports) - dovecotd = plinth.daemon.Daemon('daemon-dovecot', 'dovecot', - listen_ports=dovecot_ports) - self.add(postfixd) - self.add(dovecotd) - for name in ('rspamd', 'redis', 'clamav-daemon', 'clamav-freshclam'): - daemon = plinth.daemon.Daemon('daemon-' + name, name) + def _add_daemons(self): + for srvname in managed_services: + # Construct `listen_ports` parameter for the daemon + mixed = port_info.get(srvname, ()) + port_numbers = [v for v in mixed if isinstance(v, int)] + listen = [] + for n in port_numbers: + listen.append((n, 'tcp4')) + listen.append((n, 'tcp6')) + # Add daemon + daemon = plinth.daemon.Daemon('daemon-' + srvname, srvname, + listen_ports=listen) self.add(daemon) - # Ports - firewall = Firewall('firewall-email', info.name, - ports=all_firewalld_ports, is_external=True) + def _add_firewall_ports(self): + all_port_names = [] + for mixed in port_info.values(): + port_names = [v for v in mixed if isinstance(v, str)] + all_port_names.extend(port_names) + + firewall = Firewall('firewall-email', self.app_name, + ports=all_port_names, is_external=True) self.add(firewall) def diagnose(self): @@ -106,9 +124,11 @@ class EmailServerApp(plinth.app.App): def setup(helper, old_version=None): """Installs and configures module""" - helper.install(managed_packages) + helper.install(packages) + helper.install(packages_bloat, skip_recommends=True) helper.call('post', audit.ldap.repair) helper.call('post', audit.spam.repair) + for srvname in managed_services: + actions.superuser_run('service', ['reload', srvname]) + # Final step: expose service daemons to public internet helper.call('post', app.enable) - for service_name in managed_services: - actions.superuser_run('service', ['reload', service_name]) diff --git a/plinth/modules/email_server/audit/ldap.py b/plinth/modules/email_server/audit/ldap.py index f0af7a7bd..f737c5278 100644 --- a/plinth/modules/email_server/audit/ldap.py +++ b/plinth/modules/email_server/audit/ldap.py @@ -102,11 +102,8 @@ def action_set_sasl(): def action_set_submission(): """Handles email_server -i ldap set_submission""" - logger.info('Set postfix service: %r', default_submission_options) postconf.set_master_cf_options(service_flags=submission_flags, options=default_submission_options) - - logger.info('Set postfix service: %r', default_smtps_options) postconf.set_master_cf_options(service_flags=smtps_flags, options=default_smtps_options) @@ -139,7 +136,7 @@ def fix_alias_maps(diagnosis): for i in range(len(analysis.parsed)): if analysis.parsed[i] in (BEFORE_ALIASES, AFTER_ALIASES): analysis.parsed[i] = '' - # Does hash:/etc/aliases exist? + # Does hash:/etc/aliases exist in list? if analysis.isystem >= 0: # Put the maps around hash:/etc/aliases val = '%s %s %s' % (BEFORE_ALIASES, ETC_ALIASES, AFTER_ALIASES) diff --git a/plinth/modules/email_server/postconf.py b/plinth/modules/email_server/postconf.py index 749e6f99d..cba3a007b 100644 --- a/plinth/modules/email_server/postconf.py +++ b/plinth/modules/email_server/postconf.py @@ -1,15 +1,20 @@ """Postconf wrapper providing thread-safe operations""" # SPDX-License-Identifier: AGPL-3.0-or-later -import dataclasses +import logging import re import subprocess +from dataclasses import dataclass +from typing import ClassVar + +from . import interproc from .lock import Mutex +logger = logging.getLogger(__name__) mutex = Mutex('email-postconf') -@dataclasses.dataclass +@dataclass class ServiceFlags: service: str type: str @@ -20,10 +25,31 @@ class ServiceFlags: maxproc: str command_args: str + crash_handler: ClassVar[str] = '/dev/null/plinth-crash' + + def _get_flags_ordered(self): + return [self.service, self.type, self.private, self.unpriv, + self.chroot, self.wakeup, self.maxproc, self.command_args] + def serialize(self) -> str: - return ' '.join([self.service, self.type, self.private, self.unpriv, - self.chroot, self.wakeup, self.maxproc, - self.command_args]) + ordered = self._get_flags_ordered() + return ' '.join(ordered) + + def serialize_temp(self) -> str: + ordered = self._get_flags_ordered() + ordered[-1] = self.crash_handler + return ' '.join(ordered) + + def try_remove_crash_handler(self, line) -> str: + pattern = re.compile('([^ \\t]+)[ \\t]+([a-z]+)[ \\t]+') + match = pattern.match(line) + if match is None: + return None + if match.group(1) != self.service or match.group(2) != self.type: + return None + if not line.rstrip().endswith(self.crash_handler): + return None + return line.replace(self.crash_handler, self.command_args) def get_many(key_list): @@ -52,9 +78,16 @@ def set_many(kv_map): set_many_unsafe(kv_map) -def set_many_unsafe(kv_map): +def set_many_unsafe(kv_map, flag=''): + args = ['/sbin/postconf'] + + if not kv_map: + return + if flag: + args.append(flag) for key, value in kv_map.items(): - set_unsafe(key, value) + args.append('{}={}'.format(key, value)) + _run(args) def set_master_cf_options(service_flags, options={}): @@ -65,15 +98,19 @@ def set_master_cf_options(service_flags, options={}): validate_key(key) validate_value(value) - service_slash_type = service_flags.service + '/' + service_flags.type - flag_string = service_flags.serialize() + service_key = service_flags.service + '/' + service_flags.type + long_opts = {service_key + '/' + k: v for (k, v) in options.items()} + logger.info('Setting %s service: %r', service_flags.service, options) + + # Crash resistant config setting: + # /sbin/postconf -M "service/type=" + # /sbin/postconf -P "service/type/k=v" ... + # Delete placeholder string /dev/null/plinth-crash with mutex.lock_all(): - # /sbin/postconf -M "service/type=flag_string" - set_unsafe(service_slash_type, flag_string, '-M') - for short_key, value in options.items(): - # /sbin/postconf -P "service/type/short_key=value" - set_unsafe(service_slash_type + '/' + short_key, value, '-P') + set_unsafe(service_key, service_flags.serialize_temp(), '-M') + set_many_unsafe(long_opts, '-P') + _master_remove_crash_handler(service_flags) def get_unsafe(key): @@ -122,6 +159,14 @@ def _run(args): raise RuntimeError('Unicode decoding failed') from unicode_error +def _master_remove_crash_handler(service_flags): + with interproc.atomically_rewrite('/etc/postfix/master.cf') as writer: + with open('/etc/postfix/master.cf') as reader: + for line in reader: + cleaned = service_flags.try_remove_crash_handler(line) + writer.write(line if cleaned is None else cleaned) + + def validate_key(key): """Validate postconf key format. Raises ValueError""" if not re.match('^[a-zA-Z][a-zA-Z0-9_]*$', key): diff --git a/plinth/modules/email_server/templates/security.html b/plinth/modules/email_server/templates/email_security.html similarity index 100% rename from plinth/modules/email_server/templates/security.html rename to plinth/modules/email_server/templates/email_security.html diff --git a/plinth/modules/email_server/urls.py b/plinth/modules/email_server/urls.py index 5e12fefa1..1ae111ee4 100644 --- a/plinth/modules/email_server/urls.py +++ b/plinth/modules/email_server/urls.py @@ -7,7 +7,7 @@ from . import views urlpatterns = [ path('apps/email_server/', views.EmailServerView.as_view(), name='index'), - path('apps/email_server/security', views.TLSView.as_view()), + path('apps/email_server/email_security', views.TLSView.as_view()), path('apps/email_server/domains', views.DomainView.as_view()), path('apps/email_server/my_mail', diff --git a/plinth/modules/email_server/views.py b/plinth/modules/email_server/views.py index 872a0c2c9..3bf378b8c 100644 --- a/plinth/modules/email_server/views.py +++ b/plinth/modules/email_server/views.py @@ -21,7 +21,7 @@ class TabMixin(View): ('', _('Home')), ('my_mail', _('My Mail')), ('my_aliases', _('My Aliases')), - ('security', _('Security')), + ('email_security', _('Security')), ('domains', _('Domains')) ] @@ -218,7 +218,7 @@ class AliasView(TabMixin, TemplateView): class TLSView(TabMixin, TemplateView): - template_name = 'security.html' + template_name = 'email_security.html' class DomainView(TabMixin, TemplateView):