Remailer support
With this feature, uffd can be configured to hide mail addresses of users from certain services while still allowing the services to send mails to the users.
To these services uffd returns special remailer addresses instead of the real mail addresses. When a service sends an email to a remailer address the mail server queries uffd and replaces the remailer address with the real mail address in both envelope and headers.
This feature requires additional mail server configuration (Postfix canonical_maps) and support in uffd-socketmapd.
Merge request reports
Activity
assigned to @julian
requested review from @julian
removed review request for @julian
requested review from @nd
added type::feature label
added 2 commits
- Resolved by Julian
- Resolved by Julian
- Resolved by Julian
- Resolved by Julian
@nd I've also tried to implement db-backed remailer addresses:
diff --git a/uffd/api/views.py b/uffd/api/views.py index ac4ebb2..f902e92 100644 --- a/uffd/api/views.py +++ b/uffd/api/views.py @@ -2,7 +2,7 @@ import functools from flask import Blueprint, jsonify, request, abort -from uffd.user.models import User, lookup_user_by_remailer_address, Group +from uffd.user.models import User, UserRemailerAddress, Group from uffd.mail.models import Mail, MailReceiveAddress, MailDestinationAddress from uffd.api.models import APIClient from uffd.session.views import login_get_user, login_ratelimit @@ -88,9 +88,9 @@ def getusers(): elif key == 'email' and len(values) == 1: users = User.query.filter_by(mail=values[0]).all() if request.api_client.service.use_remailer: - user, service = lookup_user_by_remailer_address(values[0]) - if user is not None and service == request.api_client.service: - users = [user] + user_remailer_address = UserRemailerAddress.query.filter_by(address=values[0]).one_or_none() + if user_remailer_address and user_remailer_address.service == request.api_client.service: + users = [user_remailer_address.user] # The lookup above does not handle MAIL_REMAILER_LIMIT_TO_USERS. E.g. if # a remailer address is supplied for a user that is not listed in # MAIL_REMAILER_LIMIT_TO_USERS (while that setting is not None), @@ -160,7 +160,7 @@ def resolve_remailer(): values = request.values.getlist('orig_address') if len(values) != 1: abort(400) - user, _ = lookup_user_by_remailer_address(values[0]) - if user is None: + user_remailer_address = UserRemailerAddress.query.filter_by(address=values[0]).one_or_none() + if not user_remailer_address: return jsonify(address=None) - return jsonify(address=user.mail) + return jsonify(address=user_remailer_address.user.mail) diff --git a/uffd/migrations/versions/704d1245331c_remailer_setting_and_api_permission.py b/uffd/migrations/versions/704d1245331c_remailer_setting_and_api_permission.py index 7a40112..41b4d10 100644 --- a/uffd/migrations/versions/704d1245331c_remailer_setting_and_api_permission.py +++ b/uffd/migrations/versions/704d1245331c_remailer_setting_and_api_permission.py @@ -40,9 +40,21 @@ def upgrade(): batch_op.add_column(sa.Column('perm_remailer', sa.Boolean(), nullable=False, default=False)) with op.batch_alter_table('service', copy_from=service) as batch_op: batch_op.add_column(sa.Column('use_remailer', sa.Boolean(), nullable=False, default=False)) + op.create_table('user_remailer_address', + sa.Column('id', sa.Integer(), autoincrement=True, nullable=False), + sa.Column('domain', sa.String(length=128), nullable=False), + sa.Column('secret', sa.String(length=128), nullable=False), + sa.Column('user_id', sa.Integer(), nullable=False), + sa.Column('service_id', sa.Integer(), nullable=False), + sa.ForeignKeyConstraint(['service_id'], ['service.id'], name=op.f('fk_user_remailer_address_service_id_service'), onupdate='CASCADE', ondelete='CASCADE'), + sa.ForeignKeyConstraint(['user_id'], ['user.id'], name=op.f('fk_user_remailer_address_user_id_user'), onupdate='CASCADE', ondelete='CASCADE'), + sa.PrimaryKeyConstraint('id', name=op.f('pk_user_remailer_address')), + sa.UniqueConstraint('secret', name=op.f('uq_user_remailer_address_secret')) + ) def downgrade(): meta = sa.MetaData(bind=op.get_bind()) + op.drop_table('user_remailer_address') api_client = sa.Table('api_client', meta, sa.Column('id', sa.Integer(), autoincrement=True, nullable=False), sa.Column('service_id', sa.Integer(), nullable=False), diff --git a/uffd/user/models.py b/uffd/user/models.py index 388702d..69eac5c 100644 --- a/uffd/user/models.py +++ b/uffd/user/models.py @@ -1,10 +1,12 @@ import string import re +import secrets from flask import current_app, escape from flask_babel import lazy_gettext from sqlalchemy import Column, Integer, String, ForeignKey, Boolean, Text from sqlalchemy.orm import relationship +from sqlalchemy.ext.hybrid import hybrid_property from uffd.database import db from uffd.password_hash import PasswordHashAttribute, LowEntropyPasswordHash @@ -105,43 +107,32 @@ class User(db.Model): def get_mail_for_service(self, service): '''Return user mail address for a given service''' + # To prevent unintentionally committing the caller's transaction, this + # method cannot be called with a dirty session. We could instead use a + # new connection, but this would cause deadlocks with SQLite ("database + # is locked" if the caller made uncommited changes). + # session.db.begin_nested would allow a partial rollback, but not a + # partial commit. + # The assert hopefully makes these situations easier to detect in tests. + assert not db.session.dirty if not current_app.config['MAIL_REMAILER_DOMAIN'] or not service.use_remailer: return self.mail if current_app.config['MAIL_REMAILER_LIMIT_TO_USERS'] is not None and \ self.loginname not in current_app.config['MAIL_REMAILER_LIMIT_TO_USERS']: return self.mail - return f'v1-{service.id}-{self.loginname}@{current_app.config["MAIL_REMAILER_DOMAIN"]}' - - # Somehow pylint non-deterministically fails to detect that .update_groups is set in invite.modes + user_remailer_address = UserRemailerAddress.query.filter_by(user=self, service=service, domain=current_app.config['MAIL_REMAILER_DOMAIN']).order_by(UserRemailerAddress.id.desc()).first() + if user_remailer_address: + return user_remailer_address.address + else: + new_user_remailer_address = UserRemailerAddress(user=self, service=service, domain=current_app.config['MAIL_REMAILER_LIMIT_TO_USERS']) + db.session.add(new_user_remailer_address) + db.session.commit() + return new_user_remailer_address.address + + # Somehow pylint non-deterministically fails to detect that .update_groups is set in invite.models def update_groups(self): pass -def lookup_user_by_remailer_address(address): - '''Return user and service object for a given remailer address''' - # With a top-level import we get circular import problems - # pylint: disable=import-outside-toplevel - from uffd.service.models import Service - if not current_app.config['MAIL_REMAILER_DOMAIN']: - return None, None - local_part, domain = (address.rsplit('@', 1) + [None])[:2] - if not local_part or not domain or domain.lower().strip() != current_app.config['MAIL_REMAILER_DOMAIN'].lower().strip(): - return None, None - prefix, rest = (local_part.split('-', 1) + [None])[:2] - if prefix != 'v1' or not rest: - return None, None - service_id, loginname = (rest.split('-', 1) + [None])[:2] - if not service_id or not loginname: - return None, None - service = Service.query.get(service_id) - user = User.query.filter_by(loginname=loginname).first() - # We don't check if service.use_remailer is enabled, so it can be disabled - # without losing mails between disabling and the service syncing user data. - # We also don't check if user is listed in MAIL_REMAILER_LIMIT_TO_USERS, so - # resolving in easier to test. - if not service or not user: - return None, None - return user, service - def next_id_expr(column, min_value, max_value): # db.func.max(column) + 1: highest used value in range + 1, NULL if no values in range # db.func.min(..., max_value): clip to range @@ -169,6 +160,24 @@ def set_default_unix_uid(mapper, connect, target): max_uid = current_app.config['USER_MAX_UID'] target.unix_uid = next_id_expr(User.unix_uid, min_uid, max_uid) +class UserRemailerAddress(db.Model): + __tablename__ = 'user_remailer_address' + id = Column(Integer(), primary_key=True, autoincrement=True) + domain = Column(String(128), nullable=False, default=lambda: current_app.config['MAIL_REMAILER_DOMAIN']) + secret = Column(String(128), nullable=False, unique=True, default=lambda: secrets.token_hex(16)) + user_id = Column(Integer(), ForeignKey('user.id', onupdate='CASCADE', ondelete='CASCADE'), nullable=False) + user = relationship('User') + service_id = Column(Integer(), ForeignKey('service.id', onupdate='CASCADE', ondelete='CASCADE'), nullable=False) + service = relationship('Service') + + @hybrid_property + def address(self): + return 'v2-' + str(self.id) + '-' + self.secret + '@' + self.domain + + @address.expression + def address(cls): + return db.literal('v2-', type_=String) + db.cast(cls.id, String) + db.literal('-', type_=String) + cls.secret + db.literal('@', type_=String) + cls.domain + group_table = db.table('group', db.column('unix_gid')) min_gid = db.bindparam('min_gid', unique=True, callable_=lambda: current_app.config['GROUP_MIN_GID'], type_=db.Integer) max_gid = db.bindparam('max_gid', unique=True, callable_=lambda: current_app.config['GROUP_MAX_GID'], type_=db.Integer)
This has the advantage of allowing gradual migration to a new remailer domain.
I did some performance testing with this code and a few hundred users. Fetching all users via the getusers API endpoint took:
- 1.57s with use_remailer disabled
- > 15m with use_remailer enabled on first call (obviously caused by hundrets of session.commit calls)
- 3.38s with use_remailer enabled on further calls
Since we cannot get rid of the commit call in
get_mail_for_service
(we need the id foruser_remailer_address.address
), I don't see doing inserts inget_mail_for_service
as an option.We could bulk-create all missing UserRemailerAddress objects in a single transaction at the beginning of the getusers endpoint. That would still have a race condition and we would need to decide what to do when there is no UserRemailerAddress object for a user.
Creating UserRemailerAddress objects when a user or service is created is not so easy without additional code in all place where we create users (e.g. signup views, users views, users cli). Also it does not solve all of our problems, as we cannot react to changes to MAIL_REMAILER_DOMAIN this way.
I also consider the performance hit from 1.57s to 3.38s to be pretty significant. So my conclusion is that a db-backed solution needs a full rewrite of the getusers endpoint (we probably need that anyway for service-specific mail addresses).
The implementation without db-backed remailer addresses for comparison:
- 1.42s with use_remailer disabled
- 1.79s with use_remailer enabled
We could bulk-create all missing UserRemailerAddress objects in a single transaction at the beginning of the getusers endpoint.
I've tried that out:
# to be called in getusers with request.api_client.service def create_missing_remailer_addresses(service=None): from uffd.service.models import Service if not current_app.config['MAIL_REMAILER_DOMAIN']: return if service: services = [service] else: services = Service.query.all() for service in services: if not service.use_remailer: continue users = User.query.outerjoin(UserRemailerAddress, db.and_(UserRemailerAddress.user_id == User.id, UserRemailerAddress.service_id == service.id, UserRemailerAddress.domain == 'remailer9.example.com')).group_by(User.id).having(UserRemailerAddress.id == None) for user in users: db.session.add(UserRemailerAddress(user=user, service=service))
That takes 5.65s when that API is called for the first time vs. 3.84s for later calls.
added 3 commits
-
77ba184a...34ea8c17 - 2 commits from branch
master
- 12a8af1d - Remailer support
-
77ba184a...34ea8c17 - 2 commits from branch