Skip to content

fix: setup email restrict configs should only prepend once#4379

Merged
georglauterbach merged 3 commits intomasterfrom
fix/prepend-checks-once
Mar 1, 2025
Merged

fix: setup email restrict configs should only prepend once#4379
georglauterbach merged 3 commits intomasterfrom
fix/prepend-checks-once

Conversation

@polarathene
Copy link
Copy Markdown
Member

@polarathene polarathene commented Feb 20, 2025 •
edited
Loading

Description

I've noticed that /etc/postfix/main.cf has these checks prepended twice, as the Postfix parameter we update also has the sed call match our own dms_ prefix variant (introduced in #3127).

No known bugs, nor should this fix change anything other than removing the duplicates prepended to smtpd_sender_restrictions, only dms_smtpd_sender_restrictions will be configured with these restrictions.

EDIT: There was a slight mishap with recipient restrictions in this PR. It was resolved in a follow-up PR for the same 15.0.1 release: #4399

Quick reference for maintainers

To configure how the sender addresses of mail is restricted, DMS has a few similar settings due to:

  • smtpd_sender_restrictions = $dms_smtpd_sender_restrictions (main.cf)
  • -o smtpd_sender_restrictions=$mua_sender_restrictions (master.cf)

That translates to the following:

  • smtpd_sender_restrictions is now only from port 25 (inbound)
  • $mua_sender_restrictions is only for ports 587 + 465
  • $dms_smtpd_sender_restrictions is used by all 3 smtpd ports (25, 587, 465).

To keep the same behaviour DMS already has for setup email restrict on all ports, we now only prepend to dms_smtpd_sender_restrictions.

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 code
  • I have commented my code, particularly in hard-to-understand areas
  • New and existing unit tests pass locally with my changes
  • I have added information about changes made in this PR to CHANGELOG.md

@polarathene polarathene added area/scripts kind/bug/fix A fix (PR) for a confirmed bug labels Feb 20, 2025
@polarathene polarathene self-assigned this Feb 20, 2025
@polarathene
Copy link
Copy Markdown
Member Author

polarathene commented Feb 20, 2025

Context

A quick overview of our current script support related to $dms_smtpd_sender_restrictions for maintainer docs to reference.

Summary:

  • smtpd_sender_restrictions is now only from port 25 (inbound)
  • $mua_sender_restrictions is only for ports 587 + 465
  • $dms_smtpd_sender_restrictions is used by all 3 smtpd ports (25, 587, 465).

#3127 was for DMS v12:

docker-mailserver/CHANGELOG.md

Lines 505 to 507 in d2d74a2

- The value of `smtpd_sender_restrictions` for Postfix has replaced the value ([#3127](https://github.com/docker-mailserver/docker-mailserver/pull/3127)):
- In `main.cf` with `$dms_smtpd_sender_restrictions`
- In `master.cf` inbound submissions ports 465 + 587 extend this inherited `smtpd` restriction with `$mua_sender_restrictions`

docker-mailserver/target/postfix/main.cf

Line 59 in d2d74a2

smtpd_sender_restrictions = $dms_smtpd_sender_restrictions

docker-mailserver/target/postfix/main.cf

Lines 70 to 74 in d2d74a2

# Custom defined parameters for DMS:
# reject_unknown_sender_domain: https://github.com/docker-mailserver/docker-mailserver/issues/3716#issuecomment-1868033234
dms_smtpd_sender_restrictions = permit_sasl_authenticated, permit_mynetworks, reject_unknown_sender_domain
# Submission ports 587 and 465 support for SPOOF_PROTECTION=1
mua_sender_restrictions = reject_authenticated_sender_login_mismatch, $dms_smtpd_sender_restrictions

Ports 587 and 465 use a custom parameter for main.cf, to reference our own one:

docker-mailserver/target/postfix/master.cf

Line 26 in d2d74a2

-o smtpd_sender_restrictions=$mua_sender_restrictions

docker-mailserver/target/postfix/master.cf

Line 40 in d2d74a2

-o smtpd_sender_restrictions=$mua_sender_restrictions

While SPOOF_PROTECTION=0 is our current default, the default config DMS ships as shown above is with the intent of SPOOF_PROTECTION=1, when the feature is not enabled we drop the extra restriction (reject_authenticated_sender_login_mismatch) for supporting SPOOF_PROTECTION=1:

docker-mailserver/target/scripts/startup/setup.d/security/spoofing.sh

Lines 24 to 26 in d2d74a2

_log 'debug' 'Spoof protection is disabled'
# shellcheck disable=SC2016
postconf 'mua_sender_restrictions = $dms_smtpd_sender_restrictions'

Another feature appends to dms_smtpd_sender_restrictions:

docker-mailserver/target/scripts/startup/setup.d/postfix.sh

Lines 64 to 69 in d2d74a2

if [[ ${POSTFIX_REJECT_UNKNOWN_CLIENT_HOSTNAME} -eq 1 ]]; then
__postfix__log 'trace' 'Enabling reject_unknown_client_hostname to dms_smtpd_sender_restrictions'
sedfile -i -E \
's|^(dms_smtpd_sender_restrictions = .*)|\1, reject_unknown_client_hostname|' \
/etc/postfix/main.cf
fi

@polarathene
Copy link
Copy Markdown
Member Author

polarathene commented Feb 20, 2025

Technically this shouldn't be a noteworthy change/fix as I don't think the current duplication is breaking anything, but if a changelog entry is needed:

### Fixes

- **Postfix:**
  - `setup email restrict` generated configs now only prepend to `dms_smtpd_sender_restrictions` ([#4379](https://github.com/docker-mailserver/docker-mailserver/pull/4379)

@polarathene polarathene added this to the v15.1.0 milestone Feb 20, 2025
@polarathene polarathene marked this pull request as ready for review February 20, 2025 03:39
@georglauterbach georglauterbach merged commit 5686a40 into master Mar 1, 2025
@georglauterbach georglauterbach deleted the fix/prepend-checks-once branch March 1, 2025 12:55
@polarathene polarathene modified the milestones: v15.1.0, v15.0.1 Mar 2, 2025
This was referenced Mar 4, 2025
Closed
Merged
@polarathene polarathene mentioned this pull request Apr 2, 2025
Open
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@casperklein casperklein casperklein approved these changes

@georglauterbach georglauterbach georglauterbach approved these changes

Assignees

@polarathene polarathene

Labels

area/scripts kind/bug/fix A fix (PR) for a confirmed bug

Projects

None yet

Milestone

v15.0.1

Development

Successfully merging this pull request may close these issues.

3 participants

@polarathene @casperklein @georglauterbach