From: Lucas Gabriel Vuotto <lucas@sexy.is>
Subject: Re: rpki-client: detect & reject "AS0 TALs"
To: Job Snijders <job@openbsd.org>
Cc: tech@openbsd.org
Date: Sat, 30 Nov 2024 14:11:08 +0000

On Sat, Nov 30, 2024 at 01:01:20PM +0000, Job Snijders wrote:
> 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;

Shouldn't this go before the for loop? is_as0_tal is local. Also I'd
prefer the call to prune_as0_tals to be guarded by includeas0 instead,
but that falls deeper into bikeshedding territory.

> +	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;