Implement new DELIVERY config parameter.

iskunk commented 1 year ago

Implementation of the feature discussed in #121.

Some remarks:

I shortened "DELIVERYMODE" to "DELIVERY" for brevity;
I fixed one unrelated bug along the way: specifying FINGERPRINT without an argument led to a segfault instead of an error;
NULLCLIENT remains recognized in the config file, but I removed mention of it from the documentation as DELIVERY REMOTEONLY supersedes it.

Between this new parameter, use/absence of a smarthost, and a local (user) vs. remote (user@host) e-mail recipient address, there are a number of permutations of operation. The table below summarizes them, and the result:

Address	Smarthost?	DELIVERY	Result
local	no	LOCALANDREMOTE	delivered locally
local	no	LOCALONLY	delivered locally
local	no	REMOTEONLY	error (SMARTHOST required, rc=78)
local	yes	LOCALANDREMOTE	delivered locally
local	yes	LOCALONLY	delivered locally
local	yes	REMOTEONLY	sent to SMARTHOST, bounce likely (rc=70)
remote	no	LOCALANDREMOTE	sent directly to address host
remote	no	LOCALONLY	error (invalid recipient, rc=65)
remote	no	REMOTEONLY	error (SMARTHOST required, rc=78)
remote	yes	LOCALANDREMOTE	sent to SMARTHOST
remote	yes	LOCALONLY	error (invalid recipient, rc=65)
remote	yes	REMOTEONLY	sent to SMARTHOST

I used a script to test the various cases, which you might find useful for that purpose: dma-delivery-test.sh.txt

corecode commented 1 year ago

I'm a bit puzzled by: remote address + REMOTEONLY will fail without SMARTHOST. Feels to violate POLA. Maybe we should call it SMARTHOSTONLY?

iskunk commented 1 year ago

Note that the REMOTEONLY behavior is the same as NULLCLIENT (in fact, the latter is now synonymous with the former, for the sake of backward compatibility), and the failure when specifying REMOTEONLY without SMARTHOST occurs on any invocation of dma.

Renaming it to SMARTHOSTONLY would complicate the current [mental] model of "local" and "remote" effectively being checkboxes, such that you can choose one, or the other, or both (but not neither), hence the LOCALANDREMOTE option. And the direct-delivery use case is, IMO, fairly niche---in practical terms, it is only usable with private mail servers, and if you have one of those, then a suitable smarthost is usually part of the setup.

corecode commented 1 year ago

I see. Still REMOTEONLY sounds like it should be able to deliver to remote addresses, which isn't the case - it can only deliver to a smarthost. I think the patch itself is great, but we need to figure out a meaningful name that won't confuse.

iskunk commented 1 year ago

If we went with SMARTHOSTONLY, then the other two keywords would presumably be LOCALONLY and LOCALANDSMARTHOST. But then LOCALANDSMARTHOST without SMARTHOST would be, on its face, an invalid config.

If you had LOCALANDREMOTE, LOCALONLY, and SMARTHOSTONLY, then users might wonder why there is no REMOTEONLY option.

We could fix the delivery code to allow REMOTEONLY to work without SMARTHOST, thus allowing the "remote" descriptor to apply to both smarthost and direct delivery mechanisms. But that's a bit beyond the scope of this PR.

Or instead of having a DELIVERY <option> parameter, we could have three parameters to enable/disable the three different kinds of delivery, e.g.

# Uncomment to enable local delivery
DELIVERLOCAL

# Uncomment to enable direct delivery to remote mail servers
# (ignored if DELIVERSMARTHOST is set)
DELIVERDIRECT

# Uncomment to relay messages to SMARTHOST
DELIVERSMARTHOST

This would also require some changes to the delivery code, however.

tedkotz commented 1 year ago

Doesn't having a SMARTHOST configured imply DELIVERSMARTHOST?

And I think it might make more sense to invert the logic on the other 2: NOLOCALDELIVERY and NODIRECTDELIVERY.

This way "everything off" is still "normal" behavior and it doesn't break peoples configs on upgrade.

Then: 1) a NULLCLIENT is just a synonym for NOLOCALDELIVERY with a SMARTHOST defined. 2) a LOCALONLY #62 would be a synonym for NODIRECTDELIVERY without a SMARTHOST defined. 3) a SMARTHOST definition would imply NODIRECTDELIVERY

iskunk commented 1 year ago

Doesn't having a SMARTHOST configured imply DELIVERSMARTHOST?

Yes, I'm not keen on that semantic redundancy.

And I think it might make more sense to invert the logic on the other 2: NOLOCALDELIVERY and NODIRECTDELIVERY.

That seems like a reasonable approach. Between those two keywords and SMARTHOST, the invalid combinations would be...

no SMARTHOST + NOLOCALDELIVERY + NODIRECTDELIVERY (nowhere for mail to go)
no SMARTHOST + NOLOCALDELIVERY + local address (can't direct-deliver without a host)
no SMARTHOST + NODIRECTDELIVERY + remote address (no way to reach remote destination)

I think that's everything... please comment if you see other examples. (SMARTHOST + NOLOCALDELIVERY + local address is a borderline case, depending on how the smarthost qualifies the address.)

For brevity, I would suggest shortening the new keywords to NOLOCAL and NODIRECT. IMO, the DELIVERY suffix doesn't help much, and the shorter forms fit in better with the existing keywords.

2. a LOCALONLY (#62) would be a synonym for NODIRECTDELIVERY without a SMARTHOST defined.

Note that LOCALONLY exists only in PR-space, so there is no need to support it. NULLCLIENT is a different story, of course.

@corecode, what are your thoughts on this? I think we're all in agreement on what we want to make possible in dma; it is just a matter of articulating the configuration for it in as clear a manner as possible.

corecode commented 1 year ago

And I think it might make more sense to invert the logic on the other 2: NOLOCALDELIVERY and NODIRECTDELIVERY.

That seems like a reasonable approach. Between those two keywords and SMARTHOST, the invalid combinations would be...

I keep getting confused, it's not obvious how things work.

If I only set SMARTHOST, shouldn't mail only get delivered to the smarthost (I think that's how e.g. postfix does it)?

So NOLOCAL and NODIRECT are implied if I set SMARTHOST?

Can we do something like:

DELIVERY smarthost.example.org   # only use smarthost
DELIVERY LOCAL # only deliver locally
DELIVERY MX # only deliver to MX, no local
DELIVERY ALL # deliver to local or MX

@corecode, what are your thoughts on this? I think we're all in agreement on what we want to make possible in dma; it is just a matter of articulating the configuration for it in as clear a manner as possible.

Absolutely. What do you think about my suggestion? Seems like we had this before, but it also feels new?

iskunk commented 1 year ago

If I only set SMARTHOST, shouldn't mail only get delivered to the smarthost (I think that's how e.g. postfix does it)?

So NOLOCAL and NODIRECT are implied if I set SMARTHOST?

Not necessarily. There are [at least] two exception cases when SMARTHOST is set and the other two keywords aren't:

A local-only address (i.e. no @domain part): This should be delivered locally. It is possible for a smarthost to interpret such an address, but that is unusual, and likely to fail. (It certainly failed in my big org's mail server, despite there being an obvious domain that could have been inferred.)
Dead smarthost + remote address: dma could fall back to direct/MX delivery.

Can we do something like:

That wouldn't cover smarthost + local, or local + MX (both reasonable scenarios), and ALL doesn't really mean "all" as it refers to only two of the three cases.

corecode commented 1 year ago

A local-only address (i.e. no @domain part): This should be delivered locally. It is possible for a smarthost to interpret such an address, but that is unusual, and likely to fail. (It certainly failed in my big org's mail server, despite there being an obvious domain that could have been inferred.)

How do you send mail from a MUA? Usually delivery to a smarthost is configured in a way to accept all locally originating mails, and the smarthost does masquerading, etc.

Dead smarthost + remote address: dma could fall back to direct/MX delivery.

No, that makes no sense. Smarthost is used when you never want to do direct MX delivery.

That wouldn't cover smarthost + local, or local + MX (both reasonable scenarios), and ALL doesn't really mean "all" as it refers to only two of the three cases.

Maybe LOCAL,MX is better than ALL?

then we could also do LOCAL,myhost.example.org.

iskunk commented 1 year ago

How do you send mail from a MUA? Usually delivery to a smarthost is configured in a way to accept all locally originating mails, and the smarthost does masquerading, etc.

Traditionally, MUAs would invoke /usr/sbin/sendmail to send mail, on a system with a full-featured MTA+MDA. While most mail would be queued for remote delivery, you could have local recipients, too. Consider the following two mailx examples:

echo hello | mail -s "Greetings" bob

echo hello | mail -s "Greetings" fred@example.com

Typically, the first message would never leave the system. Why would it? You're already at the destination.

That said, if you do want even local addresses to go to the smarthost, this can be specified with the NOLOCAL keyword. So both cases are covered.

No, that makes no sense. Smarthost is used when you never want to do direct MX delivery.

It's conceivable to have MX fallback as a resiliency measure (which can be opted out of with NODIRECT). But I have no interest in MX mode myself, as it breaks SPF/DKIM/everything.

Maybe LOCAL,MX is better than ALL?

then we could also do LOCAL,myhost.example.org.

LOCAL,MX is better, but the comma would be new to the dma config syntax. Also, I'm not keen on the approach of specifying a hostname where a keyword might go, since then you can run into "but my smarthost's hostname is 'MX' actually" situations.

corecode / dma

Implement new DELIVERY config parameter. #125