On 2023/10/17 22:27, Philipp wrote: > [2023-10-17 17:32] Omar Polo <o...@omarpolo.com> > > > > 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. > > My reasaning for that was: When you set Null MX you explicitly opt out > from reciving mail even when you violate the spec. But if you want > it diffrent I can change it.
I think in that situation, the recipient's DNS admin has *not* set a valid nullmx, and delivery to other MXes should still be attempted. > But I don't think your proposed patch is a good solution, because the > result depend on the order of the RR in the repsonse. The problem is > when the first entry in the response is a Null MX your patch truncate > the other results and a bounce is generated. But when the Null MX is > later in the response the other entries are used to deliver the mail. > > > 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. > > To test Null MX you can use example.com. To test the localhost patch a > friend of me has set up mxtest.yannikenss.de. > > > > 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: > > When localhost and Null MX should be handled the same this could be > simplified in one check, yes. But that "localhost." and Null MX are > handled different was on purpose. Because I would say setting a Null MX > is a explicit opt out from reciving mail and setting MX to localhost > is implicit. As said before, I have no problem which changing this. > > > 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? ;) > > The idea[1] about this part was because setting "localhost." as MX is used > in a similiar way as Null MX[0]. So handle this in a simmilar way as > Null MX make sense. This way the sender will get a better bounce message. > > Philipp > > [0] https://www.netmeister.org/blog/mx-diversity.html > > [1] I still belive OpenSMTPD should only connect to public routable > addresses for relay. So don't connect to something like 127.0.0.1/8, > ::1/128, RFC1918 addresses, ULA, ... > But this is independent of this patch. Support for nullmx makes sense to me. Perhaps also the localhost handling. But some intranet setups may require connections to RFC1918/ULA addresses - I don't think an MTA should prevent these. BTW, you can already block those at the DNS resolver level in a configurable way - see unbound's "private-address" feature.
