From d0cf01fb290a5756bb547438447775ebebcb50e2 Mon Sep 17 00:00:00 2001 From: Sunil Mohan Adapa Date: Wed, 20 Oct 2021 17:00:10 -0700 Subject: [PATCH] email_server: Don't use user IDs when performing lookups - Typical mail systems are configured to work on usernames or virtual usernames. UIDs/GIDs are only needed at the final moment when delivering mails to user inboxes that need to have proper UID/GID set. - This makes it easy for dovecot to simply use PAM authentication instead of having to use LDAP. - Trying to hide UID from email headers is no longer necessary. Received: header is important for debugging mail delivery across the chain. Don't miss out. Signed-off-by: Sunil Mohan Adapa Reviewed-by: James Valleroy --- plinth/modules/email_server/aliases.py | 28 ++++++------ plinth/modules/email_server/audit/ldap.py | 45 +++++-------------- plinth/modules/email_server/audit/models.py | 13 ------ .../dovecot/conf.d/05-freedombox-auth.conf | 9 ---- .../dovecot/conf.d/90-freedombox-lmtp.conf | 4 -- .../freedombox-ldap-userdb-uid.conf.ext | 22 --------- .../freedombox-username-to-uid-number.cf | 10 ----- plinth/modules/email_server/views.py | 18 +++----- 8 files changed, 33 insertions(+), 116 deletions(-) delete mode 100644 plinth/modules/email_server/data/etc/dovecot/freedombox-ldap-userdb-uid.conf.ext delete mode 100644 plinth/modules/email_server/data/etc/postfix/freedombox-username-to-uid-number.cf diff --git a/plinth/modules/email_server/aliases.py b/plinth/modules/email_server/aliases.py index 57cba1f83..85f2910ba 100644 --- a/plinth/modules/email_server/aliases.py +++ b/plinth/modules/email_server/aliases.py @@ -11,7 +11,7 @@ from plinth import actions @dataclass class Alias: - value: int + value: str name: str enabled: bool = field(init=False) status: InitVar[int] @@ -34,11 +34,11 @@ def _get_cursor(): connection.close() -def get(uid): +def get(username): """Get all aliases of a user.""" query = 'SELECT name, value, status FROM alias WHERE value=?' with _get_cursor() as cursor: - rows = cursor.execute(query, (uid, )) + rows = cursor.execute(query, (username, )) return [Alias(**row) for row in rows] @@ -56,40 +56,40 @@ def exists(name): return cursor.fetchone()[0] != 0 -def put(uid, name): +def put(username, name): """Insert if not exists a new alias.""" query = 'INSERT INTO alias (name, value, status) VALUES (?, ?, ?)' with _get_cursor() as cursor: try: - cursor.execute(query, (name, uid, 1)) + cursor.execute(query, (name, username, 1)) except sqlite3.IntegrityError: pass # Alias exists, rare since we are already checking -def delete(uid, aliases): +def delete(username, aliases): """Delete a set of aliases.""" query = 'DELETE FROM alias WHERE value=? AND name=?' - parameter_seq = ((uid, name) for name in aliases) + parameter_seq = ((username, name) for name in aliases) with _get_cursor() as cursor: cursor.execute('BEGIN') cursor.executemany(query, parameter_seq) cursor.execute('COMMIT') -def enable(uid, aliases): +def enable(username, aliases): """Enable a list of aliases.""" - return _set_status(uid, aliases, 1) + return _set_status(username, aliases, 1) -def disable(uid, aliases): +def disable(username, aliases): """Disable a list of aliases.""" - return _set_status(uid, aliases, 0) + return _set_status(username, aliases, 0) -def _set_status(uid, aliases, status): +def _set_status(username, aliases, status): """Set the status value of a list of aliases.""" query = 'UPDATE alias SET status=? WHERE value=? AND name=?' - parameter_seq = ((status, uid, name) for name in aliases) + parameter_seq = ((status, username, name) for name in aliases) with _get_cursor() as cursor: cursor.execute('BEGIN') cursor.executemany(query, parameter_seq) @@ -106,7 +106,7 @@ PRAGMA journal_mode=WAL; BEGIN; CREATE TABLE IF NOT EXISTS alias ( name TEXT NOT NULL, - value INTEGER NOT NULL, + value TEXT NOT NULL, status INTEGER NOT NULL, PRIMARY KEY (name) ); diff --git a/plinth/modules/email_server/audit/ldap.py b/plinth/modules/email_server/audit/ldap.py index 120176d22..f7969a1a8 100644 --- a/plinth/modules/email_server/audit/ldap.py +++ b/plinth/modules/email_server/audit/ldap.py @@ -54,9 +54,7 @@ default_smtps_options = { } MAILSRV_DIR = '/var/lib/plinth/mailsrv' -ETC_ALIASES = 'hash:/etc/aliases' -BEFORE_ALIASES = 'ldap:/etc/postfix/freedombox-username-to-uid-number.cf' -AFTER_ALIASES = 'sqlite:/etc/postfix/freedombox-aliases.cf' +SQLITE_ALIASES = 'sqlite:/etc/postfix/freedombox-aliases.cf' logger = logging.getLogger(__name__) @@ -123,45 +121,26 @@ def check_alias_maps(title=''): """Check the ability to mail to usernames and user aliases""" diagnosis = models.MainCfDiagnosis(title) - analysis = models.AliasMapsAnalysis() - analysis.parsed = postconf.parse_maps_by_key_unsafe('alias_maps') - analysis.isystem = list_find(analysis.parsed, ETC_ALIASES) - analysis.ibefore = list_find(analysis.parsed, BEFORE_ALIASES) - analysis.iafter = list_find(analysis.parsed, AFTER_ALIASES) - - if analysis.ibefore == -1 or analysis.iafter == -1: - diagnosis.flag_once('alias_maps', user=analysis) + alias_maps = postconf.get_unsafe('alias_maps').replace(',', ' ').split(' ') + if SQLITE_ALIASES not in alias_maps: + diagnosis.flag_once('alias_maps', user=alias_maps) diagnosis.critical('Required maps not in list') - if analysis.ibefore > analysis.iafter: - diagnosis.flag_once('alias_maps', user=analysis) - diagnosis.critical('Insecure map order') return diagnosis def fix_alias_maps(diagnosis): - diagnosis.repair('alias_maps', rearrange_alias_maps) + + def fix_value(alias_maps): + if SQLITE_ALIASES not in alias_maps: + alias_maps.append(SQLITE_ALIASES) + + return ' '.join(alias_maps) + + diagnosis.repair('alias_maps', fix_value) diagnosis.apply_changes(postconf.set_many_unsafe) -def rearrange_alias_maps(analysis): - # Delete *all* references to BEFORE_ALIASES and AFTER_ALIASES - for i in range(len(analysis.parsed)): - if analysis.parsed[i] in (BEFORE_ALIASES, AFTER_ALIASES): - analysis.parsed[i] = '' - # 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) - analysis.parsed[analysis.isystem] = val - else: - # To the end - analysis.parsed.append(BEFORE_ALIASES) - analysis.parsed.append(AFTER_ALIASES) - # List -> string - return ' '.join(filter(None, analysis.parsed)) - - def check_local_recipient_maps(title=''): diagnosis = models.MainCfDiagnosis(title) lrcpt_maps = postconf.parse_maps_by_key_unsafe('local_recipient_maps') diff --git a/plinth/modules/email_server/audit/models.py b/plinth/modules/email_server/audit/models.py index 173e77630..60083f6e9 100644 --- a/plinth/modules/email_server/audit/models.py +++ b/plinth/modules/email_server/audit/models.py @@ -1,9 +1,7 @@ # SPDX-License-Identifier: AGPL-3.0-or-later """Models of the audit module""" -import dataclasses import logging -import typing logger = logging.getLogger(__name__) @@ -207,14 +205,3 @@ class MainCfDiagnosis(Diagnosis): contains an unresolved issue (i.e. an uncorrected key)""" if None in self.advice.values(): raise UnresolvedIssueError('Assertion failed') - - -@dataclasses.dataclass(init=False) -class AliasMapsAnalysis: - parsed = typing.List[str] - ibefore = int - isystem = int - iafter = int - - def __init__(self): - pass diff --git a/plinth/modules/email_server/data/etc/dovecot/conf.d/05-freedombox-auth.conf b/plinth/modules/email_server/data/etc/dovecot/conf.d/05-freedombox-auth.conf index 53f74960f..b8b027b42 100644 --- a/plinth/modules/email_server/data/etc/dovecot/conf.d/05-freedombox-auth.conf +++ b/plinth/modules/email_server/data/etc/dovecot/conf.d/05-freedombox-auth.conf @@ -12,15 +12,6 @@ passdb { result_success = return-ok } -userdb { - # UID number lookup (10001@example.com) - driver = ldap - args = /etc/dovecot/freedombox-ldap-userdb-uid.conf.ext - result_failure = continue - result_internalfail = return-fail - result_success = return-ok -} - userdb { driver = ldap args = /etc/dovecot/freedombox-ldap-userdb.conf.ext diff --git a/plinth/modules/email_server/data/etc/dovecot/conf.d/90-freedombox-lmtp.conf b/plinth/modules/email_server/data/etc/dovecot/conf.d/90-freedombox-lmtp.conf index 0b9c3516f..a0f2a3ca4 100644 --- a/plinth/modules/email_server/data/etc/dovecot/conf.d/90-freedombox-lmtp.conf +++ b/plinth/modules/email_server/data/etc/dovecot/conf.d/90-freedombox-lmtp.conf @@ -1,10 +1,6 @@ # Direct edits to this file will be lost! # Manage your settings on Plinth -# Privacy: remove the recipient's UID number from email headers -lmtp_add_received_header = no -lmtp_hdr_delivery_address = original - protocol lmtp { mail_plugins = $mail_plugins sieve } diff --git a/plinth/modules/email_server/data/etc/dovecot/freedombox-ldap-userdb-uid.conf.ext b/plinth/modules/email_server/data/etc/dovecot/freedombox-ldap-userdb-uid.conf.ext deleted file mode 100644 index e8a047f22..000000000 --- a/plinth/modules/email_server/data/etc/dovecot/freedombox-ldap-userdb-uid.conf.ext +++ /dev/null @@ -1,22 +0,0 @@ -# Direct edits to this file will be lost! -# Manage your settings on Plinth https://localhost/plinth/apps/email_server - -uris = ldap://127.0.0.1 -base = dc=thisbox - -user_attrs = \ - =home=%{ldap:homeDirectory}, \ - =uid=%{ldap:uidNumber}, \ - =gid=%{ldap:gidNumber}, \ - =user=%{ldap:uid}, \ - =mail=maildir:~/Maildir:LAYOUT=index - -# Support user lookup by UID number - -user_filter = \ - (&(objectClass=posixAccount)(!(uidNumber=0))(uidNumber=%n)) - -# doveadm -A - -iterate_attrs = =user=%{ldap:uid} -iterate_filter = (objectClass=posixAccount) diff --git a/plinth/modules/email_server/data/etc/postfix/freedombox-username-to-uid-number.cf b/plinth/modules/email_server/data/etc/postfix/freedombox-username-to-uid-number.cf deleted file mode 100644 index 70a59bcee..000000000 --- a/plinth/modules/email_server/data/etc/postfix/freedombox-username-to-uid-number.cf +++ /dev/null @@ -1,10 +0,0 @@ -# This file is managed by FreedomBox - -# Map user name to UID number - -bind = no -server_host = ldap://127.0.0.1 -search_base = dc=thisbox -query_filter = (&(objectClass=posixAccount)(uid=%s)) -result_attribute = uidNumber -result_format = %s@localhost diff --git a/plinth/modules/email_server/views.py b/plinth/modules/email_server/views.py index e10955c14..d86ea2925 100644 --- a/plinth/modules/email_server/views.py +++ b/plinth/modules/email_server/views.py @@ -2,7 +2,6 @@ """ Views for the email app. """ -import pwd from django.contrib import messages from django.core.exceptions import ValidationError @@ -147,13 +146,9 @@ class AliasView(FormView): return kwargs - def _get_uid(self): - """Return the UID of the user that made the request.""" - return pwd.getpwnam(self.request.user.username).pw_uid - def _get_current_aliases(self): """Return current list of aliases.""" - return aliases_module.get(self._get_uid()) + return aliases_module.get(self.request.user.username) def get_context_data(self, *args, **kwargs): context = super().get_context_data(*args, **kwargs) @@ -185,17 +180,18 @@ class AliasView(FormView): """Handle a valid alias list form operation.""" aliases = form.cleaned_data['aliases'] action = form.cleaned_data['action'] - uid = self._get_uid() + username = self.request.user.username if action == 'delete': - aliases_module.delete(uid, aliases) + aliases_module.delete(username, aliases) elif action == 'disable': - aliases_module.disable(uid, aliases) + aliases_module.disable(username, aliases) elif action == 'enable': - aliases_module.enable(uid, aliases) + aliases_module.enable(username, aliases) def _create_form_valid(self, form): """Handle a valid create alias form operation.""" - aliases_module.put(self._get_uid(), form.cleaned_data['alias']) + username = self.request.user.username + aliases_module.put(username, form.cleaned_data['alias']) class DomainsView(FormView):