foxcpp / maddy

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

Bug report: empty smtp.mailfrom on Delivery Status Notification (DSN) #629

Open oliverpool opened 11 months ago

oliverpool commented 11 months ago

Describe the bug

I am using maddy as a send-only SMTP server and it works great, thank you so much for this nice piece of software !

I want set up Delivery Status Notification to my inbox. However they are marked a spam by my email hosting (which has nothing to do with maddy) because the the lack of the mailfrom: spf=none smtp.mailfrom="" smtp.helo=maddy.example.org (I was able to add dkim signing though).

Steps to reproduce

Setup a bounce with a remote MX sending and trigger a bounce message:

Configuration file

target.queue remote_queue {
    target &outbound_delivery

    autogenerated_msg_domain $(primary_domain)
    bounce {
        destination $(local_domains) {
            modify {
                dkim {
                    domains $(local_domains)
                    selector key
                    key_path dkim-keys/{selector}.key
                }
            }
            deliver_to &outbound_delivery
        }
        default_destination {
            reject 550 5.0.0 "Refusing to send DSNs to non-local addresses"
        }
    }
}

Environment information

Relevant code

The last argument of dsnDelivery, err := q.dsnPipeline.Start(mailCtx, dsnMeta, "") is always empty.

https://github.com/foxcpp/maddy/blob/de756c8dc52b69efa511c8340aa29af76c407f93/internal/target/queue/queue.go#L962

Is there any reason this is not set to dsnEnvelope.From (which is set to "MAILER-DAEMON@" + q.autogenMsgDomain) ? If yes, would you consider an option to set it in some cases? Or is there a configuration option that I am missing?

foxcpp commented 8 months ago

RFC 5321 (SMTP spec) requires (a "MUST") all delivery status notifications to be sent with an empty MAIL FROM: https://datatracker.ietf.org/doc/html/rfc5321#section-3.6.3

Of course, SMTP servers MUST NOT send notification messages about problems transporting notification messages. One way to prevent loops in error reporting is to specify a null reverse-path in the MAIL command of a notification message. When such a message is transmitted, the reverse-path MUST be set to null (see Section 4.5.5 for additional discussion).

foxcpp commented 8 months ago

Though, you can "hack" it and rewrite MAIL FROM using modify.replace_rcpt:

modify {
  replace_rcpt static {
    entry "" "MAILER-DAEMON@example.org"
  }
}

Since the problem mentioned in the RFC does not apply to your usecase, it probably will be fine.

lukastribus commented 6 months ago

I have the same need, and I'm really struggling with this.

The debug output doesn't show the actual from used; I tried both replace_rcpt and replace_sender, before and after destination $(local_domains).

I tried static and regexp replacements, but none of it seems to work.

In most configurations I just get a:

failed to enqueue DSN {[...],"reason":"malformed address: address: missing at-sign"}

Can someone post an entire target.queue remote_queue section with this configuration?

foxcpp commented 6 months ago

Apparently, replace_* apply normalization functions to addresses and it does not allow "", hence the error.

Pushed cee5777 to master that allows this.

lukastribus commented 6 months ago

Thank you, with cee5777 this now works as expected:

target.queue remote_queue {
    target &outbound_delivery

    autogenerated_msg_domain $(primary_domain)
    bounce {
        modify {
                replace_sender static { entry "" "MAILER-DAEMON@example.org" }
        }
        destination $(local_domains) {
            modify {
                dkim $(primary_domain) $(local_domains) <YOUR-DKIM-SELECTOR>
            }

            deliver_to &remote_queue

        }
        default_destination {
            reject 550 5.0.0 "Refusing to send DSNs to non-local addresses"
        }
    }
}