Hello,
sorry for the terrifc delay.
On 2023/10/01 14:59:15 +0200, Philipp <philipp+open...@bureaucracy.de> wrote:
> Hi
>
> Setting Null MX is a way for domainowners to indicate that the domain
> does not accept mail. Currently a Null MX causes a tempfail and the
> mail will be queued and tried to resubmitted till a timeout. With the
> attached patch a Null MX causes a permfail. This way the sender will
> directly get a bounce with the message "Domain does not accept mail".
I'm not sure how much widespread the usage is, but it doesn't seem a
bad feature. It's simple to implement and it provides some (very
small) value.
In general I'm OK with the idea, but would need some OKs from other
developers too.
There is one part of the RFC7505 that I'd like to quote and discuss
with you however. The last paragraph of the section 3 says:
: A domain that advertises a null MX MUST NOT advertise any other MX
: RR.
Your implementation handles the case where there are more than one MX
by setting the `nullmx' field in the shared struct when we encounter
one, and then don't process the other MXs when that field is set.
This is fine.
However, I think we can simplify here by not honouring the Null MX
when there are other MX records. We're not violating the RFC. See
diff below to see what I mean. The only difference is in dns.c, the
rest is the same with yours.
So far I've only compile-tested the code. I don't have a spare domain
to test and don't know of anyone using a Null MX.
> Because some domains set the MX record to "localhost." to get a similar
> efect the secound patch ignores "localhost." MX entries and handles a MX
> containing only "localhost." records like a Null MX.
This could be simplified too. On top of diff below, it would need
just something among the lines of:
- if (rr.rr.mx.preference == 0 && !strcmp(buf, "")) {
+ if ((rr.rr.mx.preference == 0 && !strcmp(buf, "")) ||
+ !strcasecmp(buf, "localhost")) {
if (found)
continue;
m_create(s->p, IMSG_MTA_DNS_HOST_END, 0, 0, -1);
I'm still not convinced about this part however. It feels wrong to me
to hardcode such a check, it seems too much paranoic. The follow-up
would be to check for localhost in dns_dispatch_host too? ;)
Thanks,
Omar Polo
diff 2d025d839f99dc09ee525c11a4ed09a0f3bbe7d0
8d6138e5b1e0bc112ff2584d8528e6bc95a39b6f
commit - 2d025d839f99dc09ee525c11a4ed09a0f3bbe7d0
commit + 8d6138e5b1e0bc112ff2584d8528e6bc95a39b6f
blob - 4cf5d23d1d14b5400c6f4429dae0a4f6490073d4
blob + effc07d142895fb2b622242efcd5c69da7f3b67c
--- usr.sbin/smtpd/dns.c
+++ usr.sbin/smtpd/dns.c
@@ -259,8 +259,22 @@ dns_dispatch_mx(struct asr_result *ar, void *arg)
unpack_rr(&pack, &rr);
if (rr.rr_type != T_MX)
continue;
+
print_dname(rr.rr.mx.exchange, buf, sizeof(buf));
buf[strlen(buf) - 1] = '\0';
+
+ if (rr.rr.mx.preference == 0 && !strcmp(buf, "")) {
+ if (found)
+ continue;
+ m_create(s->p, IMSG_MTA_DNS_HOST_END, 0, 0, -1);
+ m_add_id(s->p, s->reqid);
+ m_add_int(s->p, DNS_NULLMX);
+ m_close(s->p);
+ free(s);
+ found++;
+ break;
+ }
+
dns_lookup_host(s, buf, rr.rr.mx.preference);
found++;
}
blob - c0d0fc02931b90409441035d2744923af9e42df1
blob + 25e158b68a88c8485428a2476c1c557f8c60404d
--- usr.sbin/smtpd/mta.c
+++ usr.sbin/smtpd/mta.c
@@ -1088,6 +1088,10 @@ mta_on_mx(void *tag, void *arg, void *data)
else
relay->failstr = "No MX found for domain";
break;
+ case DNS_NULLMX:
+ relay->fail = IMSG_MTA_DELIVERY_PERMFAIL;
+ relay->failstr = "Domain does not accept mail";
+ break;
default:
fatalx("bad DNS lookup error code");
break;
blob - 6781286928da45e6159bce81ff2437011ebdef72
blob + 9f4732d1c27f6ef9f62951f0207f209a83038dcf
--- usr.sbin/smtpd/smtpd.h
+++ usr.sbin/smtpd/smtpd.h
@@ -1026,6 +1026,8 @@ enum dns_error {
DNS_EINVAL,
DNS_ENONAME,
DNS_ENOTFOUND,
+ /* rfc 7505 */
+ DNS_NULLMX,
};
enum lka_resp_status {
