RE: anti-RFE

> -----Original Message-----
> From: opendkim-dev-bounce_at_lists.opendkim.org [mailto:opendkim-dev-
> bounce_at_lists.opendkim.org] On Behalf Of Daniel Black
> Sent: Saturday, June 26, 2010 10:06 PM
> To: opendkim-dev_at_lists.opendkim.org
> Subject: anti-RFE
>
> Early on, in dkim-milter/opendkim features were added for probably some
> not so
> common causes. This has increases our user base at the expense of code
> complexity. The additions of lua, dataset abstraction and
> SigningTable/KeyTable has created a very generic interface. The generic
> interfaces can do some of what has already been added as features
> earlier.

I have two concerns with this sort of pruning:

(1) Losing backward compatibility. We don't really have a reliable way to know which of these might be in use at more than a couple of places; we could ask on opendkim-users but there's no guarantee everyone using feature X is on there, much less will actually respond. But otherwise I agree, code simplicity is a good idea.

(2) The expense of Lua, which we don't know about yet. If invoking the Lua interpreter just for one line of code (see your first example below) carries substantial cost, especially since it's going to be executed for each message, then converting a simple variable check into a Lua script might bear more cost than we're assuming right now.

I think what I'll do is conduct some experiments with miltertest to measure the difference and report back. That'll give us some meaningful guidance.

> [...]
>
> As a first rough cut:
>
> ADSPNoSuchDomain - could be done in lua with a
> odkim.get_policy=DKIMF_POLICY_NXDOMAIN comparison

Might be more expensive to evaluate in a Lua script than in C.

> AllowSHA1Only - is this still needed?

I added this as a convenience for sites that would have some difficulty upgrading their OpenSSL infrastructure (i.e. restrictive package dependencies). No idea if it's actually needed these days. It's inexpensive to keep around though as it's only checked once during startup.

> AuthservIDWithJobID -needed as option? can this just default to true?

This needs to stay actually. Some sites are okay with this string changing per-message, but if you read RFC5451 it's fairly obvious that this generally needs to be a stable value.

> BodyLengths - probably on the lower priority list of things to remove.

I think there are enough people who still think this is a solution for lists that we should leave it in as a native function. It would, for example, be costly to force this into a Lua script that just always returns "true".

> BodyLengthDBFile - was designed to do l= for mailing lists. Once SF
> #2964366
> is done this should be an easy lua.

I can add support for querying that specific DB while we wait for 2964366 in the interim, if we want that. But this is the first good argument for pushing up the priority of that RFE.

> ExemptDomains - seems like it can be done in lua with the right logic.
> I assume this is done it lua with a odkim.set_result(ctx,SMFIS_ACCEPT).

That feature refers to a list of domains which should not get signed mail from you. So in Lua, you would just return from the script without doing anything in such cases, i.e. without calling odkim.sign().

> IdentityHeader/IdentityHeaderRemove - could be scriptable. needs
> odkim.sig_setidentity function though

Yep, we'd need that. There's an open RFE that probably means we'll need that function anyway, either natively as a fixed string, or through the SigningTable or KeyTable, or through Lua (or both of the last two).

> MaximumSignedBytes - still useful? Missing lua function to implement
> there
> however.

Right, this would have to be exposed, probably by an amendment to the odkim.sign() function.

> Minimum - looks like an advanced filter function.

Yes, but one that it can already do. I think the sample "final" script shows how it could be done.

> Quarantine - can "quarantine" be added to the
> accept,discard,tempfail,reject
> list of options associated with On-* ?

Yep, it could. Want to open an RFE for that?

> ReplaceRules - seems like an advanced option. It was also suggested to
> me that
> adding a [listid] to a subject line may improve acceptance. Needs lua
> function
> for it though.

Generally I think this code should be fully activated and not just an FFR, as long as someone's been using it and reports that it's stable. In fact I should probably poll the users to see which --enable features are being commonly used, and promote them to full code.

Your suggestion is interesting. ReplaceRules is currently implemented in the mlfi_header() function. We could do this in the setup script but it would require a little extra memory management. For a cleaner approach we might want to have a fourth Lua hook that's run once per header, but that makes my concern about the expense of invoking Lua that much greater.

> RemoveARFrom - is this is still desired? remove_header is missing from
> lua.

Control over AR header fields could be added in the final script, needing, as you pointed out, a remove header function. Have to be careful though not to remove header fields that are covered by current signatures.

> RemoveOldSignatures - is this still desired?

Isn't it something we're going to need with the lists BCP?

> Reputation* - is this something that is advanced scripting? While there
> is an
> add_header function is adding a AR header a worthy function to
> introduce?

Actually I think it is. There's an open RFE to control when an AR header gets added, and pushing that logic to Lua might make sense. The specific RFE is to add an AR for DKIM only if it succeeds or if DK also fails; if DKIM fails but DK succeeds, only add one for DK.

> SelectorHeader/SelectorHeaderRemove - seems like advanced functionality

Yes, this is probably not needed. Again, I need to poll the user base to see if anyone's using it.

> SenderHeaders - what was the purpose here?

It's a mechanism for controlling how the filter selects who the sender is for a message in order to decide which key to apply. The default is just "From", but if for example you wanted to pick the signing key based on what's in "Sender" or some made up "X-Sign-As-If-From" header field, you could do that too.

In some sense it's a holdover from DK, which checked Sender before From. As the right thing to do was still under discussion, I made it configurable so people could experiment.

It's another one about which we need to poll the user base.

It also impacts which header fields ADSP checks to determine the author domain. That is clearly a bug, something that needs to be fixed. There's an open bug about it.

> SubDomains - sounds like something that can be done with regex

A small sequence of strcmp() calls is probably cheaper than full regex, no?

> TrustSignaturesFrom - I'm a little unsure what this is but is seems
> like
> something a advanced lua script should do.

In the early days of dkim-milter I visited AOL to find out what their needs are with respect to DKIM. They had decided they wanted to provide DKIM service that only paid attention to author signatures. This function allows that; the filter always trusts author signatures, plus any signature from a domain in this data set, and any other signature is ignored. You could add re-signing domains you trust too. By default it trusts everything, but this is the mechanism by which you would assert this kind of logic.

The setup script can implement this already.

> VBR* - seem like Reputation* as an advanced selection criteria.
> a generic DNS interface and async callback is probably desirable

It's implemented with calls to arlib already, but not exposed via Lua.

> Even if these don't end up being removed having lua scripts by
> comparison will
> demonstrate the flexibility of options available to the user.

I definitely agree with that. The sample Lua scripts we have in the opendkim directory implement a few of these things but a more comprehensive set would certainly be valuable.
Received on Fri Jul 02 2010 - 18:17:21 PST