Skip to content

Commit d7e60aa

Browse files
committed
Merge pull request trusteddomainproject#177 from futatuki/improve-ar-parse
Improve AR parser openarc/openarc-ar.c (ares_parse): * strictly check the use of "none" (no-result) instead of one or more resinfo * record really "result comment" to result_comment field * overwrite the former result instead of dedup after writing to new result * do not record the results of unknown method (Fix issue trusteddomainproject#113) * even after the number of recorded result has reached to the limit, continue to parse for syntax checking (and update the former result if newer value is found for seen methods). Others: * Remove dedup() in openarc/opearc-ar.c * Fix main() function for ARTEST enabled in openarc-ar.c for testing above * In mlfi_eom in openarc.c, add foolproof check for ignoring unknown results trusteddomainproject#177
2 parents 27365b9 + 8d09e4f commit d7e60aa

File tree

2 files changed

+132
-58
lines changed

2 files changed

+132
-58
lines changed

openarc/openarc-ar.c

Lines changed: 121 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@
4040
#define ARES_TOKENS ";=."
4141
#define ARES_TOKENS2 "=."
4242

43-
#define ARES_MAXTOKENS 512
43+
#define ARES_MAXTOKENS 1024
4444

4545
/* tables */
4646
struct lookup
@@ -337,35 +337,37 @@ ares_xconvert(struct lookup *table, int code)
337337
}
338338

339339
/*
340-
** ARES_DEDUP -- if we've gotten multiple results of the same method,
341-
** discard the older one
340+
** ARES_METHOD_SEEN -- if we've already seen the results of the method,
341+
** returns its index
342342
**
343343
** Parameters:
344344
** ar -- pointer to a (struct authres)
345345
** n -- the last one that was loaded
346+
** m -- the method to be searched
346347
**
347348
** Return value:
348-
** TRUE iff a de-duplication happened, leaving the result referenced by
349-
** "n" open.
349+
** The index of the method in ar, if it is found, else -1
350350
*/
351351

352-
static _Bool
353-
ares_dedup(struct authres *ar, int n)
352+
static int
353+
ares_method_seen(struct authres *ar, int n, ares_method_t m)
354354
{
355355
int c;
356356

357+
if (ar->ares_result[n].result_method == ARES_METHOD_DKIM)
358+
{
359+
/* All results of DKIM should be kept */
360+
return -1;
361+
}
357362
for (c = 0; c < n; c++)
358363
{
359-
if (ar->ares_result[c].result_method == ar->ares_result[n].result_method &&
360-
ar->ares_result[c].result_method != ARES_METHOD_DKIM)
364+
if (ar->ares_result[c].result_method == m)
361365
{
362-
memcpy(&ar->ares_result[c], &ar->ares_result[n],
363-
sizeof(ar->ares_result[c]));
364-
return TRUE;
366+
return c;
365367
}
366368
}
367369

368-
return FALSE;
370+
return -1;
369371
}
370372

371373
/*
@@ -390,6 +392,8 @@ ares_parse(u_char *hdr, struct authres *ar)
390392
int r = 0;
391393
int state;
392394
int prevstate;
395+
int i; /* index of a result to be recorded */
396+
ares_method_t m;
393397
u_char tmp[ARC_MAXHEADER + 2];
394398
u_char *tokens[ARES_MAXTOKENS];
395399

@@ -406,14 +410,21 @@ ares_parse(u_char *hdr, struct authres *ar)
406410
prevstate = -1;
407411
state = 0;
408412
n = 0;
413+
i = 0;
409414

410415
for (c = 0; c < ntoks; c++)
411416
{
412417
if (tokens[c][0] == '(') /* comment */
413418
{
414-
strlcpy((char *) ar->ares_result[n - 1].result_comment,
415-
(char *) tokens[c],
416-
sizeof ar->ares_result[n - 1].result_comment);
419+
if (i >= 0 && prevstate == 5)
420+
{
421+
/* record at most one comment only */
422+
assert(state == 6);
423+
strlcpy((char *) ar->ares_result[i].result_comment,
424+
(char *) tokens[c],
425+
sizeof ar->ares_result[i].result_comment);
426+
prevstate = state;
427+
}
417428
continue;
418429
}
419430

@@ -488,27 +499,55 @@ ares_parse(u_char *hdr, struct authres *ar)
488499
break;
489500

490501
case 3: /* method/none */
491-
if (n == 0 || !ares_dedup(ar, n))
492-
n++;
493-
494-
if (n >= MAXARESULTS)
495-
return 0;
496-
497-
r = 0;
498-
499502
if (strcasecmp((char *) tokens[c], "none") == 0)
500503
{
501-
if (n > 0)
502-
n--;
504+
switch (prevstate)
505+
{
506+
case 0:
507+
case 1:
508+
case 2:
509+
prevstate = state;
510+
state = 14;
511+
continue;
512+
default:
513+
/* should not have other resinfo */
514+
return -1;
515+
}
516+
}
503517

504-
prevstate = state;
505-
state = 14;
518+
m = ares_convert(methods, (char *) tokens[c]);
519+
switch(m)
520+
{
521+
case ARES_METHOD_UNKNOWN:
522+
i = -1;
523+
break;
524+
case ARES_METHOD_DKIM:
525+
i = n;
526+
break;
527+
default:
528+
i = ares_method_seen(ar, n, m);
529+
if (i == -1)
530+
{
531+
i = n;
532+
}
533+
else
534+
{
535+
/* Reuse results field of same method */
536+
memset(&ar->ares_result[i], '\0',
537+
sizeof(ar->ares_result[i]));
538+
}
539+
}
506540

507-
continue;
541+
r = 0;
542+
if (i >= MAXARESULTS)
543+
{
544+
/* continue parsing, but don't record */
545+
i = -1;
508546
}
509547

510-
ar->ares_result[n - 1].result_method = ares_convert(methods,
511-
(char *) tokens[c]);
548+
if (i >= 0) {
549+
ar->ares_result[i].result_method = m;
550+
}
512551
prevstate = state;
513552
state = 4;
514553

@@ -525,9 +564,12 @@ ares_parse(u_char *hdr, struct authres *ar)
525564
break;
526565

527566
case 5: /* result */
528-
ar->ares_result[n - 1].result_result = ares_convert(aresults,
529-
(char *) tokens[c]);
530-
ar->ares_result[n - 1].result_comment[0] = '\0';
567+
if (i >= 0)
568+
{
569+
ar->ares_result[i].result_result = ares_convert(aresults,
570+
(char *) tokens[c]);
571+
ar->ares_result[i].result_comment[0] = '\0';
572+
}
531573
prevstate = state;
532574
state = 6;
533575

@@ -544,9 +586,12 @@ ares_parse(u_char *hdr, struct authres *ar)
544586
break;
545587

546588
case 8:
547-
strlcpy((char *) ar->ares_result[n - 1].result_reason,
548-
(char *) tokens[c],
549-
sizeof ar->ares_result[n - 1].result_reason);
589+
if (i >= 0)
590+
{
591+
strlcpy((char *) ar->ares_result[i].result_reason,
592+
(char *) tokens[c],
593+
sizeof ar->ares_result[i].result_reason);
594+
}
550595

551596
prevstate = state;
552597
state = 9;
@@ -557,6 +602,11 @@ ares_parse(u_char *hdr, struct authres *ar)
557602
if (tokens[c][0] == ';' && /* neither */
558603
tokens[c][1] == '\0')
559604
{
605+
if (i == n)
606+
{
607+
n++;
608+
}
609+
560610
prevstate = state;
561611
state = 3;
562612

@@ -585,9 +635,12 @@ ares_parse(u_char *hdr, struct authres *ar)
585635
{
586636
r--;
587637

588-
strlcat((char *) ar->ares_result[n - 1].result_value[r],
589-
(char *) tokens[c],
590-
sizeof ar->ares_result[n - 1].result_value[r]);
638+
if (i >= 0)
639+
{
640+
strlcat((char *) ar->ares_result[i].result_value[r],
641+
(char *) tokens[c],
642+
sizeof ar->ares_result[i].result_value[r]);
643+
}
591644

592645
prevstate = state;
593646
state = 13;
@@ -598,6 +651,11 @@ ares_parse(u_char *hdr, struct authres *ar)
598651
if (tokens[c][0] == ';' &&
599652
tokens[c][1] == '\0')
600653
{
654+
if (i == n)
655+
{
656+
n++;
657+
}
658+
601659
prevstate = state;
602660
state = 3;
603661

@@ -611,8 +669,10 @@ ares_parse(u_char *hdr, struct authres *ar)
611669
if (x == ARES_PTYPE_UNKNOWN)
612670
return -1;
613671

614-
if (r < MAXPROPS)
615-
ar->ares_result[n - 1].result_ptype[r] = x;
672+
if (r < MAXPROPS && i >= 0)
673+
{
674+
ar->ares_result[i].result_ptype[r] = x;
675+
}
616676

617677
prevstate = state;
618678
state = 10;
@@ -631,11 +691,11 @@ ares_parse(u_char *hdr, struct authres *ar)
631691
break;
632692

633693
case 11: /* property */
634-
if (r < MAXPROPS)
694+
if (r < MAXPROPS && i >= 0)
635695
{
636-
strlcpy((char *) ar->ares_result[n - 1].result_property[r],
696+
strlcpy((char *) ar->ares_result[i].result_property[r],
637697
(char *) tokens[c],
638-
sizeof ar->ares_result[n - 1].result_property[r]);
698+
sizeof ar->ares_result[i].result_property[r]);
639699
}
640700

641701
prevstate = state;
@@ -656,11 +716,14 @@ ares_parse(u_char *hdr, struct authres *ar)
656716
case 13: /* value */
657717
if (r < MAXPROPS)
658718
{
659-
strlcat((char *) ar->ares_result[n - 1].result_value[r],
660-
(char *) tokens[c],
661-
sizeof ar->ares_result[n - 1].result_value[r]);
719+
if (i >= 0)
720+
{
721+
strlcat((char *) ar->ares_result[i].result_value[r],
722+
(char *) tokens[c],
723+
sizeof ar->ares_result[i].result_value[r]);
724+
ar->ares_result[i].result_props = r + 1;
725+
}
662726
r++;
663-
ar->ares_result[n - 1].result_props = r;
664727
}
665728

666729
prevstate = state;
@@ -680,10 +743,10 @@ ares_parse(u_char *hdr, struct authres *ar)
680743
state == 11 || state == 12)
681744
return -1;
682745

683-
if (n > 1)
746+
if (i == n)
684747
{
685-
if (ares_dedup(ar, n - 1))
686-
n--;
748+
/* the last resinfo was added */
749+
n++;
687750
}
688751

689752
ar->ares_count = n;
@@ -750,8 +813,6 @@ ares_getptype(ares_ptype_t ptype)
750813
** EX_USAGE or EX_OK
751814
*/
752815

753-
# define NTOKENS 256
754-
755816
int
756817
main(int argc, char **argv)
757818
{
@@ -761,8 +822,8 @@ main(int argc, char **argv)
761822
char *p;
762823
char *progname;
763824
struct authres ar;
764-
u_char buf[1024];
765-
u_char *toks[NTOKENS];
825+
u_char buf[ARC_MAXHEADER + 2];
826+
u_char *toks[ARES_MAXTOKENS];
766827

767828
progname = (p = strrchr(argv[0], '/')) == NULL ? argv[0] : p + 1;
768829

@@ -772,13 +833,14 @@ main(int argc, char **argv)
772833
return EX_USAGE;
773834
}
774835

775-
c = ares_tokenize(argv[1], buf, sizeof buf, toks, NTOKENS);
836+
c = ares_tokenize(((u_char **)argv)[1], buf, sizeof buf, toks,
837+
ARES_MAXTOKENS);
776838
for (d = 0; d < c; d++)
777839
printf("token %d = '%s'\n", d, toks[d]);
778840

779841
printf("\n");
780842

781-
status = ares_parse(argv[1], &ar);
843+
status = ares_parse(((u_char **)argv)[1], &ar);
782844
if (status == -1)
783845
{
784846
printf("%s: ares_parse() returned -1\n", progname);
@@ -804,6 +866,7 @@ main(int argc, char **argv)
804866
ares_xconvert(aresults,
805867
ar.ares_result[c].result_result));
806868
printf("\treason \"%s\"\n", ar.ares_result[c].result_reason);
869+
printf("\tcomment \"%s\"\n", ar.ares_result[c].result_comment);
807870

808871
for (d = 0; d < ar.ares_result[c].result_props; d++)
809872
{

openarc/openarc.c

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3675,6 +3675,17 @@ mlfi_eom(SMFICTX *ctx)
36753675

36763676
for (n = 0; n < ar.ares_count; n++)
36773677
{
3678+
if (ar.ares_result[n].result_method == ARES_METHOD_UNKNOWN)
3679+
{
3680+
/* foolproof: should not happen */
3681+
if (conf->conf_dolog)
3682+
{
3683+
syslog(LOG_DEBUG,
3684+
"%s: internal error: unknown method is found in ares_result, ignored",
3685+
afc->mctx_jobid);
3686+
}
3687+
continue;
3688+
}
36783689
if (ar.ares_result[n].result_method == ARES_METHOD_ARC)
36793690
{
36803691
/*

0 commit comments

Comments
 (0)