On Sat, Nov 30, 2024 at 01:01:20PM +0000, Job Snijders wrote:
> On Fri, Nov 29, 2024 at 08:32:28AM -0700, Theo de Raadt wrote:
> > I fear that the -x option will eventually become a useful crutch to
> > block other behaviours.  Maybe this should be -0, to be more be
> > explicit about this AS0 issue.  In the manual page, AS0 support being
> > tied to a specific flag called -0 makes it easier to make the public
> > aware of this problem and the decision for AS0 non-support.
> 
> How about

To be honest, I don't really understand why we need this.
We will not ship an AS0-TAL by default. So the operator already opted into
this by installing this AS0-TAL by themselfs.
Now you want an extra flag on top of that?
Do you fear that on other distributions they will add bad tals by
default?
 
> Index: extern.h
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v
> diff -u -p -r1.233 extern.h
> --- extern.h	26 Nov 2024 13:59:09 -0000	1.233
> +++ extern.h	30 Nov 2024 13:00:41 -0000
> @@ -663,6 +663,7 @@ extern int noop;
>  extern int filemode;
>  extern int excludeaspa;
>  extern int experimental;
> +extern int includeas0;
>  extern const char *tals[];
>  extern const char *taldescs[];
>  extern unsigned int talrepocnt[];
> Index: main.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/main.c,v
> diff -u -p -r1.275 main.c
> --- main.c	21 Nov 2024 13:32:27 -0000	1.275
> +++ main.c	30 Nov 2024 13:00:41 -0000
> @@ -69,6 +69,7 @@ int	verbose;
>  int	noop;
>  int	excludeaspa;
>  int	filemode;
> +int	includeas0;
>  int	shortlistmode;
>  int	rrdpon = 1;
>  int	repo_timeout;
> @@ -1014,8 +1015,12 @@ main(int argc, char *argv[])
>  	    "proc exec unveil", NULL) == -1)
>  		err(1, "pledge");
>  
> -	while ((c = getopt(argc, argv, "Ab:Bcd:e:fH:jmnoP:Rs:S:t:T:vVx")) != -1)
> +	while ((c =
> +	    getopt(argc, argv, "0Ab:Bcd:e:fH:jmnoP:Rs:S:t:T:vVx")) != -1)
>  		switch (c) {
> +		case '0':
> +			includeas0 = 1;
> +			break;
>  		case 'A':
>  			excludeaspa = 1;
>  			break;
> @@ -1552,7 +1557,7 @@ main(int argc, char *argv[])
>  
>  usage:
>  	fprintf(stderr,
> -	    "usage: rpki-client [-ABcjmnoRVvx] [-b sourceaddr] [-d cachedir]"
> +	    "usage: rpki-client [-0ABcjmnoRVvx] [-b sourceaddr] [-d cachedir]"
>  	    " [-e rsync_prog]\n"
>  	    "                   [-H fqdn] [-P epoch] [-S skiplist] [-s timeout]"
>  	    " [-T table]\n"
> Index: output.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/output.c,v
> diff -u -p -r1.33 output.c
> --- output.c	22 Feb 2024 12:49:42 -0000	1.33
> +++ output.c	30 Nov 2024 13:00:41 -0000
> @@ -82,6 +82,49 @@ static int	 output_finish(FILE *);
>  static void	 sig_handler(int);
>  static void	 set_signal_handler(void);
>  
> +/*
> + * Detect & reject so-called "AS0 TALs".
> + * AS0 TALs are TALs where for each and every subordinate ROA the asID field
> + * set to 0. Such TALs introduce operational risk, as they change the fail-safe
> + * from 'fail-open' to 'fail-closed'. Some context:
> + *     https://lists.afrinic.net/pipermail/rpd/2021/013312.html
> + *     https://lists.afrinic.net/pipermail/rpd/2021/013314.html
> + */
> +static void
> +prune_as0_tals(struct vrp_tree *vrps)
> +{
> +	struct vrp *v, *tv;
> +	int talid;
> +	int is_as0_tal[TALSZ_MAX] = { 0 };
> +
> +	for (talid = 0; talid < talsz; talid++)
> +		is_as0_tal[talid] = 1;
> +
> +	if (includeas0)
> +		return;
> +
> +	RB_FOREACH(v, vrp_tree, vrps) {
> +		if (v->asid != 0)
> +			is_as0_tal[v->talid] = 0;
> +	}
> +
> +	for (talid = 0; talid < talsz; talid++) {
> +		if (is_as0_tal[talid]) {
> +			warnx("%s: Detected AS0 TAL, pruning associated VRPs",
> +			    taldescs[talid]);
> +		}
> +	}
> +
> +	RB_FOREACH_SAFE(v, vrp_tree, vrps, tv) {
> +		if (is_as0_tal[v->talid]) {
> +			RB_REMOVE(vrp_tree, vrps, v);
> +			free(v);
> +		}
> +	}
> +
> +	/* XXX: update talstats? */
> +}
> +
>  int
>  outputfiles(struct vrp_tree *v, struct brk_tree *b, struct vap_tree *a,
>      struct vsp_tree *p, struct stats *st)
> @@ -90,6 +133,8 @@ outputfiles(struct vrp_tree *v, struct b
>  
>  	atexit(output_cleantmp);
>  	set_signal_handler();
> +
> +	prune_as0_tals(v);
>  
>  	for (i = 0; outputs[i].name; i++) {
>  		FILE *fout;
> Index: rpki-client.8
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/rpki-client.8,v
> diff -u -p -r1.113 rpki-client.8
> --- rpki-client.8	4 Nov 2024 11:39:12 -0000	1.113
> +++ rpki-client.8	30 Nov 2024 13:00:41 -0000
> @@ -22,7 +22,7 @@
>  .Nd RPKI validator to support BGP routing security
>  .Sh SYNOPSIS
>  .Nm
> -.Op Fl ABcjmnoRVvx
> +.Op Fl 0ABcjmnoRVvx
>  .Op Fl b Ar sourceaddr
>  .Op Fl d Ar cachedir
>  .Op Fl e Ar rsync_prog
> @@ -63,6 +63,10 @@ in various formats.
>  .Pp
>  The options are as follows:
>  .Bl -tag -width Ds
> +.It Fl 0
> +Include potentially hazardous AS0 TALs in the output files.
> +AS0 TALs are not recommended for automatic filtering of BGP routes.
> +The default is not to include them.
>  .It Fl A
>  Exclude the ASPA-set from the output files that support it (JSON and
>  OpenBGPD).
> 

-- 
:wq Claudio