From: Claudio Jeker Subject: Re: rpki-client: add support for draft-ietf-sidrops-rpki-prefixlist To: Theo Buehler Cc: Job Snijders , tech@openbsd.org Date: Thu, 22 Feb 2024 21:42:19 +0100 On Thu, Feb 22, 2024 at 05:59:09PM +0100, Theo Buehler wrote: > > I'm happy to have something like -x to exclude / include experimental > > bits. So we don't constantly add flags and remove them again. With ASPA we > > got hit rather bad IIRC since we had to go back and forth a few times to > > get it right. > > Something like this? This short circuits the parsing of the DER of the > SPL and counts the "failure" as "skipped" instead. It's a bit dumb to > have two mutually exclusive counters, but it feels wrong to use the same > counter. I would just remove the extra counter and just not count at all. This is a temporary measure to ensure we can handle draft changes or other unexpected problems with these early code dumps. rpki-client is normally around 1 year ahead of the other RP software when it comes to new features and we don't need to be too aggressive. I though -x would enable .spl parsing so that people like Job can play with it but the general public is not affected. We had a few issues with new file types. One was around handling of a new file RTYPE. So I wonder if it is enough to "just" skip the decoder. > I thought it makes more sense to keep -x a no-op with -f mode. After > all, you hand such a file explicitly on the command line. > > Index: extern.h > =================================================================== > RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v > diff -u -p -r1.208 extern.h > --- extern.h 22 Feb 2024 12:49:42 -0000 1.208 > +++ extern.h 22 Feb 2024 16:31:37 -0000 > @@ -575,6 +575,7 @@ enum stype { > STYPE_OK, > STYPE_FAIL, > STYPE_INVALID, > + STYPE_SKIPPED, > STYPE_BGPSEC, > STYPE_TOTAL, > STYPE_UNIQUE, > @@ -612,7 +613,8 @@ struct repotalstats { > uint32_t vrps_uniqs; /* number of unique vrps */ > uint32_t spls; /* signed prefix list */ > uint32_t spls_fail; /* failing syntactic parse */ > - uint32_t spls_invalid; /* invalid asid */ > + uint32_t spls_skipped; /* skipped spls */ > + uint32_t spls_invalid; /* invalid spls */ > uint32_t vsps; /* total number of Validated SPL Payloads */ > uint32_t vsps_uniqs; /* number of unique vsps */ > }; > Index: main.c > =================================================================== > RCS file: /cvs/src/usr.sbin/rpki-client/main.c,v > diff -u -p -r1.251 main.c > --- main.c 22 Feb 2024 12:49:42 -0000 1.251 > +++ main.c 22 Feb 2024 16:31:25 -0000 > @@ -72,6 +72,7 @@ int filemode; > int shortlistmode; > int rrdpon = 1; > int repo_timeout; > +int skip_experimental; > time_t deadline; > > /* 9999-12-31 23:59:59 UTC */ > @@ -670,7 +671,10 @@ entity_process(struct ibuf *b, struct st > case RTYPE_SPL: > io_read_buf(b, &c, sizeof(c)); > if (c == 0) { > - repo_stat_inc(rp, talid, type, STYPE_FAIL); > + if (skip_experimental) > + repo_stat_inc(rp, talid, type, STYPE_SKIPPED); > + else > + repo_stat_inc(rp, talid, type, STYPE_FAIL); > break; > } > spl = spl_read(b); > @@ -772,6 +776,7 @@ sum_stats(const struct repo *rp, const s > out->vaps_pas += in->vaps_pas; > out->spls += in->spls; > out->spls_fail += in->spls_fail; > + out->spls_skipped += in->spls_skipped; > out->spls_invalid += in->spls_invalid; > out->vsps += in->vsps; > out->vsps_uniqs += in->vsps_uniqs; > @@ -996,7 +1001,7 @@ main(int argc, char *argv[]) > "proc exec unveil", NULL) == -1) > err(1, "pledge"); > > - while ((c = getopt(argc, argv, "Ab:Bcd:e:fH:jmnoP:rRs:S:t:T:vV")) != -1) > + while ((c = getopt(argc, argv, "Ab:Bcd:e:fH:jmnoP:rRs:S:t:T:vVx")) != -1) > switch (c) { > case 'A': > excludeaspa = 1; > @@ -1074,6 +1079,9 @@ main(int argc, char *argv[]) > case 'V': > fprintf(stderr, "rpki-client %s\n", RPKI_VERSION); > return 0; > + case 'x': > + skip_experimental = 1; > + break; > default: > goto usage; > } > @@ -1473,8 +1481,10 @@ main(int argc, char *argv[]) > "invalid)\n", stats.repo_tal_stats.aspas, > stats.repo_tal_stats.aspas_fail, > stats.repo_tal_stats.aspas_invalid); > - printf("Signed Prefix Lists: %u (%u failed parse, %u invalid)\n", > + printf("Signed Prefix Lists: %u (%u failed parse, %u skipped, " > + "%u invalid)\n", > stats.repo_tal_stats.spls, stats.repo_tal_stats.spls_fail, > + stats.repo_tal_stats.spls_skipped, > stats.repo_tal_stats.spls_invalid); > printf("BGPsec Router Certificates: %u\n", stats.repo_tal_stats.brks); > printf("Certificates: %u (%u invalid)\n", > @@ -1505,7 +1515,7 @@ main(int argc, char *argv[]) > > usage: > fprintf(stderr, > - "usage: rpki-client [-ABcjmnoRrVv] [-b sourceaddr] [-d cachedir]" > + "usage: rpki-client [-ABcjmnoRrVvx] [-b sourceaddr] [-d cachedir]" > " [-e rsync_prog]\n" > " [-H fqdn] [-P epoch] [-S skiplist] [-s timeout]" > " [-T table]\n" > Index: output-ometric.c > =================================================================== > RCS file: /cvs/src/usr.sbin/rpki-client/output-ometric.c,v > diff -u -p -r1.8 output-ometric.c > --- output-ometric.c 22 Feb 2024 12:49:42 -0000 1.8 > +++ output-ometric.c 22 Feb 2024 16:50:34 -0000 > @@ -87,6 +87,8 @@ set_common_stats(const struct repotalsta > OKV("type", "state"), OKV("spl", "valid"), ol); > ometric_set_int_with_labels(metric, in->spls_fail, > OKV("type", "state"), OKV("spl", "failed parse"), ol); > + ometric_set_int_with_labels(metric, in->spls_skipped, > + OKV("type", "state"), OKV("spl", "skipped"), ol); > ometric_set_int_with_labels(metric, in->spls_invalid, > OKV("type", "state"), OKV("spl", "invalid"), ol); > > Index: parser.c > =================================================================== > RCS file: /cvs/src/usr.sbin/rpki-client/parser.c,v > diff -u -p -r1.129 parser.c > --- parser.c 22 Feb 2024 12:49:42 -0000 1.129 > +++ parser.c 22 Feb 2024 15:29:45 -0000 > @@ -39,6 +39,8 @@ > #include "extern.h" > > extern int noop; > +extern int skip_experimental; > +extern int verbose; > > static X509_STORE_CTX *ctx; > static struct auth_tree auths = RB_INITIALIZER(&auths); > @@ -861,9 +863,15 @@ parse_entity(struct entityq *q, struct m > case RTYPE_SPL: > file = parse_load_file(entp, &f, &flen); > io_str_buffer(b, file); > - spl = proc_parser_spl(file, f, flen, entp); > - if (spl != NULL) > - mtime = spl->signtime; > + if (skip_experimental) { > + if (verbose) > + warnx("%s: skipped", file); > + spl = NULL; > + } else { > + spl = proc_parser_spl(file, f, flen, entp); > + if (spl != NULL) > + mtime = spl->signtime; > + } > io_simple_buffer(b, &mtime, sizeof(mtime)); > c = (spl != NULL); > io_simple_buffer(b, &c, sizeof(int)); > Index: repo.c > =================================================================== > RCS file: /cvs/src/usr.sbin/rpki-client/repo.c,v > diff -u -p -r1.53 repo.c > --- repo.c 22 Feb 2024 12:49:42 -0000 1.53 > +++ repo.c 22 Feb 2024 15:49:33 -0000 > @@ -1488,6 +1488,9 @@ repo_stat_inc(struct repo *rp, int talid > case STYPE_FAIL: > rp->stats[talid].spls_fail++; > break; > + case STYPE_SKIPPED: > + rp->stats[talid].spls_skipped++; > + break; > case STYPE_INVALID: > rp->stats[talid].spls_invalid++; > break; > Index: rpki-client.8 > =================================================================== > RCS file: /cvs/src/usr.sbin/rpki-client/rpki-client.8,v > diff -u -p -r1.101 rpki-client.8 > --- rpki-client.8 22 Feb 2024 12:49:42 -0000 1.101 > +++ rpki-client.8 22 Feb 2024 16:15:25 -0000 > @@ -22,7 +22,7 @@ > .Nd RPKI validator to support BGP routing security > .Sh SYNOPSIS > .Nm > -.Op Fl ABcjmnoRrVv > +.Op Fl ABcjmnoRrVvx > .Op Fl b Ar sourceaddr > .Op Fl d Ar cachedir > .Op Fl e Ar rsync_prog > @@ -231,6 +231,10 @@ If > .Fl f > is given, specify once to print more information about the encapsulated X.509 > certificate, twice to print the certificate in PEM format. > +.It Fl x > +Skip processing of experimental file formats. > +This option has no effect if > +.Fl f is given . > .It Ar outputdir > The directory where > .Nm > -- :wq Claudio