Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding Authentication or Software Header on Mail with Bad Syntax #194

Open
B1G-FUNGUS opened this issue Jan 19, 2024 · 2 comments
Open

Adding Authentication or Software Header on Mail with Bad Syntax #194

B1G-FUNGUS opened this issue Jan 19, 2024 · 2 comments

Comments

@B1G-FUNGUS
Copy link

B1G-FUNGUS commented Jan 19, 2024

Version:
OpenDKIM version - 2.11.0~beta2-8+deb12u1
Distro - Debian 12 (bookworm)

Relevant Config:
AlwaysAddARHeader - true
SoftwareHeader - true
WeaksyntaxChecks - true/false, same result either way

Issue:
If OpenDKIM tries to verify mail with a malformed signature (such as "DKIM-Signature: eeee"), it will give a syntax error and accept/reject the mail based off of the "On-BadSignature" option. This is expected, however, if the mail is accepted, no authentication header or software header will be added, even if these options are set.

Other:
From my understanding, one purpose of having the option to always add an authentication header or software header is to make it easier to distinguish between mail that was processed by OpenDKIM and failed, and mail that was not processed in the first place. In my case, I am using it to distinguish between local mail that doesn't need to be signed/verified and external mail that does for a Sieve filter. So, I see this as an issue, as it treats mail which failed a DKIM test the same as mail which was never subject to one. My reasoning is the same as the reasoning in this issue.

However, this might be a non-issue. There might be a reason why OpenDKIM should not or cannot add these headers in such an error which I am not aware of. In that case there are possible workarounds for users like me. Because of the Lua scripting support, you can actually fix this with a minimum of one line (something like odkin.add_header(ctx, "X-dkim-checked", "yes") if you just want to see if the mail was processed), so I guess the real question here is whether or not it's better to have the auth/software headers added on syntax errors by default or to just let users who want that behavior to configure such a workaround. Ideally the former would be implemented, since it would line up with the stance in the sourceforge issue that mail which can't be properly checked still deserves an auth header, but the latter is easier (and such behaviour may only be desired by a small niche).

@B1G-FUNGUS
Copy link
Author

B1G-FUNGUS commented Jan 29, 2024

I took took a look at the source code to see what was going on. Apologies if some of what follows is misinformed, I still feel like I'm missing a lot, but here's some information that I believe is important:

This specific issue (though I believe there may be other times where OpenDKIM exits early on a syntax error) can be located around line 12966 in mlfi_eoh() in opendkim/opendkim.c:

		if (dfc->mctx_dkimv != NULL)
		{
			status = dkim_header(dfc->mctx_dkimv,
			                     (u_char *) dkimf_dstring_get(dfc->mctx_tmpstr),
			                     dkimf_dstring_len(dfc->mctx_tmpstr));

			if (status != DKIM_STAT_OK)
			{
				ms = dkimf_libstatus(ctx, dfc->mctx_dkimv,
				                     "dkim_header()", status);
			}
		}
	}

	/* return any error status from earlier */
	if (ms != SMFIS_CONTINUE)
		return ms;

From my understanding, dkim_header() returns the error DKIM_SYNTAX, and dkimf_libstatus() then returns a sfsistat which says to accept/tempfail/reject the message. In the last two lines mlfi_eoh() returns prematurely because this is not SMFIS_CONTINUE, and dkim_testfile() then does the same. This prevents the AR/software header from being added, since that would happen later on in mlfi_eom().

The two ways I would think to fix this would be (if the status is DKIM_SYNTAX, the user has enabled AlwaysAddARHeader/SoftwareHeader, and OnBadSignature is set to accept) to either continue here as normal or to still return prematurely but add the headers in before doing so. However, maybe there's some problem with these solutions that I'm overlooking.

Finally, I do believe more firmly that this is a real issue. Enabling AlwaysAddARHeader should give you a result even if there is a syntax error with the signature, considering that RFC 8601 describes the neutral and permerror results, both of which would fit a syntax error in the signature.

@andreasschulze
Copy link

andreasschulze commented Aug 19, 2024

one observation:

  • OpenDKIM is validating messages without DKIM-Signature
  • LogWhy is "yes"

if AlwaysAddARHeader is "no", a message "no signature data" is logged
if AlwaysAddARHeader is "yes", a message "no signature data" is NOT logged

I expect, the message "no signature data" is logged in both cases.

-> wrong expectation or a bug?

@mskucherawy could you remember the requrements for AlwaysAddARHeader

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants