Skip to content
Snippets Groups Projects

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.

Edited by Julian

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • nd
  • nd
  • nd
  • Julian added 3 commits

    added 3 commits

    • 31fbc44d - Better code formatting
    • 0b1c82da - Show warning triangle with tooltip instead of disabling remailer options
    • 05cecec9 - Commented special case in api.getusers

    Compare with previous version

  • Julian added 1 commit

    added 1 commit

    Compare with previous version

  • Julian resolved all threads

    resolved all threads

    • @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 for user_remailer_address.address), I don't see doing inserts in get_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.

    • Please register or sign in to reply
  • Julian added 3 commits

    added 3 commits

    Compare with previous version

  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Please register or sign in to reply
    Loading