Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Dovecot Quota dummy accounts for aliases should check for existing users with an exact user key lookup #2640

Merged
merged 3 commits into from
Jun 15, 2022

Conversation

tomav
Copy link
Contributor

@tomav tomav commented Jun 14, 2022

Description

Fixes #2639

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation (README.md or the documentation under docs/)
  • If necessary I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@tomav tomav changed the title Fixes #2639 Fixes #2639 - Alias will not be added to '/etc/dovecot/userdb' twice Jun 14, 2022
@wernerfred wernerfred requested a review from a team June 14, 2022 11:39
@georglauterbach
Copy link
Member

georglauterbach commented Jun 14, 2022

@polarathene does this already suffice when it comes to the issue of adding „wildcard“ aliases (#2637 (comment))?

@georglauterbach georglauterbach requested a review from a team June 14, 2022 12:49
@georglauterbach georglauterbach added this to the v11.2.0 milestone Jun 14, 2022
casperklein
casperklein previously approved these changes Jun 14, 2022
Copy link
Member

@polarathene polarathene left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only match what's relevant, the user column. Ensure the value is the start of the line and ends at the delimiter : to avoid the current bug.

Additionally I don't think we want to accidentally have two users with different casing, so use case insensitive match -i.

target/scripts/helpers/accounts.sh Outdated Show resolved Hide resolved
@polarathene
Copy link
Member

polarathene commented Jun 14, 2022

@polarathene does this already suffice when it comes to the issue of adding „wildcard“ aliases (#2637 (comment))?

No. That is a separate issue.

If I understand correctly, the method would still accept a @example.com wildcard alias entry as a user and enter it into the user DB.

You would have to reject an address that starts with @ with ^@(identify wildcard) or ^[^@] (identify valid entry as any first char that is not @).


That said, this methods purpose only is as an awful workaround for supporting ENABLE_QUOTAS=1. Technically without adding the wildcard entry, you'd get the problem with the postfix policy check as Dovecot is not aware of the (fake) user @example.com.

In that sense, by not addressing it like I mentioned, yes, this PR should also fix that issue. It may not fix it fully for that user as they had recursive aliases (aliases that referenced aliases). A similar issue may be present with support for multiple recipients to an alias, as I don't think the current code handles that either.

Unless someone adds a proper policy service to resolve aliases to actual users, the quota support is going to be flakey. AFAIK even then, I think it would be an issue with multiple recipients, as you either accept or reject when at least one alias mapped user has a known quota overage. It'd seem wrong to block delivery to other recipients for the alias, but not doing so would defeat the purpose afaik regarding backscatter prevention.

The rest is irrelevant for this check. An exact match avoids accidentally matching substring of an existing user key.
@polarathene polarathene modified the milestones: v11.2.0, v11.1.0 Jun 15, 2022
@polarathene
Copy link
Member

We can squeeze in this fix for 11.1 I think.

@polarathene polarathene changed the title Fixes #2639 - Alias will not be added to '/etc/dovecot/userdb' twice fix: Dovecot Quota dummy accounts for aliases should check for existing users with an exact user key lookup Jun 15, 2022
Copy link
Member

@georglauterbach georglauterbach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this is fine to be in 11.1.0 :)

@casperklein casperklein merged commit a84b8a1 into docker-mailserver:master Jun 15, 2022
@tomav tomav deleted the issue-2639 branch June 16, 2022 19:08
@tomav
Copy link
Contributor Author

tomav commented Jun 16, 2022

Well, thanks guys.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Alias will not be added to '/etc/dovecot/userdb' twice
5 participants