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

Maddy reports successful delivery on LMTP delivery failures (losing messages) #453

Closed
sblinch opened this issue Feb 16, 2022 · 1 comment
Assignees
Labels
bug Something isn't working.

Comments

@sblinch
Copy link

sblinch commented Feb 16, 2022

Describe the bug

If you've configured Maddy for LMTP delivery (I'm currently integrating it with Dovecot) and your LMTP server returns a delivery error, Maddy currently ignores the error from the LMTP server and indicates a successful delivery. The message is then permanently lost since the sending server received a success response.

To describe what happens (likely painfully confusing, sorry):

msgpipelineDelivery.Body calls delivery.Body at msgpipeline.go:417, but if delivery is an *smtp.lmtpDelivery, this should actually be a call to delivery.BodyNonAtomic, after which the StatusCollector is checked for individual delivery errors.

The current approach of calling smtp.lmtpDelivery.Body actually invokes smtp.delivery.Body (on the embedded smtp.delivery struct), which then invokes d.conn.Data at smtp_downstream.go:276, which in turn invokes c.cl.Data (instead of c.cl.LMTPData) at smtpconn.go:369, which returns a *dataCloser in go-smtp/client.go:489 with its statusCb set to nil. Because statusCb is set to nil, any delivery errors are dropped on the floor at go-smtp/client.go:447. (All line numbers approximate; I've been tinkering.)

As a temporary workaround I've created an smtp.lmtpDelivery.Body implementation that wraps smtp.lmtpDelivery.BodyNonAtomic and returns an error if any errors are added to the StatusCollector. This keeps Maddy from outright losing messages if the LMTP server is down, but I believe it's still incorrect... I'm not familiar enough with the relevant RFCs, but I would assume that if a message has multiple recipients, and only a subset of those recipients fail, this should somehow be communicated back to the connecting SMTP server so that it only retries delivery for the subset of failed recipients.

Steps to reproduce

  1. Install Dovecot (I'm using 2.3.13 but it's not specific to any particular LMTP server).
  2. Follow the instructions in dovecot.md under the "Dovecot" and "Authentication headings integrate with Dovecot.
  3. Deliberately misconfigure Dovecot so that it fails to deliver messages. (In my case, I inadvertently used a mailbox user UID that was outside of the range first_valid_uid..last_valid_uid.)

Log files

Maddy:

Feb 15 16:28:37 mail-p1 maddy[62209]: smtp: incoming message        {"msg_id":"","sender":"sender@example.org","src_host":"example.org","src_ip":"10.11.12.13:54014"}
Feb 15 16:28:37 mail-p1 maddy[62209]: [debug] msgpipeline: sender sender@example.org matched by default rule        {"msg_id":"d9cb09f9"}
Feb 15 16:28:37 mail-p1 maddy[62209]: [debug] msgpipeline: global rcpt modifiers: recipient@example.com => recipient@example.com        {"msg_id":"d9cb09f9"}
Feb 15 16:28:37 mail-p1 maddy[62209]: [debug] msgpipeline: per-source rcpt modifiers: recipient@example.com => recipient@example.com        {"msg_id":"d9cb09f9"}
Feb 15 16:28:37 mail-p1 maddy[62209]: [debug] msgpipeline: recipient recipient@example.com matched by domain rule 'example.com'        {"msg_id":"d9cb09f9"}
Feb 15 16:28:37 mail-p1 maddy[62209]: [debug] msgpipeline: per-rcpt modifiers: recipient@example.com => recipient@example.com        {"msg_id":"d9cb09f9"}
Feb 15 16:28:37 mail-p1 maddy[62209]: [debug] target.lmtp: connected        {"downstream_server":"","msg_id":"d9cb09f9"}
Feb 15 16:28:37 mail-p1 maddy[62209]: [debug] target.lmtp: connected        {"msg_id":"d9cb09f9","remote_server":""}
Feb 15 16:28:37 mail-p1 maddy[62209]: [debug] msgpipeline: tgt.Start(sender@example.org) ok, target = target.lmtp:local_mailboxes        {"msg_id":"d9cb09f9"}
Feb 15 16:28:37 mail-p1 maddy[62209]: smtp: RCPT ok        {"msg_id":"d9cb09f9","rcpt":"recipient@example.com"}
Feb 15 16:28:37 mail-p1 maddy[62209]: smtp/pipeline: no check action        {"check":"check.dkim","msg_id":"d9cb09f9","reason":"No DKIM signatures","smtp_code":550,"smtp_enchcode":"5.7.20","smtp_msg":"No DKIM signatures"}
Feb 15 16:28:37 mail-p1 maddy[62209]: smtp/pipeline: quarantined        {"check":"check.spf","msg_id":"d9cb09f9","reason":"matched 'all'","smtp_code":550,"smtp_enchcode":"5.7.23","smtp_msg":"SPF authentication soft-failed"}
Feb 15 16:28:37 mail-p1 maddy[62209]: [debug] msgpipeline: delivery.Body ok, Delivery object = *msgpipeline.delivery        {"msg_id":"d9cb09f9"}
Feb 15 16:28:37 mail-p1 maddy[62209]: smtp: accepted        {"msg_id":"d9cb09f9"}

Dovecot:

Feb 15 16:28:37 mail-p1 dovecot[62084]: 2022-02-15 16:28:37 lmtp(62220, recipient@example.com): Error: lmtp-server: conn unix:pid=62209,uid=8 [1]: rcpt recipient@example.com: Failed to initialize user: Mail access for users with UID 8 not permitt>
Feb 15 16:28:37 mail-p1 dovecot[62084]: 2022-02-15 16:28:37 lmtp(62220): Info: Disconnect from local: Client has quit the connection (state=READY)

Configuration file

(Not configuration-dependent.)

Environment information

@sblinch sblinch added the bug Something isn't working. label Feb 16, 2022
@foxcpp foxcpp self-assigned this Feb 16, 2022
@foxcpp foxcpp closed this as completed in 90559be Feb 16, 2022
@foxcpp
Copy link
Owner

foxcpp commented Feb 16, 2022

Just pushed 90559be that should fix it.

foxcpp added a commit that referenced this issue Feb 16, 2022
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