Skip to content

mlfi_eoh: emit dkim=permerror for malformed DKIM-Signature (fixes #194)#403

Merged
thegushi merged 1 commit into
trusteddomainproject:developfrom
thegushi:fix-194-syntax-ar-header
Jun 4, 2026
Merged

mlfi_eoh: emit dkim=permerror for malformed DKIM-Signature (fixes #194)#403
thegushi merged 1 commit into
trusteddomainproject:developfrom
thegushi:fix-194-syntax-ar-header

Conversation

@thegushi

@thegushi thegushi commented Jun 4, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • When dkim_header() returns DKIM_STAT_SYNTAX for a malformed DKIM-Signature, mlfi_eoh() was setting ms and returning early, bypassing mlfi_eom() entirely. With AlwaysAddARHeader or SoftwareHeader enabled, the message was accepted with no Authentication-Results header — making it indistinguishable from mail that was never processed by OpenDKIM.
  • RFC 8601 §2.7.1 defines dkim=permerror for exactly this case.
  • The fix routes syntax errors through the existing mctx_headeronly / DKIMF_STATUS_BADFORMAT path that mlfi_eom() already handles correctly, rather than short-circuiting past it.

How it works

When dkim_header() returns DKIM_STAT_SYNTAX, instead of setting ms (which causes mlfi_eoh() to return early), the fix sets mctx_headeronly, mctx_addheader, and mctx_status=DKIMF_STATUS_BADFORMAT. mlfi_eoh() returns SMFIS_CONTINUE; mlfi_body() skips the body via the existing mctx_headeronly check; mlfi_eom() reaches its own mctx_headeronly branch which emits dkim=permerror and accepts.

A guard is also added to the DKIM_STAT_NOSIG branch of the dkim_eoh() result switch so it does not overwrite a BADFORMAT status already set from the header loop. (A handle with a bad DKIM-Signature produces NOSIG from dkim_eoh_verify() once the bad sig's set_bad flag is set.)

Test

t-test208 confirms the library behaviour the fix relies on: dkim_header() returns DKIM_STAT_SYNTAX for a malformed DKIM-Signature, the handle remains usable, and dkim_eoh() returns DKIM_STAT_NOSIG. Verified on FreeBSD.

Test plan

  • Build and run make check — t-test208 should pass
  • Manual test: send a message with DKIM-Signature: eeee through a milter instance configured with AlwaysAddARHeader yes; confirm Authentication-Results header contains dkim=permerror

…rusteddomainproject#194)

When dkim_header() returns DKIM_STAT_SYNTAX (malformed DKIM-Signature),
mlfi_eoh() was setting ms and returning early, bypassing mlfi_eom()
entirely.  With AlwaysAddARHeader or SoftwareHeader enabled this meant
the message was accepted silently with no Authentication-Results header,
making mail with a bad signature indistinguishable from mail that was
never processed by OpenDKIM.

RFC 8601 section 2.7.1 defines dkim=permerror for exactly this case.

Fix: when dkim_header() returns DKIM_STAT_SYNTAX, call dkimf_libstatus()
to determine the configured OnBadSignature action.  If the action is
accept (the common case), set mctx_headeronly, mctx_addheader, and
mctx_status=DKIMF_STATUS_BADFORMAT so mlfi_eom() emits dkim=permerror
via the existing mctx_headeronly path.  If the action is reject,
tempfail, or discard, set ms as before so the early-exit behaviour is
preserved.

Also guard the DKIM_STAT_NOSIG branch in the dkim_eoh() result switch
so it does not overwrite a BADFORMAT status already set from the header
loop (a handle with a bad DKIM-Signature yields NOSIG from
dkim_eoh_verify() once the bad sig's set_bad flag is set).

Add t-test208 to confirm the library behaviour the fix relies on:
dkim_header() returns DKIM_STAT_SYNTAX for a malformed DKIM-Signature,
the handle remains usable, and dkim_eoh() returns DKIM_STAT_NOSIG.
@thegushi thegushi force-pushed the fix-194-syntax-ar-header branch from 41cbf47 to 8b2644b Compare June 4, 2026 19:08
@thegushi thegushi merged commit f191231 into trusteddomainproject:develop Jun 4, 2026
1 check passed
thegushi added a commit that referenced this pull request Jun 4, 2026
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

Successfully merging this pull request may close these issues.

1 participant