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