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 a6456a81e853f18341b6a500d638a216c896e679 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-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 | 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 80c85c7c908fc936f01b7ab335a078759fda0865 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 88dd06ce3b5e18f8ce6405046d5a384b00cc3c45 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 d570811ad355d76e357b35f3f8411ce39def4726 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 be7d10195d0e85d8f1ad970abd4817a358ff55da 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 8d09e4fcc6d216b90c9b2907aba7c230cbed9495 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) { /*