From ccd528f9be69444d216fd28328c286efe6a02437 Mon Sep 17 00:00:00 2001 From: FUTATSUKI Yasuhito Date: Thu, 12 Sep 2024 18:15:10 +0900 Subject: [PATCH 1/2] Improve AR parsing openarc/openarc-ar.c: silence a compiler with ARTEST defined. * openarc/openarc-ar.c (main): explicitly cast pointer to signed char into pointer to u_char. openarc/openarc-ar.c: guarantiee that no-result AR has no other resifo * openarc/openarc.c (ares_parse): Check prevstate if "none" is found on input token in state 3. openarc/openarc-ar.c: Store truely result comment into result_comment field "Result comment" is a comment just after "result" of the "methodspec". No other comments that can be allowed in AR syntax should be ignored. * openarc/openarc-ar.c (ares_parse): Check prevstate before storing result comment. If it stored once update prevstate so as not to store again. openarc/openarc-ar.c: Overwriting former result instead of dedup after adding When the parser find a new result and its "method" is not "dkim", overwriting the old result of same "method" if exists, instead of check the duplicate of the latest result just finished recording and move it. This may allow to parse longer header a bit. * openarc/openarc-ar.c (ares_dedup): Removed (ares_method_seen): New. (ares_parse): As the description above. openarc/openarc-ar.c: Ignore results of unknown methods. According to RFC 8601 Section 2.7.6[1], unknown authentication methods in AR header should be ignored or deleted. With this commit, ares_parse() does not store the result of authres which method is 'unknown'. [1] https://www.rfc-editor.org/rfc/rfc8601.html#section-2.7.6 * openarc/openarc-ar.c (ares_parse): As described above. openarc/openarc-ar.c: continue parsing after reached the limit of the number of auth results. * openarc/openarc-ar.c (ares_parse): Do not return and continue parsing even if there is no space to record more results, for syntax checking, or overwrite the result of some auth method already seen. openarc/openarc-ar.c (main (for testing)): Use appropriate macros openarc/openarc-ar.c (main (for testing)): print result_comment openarc/openarc-ar.c: Inclease max number of tokens for parsing AR header openarc: Ignore unknown method in the result of parsing AR header Since we discard resinfo of which method is not known in AR header parsing now, it should never happen. However we check it as a foolproof. * openarc/openarc.c (mlfi_eom): Ignore unknown method in the result of parsing AR header, and log it. --- openarc/openarc-ar.c | 179 +++++++++++++++++++++++++++++-------------- openarc/openarc.c | 9 +++ 2 files changed, 130 insertions(+), 58 deletions(-) diff --git a/openarc/openarc-ar.c b/openarc/openarc-ar.c index 52b6cf4..ec7fdeb 100644 --- a/openarc/openarc-ar.c +++ b/openarc/openarc-ar.c @@ -40,7 +40,7 @@ #define ARES_TOKENS ";=." #define ARES_TOKENS2 "=." -#define ARES_MAXTOKENS 512 +#define ARES_MAXTOKENS 1024 /* tables */ struct lookup @@ -337,35 +337,37 @@ ares_xconvert(struct lookup *table, int code) } /* -** ARES_DEDUP -- if we've gotten multiple results of the same method, -** discard the older one +** ARES_METHOD_SEEN -- if we've already seen the results of the method, +** returns its index ** ** Parameters: ** ar -- pointer to a (struct authres) ** n -- the last one that was loaded +** m -- the method to be searched ** ** Return value: -** TRUE iff a de-duplication happened, leaving the result referenced by -** "n" open. +** The index of the method in ar, if it is found, else -1 */ -static _Bool -ares_dedup(struct authres *ar, int n) +static int +ares_method_seen(struct authres *ar, int n, ares_method_t m) { int c; + if (ar->ares_result[n].result_method == ARES_METHOD_DKIM) + { + /* All results of DKIM should be kept */ + return -1; + } for (c = 0; c < n; c++) { - if (ar->ares_result[c].result_method == ar->ares_result[n].result_method && - ar->ares_result[c].result_method != ARES_METHOD_DKIM) + if (ar->ares_result[c].result_method == m) { - memcpy(&ar->ares_result[c], &ar->ares_result[n], - sizeof(ar->ares_result[c])); - return TRUE; + return c; } } - return FALSE; + return -1; } /* @@ -390,6 +392,8 @@ ares_parse(u_char *hdr, struct authres *ar) int r = 0; int state; int prevstate; + int i; /* index of a result to be recorded */ + ares_method_t m; u_char tmp[ARC_MAXHEADER + 2]; u_char *tokens[ARES_MAXTOKENS]; @@ -406,14 +410,21 @@ ares_parse(u_char *hdr, struct authres *ar) prevstate = -1; state = 0; n = 0; + i = 0; for (c = 0; c < ntoks; c++) { if (tokens[c][0] == '(') /* comment */ { - strlcpy((char *) ar->ares_result[n - 1].result_comment, - (char *) tokens[c], - sizeof ar->ares_result[n - 1].result_comment); + if (i >= 0 && prevstate == 5) + { + /* record at most one comment only */ + assert(state == 6); + strlcpy((char *) ar->ares_result[i].result_comment, + (char *) tokens[c], + sizeof ar->ares_result[i].result_comment); + prevstate = state; + } continue; } @@ -488,27 +499,55 @@ ares_parse(u_char *hdr, struct authres *ar) break; case 3: /* method/none */ - if (n == 0 || !ares_dedup(ar, n)) - n++; - - if (n >= MAXARESULTS) - return 0; - - r = 0; - if (strcasecmp((char *) tokens[c], "none") == 0) { - if (n > 0) - n--; + switch (prevstate) + { + case 0: + case 1: + case 2: + prevstate = state; + state = 14; + continue; + default: + /* should not have other resinfo */ + return -1; + } + } - prevstate = state; - state = 14; + m = ares_convert(methods, (char *) tokens[c]); + switch(m) + { + case ARES_METHOD_UNKNOWN: + i = -1; + break; + case ARES_METHOD_DKIM: + i = n; + break; + default: + i = ares_method_seen(ar, n, m); + if (i == -1) + { + i = n; + } + else + { + /* Reuse results field of same method */ + memset(&ar->ares_result[i], '\0', + sizeof(ar->ares_result[i])); + } + } - continue; + r = 0; + if (i >= MAXARESULTS) + { + /* continue parsing, but don't record */ + i = -1; } - ar->ares_result[n - 1].result_method = ares_convert(methods, - (char *) tokens[c]); + if (i >= 0) { + ar->ares_result[i].result_method = m; + } prevstate = state; state = 4; @@ -525,9 +564,12 @@ ares_parse(u_char *hdr, struct authres *ar) break; case 5: /* result */ - ar->ares_result[n - 1].result_result = ares_convert(aresults, - (char *) tokens[c]); - ar->ares_result[n - 1].result_comment[0] = '\0'; + if (i >= 0) + { + ar->ares_result[i].result_result = ares_convert(aresults, + (char *) tokens[c]); + ar->ares_result[i].result_comment[0] = '\0'; + } prevstate = state; state = 6; @@ -544,9 +586,12 @@ ares_parse(u_char *hdr, struct authres *ar) break; case 8: - strlcpy((char *) ar->ares_result[n - 1].result_reason, - (char *) tokens[c], - sizeof ar->ares_result[n - 1].result_reason); + if (i >= 0) + { + strlcpy((char *) ar->ares_result[i].result_reason, + (char *) tokens[c], + sizeof ar->ares_result[i].result_reason); + } prevstate = state; state = 9; @@ -557,6 +602,11 @@ ares_parse(u_char *hdr, struct authres *ar) if (tokens[c][0] == ';' && /* neither */ tokens[c][1] == '\0') { + if (i == n) + { + n++; + } + prevstate = state; state = 3; @@ -585,9 +635,12 @@ ares_parse(u_char *hdr, struct authres *ar) { r--; - strlcat((char *) ar->ares_result[n - 1].result_value[r], - (char *) tokens[c], - sizeof ar->ares_result[n - 1].result_value[r]); + if (i >= 0) + { + strlcat((char *) ar->ares_result[i].result_value[r], + (char *) tokens[c], + sizeof ar->ares_result[i].result_value[r]); + } prevstate = state; state = 13; @@ -598,6 +651,11 @@ ares_parse(u_char *hdr, struct authres *ar) if (tokens[c][0] == ';' && tokens[c][1] == '\0') { + if (i == n) + { + n++; + } + prevstate = state; state = 3; @@ -611,8 +669,10 @@ ares_parse(u_char *hdr, struct authres *ar) if (x == ARES_PTYPE_UNKNOWN) return -1; - if (r < MAXPROPS) - ar->ares_result[n - 1].result_ptype[r] = x; + if (r < MAXPROPS && i >= 0) + { + ar->ares_result[i].result_ptype[r] = x; + } prevstate = state; state = 10; @@ -631,11 +691,11 @@ ares_parse(u_char *hdr, struct authres *ar) break; case 11: /* property */ - if (r < MAXPROPS) + if (r < MAXPROPS && i >= 0) { - strlcpy((char *) ar->ares_result[n - 1].result_property[r], + strlcpy((char *) ar->ares_result[i].result_property[r], (char *) tokens[c], - sizeof ar->ares_result[n - 1].result_property[r]); + sizeof ar->ares_result[i].result_property[r]); } prevstate = state; @@ -656,11 +716,14 @@ ares_parse(u_char *hdr, struct authres *ar) case 13: /* value */ if (r < MAXPROPS) { - strlcat((char *) ar->ares_result[n - 1].result_value[r], - (char *) tokens[c], - sizeof ar->ares_result[n - 1].result_value[r]); + if (i >= 0) + { + strlcat((char *) ar->ares_result[i].result_value[r], + (char *) tokens[c], + sizeof ar->ares_result[i].result_value[r]); + ar->ares_result[i].result_props = r + 1; + } r++; - ar->ares_result[n - 1].result_props = r; } prevstate = state; @@ -680,10 +743,10 @@ ares_parse(u_char *hdr, struct authres *ar) state == 11 || state == 12) return -1; - if (n > 1) + if (i == n) { - if (ares_dedup(ar, n - 1)) - n--; + /* the last resinfo was added */ + n++; } ar->ares_count = n; @@ -750,8 +813,6 @@ ares_getptype(ares_ptype_t ptype) ** EX_USAGE or EX_OK */ -# define NTOKENS 256 - int main(int argc, char **argv) { @@ -761,8 +822,8 @@ main(int argc, char **argv) char *p; char *progname; struct authres ar; - u_char buf[2048]; - u_char *toks[NTOKENS]; + u_char buf[ARC_MAXHEADER + 2]; + u_char *toks[ARES_MAXTOKENS]; progname = (p = strrchr(argv[0], '/')) == NULL ? argv[0] : p + 1; @@ -772,13 +833,14 @@ main(int argc, char **argv) return EX_USAGE; } - c = ares_tokenize(argv[1], buf, sizeof buf, toks, NTOKENS); + c = ares_tokenize(((u_char **)argv)[1], buf, sizeof buf, toks, + ARES_MAXTOKENS); for (d = 0; d < c; d++) printf("token %d = '%s'\n", d, toks[d]); printf("\n"); - status = ares_parse(argv[1], &ar); + status = ares_parse(((u_char **)argv)[1], &ar); if (status == -1) { printf("%s: ares_parse() returned -1\n", progname); @@ -804,6 +866,7 @@ main(int argc, char **argv) ares_xconvert(aresults, ar.ares_result[c].result_result)); printf("\treason \"%s\"\n", ar.ares_result[c].result_reason); + printf("\tcomment \"%s\"\n", ar.ares_result[c].result_comment); for (d = 0; d < ar.ares_result[c].result_props; d++) { diff --git a/openarc/openarc.c b/openarc/openarc.c index 2a4eebb..b61d85f 100644 --- a/openarc/openarc.c +++ b/openarc/openarc.c @@ -3717,7 +3717,16 @@ mlfi_eom(SMFICTX *ctx) for (n = 0; n < ar.ares_count; n++) { if (ar.ares_result[n].result_method == ARES_METHOD_UNKNOWN) + { + /* foolproof: should not happen */ + if (conf->conf_dolog) + { + syslog(LOG_DEBUG, + "%s: internal error: unknown method is found in ares_result, ignored", + afc->mctx_jobid); + } continue; + } if (ar.ares_result[n].result_method == ARES_METHOD_ARC) { /* From 52b28e96612e470e06c70f710a3193950161b454 Mon Sep 17 00:00:00 2001 From: Paul Arthur Date: Thu, 10 Oct 2024 15:29:34 +0000 Subject: [PATCH 2/2] Rework AR parsing * Add tests. * Use named states to make it less terribad to read. * Track the current method in a temporary struct instead of putting partial results into `ar`. * Don't parse headers if we don't care about them. * Comprehensively dedup authentication results, instead of just within a single parsed header. * Store all interesting comments. * Clean up whitespace inside of comments. * Fix non-terminal state conditions in the tokenizer and parser. * Output "reason" in the correct location. * Return ARES_RESULT_UNDEFINED when mapping result strings to values does not find a match; ARES_RESULT_UNKNOWN is an actual result. * Parsed values can also need quoting and escaping on output. * Add missing IANA registered values from https://www.iana.org/assignments/email-auth/email-auth.xhtml --- openarc/openarc-ar.c | 475 ++++++++++-------- openarc/openarc-ar.h | 118 ++--- openarc/openarc.c | 136 ++--- ... => test_milter_ar_override_disabled.conf} | 0 test/test_milter.py | 203 +++++++- 5 files changed, 568 insertions(+), 364 deletions(-) rename test/files/{test_milter_ar_disabled.conf => test_milter_ar_override_disabled.conf} (100%) diff --git a/openarc/openarc-ar.c b/openarc/openarc-ar.c index ec7fdeb..fea9b0e 100644 --- a/openarc/openarc-ar.c +++ b/openarc/openarc-ar.c @@ -57,41 +57,63 @@ struct lookup methods[] = { "dkim-adsp", ARES_METHOD_DKIMADSP }, { "dkim-atps", ARES_METHOD_DKIMATPS }, { "dmarc", ARES_METHOD_DMARC }, + { "dnswl", ARES_METHOD_DNSWL }, { "domainkeys", ARES_METHOD_DOMAINKEYS }, { "iprev", ARES_METHOD_IPREV }, { "rrvs", ARES_METHOD_RRVS }, { "sender-id", ARES_METHOD_SENDERID }, { "smime", ARES_METHOD_SMIME }, { "spf", ARES_METHOD_SPF }, + { "vbr", ARES_METHOD_VBR }, { NULL, ARES_METHOD_UNKNOWN } }; struct lookup aresults[] = { - { "none", ARES_RESULT_NONE }, - { "pass", ARES_RESULT_PASS }, + { "discard", ARES_RESULT_DISCARD }, { "fail", ARES_RESULT_FAIL }, - { "policy", ARES_RESULT_POLICY }, { "neutral", ARES_RESULT_NEUTRAL }, - { "temperror", ARES_RESULT_TEMPERROR }, - { "permerror", ARES_RESULT_PERMERROR }, + { "none", ARES_RESULT_NONE }, { "nxdomain", ARES_RESULT_NXDOMAIN }, + { "pass", ARES_RESULT_PASS }, + { "permerror", ARES_RESULT_PERMERROR }, + { "policy", ARES_RESULT_POLICY }, { "signed", ARES_RESULT_SIGNED }, - { "unknown", ARES_RESULT_UNKNOWN }, - { "discard", ARES_RESULT_DISCARD }, { "softfail", ARES_RESULT_SOFTFAIL }, - { NULL, ARES_RESULT_UNKNOWN } + { "temperror", ARES_RESULT_TEMPERROR }, + { "unknown", ARES_RESULT_UNKNOWN }, + { NULL, ARES_RESULT_UNDEFINED } }; struct lookup ptypes[] = { - { "smtp", ARES_PTYPE_SMTP }, - { "header", ARES_PTYPE_HEADER }, { "body", ARES_PTYPE_BODY }, + { "dns", ARES_PTYPE_DNS }, + { "header", ARES_PTYPE_HEADER }, { "policy", ARES_PTYPE_POLICY }, + { "smtp", ARES_PTYPE_SMTP }, { NULL, ARES_PTYPE_UNKNOWN } }; + +enum ar_parser_state { + ARP_STATE_AUTHSERVID, + ARP_STATE_AUTHRESVERSION_OR_AUTHSERVID, + ARP_STATE_RESINFO, + ARP_STATE_METHODSPEC, + ARP_STATE_METHODSPEC_EQUALS, + ARP_STATE_RESULT, + ARP_STATE_REASONSPEC_EQUALS, + ARP_STATE_REASONSPEC_VALUE, + ARP_STATE_PROP_OR_REASON, + ARP_STATE_PTYPE, + ARP_STATE_PROPSPEC_DOT, + ARP_STATE_PROPERTY, + ARP_STATE_PROPSPEC_EQUALS, + ARP_STATE_PVALUE, + ARP_STATE_DONE, +}; + /* ** ARES_TOKENIZE -- tokenize a string ** @@ -103,7 +125,7 @@ struct lookup ptypes[] = ** ntokens -- number of token pointers available at "tokens" ** ** Return value: -** -1 -- not enough space at "outbuf" for tokenizing +** -1 -- bad syntax or not enough space at "outbuf" for tokenizing ** other -- number of tokens identified; may be greater than ** "ntokens" if there were more tokens found than there were ** pointers available. @@ -142,11 +164,16 @@ ares_tokenize(u_char *input, u_char *outbuf, size_t outbuflen, intok = TRUE; } + if (*p == '\\' || *p == '"') { + /* Needs to remain escaped. */ + *q = '\\'; + q++; + } *q = *p; q++; escaped = FALSE; } - else if (*p == '\\') /* escape */ + else if (*p == '\\' && quoted) /* escape */ { escaped = TRUE; } @@ -204,20 +231,30 @@ ares_tokenize(u_char *input, u_char *outbuf, size_t outbuflen, } else if (isascii(*p) && isspace(*p)) /* whitespace */ { - if (quoted || parens > 0) + if (intok) { - if (intok) + if (quoted) { *q = *p; q++; } - } - else if (intok) - { - intok = FALSE; - *q = '\0'; - q++; - n++; + else if (parens > 0) + { + /* turn all whitespace in comments into single spaces */ + *q = ' '; + q++; + while isspace(p[1]) + { + p++; + } + } + else + { + intok = FALSE; + *q = '\0'; + q++; + n++; + } } } else if (strchr(ARES_TOKENS, *p) != NULL) /* delimiter */ @@ -268,8 +305,15 @@ ares_tokenize(u_char *input, u_char *outbuf, size_t outbuflen, } } + if (quoted || parens > 0) + { + return -1; + } + if (q >= end) + { return -1; + } if (intok) { @@ -337,37 +381,38 @@ ares_xconvert(struct lookup *table, int code) } /* -** ARES_METHOD_SEEN -- if we've already seen the results of the method, -** returns its index +** ARES_METHOD_ADD -- add a parsed method to the results if there's room +** and we haven't already seen it. ** ** Parameters: -** ar -- pointer to a (struct authres) -** n -- the last one that was loaded -** m -- the method to be searched +** ar -- authentication results +** r -- result to add ** ** Return value: -** The index of the method in ar, if it is found, else -1 +** Whether the method was added */ -static int -ares_method_seen(struct authres *ar, int n, ares_method_t m) +static _Bool +ares_method_add(struct authres *ar, struct result *r) { - int c; - - if (ar->ares_result[n].result_method == ARES_METHOD_DKIM) + if (r->result_method == ARES_METHOD_UNKNOWN || ar->ares_count >= MAXARESULTS) { - /* All results of DKIM should be kept */ - return -1; + return FALSE; } - for (c = 0; c < n; c++) + if (r->result_method != ARES_METHOD_DKIM) { - if (ar->ares_result[c].result_method == m) + for (int i = 0; i < ar->ares_count; i++) { - return c; + if (ar->ares_result[i].result_method == r->result_method) + { + return FALSE; + } } } - return -1; + memcpy(ar->ares_result + ar->ares_count, r, sizeof ar->ares_result[ar->ares_count]); + ar->ares_count++; + return TRUE; } /* @@ -378,98 +423,111 @@ ares_method_seen(struct authres *ar, int n, ares_method_t m) ** hdr -- NULL-terminated contents of an Authentication-Results: ** header field ** ar -- a pointer to a (struct authres) loaded by values after parsing +** authserv -- string containing the authserv-id we care about ** ** Return value: -** 0 on success, -1 on failure. +** 0 on success, -1 on failure, -2 when a header is uninteresting. */ int -ares_parse(u_char *hdr, struct authres *ar) +ares_parse(u_char *hdr, struct authres *ar, const char *authserv) { - int n; int ntoks; - int c; - int r = 0; - int state; - int prevstate; - int i; /* index of a result to be recorded */ - ares_method_t m; + enum ar_parser_state state; + enum ar_parser_state prevstate; + ares_method m; u_char tmp[ARC_MAXHEADER + 2]; u_char *tokens[ARES_MAXTOKENS]; + u_char ares_host[ARC_MAXHOSTNAMELEN + 1]; + struct result cur; + int initial_ares_count; assert(hdr != NULL); assert(ar != NULL); - memset(ar, '\0', sizeof *ar); memset(tmp, '\0', sizeof tmp); + memset(&cur, '\0', sizeof cur); + memset(ares_host, '\0', sizeof ares_host); ntoks = ares_tokenize(hdr, tmp, sizeof tmp, tokens, ARES_MAXTOKENS); if (ntoks == -1 || ntoks > ARES_MAXTOKENS) return -1; - prevstate = -1; - state = 0; - n = 0; - i = 0; + prevstate = ARP_STATE_AUTHSERVID; + state = ARP_STATE_AUTHSERVID; + initial_ares_count = ar->ares_count; - for (c = 0; c < ntoks; c++) + for (int c = 0; c < ntoks; c++) { - if (tokens[c][0] == '(') /* comment */ + if (tokens[c][0] == '(') { - if (i >= 0 && prevstate == 5) - { - /* record at most one comment only */ - assert(state == 6); - strlcpy((char *) ar->ares_result[i].result_comment, + /* Comments are valid in a lot of places, but we're + * only interested in storing ones that are placed + * like properties + */ + if (cur.result_props < MAXPROPS && + (state == ARP_STATE_PROP_OR_REASON || + state == ARP_STATE_PTYPE)) { + cur.result_ptype[cur.result_props] = ARES_PTYPE_COMMENT; + strlcpy((char *) cur.result_value[cur.result_props], (char *) tokens[c], - sizeof ar->ares_result[i].result_comment); - prevstate = state; + sizeof cur.result_value[cur.result_props]); + cur.result_props++; } continue; } switch (state) { - case 0: /* authserv-id */ - if (!isascii(tokens[c][0]) || - !isalnum(tokens[c][0])) + case ARP_STATE_AUTHSERVID: + if (!isascii(tokens[c][0]) || !isalnum(tokens[c][0])) + { + ar->ares_count = initial_ares_count; return -1; + } + if (tokens[c][0] == ';') { prevstate = state; - state = 3; + state = ARP_STATE_METHODSPEC; } else { - strlcat((char *) ar->ares_host, - (char *) tokens[c], - sizeof ar->ares_host); + strlcat((char *) ares_host, (char *) tokens[c], sizeof ares_host); prevstate = state; - state = 1; + state = ARP_STATE_AUTHRESVERSION_OR_AUTHSERVID; } break; - case 1: /* [version] */ + case ARP_STATE_AUTHRESVERSION_OR_AUTHSERVID: if (tokens[c][0] == '.' && - tokens[c][1] == '\0' && prevstate == 0) + tokens[c][1] == '\0' && prevstate == ARP_STATE_AUTHSERVID) { - strlcat((char *) ar->ares_host, - (char *) tokens[c], - sizeof ar->ares_host); + strlcat((char *) ares_host, (char *) tokens[c], sizeof ares_host); prevstate = state; - state = 0; + state = ARP_STATE_AUTHSERVID; break; } + /* We've successfully assembled the authserv-id, + * see if it's what we're looking for. + */ + if (authserv && strcasecmp(authserv, ares_host) != 0) + { + ar->ares_count = initial_ares_count; + return -2; + } + strlcpy(ar->ares_host, ares_host, sizeof ar->ares_host); + if (tokens[c][0] == ';') { prevstate = state; - state = 3; + state = ARP_STATE_METHODSPEC; } else if (isascii(tokens[c][0]) && isdigit(tokens[c][0])) @@ -479,136 +537,102 @@ ares_parse(u_char *hdr, struct authres *ar) sizeof ar->ares_version); prevstate = state; - state = 2; + state = ARP_STATE_RESINFO; } else { + ar->ares_count = initial_ares_count; return -1; } break; - case 2: /* ; */ - if (tokens[c][0] != ';' || - tokens[c][1] != '\0') + case ARP_STATE_RESINFO: + if (tokens[c][0] != ';' || tokens[c][1] != '\0') + { + ar->ares_count = initial_ares_count; return -1; + } prevstate = state; - state = 3; + state = ARP_STATE_METHODSPEC; break; - case 3: /* method/none */ + case ARP_STATE_METHODSPEC: if (strcasecmp((char *) tokens[c], "none") == 0) { switch (prevstate) { - case 0: - case 1: - case 2: + case ARP_STATE_AUTHSERVID: + case ARP_STATE_AUTHRESVERSION_OR_AUTHSERVID: + case ARP_STATE_RESINFO: prevstate = state; - state = 14; + state = ARP_STATE_DONE; continue; default: /* should not have other resinfo */ + ar->ares_count = initial_ares_count; return -1; } } - m = ares_convert(methods, (char *) tokens[c]); - switch(m) - { - case ARES_METHOD_UNKNOWN: - i = -1; - break; - case ARES_METHOD_DKIM: - i = n; - break; - default: - i = ares_method_seen(ar, n, m); - if (i == -1) - { - i = n; - } - else - { - /* Reuse results field of same method */ - memset(&ar->ares_result[i], '\0', - sizeof(ar->ares_result[i])); - } - } + memset(&cur, '\0', sizeof cur); - r = 0; - if (i >= MAXARESULTS) - { - /* continue parsing, but don't record */ - i = -1; - } + m = ares_convert(methods, (char *) tokens[c]); - if (i >= 0) { - ar->ares_result[i].result_method = m; - } + cur.result_method = m; prevstate = state; - state = 4; + state = ARP_STATE_METHODSPEC_EQUALS; break; - case 4: /* = */ - if (tokens[c][0] != '=' || - tokens[c][1] != '\0') + case ARP_STATE_METHODSPEC_EQUALS: + if (tokens[c][0] != '=' || tokens[c][1] != '\0') + { + ar->ares_count = initial_ares_count; return -1; + } prevstate = state; - state = 5; + state = ARP_STATE_RESULT; break; - case 5: /* result */ - if (i >= 0) - { - ar->ares_result[i].result_result = ares_convert(aresults, - (char *) tokens[c]); - ar->ares_result[i].result_comment[0] = '\0'; - } + case ARP_STATE_RESULT: + cur.result_result = ares_convert(aresults, (char *) tokens[c]); prevstate = state; - state = 6; + state = ARP_STATE_PROP_OR_REASON; break; - case 7: /* = (reason) */ - if (tokens[c][0] != '=' || - tokens[c][1] != '\0') + case ARP_STATE_REASONSPEC_EQUALS: + if (tokens[c][0] != '=' || tokens[c][1] != '\0') + { + ar->ares_count = initial_ares_count; return -1; - + } prevstate = state; - state = 8; + state = ARP_STATE_REASONSPEC_VALUE; break; - case 8: - if (i >= 0) - { - strlcpy((char *) ar->ares_result[i].result_reason, - (char *) tokens[c], - sizeof ar->ares_result[i].result_reason); - } + case ARP_STATE_REASONSPEC_VALUE: + strlcpy((char *) cur.result_reason, (char *) tokens[c], sizeof cur.result_reason); prevstate = state; - state = 9; + state = ARP_STATE_PTYPE; break; - case 6: /* reason/propspec */ + case ARP_STATE_PROP_OR_REASON: if (tokens[c][0] == ';' && /* neither */ tokens[c][1] == '\0') { - if (i == n) - { - n++; - } - + ares_method_add(ar, &cur); + memset(&cur, '\0', sizeof cur); prevstate = state; - state = 3; + state = ARP_STATE_METHODSPEC; continue; } @@ -616,144 +640,165 @@ ares_parse(u_char *hdr, struct authres *ar) if (strcasecmp((char *) tokens[c], "reason") == 0) { /* reason */ prevstate = state; - state = 7; - + state = ARP_STATE_REASONSPEC_EQUALS; continue; } else { prevstate = state; - state = 9; + state = ARP_STATE_PTYPE; } /* FALLTHROUGH */ - case 9: /* ptype */ - if (prevstate == 13 && + case ARP_STATE_PTYPE: + if (prevstate == ARP_STATE_PVALUE && strchr(ARES_TOKENS2, tokens[c][0]) != NULL && tokens[c][1] == '\0') { - r--; - - if (i >= 0) - { - strlcat((char *) ar->ares_result[i].result_value[r], - (char *) tokens[c], - sizeof ar->ares_result[i].result_value[r]); - } + /* actually a part of the previous value */ + cur.result_props--; + strlcat((char *) cur.result_value[cur.result_props], + (char *) tokens[c], + sizeof cur.result_value[cur.result_props]); prevstate = state; - state = 13; - + state = ARP_STATE_PVALUE; continue; } if (tokens[c][0] == ';' && tokens[c][1] == '\0') { - if (i == n) - { - n++; - } - + ares_method_add(ar, &cur); + memset(&cur, '\0', sizeof(cur)); prevstate = state; - state = 3; - + state = ARP_STATE_METHODSPEC; continue; } else { - ares_ptype_t x; + ares_ptype x; x = ares_convert(ptypes, (char *) tokens[c]); if (x == ARES_PTYPE_UNKNOWN) + { + ar->ares_count = initial_ares_count; return -1; + } - if (r < MAXPROPS && i >= 0) + if (cur.result_props < MAXPROPS) { - ar->ares_result[i].result_ptype[r] = x; + cur.result_ptype[cur.result_props] = x; } prevstate = state; - state = 10; + state = ARP_STATE_PROPSPEC_DOT; } break; - case 10: /* . */ - if (tokens[c][0] != '.' || - tokens[c][1] != '\0') + case ARP_STATE_PROPSPEC_DOT: + if (tokens[c][0] != '.' || tokens[c][1] != '\0') + { + ar->ares_count = initial_ares_count; return -1; + } prevstate = state; - state = 11; + state = ARP_STATE_PROPERTY; break; - case 11: /* property */ - if (r < MAXPROPS && i >= 0) + case ARP_STATE_PROPERTY: + if (cur.result_props < MAXPROPS) { - strlcpy((char *) ar->ares_result[i].result_property[r], + strlcpy((char *) cur.result_property[cur.result_props], (char *) tokens[c], - sizeof ar->ares_result[i].result_property[r]); + sizeof cur.result_property[cur.result_props]); } prevstate = state; - state = 12; + state = ARP_STATE_PROPSPEC_EQUALS; break; - case 12: /* = */ - if (tokens[c][0] != '=' || - tokens[c][1] != '\0') + case ARP_STATE_PROPSPEC_EQUALS: + if (tokens[c][0] != '=' || tokens[c][1] != '\0') + { + ar->ares_count = initial_ares_count; return -1; + } prevstate = state; - state = 13; + state = ARP_STATE_PVALUE; break; - case 13: /* value */ - if (r < MAXPROPS) + case ARP_STATE_PVALUE: + if (cur.result_props < MAXPROPS) { - if (i >= 0) - { - strlcat((char *) ar->ares_result[i].result_value[r], - (char *) tokens[c], - sizeof ar->ares_result[i].result_value[r]); - ar->ares_result[i].result_props = r + 1; - } - r++; + strlcat((char *) cur.result_value[cur.result_props], + (char *) tokens[c], + sizeof cur.result_value[cur.result_props]); + cur.result_props++; } prevstate = state; - state = 9; + state = ARP_STATE_PTYPE; break; - case 14: /* only reached in case of a malformed A-R */ + case ARP_STATE_DONE: + /* unexpected content after a singleton value */ + ar->ares_count = initial_ares_count; return -1; - - break; /* not reached, just to make some lint-like sw happy */ } } /* error out on non-terminal states */ - if (state == 4 || state == 7 || state == 10 || - state == 11 || state == 12) - return -1; - - if (i == n) + if (state != ARP_STATE_METHODSPEC && + state != ARP_STATE_PROP_OR_REASON && + state != ARP_STATE_PTYPE && + state != ARP_STATE_DONE) { - /* the last resinfo was added */ - n++; + ar->ares_count = initial_ares_count; + return -1; } - ar->ares_count = n; + ares_method_add(ar, &cur); return 0; } +/* +** ARES_ISTOKEN -- check whether a string is a valid token +** +** Parameters: +** str -- string to check +** +** Return value: +** TRUE if the string contains no characters that require quoting, +** FALSE otherwise. +*/ +_Bool +ares_istoken(const char *str) +{ + for (const char *c = str; *c != '\0'; c++) + { + if (iscntrl(*c)) { + return FALSE; + } + /* ' ' and tspecials from RFC 2045 except @ + * (local-part@domain-name doesn't require quoting) + */ + if (strchr(" ()<>,;:\\\"/[]?=", *c) != NULL) { + return FALSE; + } + } + return TRUE; +} + /* ** ARES_GETMETHOD -- translate a method code to its name ** @@ -765,7 +810,7 @@ ares_parse(u_char *hdr, struct authres *ar) */ const char * -ares_getmethod(ares_method_t method) +ares_getmethod(ares_method method) { return (const char *) ares_xconvert(methods, method); } @@ -781,7 +826,7 @@ ares_getmethod(ares_method_t method) */ const char * -ares_getresult(ares_result_t result) +ares_getresult(ares_result result) { return (const char *) ares_xconvert(aresults, result); } @@ -797,7 +842,7 @@ ares_getresult(ares_result_t result) */ const char * -ares_getptype(ares_ptype_t ptype) +ares_getptype(ares_ptype ptype) { return (const char *) ares_xconvert(ptypes, ptype); } @@ -840,7 +885,7 @@ main(int argc, char **argv) printf("\n"); - status = ares_parse(((u_char **)argv)[1], &ar); + status = ares_parse(((u_char **)argv)[1], &ar, NULL); if (status == -1) { printf("%s: ares_parse() returned -1\n", progname); diff --git a/openarc/openarc-ar.h b/openarc/openarc-ar.h index 1c34d0c..f0d02ba 100644 --- a/openarc/openarc-ar.h +++ b/openarc/openarc-ar.h @@ -21,59 +21,61 @@ #define MAXPROPS 16 #define MAXAVALUE 256 -/* ARES_METHOD_T -- type for specifying an authentication method */ -typedef int ares_method_t; - -#define ARES_METHOD_UNKNOWN (-1) -#define ARES_METHOD_AUTH 0 -#define ARES_METHOD_DKIM 1 -#define ARES_METHOD_DOMAINKEYS 2 -#define ARES_METHOD_SENDERID 3 -#define ARES_METHOD_SPF 4 -#define ARES_METHOD_DKIMADSP 5 -#define ARES_METHOD_IPREV 6 -#define ARES_METHOD_DKIMATPS 7 -#define ARES_METHOD_DMARC 8 -#define ARES_METHOD_SMIME 9 -#define ARES_METHOD_RRVS 10 -#define ARES_METHOD_ARC 11 - -/* ARES_RESULT_T -- type for specifying an authentication result */ -typedef int ares_result_t; - -#define ARES_RESULT_UNDEFINED (-1) -#define ARES_RESULT_PASS 0 -#define ARES_RESULT_UNASSIGNED 1 /* UNASSIGNED */ -#define ARES_RESULT_SOFTFAIL 2 -#define ARES_RESULT_NEUTRAL 3 -#define ARES_RESULT_TEMPERROR 4 -#define ARES_RESULT_PERMERROR 5 -#define ARES_RESULT_NONE 6 -#define ARES_RESULT_FAIL 7 -#define ARES_RESULT_POLICY 8 -#define ARES_RESULT_NXDOMAIN 9 -#define ARES_RESULT_SIGNED 10 -#define ARES_RESULT_UNKNOWN 11 -#define ARES_RESULT_DISCARD 12 - -/* ARES_PTYPE_T -- type for specifying an authentication property */ -typedef int ares_ptype_t; - -#define ARES_PTYPE_UNKNOWN (-1) -#define ARES_PTYPE_SMTP 0 -#define ARES_PTYPE_HEADER 1 -#define ARES_PTYPE_BODY 2 -#define ARES_PTYPE_POLICY 3 +/* ARES_METHOD -- type for specifying an authentication method */ +typedef enum { + ARES_METHOD_UNKNOWN, + ARES_METHOD_ARC, + ARES_METHOD_AUTH, + ARES_METHOD_DKIM, + ARES_METHOD_DKIMADSP, + ARES_METHOD_DKIMATPS, + ARES_METHOD_DMARC, + ARES_METHOD_DNSWL, + ARES_METHOD_DOMAINKEYS, + ARES_METHOD_IPREV, + ARES_METHOD_RRVS, + ARES_METHOD_SENDERID, + ARES_METHOD_SMIME, + ARES_METHOD_SPF, + ARES_METHOD_VBR, +} ares_method; + +/* ARES_RESULT -- type for specifying an authentication result */ +typedef enum { + ARES_RESULT_UNDEFINED, /* "unknown" is an actual result */ + ARES_RESULT_DISCARD, + ARES_RESULT_FAIL, + ARES_RESULT_NEUTRAL, + ARES_RESULT_NONE, + ARES_RESULT_NXDOMAIN, + ARES_RESULT_PASS, + ARES_RESULT_PERMERROR, + ARES_RESULT_POLICY, + ARES_RESULT_SIGNED, + ARES_RESULT_SOFTFAIL, + ARES_RESULT_TEMPERROR, + ARES_RESULT_UNKNOWN, +} ares_result; + +/* ARES_PTYPE -- type for specifying an authentication property */ +typedef enum { + ARES_PTYPE_COMMENT = -1, + ARES_PTYPE_UNKNOWN, + ARES_PTYPE_BODY, + ARES_PTYPE_DNS, + ARES_PTYPE_HEADER, + ARES_PTYPE_POLICY, + ARES_PTYPE_SMTP, +} ares_ptype; /* RESULT structure -- a single result */ struct result { int result_props; - ares_method_t result_method; - ares_result_t result_result; - ares_ptype_t result_ptype[MAXPROPS]; + ares_method result_method; + ares_result result_result; + ares_ptype result_ptype[MAXPROPS]; unsigned char result_reason[MAXAVALUE + 1]; - unsigned char result_comment[MAXAVALUE + 1]; unsigned char result_property[MAXPROPS][MAXAVALUE + 1]; unsigned char result_value[MAXPROPS][MAXAVALUE + 1]; }; @@ -87,23 +89,11 @@ struct authres struct result ares_result[MAXARESULTS]; }; -/* -** ARES_PARSE -- parse an Authentication-Results: header, return a -** structure containing a parsed result -** -** Parameters: -** hdr -- NULL-terminated contents of an Authentication-Results: -** header field -** ar -- a pointer to a (struct authres) loaded by values after parsing -** -** Return value: -** 0 on success, -1 on failure. -*/ - -extern int ares_parse __P((u_char *, struct authres *)); +extern int ares_parse __P((u_char *, struct authres *, const char *)); +extern _Bool ares_istoken __P((const char *)); -extern const char *ares_getmethod __P((ares_method_t)); -extern const char *ares_getresult __P((ares_result_t)); -extern const char *ares_getptype __P((ares_ptype_t)); +extern const char *ares_getmethod __P((ares_method)); +extern const char *ares_getresult __P((ares_result)); +extern const char *ares_getptype __P((ares_ptype)); #endif /* _OPENARC_AR_H_ */ diff --git a/openarc/openarc.c b/openarc/openarc.c index b61d85f..92cbb73 100644 --- a/openarc/openarc.c +++ b/openarc/openarc.c @@ -3685,18 +3685,18 @@ mlfi_eom(SMFICTX *ctx) if (BITSET(ARC_MODE_SIGN, cc->cctx_mode)) { - int arfound = 0; - + _Bool arfound = FALSE; + memset(&ar, '\0', sizeof ar); arcf_dstring_blank(afc->mctx_tmpstr); /* assemble authentication results */ - for (c = 0; ; c++) + for (int i = 0; ; i++) { - hdr = arcf_findheader(afc, AUTHRESULTSHDR, c); + hdr = arcf_findheader(afc, AUTHRESULTSHDR, i); if (hdr == NULL) break; - status = ares_parse(hdr->hdr_val, &ar); - if (status != 0) + status = ares_parse(hdr->hdr_val, &ar, conf->conf_authservid); + if (status == -1) { if (conf->conf_dolog) { @@ -3707,93 +3707,65 @@ mlfi_eom(SMFICTX *ctx) continue; } + } - if (strcasecmp(conf->conf_authservid, - ar.ares_host) == 0) + for (int i = 0; i < ar.ares_count; i++) + { + if (ar.ares_result[i].result_method == ARES_METHOD_ARC) { - int m; - int n; + if (!conf->conf_overridecv) { + continue; + } - for (n = 0; n < ar.ares_count; n++) + arfound = TRUE; + if (reconcile_arc_state(afc, &ar.ares_result[i]) && conf->conf_dolog) { - if (ar.ares_result[n].result_method == ARES_METHOD_UNKNOWN) - { - /* foolproof: should not happen */ - if (conf->conf_dolog) - { - syslog(LOG_DEBUG, - "%s: internal error: unknown method is found in ares_result, ignored", - afc->mctx_jobid); - } - continue; - } - if (ar.ares_result[n].result_method == ARES_METHOD_ARC) - { - /* - ** If it's an ARC result under - ** our authserv-id, use that - ** value as the chain state. - */ - - if (!conf->conf_overridecv) { - continue; - } + syslog(LOG_INFO, + "%s: chain state forced to \"%s\" due to prior result found", + afc->mctx_jobid, + arc_chain_status_str(afc->mctx_arcmsg)); + } + } - arfound += 1; - if (arfound > 1) - { - continue; - } + if (arcf_dstring_len(afc->mctx_tmpstr) > 0) + { + arcf_dstring_cat(afc->mctx_tmpstr, + "; "); + } - if (reconcile_arc_state(afc, &ar.ares_result[n]) && conf->conf_dolog) - { - syslog(LOG_INFO, - "%s: chain state forced to \"%s\" due to prior result found", - afc->mctx_jobid, - arc_chain_status_str(afc->mctx_arcmsg)); - } - } + arcf_dstring_printf(afc->mctx_tmpstr, + "%s=%s", + ares_getmethod(ar.ares_result[i].result_method), + ares_getresult(ar.ares_result[i].result_result)); - if (arcf_dstring_len(afc->mctx_tmpstr) > 0) - { - arcf_dstring_cat(afc->mctx_tmpstr, - "; "); - } + if (ar.ares_result[i].result_reason[0] != '\0') + { + arcf_dstring_printf(afc->mctx_tmpstr, + " reason=\"%s\"", + ar.ares_result[i].result_reason); + } + for (int j = 0; j < ar.ares_result[i].result_props; j++) + { + if (ar.ares_result[i].result_ptype[j] == ARES_PTYPE_COMMENT) + { arcf_dstring_printf(afc->mctx_tmpstr, - "%s=%s", - ares_getmethod(ar.ares_result[n].result_method), - ares_getresult(ar.ares_result[n].result_result)); - - if (ar.ares_result[n].result_comment[0] != '\0') - { - arcf_dstring_printf(afc->mctx_tmpstr, - " %s", - ar.ares_result[n].result_comment); - } - - for (m = 0; - m < ar.ares_result[n].result_props; - m++) - { - arcf_dstring_printf(afc->mctx_tmpstr, - " %s.%s=%s", - ares_getptype(ar.ares_result[n].result_ptype[m]), - ar.ares_result[n].result_property[m], - ar.ares_result[n].result_value[m]); - } - - if (ar.ares_result[n].result_reason[0] != '\0') - { - arcf_dstring_printf(afc->mctx_tmpstr, - " reason=\"%s\"", - ar.ares_result[n].result_reason); - } + " %s", + ar.ares_result[i].result_value[j]); + } else { + _Bool quote = !ares_istoken(ar.ares_result[i].result_value[j]); + arcf_dstring_printf(afc->mctx_tmpstr, + " %s.%s=%s%s%s", + ares_getptype(ar.ares_result[i].result_ptype[j]), + ar.ares_result[i].result_property[j], + quote ? "\"" : "", + ar.ares_result[i].result_value[j], + quote ? "\"" : ""); } } } - if (arfound == 0) { + if (!arfound) { /* Record the ARC status */ if (arcf_dstring_len(afc->mctx_tmpstr) > 0) { @@ -3899,7 +3871,7 @@ mlfi_eom(SMFICTX *ctx) if (ipout != NULL) { - _Bool quote = strchr(ipout, ':') != NULL; + _Bool quote = !ares_istoken(ipout); arcf_dstring_printf(afc->mctx_tmpstr, " smtp.remote-ip=%s%s%s", @@ -3910,7 +3882,7 @@ mlfi_eom(SMFICTX *ctx) if (conf->conf_finalreceiver && arcchainlen > 0) { - _Bool quote = strchr(arcchainbuf, ':') != NULL; + _Bool quote = !ares_istoken(arcchainbuf); arcf_dstring_printf(afc->mctx_tmpstr, " arc.chain=%s%s%s", diff --git a/test/files/test_milter_ar_disabled.conf b/test/files/test_milter_ar_override_disabled.conf similarity index 100% rename from test/files/test_milter_ar_disabled.conf rename to test/files/test_milter_ar_override_disabled.conf diff --git a/test/test_milter.py b/test/test_milter.py index 1cc5346..660bda6 100644 --- a/test/test_milter.py +++ b/test/test_milter.py @@ -158,7 +158,204 @@ def test_milter_mode_none_sign(run_miltertest): assert res['headers'][2] == ['ARC-Authentication-Results', 'i=1; example.com; arc=none'] -def test_milter_ar(run_miltertest): +@pytest.mark.parametrize( + 'data', + [ + # Single header + [ + [( + 'example.com;' + ' iprev=pass\n\tpolicy.iprev=192.0.2.1 (mail.example.com);' + '\n\tspf=pass (domain of foo@example.com\n\t designates 192.0.2.1 as permitted sender);' + ' dkim=pass header.i=@example.com header.s=foo' + )], + ( + 'iprev=pass policy.iprev=192.0.2.1 (mail.example.com);' + ' spf=pass (domain of foo@example.com designates 192.0.2.1 as permitted sender);' + ' dkim=pass header.i=@example.com header.s=foo; arc=none' + ), + ], + # Multiple headers + [ + [ + 'example.com; iprev=pass\n\tpolicy.iprev=192.0.2.1 (mail.example.com)', + 'example.com; spf=pass (domain of foo@example.com\n\t designates 192.0.2.1 as permitted sender)', + 'example.com; dkim=pass header.i=@example.com header.s=foo', + ], + ( + 'iprev=pass policy.iprev=192.0.2.1 (mail.example.com);' + ' spf=pass (domain of foo@example.com designates 192.0.2.1 as permitted sender);' + ' dkim=pass header.i=@example.com header.s=foo; arc=none' + ), + ], + # Multiple headers for the same method + [ + [ + 'example.com; spf=pass', + 'example.com; spf=fail', + 'example.com; spf=none', + ], + 'spf=pass; arc=none', + ], + # Same method multiple times + [ + ['example.com; spf=pass; spf=fail; spf=none'], + 'spf=pass; arc=none', + ], + # Header with more results than we're willing to store + [ + [( + 'example.com;' + ' dkim=pass header.i=@example.com header.s=foo;' + ' dkim=pass header.i=@example.com header.s=bar;' + ' dkim=pass header.i=@example.com header.s=baz;' + ' dkim=pass header.i=@example.com header.s=qux;' + ' dkim=pass header.i=@example.com header.s=quux;' + ' dkim=pass header.i=@example.com header.s=quuux;' + ' dkim=fail header.i=@example.com header.s=foo;' + ' dkim=fail header.i=@example.com header.s=bar;' + ' dkim=fail header.i=@example.com header.s=baz;' + ' dkim=fail header.i=@example.com header.s=qux;' + ' dkim=fail header.i=@example.com header.s=quux;' + ' dkim=fail header.i=@example.com header.s=quuux;' + ' dkim=policy header.i=@example.com header.s=foo;' + ' dkim=policy header.i=@example.com header.s=bar;' + ' dkim=policy header.i=@example.com header.s=baz;' + ' dkim=policy header.i=@example.com header.s=qux;' + ' dkim=policy header.i=@example.com header.s=quux;' + ' dkim=policy header.i=@example.com header.s=quuux;' + ' spf=pass' + )], + ( + 'dkim=pass header.i=@example.com header.s=foo;' + ' dkim=pass header.i=@example.com header.s=bar;' + ' dkim=pass header.i=@example.com header.s=baz;' + ' dkim=pass header.i=@example.com header.s=qux;' + ' dkim=pass header.i=@example.com header.s=quux;' + ' dkim=pass header.i=@example.com header.s=quuux;' + ' dkim=fail header.i=@example.com header.s=foo;' + ' dkim=fail header.i=@example.com header.s=bar;' + ' dkim=fail header.i=@example.com header.s=baz;' + ' dkim=fail header.i=@example.com header.s=qux;' + ' dkim=fail header.i=@example.com header.s=quux;' + ' dkim=fail header.i=@example.com header.s=quuux;' + ' dkim=policy header.i=@example.com header.s=foo;' + ' dkim=policy header.i=@example.com header.s=bar;' + ' dkim=policy header.i=@example.com header.s=baz;' + ' dkim=policy header.i=@example.com header.s=qux;' + ' arc=none' + ) + ], + # Non-matching authserv-id + [ + [ + 'example.com.example.net; spf=pass', + 'otheradmd.example.com; spf=tempfail', + 'example.net; spf=permfail', + ], + 'arc=none', + ], + # CFWS + [ + ['example.com; (a)spf (Sender Policy Framework) = pass (good) smtp (mail transfer) . (protocol) mailfrom = foo@example.com;'], + 'spf=pass (good) smtp.mailfrom=foo@example.com; arc=none', + ], + # Unknown method + [ + ['example.com; spf=pass; superspf=pass; arc=pass; superarc=fail policy.krypton=foo;'], + 'spf=pass; arc=pass', + ], + # Unknown ptype + [ + [ + 'example.com; spf=pass imap.override=true', + 'example.com; spf=pass; iprev=pass dnssec.signed=true', + ], + 'arc=none', + ], + # reason + [ + ['example.com; spf=pass (ip4)reason="192.0.2.1 matched ip4:192.0.2.0/27 in _spf.example.com"; dmarc=pass'], + 'spf=pass reason="192.0.2.1 matched ip4:192.0.2.0/27 in _spf.example.com" (ip4); dmarc=pass; arc=none', + ], + # misplaced reason + [ + ['example.com; spf=pass; iprev=pass policy.iprev=192.0.2.1 reason="because"'], + 'arc=none', + ], + # no-result + [ + [ + 'example.com; none', + 'example.com; none; spf=pass', + 'example.com; spf=fail; none', + ], + 'arc=none', + ], + # truncations + [ + [ + 'example.com', + 'example.com; spf', + 'example.com; spf=', + 'example.com; dmarc=pass; iprev=pass policy', + 'example.com; dmarc=pass; iprev=pass policy.', + 'example.com; dmarc=pass; iprev=pass policy.iprev', + 'example.com; dmarc=pass; iprev=pass policy.iprev=', + 'example.com; dmarc=pass; iprev=pass policy.iprev="', + 'example.com; dmarc=pass; iprev=pass policy.iprev="1', + 'example.com; dmarc=pass; iprev=pass policy.iprev="1" (', + 'example.com; dmarc=pass; iprev=pass policy.iprev="1" ( a c', + ], + 'arc=none', + ], + # bad sequences + [ + [ + 'example.com; dmarc=pass; spf pass;', + 'example.com; dmarc=pass; iprev=pass policy.iprev.192.0.2.1', + 'example.com; dmarc=pass; iprev=pass policy=iprev=192.0.2.1', + 'example.com; dmarc=pass reason "because";', + ], + 'arc=none', + ], + # RFC 8904 + [ + ['example.com; dnswl=pass dns.zone=accept.example.com policy.ip=192.0.2.1 policy.txt="sure, yeah" dns.sec=yes'], + 'dnswl=pass dns.zone=accept.example.com policy.ip=192.0.2.1 policy.txt="sure, yeah" dns.sec=yes; arc=none', + ], + # quoted-string + [ + ['example.com; auth=pass smtp.auth="花木蘭\\"\\\\ []"'], + 'auth=pass smtp.auth="花木蘭\\"\\\\ []"; arc=none', + ], + # version + [ + [ + 'example.com 1; spf=pass', + 'example.com 1 ; dmarc=pass', + ], + 'spf=pass; dmarc=pass; arc=none', + ], + # invalid version + [ + [ + 'example.com 12.0; spf=pass', + 'example.com a; spf=pass', + 'example.com 1 1; spf=pass', + ], + 'arc=none', + ], + ] +) +def test_milter_ar(run_miltertest, data): + """Test Authentication-Results parsing""" + res = run_miltertest([['Authentication-Results', x] for x in data[0]]) + + assert res['headers'][3] == ['ARC-Authentication-Results', f'i=1; example.com; {data[1]}'] + + +def test_milter_ar_override(run_miltertest): """Override the chain validation state with Authentication-Results""" res = run_miltertest() @@ -182,7 +379,7 @@ def test_milter_ar(run_miltertest): assert len(res['headers']) == 1 -def test_milter_ar_disabled(run_miltertest): +def test_milter_ar_override_disabled(run_miltertest): """`PermitAuthenticationOverrides = no` preserves the actual state""" res = run_miltertest() @@ -197,7 +394,7 @@ def test_milter_ar_disabled(run_miltertest): assert res['headers'][3] == ['ARC-Authentication-Results', 'i=2; example.com; arc=pass'] -def test_milter_ar_multi(run_miltertest): +def test_milter_ar_override_multi(run_miltertest): """Only the most recent A-R header should matter""" res = run_miltertest()