From: Theo Buehler Subject: Re: rpki-client: add support for draft-ietf-sidrops-rpki-prefixlist To: Job Snijders , tech@openbsd.org Date: Thu, 22 Feb 2024 22:31:37 +0100 On Thu, Feb 22, 2024 at 09:42:19PM +0100, Claudio Jeker wrote: > 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. Makes sense. The diff below still counts the total. > 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. This is being tested right now: job deployed .spl in a .mft on chloe (about a week back, I think?) and nothing blew up on stable or recent current. We've been handling unknown filetypes gracefully for more than 1.5 years now. queue_add_from_mft() explicitly skips files with RTYPE_INVALID and there are other safeguards that avoid erroring on encountering an unknown product. The big issue with ASPA was that the file format was changed brutally and the old parser (which was enabled in stable) would error. With SPL I do not expect a trash fire of similar dimension as ASPA, but yes, making its handling opt-in seems totally reasonable. We could consider defaulting to experimental = 1 until shortly before lock. But maybe this can also wait until after release. The diff below is the conservative opt-in. 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 20:54:42 -0000 @@ -72,6 +72,7 @@ int filemode; int shortlistmode; int rrdpon = 1; int repo_timeout; +int experimental; time_t deadline; /* 9999-12-31 23:59:59 UTC */ @@ -670,7 +671,8 @@ 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 (experimental) + repo_stat_inc(rp, talid, type, STYPE_FAIL); break; } spl = spl_read(b); @@ -996,7 +998,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 +1076,9 @@ main(int argc, char *argv[]) case 'V': fprintf(stderr, "rpki-client %s\n", RPKI_VERSION); return 0; + case 'x': + experimental = 1; + break; default: goto usage; } @@ -1505,7 +1510,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-json.c =================================================================== RCS file: /cvs/src/usr.sbin/rpki-client/output-json.c,v diff -u -p -r1.43 output-json.c --- output-json.c 22 Feb 2024 12:49:42 -0000 1.43 +++ output-json.c 22 Feb 2024 20:55:17 -0000 @@ -23,6 +23,8 @@ #include "extern.h" #include "json.h" +extern int experimental; + static void outputheader_json(struct stats *st) { @@ -112,6 +114,31 @@ output_aspa(struct vap_tree *vaps) json_do_end(); } +static void +output_spl(struct vsp_tree *vsps) +{ + struct vsp *vsp; + char buf[64]; + size_t i; + + json_do_array("signedprefixlists"); + RB_FOREACH(vsp, vsp_tree, vsps) { + json_do_object("vsp", 1); + json_do_int("origin_as", vsp->asid); + json_do_array("prefixes"); + for (i = 0; i < vsp->prefixesz; i++) { + ip_addr_print(&vsp->prefixes[i].prefix, + vsp->prefixes[i].afi, buf, sizeof(buf)); + json_do_string("prefix", buf); + } + json_do_end(); + json_do_int("expires", vsp->expires); + json_do_string("ta", taldescs[vsp->talid]); + json_do_end(); + } + json_do_end(); +} + int output_json(FILE *out, struct vrp_tree *vrps, struct brk_tree *brks, struct vap_tree *vaps, struct vsp_tree *vsps, struct stats *st) @@ -119,8 +146,6 @@ output_json(FILE *out, struct vrp_tree * char buf[64]; struct vrp *v; struct brk *b; - struct vsp *vsp; - size_t i; json_do_start(out); outputheader_json(st); @@ -154,22 +179,8 @@ output_json(FILE *out, struct vrp_tree * if (!excludeaspa) output_aspa(vaps); - json_do_array("signedprefixlists"); - RB_FOREACH(vsp, vsp_tree, vsps) { - json_do_object("vsp", 1); - json_do_int("origin_as", vsp->asid); - json_do_array("prefixes"); - for (i = 0; i < vsp->prefixesz; i++) { - ip_addr_print(&vsp->prefixes[i].prefix, - vsp->prefixes[i].afi, buf, sizeof(buf)); - json_do_string("prefix", buf); - } - json_do_end(); - json_do_int("expires", vsp->expires); - json_do_string("ta", taldescs[vsp->talid]); - json_do_end(); - } - json_do_end(); + if (experimental) + output_spl(vsps); return json_do_finish(); } 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 20:55:58 -0000 @@ -39,6 +39,8 @@ #include "extern.h" extern int noop; +extern int 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 (experimental) { + spl = proc_parser_spl(file, f, flen, entp); + if (spl != NULL) + mtime = spl->signtime; + } else { + if (verbose) + warnx("%s: skipped", file); + spl = NULL; + } io_simple_buffer(b, &mtime, sizeof(mtime)); c = (spl != NULL); io_simple_buffer(b, &c, sizeof(int)); 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 20:59:10 -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 +Enable processing of experimental file formats. +This option is implied by +.Fl f . .It Ar outputdir The directory where .Nm