Comment 16 for bug 1770532

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

I studied the three patches - thanks to all the people linking to them.

Yes Fedora took "only" the old version [1], but given how the mail threads read we should IMHO go go with one of the newer ones.

Sticking to the names coined by Simon Deziel:
Giovanni's old: working but a few edge cases left (before dkim_signatures_valid)
Adam Jacobs: picked a new place for the same change further improving the situation (in dkim_make_signatures)
Giovanni's new: added the same also at /^MAIL\z/ section of process_smtp_request
Adam Jacobs new: a new place at the beginning of dkim_make_signatures

I've read through the three of them and agree to Adam as being the most complrehensive that should cover all cases. Unless I failed to track the control flow in this 35kloc perl thing :-/!!!

The former patches might have changed other code paths, as usually "$msginfo->originating(c('originating')) if $msginfo;" would have been set by load_policy_bank depending on some attributes of the message. But if we are in dkim_make_signatures there is no way this should not be set (not trying to get mad at the control flow) it seems safe to set it if we are in that path not bothering too much how we got here - that would be for upstream which seems dead reading between the lines of the linked discussions.

I think any of these might even work "for the case reported here", and due to that this needs to test a bunch of non-dkim-signing setups if they are affected in any bad way more than the actual case test.

I'll prep an MP [2] with for review and an associated cosmic PPA to test [3].

[1]: https://src.fedoraproject.org/cgit/rpms/amavisd-new.git/tree/amavisd-new-2.11.0-detect_originating_email.patch
[2]: https://code.launchpad.net/~paelzer/ubuntu/+source/amavisd-new/+git/amavisd-new/+merge/355543
[3]: https://launchpad.net/~ci-train-ppa-service/+archive/ubuntu/3436