Index | Thread | Search

From:
Job Snijders <job@bsd.nl>
Subject:
rpki-client: refactor non-functional CA metrics accounting
To:
tech@openbsd.org
Date:
Sun, 21 Jun 2026 21:47:13 +0000

Download raw body.

Thread
Rpki-client detects non-functional CAs by initially marking every CA as
non-functional, and then later on (after a valid manifest is discovered)
clearing that mark. Statistics were done along the same lines: first
increment and later on substract for all functional ones. This resulted
in rather unwieldy code, e.g., a repo pointer needed to be hoisted
around which is getting in the way of upcoming work.

The below diff simplifies the dance described above, and also fixes a
subtle defect: non-functional CAs were counted towards the repository
those broken CAs were pointing towards, instead of being counted
towards the repository that contained the broken CA. Attributing the
non-functional CA to the issuing parent makes more sense to me, because
the issuing parent can actually do something about it, for example by
revoking the broken child.

In short, I think this diff helps better show which repository is
responsible for non-functional CAs. See the metrics output that follows.

Openmetrics while attributing to parent (behavior with the below diff):

	rpki_client_repository_objects{type="cert",state="non-functional",name="apnic",carepo="rsync://rpki.cernet.edu.cn/repo"} 1
	rpki_client_repository_objects{type="cert",state="non-functional",name="apnic",carepo="rsync://rpki.apnic.net/repository"} 1
	rpki_client_repository_objects{type="cert",state="non-functional",name="arin",carepo="rsync://rpki.sunoaki.net/repo"} 3
	rpki_client_repository_objects{type="cert",state="non-functional",name="arin",carepo="rsync://rsync.paas.rpki.ripe.net/repository"} 2
	rpki_client_repository_objects{type="cert",state="non-functional",name="arin",carepo="rsync://rpki.arin.net/repository"} 16
	rpki_client_repository_objects{type="cert",state="non-functional",name="lacnic",carepo="rsync://rpki-repo.registro.br/repo"} 46
	rpki_client_repository_objects{type="cert",state="non-functional",name="ripe",carepo="rsync://rpki.komorebi.network/repo"} 2
	rpki_client_repository_objects{type="cert",state="non-functional",name="ripe",carepo="rsync://rpki.cc/repo"} 1
	rpki_client_repository_objects{type="cert",state="non-functional",name="ripe",carepo="rsync://krill.47272.net/repo"} 2
	rpki_client_repository_objects{type="cert",state="non-functional",name="ripe",carepo="rsync://cloudie-repo.rpki.app/repo"} 2
	rpki_client_repository_objects{type="cert",state="non-functional",name="ripe",carepo="rsync://rsync.paas.rpki.ripe.net/repository"} 20
	rpki_client_repository_objects{type="cert",state="non-functional",name="ripe",carepo="rsync://rpki-rps.arin.net/repository"} 4
	rpki_client_repository_objects{type="cert",state="non-functional",name="ripe",carepo="rsync://rpki.ripe.net/repository"} 46

Openmetrics while attributing to child (old behavior):

	rpki_client_repository_objects{type="cert",state="non-functional",name="apnic",carepo="rsync://alias.rpki-measurement.site/repo"} 1
	rpki_client_repository_objects{type="cert",state="non-functional",name="apnic",carepo="rsync://rsync.rp.ki/repo"} 1
	rpki_client_repository_objects{type="cert",state="non-functional",name="arin",carepo="rsync://rpki-repo.canops.org/repo"} 1
	rpki_client_repository_objects{type="cert",state="non-functional",name="arin",carepo="rsync://krill.ca-bc-01.ssmidge.xyz/repo"} 2
	rpki_client_repository_objects{type="cert",state="non-functional",name="arin",carepo="rsync://rpki.leitecastro.com/repo"} 1
	rpki_client_repository_objects{type="cert",state="non-functional",name="arin",carepo="rsync://rpki.sunoaki.net/repo"} 3
	rpki_client_repository_objects{type="cert",state="non-functional",name="arin",carepo="rsync://repo.kagl.me/rpki"} 1
	rpki_client_repository_objects{type="cert",state="non-functional",name="arin",carepo="rsync://rpki-repo.registro.br/repo"} 1
	rpki_client_repository_objects{type="cert",state="non-functional",name="arin",carepo="rsync://rsync.rp.ki/repo"} 2
	rpki_client_repository_objects{type="cert",state="non-functional",name="arin",carepo="rsync://rpki-rps.arin.net/repository"} 9
	rpki_client_repository_objects{type="cert",state="non-functional",name="arin",carepo="rsync://rsync.paas.rpki.ripe.net/repository"} 1
	rpki_client_repository_objects{type="cert",state="non-functional",name="lacnic",carepo="rsync://rpki-repo.registro.br/repo"} 46
	rpki_client_repository_objects{type="cert",state="non-functional",name="ripe",carepo="rsync://krill.ca-bc-01.ssmidge.xyz/repo"} 1
	rpki_client_repository_objects{type="cert",state="non-functional",name="ripe",carepo="rsync://rpki.qs.nu/repo"} 1
	rpki_client_repository_objects{type="cert",state="non-functional",name="ripe",carepo="rsync://rki.plasmanodes.com/repo"} 1
	rpki_client_repository_objects{type="cert",state="non-functional",name="ripe",carepo="rsync://rpki.leitecastro.com/repo"} 1
	rpki_client_repository_objects{type="cert",state="non-functional",name="ripe",carepo="rsync://oto.wakuwaku.ne.jp/pki"} 1
	rpki_client_repository_objects{type="cert",state="non-functional",name="ripe",carepo="rsync://krill.ipgua.com/repo"} 4
	rpki_client_repository_objects{type="cert",state="non-functional",name="ripe",carepo="rsync://pub.krill.ausra.cloud/repo"} 1
	rpki_client_repository_objects{type="cert",state="non-functional",name="ripe",carepo="rsync://repo.kagl.me/rpki"} 1
	rpki_client_repository_objects{type="cert",state="non-functional",name="ripe",carepo="rsync://rpki.axivora.net/repo"} 4
	rpki_client_repository_objects{type="cert",state="non-functional",name="ripe",carepo="rsync://rpki.folf.systems/repo"} 1
	rpki_client_repository_objects{type="cert",state="non-functional",name="ripe",carepo="rsync://rpki.zappiehost.com/repo"} 1
	rpki_client_repository_objects{type="cert",state="non-functional",name="ripe",carepo="rsync://krill.signalx.cloud/repo"} 3
	rpki_client_repository_objects{type="cert",state="non-functional",name="ripe",carepo="rsync://rsync.rp.ki/repo"} 2
	rpki_client_repository_objects{type="cert",state="non-functional",name="ripe",carepo="rsync://rpki-rps.arin.net/repository"} 2
	rpki_client_repository_objects{type="cert",state="non-functional",name="ripe",carepo="rsync://rsync.paas.rpki.ripe.net/repository"} 53

OK?

Index: cert.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v
diff -u -p -r1.239 cert.c
--- cert.c	15 Jun 2026 17:30:04 -0000	1.239
+++ cert.c	21 Jun 2026 21:21:26 -0000
@@ -2113,7 +2113,7 @@ RB_GENERATE(brk_tree, brk, entry, brkcmp
  * Add each CA cert into the non-functional CA tree.
  */
 void
-cert_insert_nca(struct nca_tree *tree, const struct cert *cert, struct repo *rp)
+cert_insert_nca(struct nca_tree *tree, const struct cert *cert)
 {
 	struct nonfunc_ca *nca;
 
@@ -2127,22 +2127,21 @@ cert_insert_nca(struct nca_tree *tree, c
 		err(1, NULL);
 	if ((nca->ski = strdup(cert->ski)) == NULL)
 		err(1, NULL);
+	nca->repoid = cert->repoid;
 	nca->certid = cert->certid;
 	nca->talid = cert->talid;
 
 	if (RB_INSERT(nca_tree, tree, nca) != NULL)
 		errx(1, "non-functional CA tree corrupted");
-	repo_stat_inc(rp, nca->talid, RTYPE_CER, STYPE_NONFUNC);
 }
 
 void
-cert_remove_nca(struct nca_tree *tree, int cid, struct repo *rp)
+cert_remove_nca(struct nca_tree *tree, int cid)
 {
 	struct nonfunc_ca *found, needle = { .certid = cid };
 
 	if ((found = RB_FIND(nca_tree, tree, &needle)) != NULL) {
 		RB_REMOVE(nca_tree, tree, found);
-		repo_stat_inc(rp, found->talid, RTYPE_CER, STYPE_FUNC);
 		free(found->location);
 		free(found->carepo);
 		free(found->mfturi);
Index: extern.h
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v
diff -u -p -r1.282 extern.h
--- extern.h	15 Jun 2026 14:30:53 -0000	1.282
+++ extern.h	21 Jun 2026 21:21:26 -0000
@@ -159,6 +159,7 @@ struct nonfunc_ca {
 	char			*mfturi;
 	char			*ski;
 	int			 certid;
+	unsigned int		 repoid;
 	int			 talid;
 };
 
@@ -725,9 +726,8 @@ struct cert	*ta_validate(const char *, s
 		    size_t);
 struct cert	*cert_read(struct ibuf *);
 void		 cert_insert_brks(struct brk_tree *, struct cert *);
-void		 cert_insert_nca(struct nca_tree *, const struct cert *,
-		    struct repo *);
-void		 cert_remove_nca(struct nca_tree *, int, struct repo *);
+void		 cert_insert_nca(struct nca_tree *, const struct cert *);
+void		 cert_remove_nca(struct nca_tree *, int);
 
 enum rtype	 rtype_from_file_extension(const char *);
 void		 mft_buffer(struct ibuf *, const struct mft *);
@@ -895,6 +895,7 @@ void		 repo_cleanup(struct filepath_tree
 int		 repo_check_timeout(int);
 void		 repostats_new_files_inc(struct repo *, const char *);
 void		 repo_stat_inc(struct repo *, int, enum rtype, enum stype);
+void		 repo_stat_inc_nca(struct nonfunc_ca *);
 void		 repo_tal_stats_collect(void (*)(const struct repo *,
 		    const struct repotalstats *, void *), int, void *);
 void		 repo_stats_collect(void (*)(const struct repo *,
Index: main.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/main.c,v
diff -u -p -r1.306 main.c
--- main.c	9 May 2026 01:22:32 -0000	1.306
+++ main.c	21 Jun 2026 21:21:26 -0000
@@ -575,7 +575,8 @@ queue_add_from_cert(const struct cert *c
 			err(1, NULL);
 	}
 
-	cert_insert_nca(ncas, cert, repo);
+	cert_insert_nca(ncas, cert);
+
 	entityq_add(npath, nfile, RTYPE_MFT, DIR_UNKNOWN, repo, NULL, 0,
 	    cert->talid, cert->certid, NULL);
 }
@@ -675,7 +676,7 @@ entity_process(struct ibuf *b, struct va
 		if (mft->seqnum_gap)
 			repo_stat_inc(rp, talid, type, STYPE_SEQNUM_GAP);
 		queue_add_from_mft(mft);
-		cert_remove_nca(&vd->ncas, mft->certid, rp);
+		cert_remove_nca(&vd->ncas, mft->certid);
 		ccr_insert_mft(&vd->ccr.mfts, mft);
 		mft_free(mft);
 		break;
@@ -1030,6 +1031,7 @@ main(int argc, char *argv[])
 	struct rusage	 ru;
 	struct timespec	 start_time, now_time;
 	struct validation_data vd = { 0 };
+	struct nonfunc_ca	*nca;
 
 	clock_gettime(CLOCK_MONOTONIC, &start_time);
 
@@ -1538,6 +1540,12 @@ main(int argc, char *argv[])
 	/* if processing in filemode the process is done, no cleanup */
 	if (filemode)
 		return rc;
+
+	/*
+	 * Account non-functional CAs statistics.
+	 */
+	RB_FOREACH(nca, nca_tree, &vd.ncas)
+		repo_stat_inc_nca(nca);
 
 	logx("all files parsed: generating output");
 
Index: repo.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/repo.c,v
diff -u -p -r1.84 repo.c
--- repo.c	15 Jun 2026 14:45:19 -0000	1.84
+++ repo.c	21 Jun 2026 21:21:26 -0000
@@ -1541,6 +1541,19 @@ repostats_new_files_inc(struct repo *rp,
 		rp->repostats.new_files++;
 }
 
+void
+repo_stat_inc_nca(struct nonfunc_ca *nca)
+{
+	struct repo *rp;
+
+	SLIST_FOREACH(rp, &repos, entry) {
+		if (rp->id == nca->repoid) {
+			rp->stats[nca->talid].certs_nonfunc++;
+			break;
+		}
+	}
+}
+
 /*
  * Update stats object of repository depending on rtype and subtype.
  */
@@ -1556,10 +1569,6 @@ repo_stat_inc(struct repo *rp, int talid
 			rp->stats[talid].certs++;
 		if (subtype == STYPE_FAIL)
 			rp->stats[talid].certs_fail++;
-		if (subtype == STYPE_NONFUNC)
-			rp->stats[talid].certs_nonfunc++;
-		if (subtype == STYPE_FUNC)
-			rp->stats[talid].certs_nonfunc--;
 		if (subtype == STYPE_BGPSEC) {
 			rp->stats[talid].certs--;
 			rp->stats[talid].brks++;