email: Simplify and rename postfix configuration module

- Remove unnecessary complex crash handler needed due to setting the service
configuration in two steps. Merge the two step into one after which crash
handler is not needed.

- Drop '_unsafe' API and verify all keys and values for sanity.

Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: James Valleroy <jvalleroy@mailbox.org>

plinth/modules/email/postconf.py

@@ -1,190 +0,0 @@
-"""Postconf wrapper providing thread-safe operations"""
-# SPDX-License-Identifier: AGPL-3.0-or-later
-
-import logging
-import re
-import subprocess
-from dataclasses import dataclass
-from typing import ClassVar
-
-from . import interproc
-
-logger = logging.getLogger(__name__)
-
-
-@dataclass
-class ServiceFlags:
-    service: str
-    type: str
-    private: str
-    unpriv: str
-    chroot: str
-    wakeup: str
-    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:
-        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):
-    """Acquire resource lock. Get the list of postconf values as specified.
-    Return a key-value map"""
-    for key in key_list:
-        validate_key(key)
-
-    return get_many_unsafe(key_list)
-
-
-def get_many_unsafe(key_iterator, flag=''):
-    result = {}
-    args = ['/sbin/postconf']
-    if flag:
-        args.append(flag)
-
-    number_of_keys = 0
-    for key in key_iterator:
-        args.append(key)
-        number_of_keys += 1
-
-    stdout = _run(args)
-    for line in filter(None, stdout.split('\n')):
-        key, sep, value = line.partition('=')
-        if not sep:
-            raise ValueError('Invalid output detected')
-        result[key.strip()] = value.strip()
-
-    if len(result) != number_of_keys:
-        raise ValueError('Some keys were missing from the output')
-    return result
-
-
-def set_many(kv_map):
-    """Acquire resource lock. Set the list of postconf values as specified"""
-    for key, value in kv_map.items():
-        validate_key(key)
-        validate_value(value)
-
-    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():
-        args.append('{}={}'.format(key, value))
-    _run(args)
-
-
-def set_master_cf_options(service_flags, options={}):
-    """Acquire resource lock. Set master.cf service options"""
-    if not isinstance(service_flags, ServiceFlags):
-        raise TypeError('service_flags')
-    for key, value in options.items():
-        validate_key(key)
-        validate_value(value)
-
-    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=<temp flag string>"
-    # /sbin/postconf -P "service/type/k=v" ...
-    # Delete placeholder string /dev/null/plinth-crash
-    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):
-    """Get postconf value (no locking, no sanitization)"""
-    result = _run(['/sbin/postconf', key])
-    match = key + ' ='
-    if not result.startswith(match):
-        raise KeyError(key)
-    return result[len(match):].strip()
-
-
-def set_unsafe(key, value, flag=''):
-    """Set postconf value (assuming root, no locking, no sanitization)"""
-    if flag:
-        _run(['/sbin/postconf', flag, '{}={}'.format(key, value)])
-    else:
-        _run(['/sbin/postconf', '{}={}'.format(key, value)])
-
-
-def parse_maps(raw_value):
-    if '{' in raw_value or '}' in raw_value:
-        raise ValueError('Unsupported map list format')
-
-    value_list = []
-    for segment in raw_value.split(','):
-        for sub_segment in segment.strip().split(' '):
-            sub_segment = sub_segment.strip()
-            if sub_segment:
-                value_list.append(sub_segment)
-    return value_list
-
-
-def _run(args):
-    """Run process. Capture and return standard output as a string. Raise a
-    RuntimeError on non-zero exit codes"""
-    try:
-        result = subprocess.run(args, check=True, capture_output=True)
-        return result.stdout.decode('utf-8')
-    except subprocess.SubprocessError as subprocess_error:
-        raise RuntimeError('Subprocess failed') from subprocess_error
-    except UnicodeDecodeError as unicode_error:
-        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):
-        raise ValueError('Invalid postconf key format')
-
-
-def validate_value(value):
-    """Validate postconf value format. Raises ValueError"""
-    for c in value:
-        if ord(c) < 32:
-            raise ValueError('Value contains control characters')

plinth/modules/email/postfix.py

@@ -0,0 +1,125 @@
+# SPDX-License-Identifier: AGPL-3.0-or-later
+"""
+Set and get postfix configuration using postconf.
+"""
+
+import re
+import subprocess
+from dataclasses import dataclass
+
+
+@dataclass
+class Service:  # NOQA, pylint: disable=too-many-instance-attributes
+    """Representation of a postfix daemon and its options."""
+    service: str
+    type_: str
+    private: str
+    unpriv: str
+    chroot: str
+    wakeup: str
+    maxproc: str
+    command: str
+    options: str
+
+    def __str__(self) -> str:
+        parts = [
+            self.service, self.type_, self.private, self.unpriv, self.chroot,
+            self.wakeup, self.maxproc, self.command
+        ]
+        for key, value in self.options.items():
+            _validate_key(key)
+            _validate_value(value)
+            parts.extend(['-o', f'{key}={value}'])
+
+        return ' '.join(parts)
+
+
+def get_config(keys: list) -> dict:
+    """Get postfix configuration using the postconf command."""
+    for key in keys:
+        _validate_key(key)
+
+    args = ['/sbin/postconf']
+    for key in keys:
+        args.append(key)
+
+    output = _run(args)
+    result = {}
+    for line in filter(None, output.split('\n')):
+        key, sep, value = line.partition('=')
+        if not sep:
+            raise ValueError('Invalid output detected')
+
+        result[key.strip()] = value.strip()
+
+    if set(keys) != set(result.keys()):
+        raise ValueError('Some keys were missing from the output')
+
+    return result
+
+
+def set_config(config: dict, flag=None):
+    """Set postfix configuration using the postconf command."""
+    if not config:
+        return
+
+    for key, value in config.items():
+        _validate_key(key)
+        _validate_value(value)
+
+    args = ['/sbin/postconf']
+    if flag:
+        args.append(flag)
+
+    for key, value in config.items():
+        args.append('{}={}'.format(key, value))
+
+    _run(args)
+
+
+def set_master_config(service: Service):
+    """Set daemons and their options in postfix master.cf."""
+    service_key = service.service + '/' + service.type_
+    set_config({service_key: str(service)}, '-M')
+
+
+def parse_maps(raw_value):
+    """Parse postfix configuration values that are maps."""
+    if '{' in raw_value or '}' in raw_value:
+        raise ValueError('Unsupported map list format')
+
+    value_list = []
+    for segment in raw_value.split(','):
+        for sub_segment in segment.strip().split(' '):
+            sub_segment = sub_segment.strip()
+            if sub_segment:
+                value_list.append(sub_segment)
+
+    return value_list
+
+
+def _run(args):
+    """Run process. Capture and return standard output as a string.
+
+    Raise a RuntimeError on non-zero exit codes.
+    """
+    try:
+        result = subprocess.run(args, check=True, capture_output=True)
+        return result.stdout.decode()
+    except subprocess.SubprocessError as subprocess_error:
+        raise RuntimeError('Subprocess failed') from subprocess_error
+    except UnicodeDecodeError as unicode_error:
+        raise RuntimeError('Unicode decoding failed') from unicode_error
+
+
+def _validate_key(key):
+    """Validate postconf key format or raise ValueError."""
+    if not re.match(r'^[a-zA-Z][a-zA-Z0-9_/]*$', key):
+        raise ValueError('Invalid postconf key format')
+
+
+def _validate_value(value):
+    """Validate postconf value format or raise ValueError."""
+    for char in value:
+        if ord(char) < 32:
+            raise ValueError('Value contains control characters')

plinth/modules/email/privileged/domain.py

@@ -7,7 +7,7 @@ import subprocess
 
 from plinth.actions import superuser_run
 from plinth.modules import config
-from plinth.modules.email import postconf
+from plinth.modules.email import postfix
 from plinth.modules.names.components import DomainName
 
 from . import tls
@@ -15,8 +15,8 @@ from . import tls
 
 def get_domains():
     """Return the current domain configuration."""
-    conf = postconf.get_many(['mydomain', 'mydestination'])
+    conf = postfix.get_config(['mydomain', 'mydestination'])
-    domains = set(postconf.parse_maps(conf['mydestination']))
+    domains = set(postfix.parse_maps(conf['mydestination']))
     defaults = {'$myhostname', 'localhost.$mydomain', 'localhost'}
     domains.difference_update(defaults)
     return {'primary_domain': conf['mydomain'], 'all_domains': domains}
@@ -48,7 +48,7 @@ def action_set_domains(primary_domain, all_domains):
         'mydomain': primary_domain,
         'mydestination': my_destination
     }
-    postconf.set_many(conf)
+    postfix.set_config(conf)
     pathlib.Path('/etc/mailname').write_text(primary_domain + '\n')
     tls.set_postfix_config(primary_domain, all_domains)
     tls.set_dovecot_config(primary_domain, all_domains)

plinth/modules/email/privileged/ldap.py

@@ -4,10 +4,10 @@ Configure postfix to use auth and local delivery with dovecot. Start smtps and
 submission services. Setup aliases database.
 """
 
-import plinth.modules.email.aliases as aliases
-import plinth.modules.email.postconf as postconf
 from plinth import actions
 
+from .. import aliases, postfix
+
 default_config = {
     'smtpd_sasl_auth_enable':
         'yes',
@@ -26,28 +26,27 @@ default_config = {
         ])
 }
 
-submission_flags = postconf.ServiceFlags(service='submission', type='inet',
+submission_options = {
-                                         private='n', unpriv='-', chroot='y',
-                                         wakeup='-', maxproc='-',
-                                         command_args='smtpd')
-
-default_submission_options = {
     'syslog_name': 'postfix/submission',
     'smtpd_tls_security_level': 'encrypt',
     'smtpd_client_restrictions': 'permit_sasl_authenticated,reject',
     'smtpd_relay_restrictions': 'permit_sasl_authenticated,reject'
 }
+submission_service = postfix.Service(service='submission', type_='inet',
+                                     private='n', unpriv='-', chroot='y',
+                                     wakeup='-', maxproc='-', command='smtpd',
+                                     options=submission_options)
 
-smtps_flags = postconf.ServiceFlags(service='smtps', type='inet', private='n',
+smtps_options = {
-                                    unpriv='-', chroot='y', wakeup='-',
-                                    maxproc='-', command_args='smtpd')
-
-default_smtps_options = {
     'syslog_name': 'postfix/smtps',
     'smtpd_tls_wrappermode': 'yes',
     'smtpd_sasl_auth_enable': 'yes',
     'smtpd_relay_restrictions': 'permit_sasl_authenticated,reject'
 }
+smtps_service = postfix.Service(service='smtps', type_='inet', private='n',
+                                unpriv='-', chroot='y', wakeup='-',
+                                maxproc='-', command='smtpd',
+                                options=smtps_options)
 
 SQLITE_ALIASES = 'sqlite:/etc/postfix/freedombox-aliases.cf'
 
@@ -59,23 +58,22 @@ def repair():
 
 
 def action_setup():
-    postconf.set_many_unsafe(default_config)
+    postfix.set_config(default_config)
     _setup_submission()
     _setup_alias_maps()
 
 
 def _setup_submission():
     """Update configuration for smtps and smtp-submission."""
-    postconf.set_master_cf_options(service_flags=submission_flags,
+    postfix.set_master_config(submission_service)
-                                   options=default_submission_options)
+    postfix.set_master_config(smtps_service)
-    postconf.set_master_cf_options(service_flags=smtps_flags,
-                                   options=default_smtps_options)
 
 
 def _setup_alias_maps():
     """Setup alias maps to include an sqlite DB."""
-    alias_maps = postconf.get_unsafe('alias_maps').replace(',', ' ').split(' ')
+    alias_maps = postfix.get_config(['alias_maps'])['alias_maps']
+    alias_maps = alias_maps.replace(',', ' ').split(' ')
     if SQLITE_ALIASES not in alias_maps:
         alias_maps.append(SQLITE_ALIASES)
 
-    postconf.set_many_unsafe({'alias_maps': ' '.join(alias_maps)})
+    postfix.set_config({'alias_maps': ' '.join(alias_maps)})

plinth/modules/email/privileged/spam.py

@@ -6,7 +6,7 @@ import re
 import subprocess
 
 from plinth import actions
-from plinth.modules.email import postconf
+from plinth.modules.email import postfix
 
 _milter_config = {
     'smtpd_milters': 'inet:127.0.0.1:11332',
@@ -21,7 +21,7 @@ def repair():
 def action_set_filter():
     _compile_sieve()
     _setup_rspamd()
-    postconf.set_many(_milter_config)
+    postfix.set_config(_milter_config)
 
 
 def _compile_sieve():

plinth/modules/email/privileged/tls.py

@@ -1,7 +1,7 @@
 """TLS configuration for postfix and dovecot."""
 # SPDX-License-Identifier: AGPL-3.0-or-later
 
-from plinth.modules.email import interproc, postconf
+from .. import interproc, postfix
 
 # Mozilla Guideline v5.6, Postfix 1.17.7, OpenSSL 1.1.1d, intermediate
 # Generated 2021-08
@@ -49,7 +49,7 @@ def set_postfix_config(primary_domain, all_domains):
         'smtpd_tls_chain_files':
             f'/etc/postfix/letsencrypt/{primary_domain}/chain.pem'
     })
-    postconf.set_many_unsafe(config)
+    postfix.set_config(config)
     content = '# This file is managed by FreedomBox\n'
     for domain in all_domains:
         content += f'{domain} /etc/postfix/letsencrypt/{domain}/chain.pem\n'