foxcpp / maddy

✉️ Composable all-in-one mail server.
https://maddy.email
GNU General Public License v3.0
5.14k stars 249 forks source link

Dangerous interaction between replace_rcpt and SMTP return code #704

Open SOwOphie opened 6 months ago

SOwOphie commented 6 months ago

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

foxcpp commented 6 months ago

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 commented 6 months ago

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?