Skip to content

Commit becddec

Browse files
committed
milter: make responses configurable
1 parent 59249b8 commit becddec

9 files changed

+175
-22
lines changed

CHANGELOG.md

+4
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ All notable changes to this project will be documented in this file.
1111
- milter - `AuthResIP` configuration option.
1212
- milter - `RequireSafeKeys` configuration option.
1313
- milter - `MinimumKeySizeRSA` configuration option.
14+
- milter - `ResponseDisabled`, `ResponseUnable`, and `ResponseUnwilling`
15+
configuration options.
1416

1517
### Changed
1618
- Custom OpenSSL locations must be configured using `OPENSSL_CFLAGS`
@@ -37,6 +39,8 @@ All notable changes to this project will be documented in this file.
3739
`header.oldest-pass` when appropriate.
3840
- milter - An `ar-test` program for seeing how `Authentication-Results`
3941
headers are parsed is built without making you jump through weird hoops.
42+
- milter - The default behaviour for messages that fail basic validity checks
43+
(malformed headers, too many headers) is to reject them.
4044

4145
### Removed
4246
- libopenarc - `arc_mail_parse()`

openarc/openarc-config.h

+3
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,9 @@ struct configdef arcf_config[] = {
3939
{"PermitAuthenticationOverrides", CONFIG_TYPE_BOOLEAN, false},
4040
{"PidFile", CONFIG_TYPE_STRING, false},
4141
{"RequireSafeKeys", CONFIG_TYPE_BOOLEAN, false},
42+
{"ResponseDisabled", CONFIG_TYPE_STRING, false},
43+
{"ResponseUnable", CONFIG_TYPE_STRING, false},
44+
{"ResponseUnwilling", CONFIG_TYPE_STRING, false},
4245
{"SealHeaderChecks", CONFIG_TYPE_STRING, false},
4346
{"Selector", CONFIG_TYPE_STRING, false},
4447
{"SignatureAlgorithm", CONFIG_TYPE_STRING, false},

openarc/openarc.c

+86-21
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,9 @@ struct arcf_config
135135
size_t conf_keylen; /* key length */
136136
int conf_maxhdrsz; /* max. header size */
137137
int conf_minkeysz; /* min. key size */
138+
int conf_ret_disabled; /* configured not to process */
139+
int conf_ret_unable; /* internal error */
140+
int conf_ret_unwilling; /* badly formed message */
138141
struct config *conf_data; /* configuration data */
139142
ARC_LIB *conf_libopenarc; /* shared library instance */
140143
struct conflist conf_peers; /* peers hosts */
@@ -221,6 +224,14 @@ struct nametable arcf_chainstates[] = {
221224
{NULL, -1 }
222225
};
223226

227+
struct nametable arcf_responses[] = {
228+
{"accept", SMFIS_ACCEPT },
229+
{"discard", SMFIS_DISCARD },
230+
{"reject", SMFIS_REJECT },
231+
{"tempfail", SMFIS_TEMPFAIL},
232+
{NULL, -1 }
233+
};
234+
224235
/* PROTOTYPES */
225236
sfsistat mlfi_abort(SMFICTX *);
226237
sfsistat mlfi_close(SMFICTX *);
@@ -1156,6 +1167,10 @@ arcf_config_new(void)
11561167
new->conf_safekeys = true;
11571168
new->conf_authresip = true;
11581169

1170+
new->conf_ret_disabled = SMFIS_ACCEPT;
1171+
new->conf_ret_unable = SMFIS_TEMPFAIL;
1172+
new->conf_ret_unwilling = SMFIS_REJECT;
1173+
11591174
LIST_INIT(&new->conf_peers);
11601175
LIST_INIT(&new->conf_internal);
11611176

@@ -1543,6 +1558,52 @@ arcf_config_load(struct config *data,
15431558
conf->conf_fixedtime = strtoul(str, &end, 10);
15441559
}
15451560

1561+
str = NULL;
1562+
config_get(data, "ResponseDisabled", &str, sizeof str);
1563+
if (str)
1564+
{
1565+
int resp = arc_name_to_code(arcf_responses, str);
1566+
if (resp == -1)
1567+
{
1568+
snprintf(err, errlen, "%s: invalid response value", str);
1569+
}
1570+
else
1571+
{
1572+
conf->conf_ret_disabled = arc_name_to_code(arcf_responses, str);
1573+
}
1574+
}
1575+
1576+
str = NULL;
1577+
config_get(data, "ResponseUnable", &str, sizeof str);
1578+
if (str)
1579+
{
1580+
int resp = arc_name_to_code(arcf_responses, str);
1581+
if (resp == -1)
1582+
{
1583+
snprintf(err, errlen, "%s: invalid response value", str);
1584+
}
1585+
else
1586+
{
1587+
conf->conf_ret_unable = arc_name_to_code(arcf_responses, str);
1588+
}
1589+
}
1590+
1591+
str = NULL;
1592+
config_get(data, "ResponseUnwilling", &str, sizeof str);
1593+
if (str)
1594+
{
1595+
int resp = arc_name_to_code(arcf_responses, str);
1596+
if (resp == -1)
1597+
{
1598+
snprintf(err, errlen, "%s: invalid response value", str);
1599+
}
1600+
else
1601+
{
1602+
conf->conf_ret_unwilling = arc_name_to_code(arcf_responses,
1603+
str);
1604+
}
1605+
}
1606+
15461607
(void) config_get(data, "TestKeys", &conf->conf_testkeys,
15471608
sizeof conf->conf_testkeys);
15481609

@@ -2785,10 +2846,10 @@ mlfi_connect(SMFICTX *ctx, char *host, _SOCK_ADDR *ip)
27852846
syslog(LOG_ERR, "%s malloc(): %s", host, strerror(errno));
27862847
}
27872848

2849+
int retval = curconf->conf_ret_unable;
27882850
pthread_mutex_unlock(&conf_lock);
27892851

2790-
/* XXX -- result should be selectable */
2791-
return SMFIS_TEMPFAIL;
2852+
return retval;
27922853
}
27932854

27942855
pthread_mutex_lock(&conf_lock);
@@ -2836,9 +2897,11 @@ mlfi_connect(SMFICTX *ctx, char *host, _SOCK_ADDR *ip)
28362897
{
28372898
if (curconf->conf_dolog)
28382899
{
2839-
syslog(LOG_INFO, "ignoring connection from %s", host);
2900+
syslog(
2901+
LOG_INFO, "peer connection from %s, returning %s", host,
2902+
arc_code_to_name(arcf_responses, curconf->conf_ret_disabled));
28402903
}
2841-
return SMFIS_ACCEPT;
2904+
return curconf->conf_ret_disabled;
28422905
}
28432906

28442907
/* infer operating mode if not explicitly set */
@@ -2991,15 +3054,13 @@ mlfi_header(SMFICTX *ctx, char *headerf, char *headerv)
29913054
afc->mctx_hdrbytes + strlen(headerf) + strlen(headerv) + 2 >
29923055
conf->conf_maxhdrsz)
29933056
{
2994-
/* FIXME: this should be configurable, and it might be better to
2995-
* default to rejecting the message.
2996-
*/
29973057
if (conf->conf_dolog)
29983058
{
2999-
syslog(LOG_NOTICE, "too much header data; accepting");
3059+
syslog(LOG_NOTICE, "too much header data, returning %s",
3060+
arc_code_to_name(arcf_responses, conf->conf_ret_unwilling));
30003061
}
30013062

3002-
return SMFIS_ACCEPT;
3063+
return conf->conf_ret_unwilling;
30033064
}
30043065

30053066
/*
@@ -3026,7 +3087,7 @@ mlfi_header(SMFICTX *ctx, char *headerf, char *headerv)
30263087
}
30273088

30283089
arcf_cleanup(ctx);
3029-
return SMFIS_TEMPFAIL;
3090+
return conf->conf_ret_unable;
30303091
}
30313092

30323093
newhdr->hdr_hdr = ARC_STRDUP(headerf);
@@ -3046,7 +3107,7 @@ mlfi_header(SMFICTX *ctx, char *headerf, char *headerv)
30463107

30473108
arcf_cleanup(ctx);
30483109

3049-
return SMFIS_TEMPFAIL;
3110+
return conf->conf_ret_unable;
30503111
}
30513112
}
30523113
else
@@ -3111,7 +3172,7 @@ mlfi_header(SMFICTX *ctx, char *headerf, char *headerv)
31113172
ARC_FREE(newhdr->hdr_val);
31123173
ARC_FREE(newhdr);
31133174
arcf_cleanup(ctx);
3114-
return SMFIS_TEMPFAIL;
3175+
return conf->conf_ret_unable;
31153176
}
31163177

31173178
afc->mctx_hdrbytes += strlen(newhdr->hdr_hdr) + 1;
@@ -3359,7 +3420,7 @@ mlfi_eoh(SMFICTX *ctx)
33593420
afc->mctx_jobid);
33603421
}
33613422

3362-
return SMFIS_ACCEPT;
3423+
return conf->conf_ret_disabled;
33633424
}
33643425
}
33653426
#endif /* USE_JANSSON */
@@ -3381,7 +3442,7 @@ mlfi_eoh(SMFICTX *ctx)
33813442
afc->mctx_jobid, err);
33823443
}
33833444

3384-
return SMFIS_TEMPFAIL;
3445+
return conf->conf_ret_unable;
33853446
}
33863447

33873448
for (hdr = afc->mctx_hqhead; hdr != NULL; hdr = hdr->hdr_next)
@@ -3397,7 +3458,7 @@ mlfi_eoh(SMFICTX *ctx)
33973458
afc->mctx_jobid);
33983459
}
33993460

3400-
return SMFIS_TEMPFAIL;
3461+
return conf->conf_ret_unable;
34013462
}
34023463
}
34033464
else
@@ -3438,7 +3499,11 @@ mlfi_eoh(SMFICTX *ctx)
34383499
afc->mctx_jobid, hdr->hdr_hdr);
34393500
}
34403501

3441-
return SMFIS_TEMPFAIL;
3502+
if (status == ARC_STAT_SYNTAX)
3503+
{
3504+
return conf->conf_ret_unwilling;
3505+
}
3506+
return conf->conf_ret_unable;
34423507
}
34433508
}
34443509

@@ -3512,7 +3577,7 @@ mlfi_body(SMFICTX *ctx, unsigned char *bodyp, size_t bodylen)
35123577
afc->mctx_jobid);
35133578
}
35143579

3515-
return SMFIS_TEMPFAIL;
3580+
return conf->conf_ret_unable;
35163581
}
35173582
}
35183583

@@ -3657,7 +3722,7 @@ mlfi_eom(SMFICTX *ctx)
36573722
syslog(LOG_ERR, "arc_dstring_new() failed");
36583723
}
36593724

3660-
return SMFIS_TEMPFAIL;
3725+
return conf->conf_ret_unable;
36613726
}
36623727
}
36633728

@@ -3693,7 +3758,7 @@ mlfi_eom(SMFICTX *ctx)
36933758
afc->mctx_jobid);
36943759
}
36953760

3696-
return SMFIS_TEMPFAIL;
3761+
return conf->conf_ret_unable;
36973762
}
36983763

36993764
if (BITSET(ARC_MODE_SIGN, cc->cctx_mode))
@@ -3806,7 +3871,7 @@ mlfi_eom(SMFICTX *ctx)
38063871
afc->mctx_jobid);
38073872
}
38083873

3809-
return SMFIS_TEMPFAIL;
3874+
return conf->conf_ret_unable;
38103875
}
38113876

38123877
for (sealhdr = seal; sealhdr != NULL; sealhdr = arc_hdr_next(sealhdr))
@@ -3858,7 +3923,7 @@ mlfi_eom(SMFICTX *ctx)
38583923
afc->mctx_jobid, "");
38593924
}
38603925

3861-
return SMFIS_TEMPFAIL;
3926+
return conf->conf_ret_unable;
38623927
}
38633928

38643929
arc_dstring_blank(afc->mctx_tmpstr);

openarc/openarc.conf.5.in

+29-1
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,34 @@ containing the process ID.
248248
Controls whether the filter requires that private keys have safe file
249249
permissions.
250250

251+
.It Cm ResponseDisabled Pq string
252+
.Brq Cm accept | discard | reject | tempfail
253+
254+
Response to send after determining that this message is one that
255+
the filter is configured not to process.
256+
See
257+
.Cm SealHeaderChecks
258+
and
259+
.Cm PeerList .
260+
The default is
261+
.Cm accept .
262+
263+
.It Cm ResponseUnable Pq string
264+
.Brq Cm accept | discard | reject | tempfail
265+
266+
Response to send after an internal error occurs that makes it
267+
impossible to finish processing the message.
268+
The default is
269+
.Cm tempfail .
270+
271+
.It Cm ResponseUnwilling Pq string
272+
.Brq Cm accept | discard | reject | tempfail
273+
274+
Response to send after a message fails basic validity checks, such as
275+
.Cm MaximumHeaders .
276+
The default is
277+
.Cm reject .
278+
251279
.It Cm SealHeaderChecks Pq string
252280
Identifies a file containing header checks that should be done to determine
253281
whether to seal a message.
@@ -268,7 +296,7 @@ Use
268296
.Ql openarc \-V
269297
to see the list of supported algorithms.
270298
The default is
271-
.Cm rsa-sha256.
299+
.Cm rsa-sha256 .
272300
Other values are not useful if you are intending to interoperate with other
273301
implementers of the ARC protocol.
274302

openarc/openarc.conf.sample

+24
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,30 @@ KeyFile /var/db/dkim/example.private
216216

217217
# RequireSafeKeys yes
218218

219+
## ResponseDisabled { accept | discard | reject | tempfail }
220+
## default "accept"
221+
##
222+
## Response to send after determining that this message is one that
223+
## the filter is configured not to process.
224+
225+
# ResponseDisabled accept
226+
227+
## ResponseUnable { accept | discard | reject | tempfail }
228+
## default "tempfail"
229+
##
230+
## Response to send after an internal error occurs that makes it impossible
231+
## to finish processing the message.
232+
233+
# ResponseUnable tempfail
234+
235+
## ResponseUnwilling { accept | discard | reject | tempfail }
236+
## default "reject"
237+
##
238+
## Response to send after a message fails basic validity checks, such as
239+
## MaximumHeaders.
240+
241+
# ResponseUnwilling reject
242+
219243
## SealHeaderChecks filename
220244
## default (none)
221245
##
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
{
2+
"MaximumHeaders": "1"
3+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{
2+
"PeerList": "peerlist",
3+
"ResponseDisabled": "reject"
4+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{
2+
"MaximumHeaders": "1",
3+
"ResponseUnwilling": "accept"
4+
}

test/test_milter.py

+18
Original file line numberDiff line numberDiff line change
@@ -564,6 +564,12 @@ def test_milter_finalreceiver(run_miltertest):
564564
assert res['headers'][0][1] == ' example.com; arc=pass header.oldest-pass=0 smtp.remote-ip=127.0.0.1 arc.chain="example.com:example.com"'
565565

566566

567+
def test_milter_maximumheaders(run_miltertest):
568+
"""Oversized headers result in message rejection"""
569+
with pytest.raises(miltertest.MilterError, match="Unexpected reply to L: \\('r'"):
570+
run_miltertest()
571+
572+
567573
def test_milter_minimum_key_bits(run_miltertest):
568574
"""A 2048-bit key passes when that is the minimum"""
569575
res = run_miltertest()
@@ -584,6 +590,18 @@ def test_milter_peerlist(run_miltertest):
584590
run_miltertest()
585591

586592

593+
def test_milter_responsedisabled(run_miltertest):
594+
"""Configured to reject messages from peers"""
595+
with pytest.raises(miltertest.MilterError, match='unexpected response: r'):
596+
run_miltertest()
597+
598+
599+
def test_milter_responseunwilling(run_miltertest):
600+
"""Configured to accept messages with too many headers"""
601+
with pytest.raises(miltertest.MilterError, match="Unexpected reply to L: \\('a'"):
602+
run_miltertest()
603+
604+
587605
def test_milter_softwareheader(run_miltertest):
588606
"""Advertise software name, version"""
589607
res = run_miltertest()

0 commit comments

Comments
 (0)