From 6a42f8d56cb8c1609e9e72750438a34be701c30a Mon Sep 17 00:00:00 2001 From: FUTATSUKI Yasuhito Date: Thu, 12 Sep 2024 18:15:10 +0900 Subject: [PATCH 01/10] 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 | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/openarc/openarc-ar.c b/openarc/openarc-ar.c index 48acec62..11eea0f1 100644 --- a/openarc/openarc-ar.c +++ b/openarc/openarc-ar.c @@ -772,13 +772,13 @@ 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, NTOKENS); 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); From 120f26a41bbb4109f39049fd4ad5231c5a2df99b Mon Sep 17 00:00:00 2001 From: FUTATSUKI Yasuhito Date: Thu, 12 Sep 2024 21:49:31 +0900 Subject: [PATCH 02/10] 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 | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/openarc/openarc-ar.c b/openarc/openarc-ar.c index 11eea0f1..4c5daffb 100644 --- a/openarc/openarc-ar.c +++ b/openarc/openarc-ar.c @@ -501,10 +501,18 @@ ares_parse(u_char *hdr, struct authres *ar) if (n > 0) n--; - prevstate = state; - state = 14; - - continue; + switch (prevstate) + { + case 0: + case 1: + case 2: + prevstate = state; + state = 14; + continue; + default: + /* should not have other resinfo */ + return -1; + } } ar->ares_result[n - 1].result_method = ares_convert(methods, From 0061b2ba4cd66b71b665f939e05ec73272e8f777 Mon Sep 17 00:00:00 2001 From: FUTATSUKI Yasuhito Date: Thu, 12 Sep 2024 21:59:20 +0900 Subject: [PATCH 03/10] 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 | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/openarc/openarc-ar.c b/openarc/openarc-ar.c index 4c5daffb..13767eca 100644 --- a/openarc/openarc-ar.c +++ b/openarc/openarc-ar.c @@ -411,9 +411,15 @@ ares_parse(u_char *hdr, struct authres *ar) { 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 (prevstate == 5) + { + /* record at most one comment only */ + assert(state == 6); + strlcpy((char *) ar->ares_result[n - 1].result_comment, + (char *) tokens[c], + sizeof ar->ares_result[n - 1].result_comment); + prevstate = state; + } continue; } From 6f7080d794a381a80d392803246007d57d5dde19 Mon Sep 17 00:00:00 2001 From: FUTATSUKI Yasuhito Date: Thu, 12 Sep 2024 23:37:06 +0900 Subject: [PATCH 04/10] 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 | 119 +++++++++++++++++++++++++++---------------- 1 file changed, 76 insertions(+), 43 deletions(-) diff --git a/openarc/openarc-ar.c b/openarc/openarc-ar.c index 13767eca..f7dee45a 100644 --- a/openarc/openarc-ar.c +++ b/openarc/openarc-ar.c @@ -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,6 +410,7 @@ ares_parse(u_char *hdr, struct authres *ar) prevstate = -1; state = 0; n = 0; + i = 0; for (c = 0; c < ntoks; c++) { @@ -415,9 +420,9 @@ ares_parse(u_char *hdr, struct authres *ar) { /* record at most one comment only */ assert(state == 6); - strlcpy((char *) ar->ares_result[n - 1].result_comment, + strlcpy((char *) ar->ares_result[i].result_comment, (char *) tokens[c], - sizeof ar->ares_result[n - 1].result_comment); + sizeof ar->ares_result[i].result_comment); prevstate = state; } continue; @@ -494,19 +499,8 @@ 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: @@ -521,8 +515,35 @@ ares_parse(u_char *hdr, struct authres *ar) } } - ar->ares_result[n - 1].result_method = ares_convert(methods, - (char *) tokens[c]); + m = ares_convert(methods, (char *) tokens[c]); + if (m == ARES_METHOD_DKIM) + { + /* "dkim" should always added */ + i = n; + break; + } + else + { + 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])); + } + } + + r = 0; + if (i >= MAXARESULTS) + { + return 0; + } + + ar->ares_result[i].result_method = m; prevstate = state; state = 4; @@ -539,9 +560,9 @@ 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'; + ar->ares_result[i].result_result = ares_convert(aresults, + (char *) tokens[c]); + ar->ares_result[i].result_comment[0] = '\0'; prevstate = state; state = 6; @@ -558,9 +579,9 @@ ares_parse(u_char *hdr, struct authres *ar) break; case 8: - strlcpy((char *) ar->ares_result[n - 1].result_reason, + strlcpy((char *) ar->ares_result[i].result_reason, (char *) tokens[c], - sizeof ar->ares_result[n - 1].result_reason); + sizeof ar->ares_result[i].result_reason); prevstate = state; state = 9; @@ -571,6 +592,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; @@ -599,9 +625,9 @@ ares_parse(u_char *hdr, struct authres *ar) { r--; - strlcat((char *) ar->ares_result[n - 1].result_value[r], + strlcat((char *) ar->ares_result[i].result_value[r], (char *) tokens[c], - sizeof ar->ares_result[n - 1].result_value[r]); + sizeof ar->ares_result[i].result_value[r]); prevstate = state; state = 13; @@ -612,6 +638,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; @@ -626,7 +657,9 @@ ares_parse(u_char *hdr, struct authres *ar) return -1; if (r < MAXPROPS) - ar->ares_result[n - 1].result_ptype[r] = x; + { + ar->ares_result[i].result_ptype[r] = x; + } prevstate = state; state = 10; @@ -647,9 +680,9 @@ ares_parse(u_char *hdr, struct authres *ar) case 11: /* property */ if (r < MAXPROPS) { - 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; @@ -670,11 +703,11 @@ ares_parse(u_char *hdr, struct authres *ar) case 13: /* value */ if (r < MAXPROPS) { - strlcat((char *) ar->ares_result[n - 1].result_value[r], + strlcat((char *) ar->ares_result[i].result_value[r], (char *) tokens[c], - sizeof ar->ares_result[n - 1].result_value[r]); + 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; @@ -694,10 +727,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; From 3c99ee4ea9a1b31cd644578b12e7feaea53263d3 Mon Sep 17 00:00:00 2001 From: FUTATSUKI Yasuhito Date: Thu, 12 Sep 2024 23:57:19 +0900 Subject: [PATCH 05/10] openarc/openarc.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.c (ares_parse): As described above. --- openarc/openarc-ar.c | 59 +++++++++++++++++++++++++++----------------- 1 file changed, 37 insertions(+), 22 deletions(-) diff --git a/openarc/openarc-ar.c b/openarc/openarc-ar.c index f7dee45a..556631a8 100644 --- a/openarc/openarc-ar.c +++ b/openarc/openarc-ar.c @@ -416,7 +416,7 @@ ares_parse(u_char *hdr, struct authres *ar) { if (tokens[c][0] == '(') /* comment */ { - if (prevstate == 5) + if (i >= 0 && prevstate == 5) { /* record at most one comment only */ assert(state == 6); @@ -516,14 +516,15 @@ ares_parse(u_char *hdr, struct authres *ar) } m = ares_convert(methods, (char *) tokens[c]); - if (m == ARES_METHOD_DKIM) + switch(m) { - /* "dkim" should always added */ + case ARES_METHOD_UNKNOWN: + i = -1; + break; + case ARES_METHOD_DKIM: i = n; break; - } - else - { + default: i = ares_method_seen(ar, n, m); if (i == -1) { @@ -543,7 +544,9 @@ ares_parse(u_char *hdr, struct authres *ar) return 0; } - ar->ares_result[i].result_method = m; + if (i >= 0) { + ar->ares_result[i].result_method = m; + } prevstate = state; state = 4; @@ -560,9 +563,12 @@ ares_parse(u_char *hdr, struct authres *ar) break; case 5: /* result */ - ar->ares_result[i].result_result = ares_convert(aresults, - (char *) tokens[c]); - ar->ares_result[i].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; @@ -579,9 +585,12 @@ ares_parse(u_char *hdr, struct authres *ar) break; case 8: - strlcpy((char *) ar->ares_result[i].result_reason, - (char *) tokens[c], - sizeof ar->ares_result[i].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; @@ -625,9 +634,12 @@ ares_parse(u_char *hdr, struct authres *ar) { r--; - strlcat((char *) ar->ares_result[i].result_value[r], - (char *) tokens[c], - sizeof ar->ares_result[i].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; @@ -656,7 +668,7 @@ ares_parse(u_char *hdr, struct authres *ar) if (x == ARES_PTYPE_UNKNOWN) return -1; - if (r < MAXPROPS) + if (r < MAXPROPS && i >= 0) { ar->ares_result[i].result_ptype[r] = x; } @@ -678,7 +690,7 @@ 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[i].result_property[r], (char *) tokens[c], @@ -703,10 +715,13 @@ ares_parse(u_char *hdr, struct authres *ar) case 13: /* value */ if (r < MAXPROPS) { - 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; + 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++; } From f7a891ee3960d377489267c561841b6f9ee33eb0 Mon Sep 17 00:00:00 2001 From: FUTATSUKI Yasuhito Date: Thu, 12 Sep 2024 20:09:22 +0900 Subject: [PATCH 06/10] 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 | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/openarc/openarc-ar.c b/openarc/openarc-ar.c index 556631a8..05a8ff65 100644 --- a/openarc/openarc-ar.c +++ b/openarc/openarc-ar.c @@ -541,7 +541,8 @@ ares_parse(u_char *hdr, struct authres *ar) r = 0; if (i >= MAXARESULTS) { - return 0; + /* continue parsing, but don't record */ + i = -1; } if (i >= 0) { From 4084c23caf245c495a0fe29413b46b5481f9a77a Mon Sep 17 00:00:00 2001 From: FUTATSUKI Yasuhito Date: Fri, 13 Sep 2024 14:24:39 +0900 Subject: [PATCH 07/10] openarc/openarc-ar.c (main (for testing)): Use appropriate macros --- openarc/openarc-ar.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/openarc/openarc-ar.c b/openarc/openarc-ar.c index 05a8ff65..bd99cfba 100644 --- a/openarc/openarc-ar.c +++ b/openarc/openarc-ar.c @@ -813,8 +813,6 @@ ares_getptype(ares_ptype_t ptype) ** EX_USAGE or EX_OK */ -# define NTOKENS 256 - int main(int argc, char **argv) { @@ -824,8 +822,8 @@ main(int argc, char **argv) char *p; char *progname; struct authres ar; - u_char buf[1024]; - 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; @@ -835,7 +833,8 @@ main(int argc, char **argv) return EX_USAGE; } - c = ares_tokenize(((u_char **)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]); From 0c517679151ff0d2a48f1cc9e9ffccb40d341e2d Mon Sep 17 00:00:00 2001 From: FUTATSUKI Yasuhito Date: Fri, 13 Sep 2024 14:31:27 +0900 Subject: [PATCH 08/10] openarc/openarc-ar.c (main (for testing)): print result_comment --- openarc/openarc-ar.c | 1 + 1 file changed, 1 insertion(+) diff --git a/openarc/openarc-ar.c b/openarc/openarc-ar.c index bd99cfba..c9e7294c 100644 --- a/openarc/openarc-ar.c +++ b/openarc/openarc-ar.c @@ -866,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++) { From 9f54ab29c10ab011c76ebd2423ed9fb616a954a4 Mon Sep 17 00:00:00 2001 From: FUTATSUKI Yasuhito Date: Fri, 13 Sep 2024 14:32:32 +0900 Subject: [PATCH 09/10] openarc/openarc-ar.c: Inclease max number of tokens for parsing AR header --- openarc/openarc-ar.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openarc/openarc-ar.c b/openarc/openarc-ar.c index c9e7294c..ec7fdeb2 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 From 13a719958304dde5ab66e3bb2de60ef3e9514baa Mon Sep 17 00:00:00 2001 From: FUTATSUKI Yasuhito Date: Fri, 13 Sep 2024 14:51:07 +0900 Subject: [PATCH 10/10] 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.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/openarc/openarc.c b/openarc/openarc.c index 7e191dc1..60dde2c7 100644 --- a/openarc/openarc.c +++ b/openarc/openarc.c @@ -3660,6 +3660,17 @@ 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) { /*