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

Dangerous interaction between replace_rcpt and SMTP return code #704

Open
SOwOphie opened this issue May 13, 2024 · 2 comments
Open

Dangerous interaction between replace_rcpt and SMTP return code #704

SOwOphie opened this issue May 13, 2024 · 2 comments
Labels
bug Something isn't working.

Comments

@SOwOphie
Copy link

Describe the bug

If replace_rcpt introduces a mix of deliverable and undeliverable mail addresses for outgoing mail, the client will receive an error and might retry the delivery over and over, spamming all deliverable mail addresses.

Details

I'm running the digital infrastructure for a local sailing club, and recently tried to implement a simple mailing list using maddy and a replace_rcpt table. Mails to the alias can only be sent from internal addresses, but are sent out to external addresses.

On the first test run, inevitably some mail was rejected by remote servers, both with 4xx and 5xx status codes. Everybody else received an increasing number of copies of the same message, until I stopped maddy and cleared the remote queue. The sender also received a number of DSNs. As a result of this incident, multiple mail providers temporarily blocked mail from us.

Investigating the maddy logs revealed that the mail client (the Android Google Mail App) had sent a total of 12 messages to maddy, despite the sender insisting they only tried to send two messages. Apart from this, maddy behaved correctly. It only retried the delivery to recipients that previously failed to receive the message.

Interpretation

My guess is that the client received an error code from maddy, and this triggered repeated retries. Unfortunately I did not have debug logging enabled at the moment to tell exactly, and I'm hesitant to recreate the situation.

Thinking about this for a bit, returning an error code is correct behavior if the recipient did not receive the message correctly. But for rewritten recipients, I don't think this behavior is desirable if at least one recipient received the message, because it can lead to situations as described above. A DSN should be sufficient. At a glance, internal/modify/replace_addr.go does not seem to be handling this case specially, so I'm somewhat confident this is the root of the issue.

If changing the current behavior is deemed undesirable by the maintainers, I would at least urge to put a prominent warning on the documentation of the replace_rcpt module, as the consequences of this interaction can be considerable, up to possibly landing the server on spam block lists.

Steps to reproduce

Use replace_rcpt to rewrite the recipients of outgoing mail, with a 1:n mapping that includes some undeliverable addresses.

Log files

My analysis of the logs can be found above. I'm hesitant to upload them outright as they contain a lot of personal info from other people.

Configuration file

# ------------------------------------------------------------------------------
# Basics
# ------------------------------------------------------------------------------

hostname [redacted]
autogenerated_msg_domain [redacted]
$(local_domains) = [redacted] [redacted]

tls file /etc/letsencrypt/live/[redacted]/fullchain.pem /etc/letsencrypt/live/[redacted]/privkey.pem

log stderr

# ------------------------------------------------------------------------------
# Databases
# ------------------------------------------------------------------------------

state_dir /var/lib/maddy
runtime_dir /tmp

auth.pass_table auth {
	table file /mnt/git-mirror/priv/maddy-config/auth
}

table.file alias {
	file /mnt/git-mirror/priv/maddy-config/alias
}

table.file lists {
	file /mnt/git-mirror/priv/maddy-config/lists
}

storage.imapsql inbox {
	driver sqlite3
	dsn imapsql.db
}

# ------------------------------------------------------------------------------
# Rewriting
# ------------------------------------------------------------------------------

msgpipeline to_local {
	destination postmaster $(local_domains) {
		modify {
			replace_rcpt static {
				entry postmaster postmaster@[redacted]
			}
			replace_rcpt &alias
		}
		deliver_to &inbox
	}
	default_destination {
		reject 550 5.1.1 "User does not exist"
	}
}

# ------------------------------------------------------------------------------
# Outbound mail
# ------------------------------------------------------------------------------

target.queue remote_queue {
	target &outbound

	bounce {
		destination postmaster $(local_domains) {
			deliver_to &to_local
		}
		default_destination {
			reject 550 5.0.0 "Refusing to send DSNs to non-local addresses"
		}
	}
}

target.remote outbound {
	limits {
		destination rate 20 1s
		destination concurrency 10
	}
	force_ipv4 true
	mx_auth {
		mtasts
		dane
		local_policy {
			min_tls_level none
			min_mx_level none
		}
	}
}

# ------------------------------------------------------------------------------
# Mail Retrieval
# ------------------------------------------------------------------------------

imap tls://0.0.0.0:993 tcp://0.0.0.0:143 {
	auth &auth
	storage &inbox
}

# ------------------------------------------------------------------------------
# Mail Submission
# ------------------------------------------------------------------------------

submission tls://0.0.0.0:465 tcp://0.0.0.0:587 {
	auth &auth
	dmarc no

	source $(local_domains) {
		check {
			authorize_sender {
				auth_normalize auto
				prepare_email &alias
				user_to_email identity
			}
		}

		destination_in &lists {
			modify {
				replace_rcpt &lists
			}
			reroute {
				destination $(local_domains) {
					deliver_to &to_local
				}
				default_destination {
					deliver_to &remote_queue
				}
			}
		}
		destination postmaster $(local_domains) {
			deliver_to &to_local
		}
		default_destination {
			modify {
				dkim {
					domains $(local_domains)
					selector default
				}
			}
			deliver_to &remote_queue
		}
	}
	default_source {
		reject 501 5.1.8 "Non-local sender domain"
	}
}

# ------------------------------------------------------------------------------
# Incoming Mail
# ------------------------------------------------------------------------------

smtp tcp://0.0.0.0:25 {
	limits {
		all rate 20 1s
		all concurrency 10
	}

	dmarc yes
	check {
		require_mx_record {
			fail_action reject
		}
		require_matching_rdns {
			fail_action ignore
		}
		dkim
		spf
		dnsbl {
			reject_threshold 1
			[redacted].zen.dq.spamhaus.net {
				client_ipv4 yes
				client_ipv6 yes
				ehlo no
				mailfrom no
			}
			[redacted].dbl.dq.spamhaus.net {
				client_ipv4 no
				client_ipv6 no
				ehlo yes
				mailfrom yes
			}
		}
	}

	source $(local_domains) {
		reject 501 5.1.8 "Use submission for outgoing SMTP"
	}
	default_source {
		destination $(local_domains) {
			deliver_to &to_local
		}
		default_destination {
			reject 550 5.1.1 "User does not exist"
		}
	}
}

Environment information

  • maddy version: 0.7.1
  • Distro: Alpine Edge
@SOwOphie SOwOphie added the bug Something isn't working. label May 13, 2024
@foxcpp
Copy link
Owner

foxcpp commented May 17, 2024

Non-debug logs should be enough to determine if the sender received any error from maddy - look for "submission: DATA error", "RCPT error", etc.

Note that target.queue won't produce an error on initial delivery if the message can't be delivered to some addresses - if the message is enqueued successfully, the sender will see "OK" status.

@SOwOphie
Copy link
Author

Okay, now I am completely stumped, and have to apologize for jumping to conclusions. The log contains no such lines (I'm guessing "submission: accepted" denotes a positive response?). But why on earth would the client then try to retransmit the message?

I went through the process of anonymizing the log file, and hope this contains no more personally identifying information. It can be found here. Does this help?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working.
Projects
None yet
Development

No branches or pull requests

2 participants