Skip to content

Commit 41cbf47

Browse files
committed
mlfi_eoh: emit dkim=permerror for malformed DKIM-Signature headers (#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, set mctx_headeronly, mctx_addheader, and mctx_status=DKIMF_STATUS_BADFORMAT instead of setting ms. mlfi_eoh() returns SMFIS_CONTINUE, the body is skipped via the existing mctx_headeronly path in mlfi_body(), and mlfi_eom()'s mctx_headeronly branch emits dkim=permerror and accepts. 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.
1 parent 9def476 commit 41cbf47

3 files changed

Lines changed: 111 additions & 6 deletions

File tree

libopendkim/tests/Makefile.am

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ check_PROGRAMS = t-test00 t-test01 t-test02 t-test03 t-test04 \
4444
t-test157 t-test158 t-test159 t-test160 \
4545
t-test200 t-test201 t-test202 t-test203 \
4646
t-test204 t-test205 \
47-
t-test206 t-test207 \
47+
t-test206 t-test207 t-test208 \
4848
t-signperf t-verifyperf
4949
check_SCRIPTS = t-signperf-sha1 t-signperf-relaxed-relaxed \
5050
t-signperf-simple-simple \
@@ -243,6 +243,7 @@ t_test204_SOURCES = t-test204.c t-testdata.h
243243
t_test205_SOURCES = t-test205.c t-testdata.h
244244
t_test206_SOURCES = t-test206.c t-testdata.h
245245
t_test207_SOURCES = t-test207.c t-testdata.h
246+
t_test208_SOURCES = t-test208.c t-testdata.h
246247

247248
MOSTLYCLEANFILES=
248249

libopendkim/tests/t-test208.c

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
/*
2+
** Copyright (c) 2005-2008 Sendmail, Inc. and its suppliers.
3+
** All rights reserved.
4+
**
5+
** Copyright (c) 2009, 2011-2013, 2026, The Trusted Domain Project.
6+
** All rights reserved.
7+
*/
8+
9+
/*
10+
** t-test208 -- verify that a malformed DKIM-Signature header produces
11+
** DKIM_STAT_SYNTAX from dkim_header() and DKIM_STAT_NOSIG
12+
** from dkim_eoh(), confirming the library behaviour that
13+
** the mlfi_eoh() fix for issue #194 relies on.
14+
*/
15+
16+
#include "build-config.h"
17+
18+
/* system includes */
19+
#include <sys/types.h>
20+
#include <assert.h>
21+
#include <string.h>
22+
#include <stdio.h>
23+
24+
#ifdef USE_GNUTLS
25+
# include <gnutls/gnutls.h>
26+
#endif /* USE_GNUTLS */
27+
28+
/* libopendkim includes */
29+
#include "../dkim.h"
30+
#include "t-testdata.h"
31+
32+
int
33+
main(int argc, char **argv)
34+
{
35+
DKIM_STAT status;
36+
DKIM *dkim;
37+
DKIM_LIB *lib;
38+
39+
printf("*** malformed DKIM-Signature: SYNTAX from dkim_header, NOSIG from dkim_eoh\n");
40+
41+
#ifdef USE_GNUTLS
42+
(void) gnutls_global_init();
43+
#endif /* USE_GNUTLS */
44+
45+
lib = dkim_init(NULL, NULL);
46+
assert(lib != NULL);
47+
48+
/*
49+
** Feed a message whose only DKIM-Signature has no valid tag-value
50+
** pairs. dkim_header() must return DKIM_STAT_SYNTAX; the handle
51+
** must remain usable; dkim_eoh() must return DKIM_STAT_NOSIG
52+
** (the bad sig is skipped, leaving no valid signatures).
53+
*/
54+
55+
dkim = dkim_verify(lib, JOBID, NULL, &status);
56+
assert(dkim != NULL);
57+
58+
status = dkim_header(dkim,
59+
(u_char *) "DKIM-Signature: eeee",
60+
strlen("DKIM-Signature: eeee"));
61+
assert(status == DKIM_STAT_SYNTAX);
62+
63+
status = dkim_header(dkim, (u_char *) HEADER05, strlen(HEADER05));
64+
assert(status == DKIM_STAT_OK);
65+
66+
status = dkim_header(dkim, (u_char *) HEADER06, strlen(HEADER06));
67+
assert(status == DKIM_STAT_OK);
68+
69+
status = dkim_header(dkim, (u_char *) HEADER07, strlen(HEADER07));
70+
assert(status == DKIM_STAT_OK);
71+
72+
status = dkim_header(dkim, (u_char *) HEADER08, strlen(HEADER08));
73+
assert(status == DKIM_STAT_OK);
74+
75+
status = dkim_eoh(dkim);
76+
assert(status == DKIM_STAT_NOSIG);
77+
78+
status = dkim_free(dkim);
79+
assert(status == DKIM_STAT_OK);
80+
81+
dkim_close(lib);
82+
83+
return 0;
84+
}

opendkim/opendkim.c

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13477,8 +13477,24 @@ mlfi_eoh(SMFICTX *ctx)
1347713477

1347813478
if (status != DKIM_STAT_OK)
1347913479
{
13480-
ms = dkimf_libstatus(ctx, dfc->mctx_dkimv,
13481-
"dkim_header()", status);
13480+
if (status == DKIM_STAT_SYNTAX)
13481+
{
13482+
/*
13483+
** Malformed DKIM-Signature: don't
13484+
** return early. Mark the context so
13485+
** mlfi_eom() emits dkim=permerror via
13486+
** the mctx_headeronly path.
13487+
*/
13488+
dfc->mctx_status = DKIMF_STATUS_BADFORMAT;
13489+
dfc->mctx_addheader = TRUE;
13490+
dfc->mctx_headeronly = TRUE;
13491+
}
13492+
else
13493+
{
13494+
ms = dkimf_libstatus(ctx, dfc->mctx_dkimv,
13495+
"dkim_header()",
13496+
status);
13497+
}
1348213498
}
1348313499
}
1348413500
}
@@ -13571,9 +13587,13 @@ mlfi_eoh(SMFICTX *ctx)
1357113587
return SMFIS_CONTINUE;
1357213588

1357313589
case DKIM_STAT_NOSIG:
13574-
dfc->mctx_status = DKIMF_STATUS_NOSIGNATURE;
13575-
if (conf->conf_alwaysaddar)
13576-
dfc->mctx_addheader = TRUE;
13590+
/* don't overwrite BADFORMAT set by a dkim_header() syntax error */
13591+
if (dfc->mctx_status != DKIMF_STATUS_BADFORMAT)
13592+
{
13593+
dfc->mctx_status = DKIMF_STATUS_NOSIGNATURE;
13594+
if (conf->conf_alwaysaddar)
13595+
dfc->mctx_addheader = TRUE;
13596+
}
1357713597
return SMFIS_CONTINUE;
1357813598

1357913599
case DKIM_STAT_NOKEY:

0 commit comments

Comments
 (0)