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.

Sign up for GitHub

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

Jump to bottom

security(Postfix): Protect against "SMTP Smuggling" attack #3727

Merged
merged 6 commits into from
Jan 3, 2024
Merged

security(Postfix): Protect against "SMTP Smuggling" attack #3727

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f7df902
prevent published attack form of SMTP smuggling
georglauterbach Dec 27, 2023
d466ec5
fix(main.cf): Use long-term fix for SMTP smuggling attack
polarathene Jan 2, 2024
b409de3
chore: Revert change to `postfix-amavis.cf`
polarathene Jan 2, 2024
b19c133
Merge branch 'master' into postfix/prevent-smtp-smuggling
polarathene Jan 3, 2024
d447294
chore: Add entry to `CHANGELOG.md`
polarathene Jan 3, 2024
497a5a8
chore: Indicate upstream default for excluded clients
polarathene Jan 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion target/amavis/postfix-amavis.cf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ smtp-amavis unix - - n - 2 smtp
-o smtpd_helo_restrictions=
-o smtpd_sender_restrictions=
-o smtpd_recipient_restrictions=permit_mynetworks,reject
-o smtpd_data_restrictions=reject_unauth_pipelining
-o smtpd_end_of_data_restrictions=
-o mynetworks=127.0.0.0/8
-o smtpd_error_sleep_time=0
Expand Down
3 changes: 3 additions & 0 deletions target/postfix/main.cf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,9 @@ smtpd_client_restrictions = permit_mynetworks, permit_sasl_authenticated, reject
smtpd_sender_restrictions = $dms_smtpd_sender_restrictions
smtpd_discard_ehlo_keywords = silent-discard, dsn
disable_vrfy_command = yes
smtpd_data_restrictions = reject_unauth_pipelining
Copy link
Member

@polarathene polarathene Dec 27, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Note that this exists already on smtpd_client_restrictions and smtpd_recipient_restrictions, introduced 7 years ago in #165

Both of those restrictions occur before smtpd_data_restrictions (section describes order), and probably should be removed in favor of smtpd_data_restrictions (based on information that follows below)

The PR that introduced reject_unauth_pipelining to the those two restrictions has a dead link, and while it does seem common enough to see in configs online, I'm not sure if they're correct. This comment also advises moving the restriction to smtpd_data_restrictions.

Amavis config for Postfix clearly communicates this configuration requirement for smtpd_data_restrictions:

The Postfix docs note that it is applicable during any SMTP command context, but also mentions:

With Postfix 2.6 and later, the SMTP server sets a per-session flag whenever it detects illegal pipelining, including pipelined HELO or EHLO commands.
The reject_unauth_pipelining feature simply tests whether the flag was set at any point in time during the session.

Since smtpd_data_restrictions is pretty much at the end of the session, I think that is why earlier reject_unauth_pipelining restrictions aren't likely to provide any benefit? 🤷‍♂️ (other than rejecting at an earlier stage, which should technically only be RCPT TO due to default smtpd_delay_reject=yes, which we for some reason set explicitly)

Also related I think is the postscreen Command pipelining test (Postfix docs for postscreen):

Unlike the Postfix SMTP server, postscreen(8) does not announce support for ESMTP command pipelining.
Therefore, clients are not allowed to send multiple commands.

With postscreen_pipelining_enable = yes, postscreen(8) detects zombies that send multiple commands, instead of sending one command and waiting for the server to reply.

This test is opportunistically enabled when postscreen(8) has to use the built-in SMTP engine anyway. This is to make postscreen(8) logging more informative.

The postscreen_pipelining_action parameter specifies the action that is taken next.

Copy link
Member

Choose a reason for hiding this comment

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

Might be worth adding some context of the restriction.

Suggested change
smtpd_data_restrictions = reject_unauth_pipelining
# Block clients that speak too early (don't wait their turn).
smtpd_data_restrictions = reject_unauth_pipelining

Keeping it in main.cf should be fine AFAIK, Amavis will inherit it and it may still make sense when Amavis is not used? (although I kinda prefer to keep it explicit on the Amavis snippet, instead of implicitly inferring it from main.cf)

The documented short-term workaround for the SMTP smuggling vulnerability is:

Suggested change
smtpd_data_restrictions = reject_unauth_pipelining
smtpd_data_restrictions = reject_unauth_pipelining
# This will conflict with existing line:
# smtpd_discard_ehlo_keywords = silent-discard, dsn
smtpd_discard_ehlo_keywords = chunking
# Should replace original (and add contextual comment to PR for the dsn config)
# Also requires checking that the logic for that PR isn't broken from any assumptions that prepending/appending 'chunking' here may break:
# smtpd_discard_ehlo_keywords = chunking, silent-discard, dsn

but once we have v14 of DMS we'll have Postfix 3.7.6 and can use the original short-term workaround:

Suggested change
smtpd_data_restrictions = reject_unauth_pipelining
# Block clients that speak too early (don't wait their turn).
# Amavis config expects this (implicitly inherits from main.cf)
smtpd_data_restrictions = reject_unauth_pipelining
# Security - Prevent "SMTP Smuggling" exploit
# https://github.com/docker-mailserver/docker-mailserver/issues/3719
smtpd_forbid_unauth_pipelining = yes
# See prior suggestion for concerns
smtpd_discard_ehlo_keywords = chunking

Only benefit then is to get this in for 13.1, but you've got it marked for v14? In which case the Postfix 3.7.6 fix is more appropriate.

# TODO enable when possible, see https://github.com/docker-mailserver/docker-mailserver/issues/3719#issuecomment-1868287208
#smtpd_forbid_bare_newline = yes
Copy link
Member

Choose a reason for hiding this comment

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

"When possible" could better clarify that the blocker is waiting on Debian to provide an updated package of Postfix (3.8.4, 3.7.9, 3.6.13 and 3.5.23). For Bookworm that'd be 3.7.9.

The comment also only refers to enabling the parameter, despite obsoleting the short-term workarounds.

The longterm-fix also references an optional 2nd parameter to configure:

smtpd_forbid_bare_newline = yes
smtpd_forbid_bare_newline_exclusions = $mynetworks

We shouldn't configure it by default in the image though (since Docker with IPv6 hosts but IPv4 only containers with default user-proxy: true daemon config) can be part of the trusted docker subnet (depending on our PERMIT_DOCKER ENV). Those that need it (like our test-suite) could enable it, and we can document that in our docs somewhere.


# Custom defined parameters for DMS:
dms_smtpd_sender_restrictions = permit_sasl_authenticated, permit_mynetworks, reject_unknown_sender_domain
Expand Down
Loading