‘KeepAuthResults = no’ (the default setting) may delete the wrong headers

[glts]

Authentication-Results headers are deleted by iterating over them in forward direction. But because each deletion shifts headers that come after it, this procedure deletes the wrong headers after the first one. I verified this with Postfix.

https://github.com/trusteddomainproject/OpenDKIM/blob/rel-opendkim-2-11-0-Beta2/opendkim/opendkim.c#L13636-L13639

(My solution: disable this code with KeepAuthResults = yes and prepend a milter https://crates.io/crates/spf-milter that does this correctly)

[Rombobeorn]

I made the same discovery while trying to figure out how to get RemoveARFrom to work. This defect caused me quite some consternation before I figured out what was going on.

With this line in /etc/opendkim.conf:

RemoveARFrom 2.example,3.example,5.example

this set of header fields:

Authentication-Results: 1.example; dkim=pass
Authentication-Results: 2.example; dkim=pass
Authentication-Results: 3.example; dkim=pass
Authentication-Results: 4.example; dkim=pass
Authentication-Results: 5.example; dkim=pass
Authentication-Results: 6.example; dkim=pass
Authentication-Results: 7.example; dkim=pass
Authentication-Results: 8.example; dkim=pass

gets transformed into this:

Authentication-Results: 1.example; dkim=pass
Authentication-Results: 3.example; dkim=pass
Authentication-Results: 5.example; dkim=pass
Authentication-Results: 6.example; dkim=pass
Authentication-Results: 8.example; dkim=pass

That is, the first matching header field gets removed correctly, but for the second match the one below it gets removed, and for the third match the one two steps below gets removed.

I found this with OpenDKIM 2.11.0-Beta2, Libmilter 8.15.2 and Postfix 3.5.13.

Evidently, when Postfix removes a header field, the ordinal numbers of the following fields change, and Postfix's and OpenDKIM's ideas of the fields' ordinal numbers get out of sync.

When removing elements from a list, it's generally safer to process the list in reverse so that the elements that get renumbered are those you have already processed.

Suggestions for a solution:

1: Count instances of Authentication-Results in mlfi_header. In mlfi_eom, walk the list of header fields backwards and count down instead of up.

2: When an instance to remove is found, push its index to a stack. When done walking the list of header fields, pop indices from the stack and call dkimf_chgheader. The use of a stack causes the last instance to be removed first.

3: If you are confident that all current and future MTAs count the header fields anew each time, then OpenDKIM can compensate by decreasing c if dkimf_chgheader succeeds.

[Rombobeorn]

CVE-2022-48521 has been assigned to this vulnerability.

[mdomsch]

Fedora bugs https://bugzilla.redhat.com/show_bug.cgi?id=2223270 and https://bugzilla.redhat.com/show_bug.cgi?id=2223352 are reported for this.

[vvvllll]

So is the whole TrustedDomainProject dead?

Nothing for years and distributions are only putting few fixes separately. Opendkim suffers from multiple bugs including this security issue..

[mdomsch]

Best as I can tell, yes, it's dead. I'm in the process of orphaning the projects in Fedora and EPEL as I'm not able to maintain these myself and upstream has had no updates in years. Red Hat has a couple engineers who are applying one recent proposed security-related patch, but it would be better if there were an actual patch from the upstream developers.

Thanks, Matt

On Fri, Dec 8, 2023 at 1:36 PM vvvllll @.***> wrote:

So is the whole TrustedDomainProject dead?

Nothing for years and distributions are only putting few fixes separately. Opendkim suffers from multiple bugs including this security issue..

— Reply to this email directly, view it on GitHub https://github.com/trusteddomainproject/OpenDKIM/issues/148#issuecomment-1847650531, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFIHKRF7PIECEUM4EKTW2TYINM4JAVCNFSM5Q4ANN72U5DIOJSWCZC7NNSXTN2JONZXKZKDN5WW2ZLOOQ5TCOBUG43DKMBVGMYQ . You are receiving this because you commented.Message ID: @.***>