Download raw body.
rpki-client: collect non-functional CAs
On Wed, Mar 26, 2025 at 11:03:44AM +0100, Claudio Jeker wrote:
[...]
> > The other annoying bit is to get the path of the cert without .rsync/
> > and .rrdp/*/ artifacts prepended to it. While this can be obtained by
> > chopping up the file in entity_process(), I think it's cleaner to
> > construct the DIR_VALID path and pass that over the pipe.
>
> I dislike this part, we start to ship around paths back and forth more and
> more and there is a fair amount of data duplication because of that.
I agree completely.
> Also
> the path is not the best identifier (apart from using it for logging).
It is uniquely used for logging. Job rightly pointed out that we should
not leak the internal .rsync and .rrdp/*/ components into the json. I
dislike parsing and modifying something under our own control with strncmp
and strcspn and friends even more than the wasteful passing over the pipe.
> This is not something to be fixed now but at some point we probably need
> to rethink and rewrite some codepaths.
Agreed.
> > The third annoying bit is the number of trees we need to pass to the
> > output functions. We should probably hang all the trees off a single
> > struct so we can avoid this churn when we add the next tree.
>
> Agreed. The context needed starts to be come rather big and so having a
> single state struct starts to make sense here.
>
> I also would like to switch from RB_GENERATE() to RB_GENERATE_STATIC() and
> hide the RB tree internals from all other modules but then the
> RB_FOREACH() used in output*.c need to be wrapped somehow.
I'll ponder it.
> > + json_do_array("nonfunc_cas");
> > + RB_FOREACH(nca, nca_tree, ncas) {
> > + json_do_object("nca", 1);
> > + json_do_string("location", nca->location);
> > + json_do_string("ta", taldescs[nca->talid]);
> > + json_do_string("caRepository", nca->carepo);
> > + json_do_string("rpkiManifest", nca->mfturi);
> > + json_do_string("ski", nca->ski);
> > + json_do_end();
> > + };
>
> Please remove the ; here.
dropped
> > + cert->path = parse_filepath(entp->repoid, entp->path, entp->file,
> > + DIR_VALID);
> > + if (cert->path == NULL) {
> > + warnx("%s: failed to create file path", file);
> > + goto out;
> > + }
> > +
>
> I think this block needs to be moved up before the auth_insert() call.
> Else you may end up with a broken auth tree if the file path creation
> fails.
>
> I think this should be right after
> cert->talid = a->cert->talid;
> since this is where we initalize the cert.
Ouch. Thanks for catching that. I'm not sure it's actually reachable,
but it should be fixed in any case. I moved it up to where you suggested
> > return cert;
> >
> > out:
> > @@ -677,6 +684,9 @@ proc_parser_root_cert(struct entity *ent
> > }
> >
> > if ((cmp = proc_parser_ta_cmp(cert1, cert2)) > 0) {
> > + if ((cert1->path = strdup(file2)) == NULL)
> > + err(1, NULL);
> > +
> > cert_free(cert2);
> > free(file2);
> >
> > @@ -694,6 +704,8 @@ proc_parser_root_cert(struct entity *ent
> > if (cert2 != NULL) {
> > cert2->talid = entp->talid;
> > auth_insert(file2, &auths, cert2, NULL);
> > + if ((cert2->path = strdup(file2)) == NULL)
> > + err(1, NULL);
>
> Again I would move this before auth_insert() even though it does not
> matter here.
Also moved.
>
> > }
> >
> > *out_cert = cert2;
> >
>
> With those fixes OK claudio@
I think this diff addresses all the comments. I am going to commit
this tomorrow unless I hear an objection.
Index: cert.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v
diff -u -p -r1.156 cert.c
--- cert.c 21 Mar 2025 18:44:15 -0000 1.156
+++ cert.c 26 Mar 2025 10:20:46 -0000
@@ -1149,6 +1149,7 @@ cert_free(struct cert *p)
free(p->crl);
free(p->repo);
+ free(p->path);
free(p->mft);
free(p->notify);
free(p->ips);
@@ -1179,6 +1180,7 @@ cert_buffer(struct ibuf *b, const struct
io_simple_buffer(b, p->ips, p->num_ips * sizeof(p->ips[0]));
io_simple_buffer(b, p->ases, p->num_ases * sizeof(p->ases[0]));
+ io_str_buffer(b, p->path);
io_str_buffer(b, p->mft);
io_str_buffer(b, p->notify);
io_str_buffer(b, p->repo);
@@ -1222,6 +1224,7 @@ cert_read(struct ibuf *b)
io_read_buf(b, p->ases, p->num_ases * sizeof(p->ases[0]));
}
+ io_read_str(b, &p->path);
io_read_str(b, &p->mft);
io_read_str(b, &p->notify);
io_read_str(b, &p->repo);
@@ -1387,3 +1390,55 @@ brkcmp(struct brk *a, struct brk *b)
}
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 nonfunc_ca *nca;
+
+ if ((nca = calloc(1, sizeof(*nca))) == NULL)
+ err(1, NULL);
+ if ((nca->location = strdup(cert->path)) == NULL)
+ err(1, NULL);
+ if ((nca->carepo = strdup(cert->repo)) == NULL)
+ err(1, NULL);
+ if ((nca->mfturi = strdup(cert->mft)) == NULL)
+ err(1, NULL);
+ if ((nca->ski = strdup(cert->ski)) == NULL)
+ err(1, NULL);
+ nca->certid = cert->certid;
+ nca->talid = cert->talid;
+
+ if (RB_INSERT(nca_tree, tree, nca) != NULL)
+ errx(1, "non-functional CA tree corrupted");
+}
+
+void
+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);
+ free(found->location);
+ free(found->carepo);
+ free(found->mfturi);
+ free(found->ski);
+ free(found);
+ }
+}
+
+static inline int
+ncacmp(const struct nonfunc_ca *a, const struct nonfunc_ca *b)
+{
+ if (a->certid < b->certid)
+ return -1;
+ if (a->certid > b->certid)
+ return 1;
+ return 0;
+}
+
+RB_GENERATE(nca_tree, nonfunc_ca, entry, ncacmp);
Index: extern.h
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v
diff -u -p -r1.237 extern.h
--- extern.h 25 Feb 2025 15:55:26 -0000 1.237
+++ extern.h 11 Mar 2025 18:36:39 -0000
@@ -129,6 +129,7 @@ struct cert {
int talid; /* cert is covered by which TAL */
int certid;
unsigned int repoid; /* repository of this cert file */
+ char *path; /* filename without .rrdp and .rsync prefix */
char *repo; /* CA repository (rsync:// uri) */
char *mft; /* manifest (rsync:// uri) */
char *notify; /* RRDP notify (https:// uri) */
@@ -145,6 +146,27 @@ struct cert {
};
/*
+ * Non-functional CA tree element.
+ * Initially all CA and TA certs are added to this tree.
+ * They are removed once they are the issuer of a valid mft.
+ */
+struct nonfunc_ca {
+ RB_ENTRY(nonfunc_ca) entry;
+ char *location;
+ char *carepo;
+ char *mfturi;
+ char *ski;
+ int certid;
+ int talid;
+};
+
+/*
+ * Tree of nonfunc CAs, sorted by certid.
+ */
+RB_HEAD(nca_tree, nonfunc_ca);
+RB_PROTOTYPE(nca_tree, nonfunc_ca, entry, ncacmp);
+
+/*
* The TAL file conforms to RFC 7730.
* It is the top-level structure of RPKI and defines where we can find
* certificates for TAs (trust anchors).
@@ -687,6 +709,8 @@ struct cert *ta_parse(const char *, stru
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 *);
+void cert_remove_nca(struct nca_tree *, int);
enum rtype rtype_from_file_extension(const char *);
void mft_buffer(struct ibuf *, const struct mft *);
@@ -966,18 +990,24 @@ extern int outformats;
#define FORMAT_OMETRIC 0x10
int outputfiles(struct vrp_tree *v, struct brk_tree *b,
- struct vap_tree *, struct vsp_tree *, struct stats *);
+ struct vap_tree *, struct vsp_tree *, struct nca_tree *,
+ struct stats *);
int outputheader(FILE *, struct stats *);
int output_bgpd(FILE *, struct vrp_tree *, struct brk_tree *,
- struct vap_tree *, struct vsp_tree *, struct stats *);
+ struct vap_tree *, struct vsp_tree *, struct nca_tree *,
+ struct stats *);
int output_bird(FILE *, struct vrp_tree *, struct brk_tree *,
- struct vap_tree *, struct vsp_tree *, struct stats *);
+ struct vap_tree *, struct vsp_tree *, struct nca_tree *,
+ struct stats *);
int output_csv(FILE *, struct vrp_tree *, struct brk_tree *,
- struct vap_tree *, struct vsp_tree *, struct stats *);
+ struct vap_tree *, struct vsp_tree *, struct nca_tree *,
+ struct stats *);
int output_json(FILE *, struct vrp_tree *, struct brk_tree *,
- struct vap_tree *, struct vsp_tree *, struct stats *);
+ struct vap_tree *, struct vsp_tree *, struct nca_tree *,
+ struct stats *);
int output_ometric(FILE *, struct vrp_tree *, struct brk_tree *,
- struct vap_tree *, struct vsp_tree *, struct stats *);
+ struct vap_tree *, struct vsp_tree *, struct nca_tree *,
+ struct stats *);
void logx(const char *fmt, ...)
__attribute__((format(printf, 1, 2)));
Index: main.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/main.c,v
diff -u -p -r1.279 main.c
--- main.c 27 Feb 2025 14:23:02 -0000 1.279
+++ main.c 11 Mar 2025 18:36:39 -0000
@@ -488,7 +488,7 @@ queue_add_from_tal(struct tal *tal)
* Add a manifest (MFT) found in an X509 certificate, RFC 6487.
*/
static void
-queue_add_from_cert(const struct cert *cert)
+queue_add_from_cert(const struct cert *cert, struct nca_tree *ncas)
{
struct repo *repo;
struct fqdnlistentry *le;
@@ -549,6 +549,7 @@ queue_add_from_cert(const struct cert *c
err(1, NULL);
}
+ cert_insert_nca(ncas, cert);
entityq_add(npath, nfile, RTYPE_MFT, DIR_UNKNOWN, repo, NULL, 0,
cert->talid, cert->certid, NULL);
}
@@ -562,7 +563,7 @@ queue_add_from_cert(const struct cert *c
static void
entity_process(struct ibuf *b, struct stats *st, struct vrp_tree *tree,
struct brk_tree *brktree, struct vap_tree *vaptree,
- struct vsp_tree *vsptree)
+ struct vsp_tree *vsptree, struct nca_tree *ncatree)
{
enum rtype type;
struct tal *tal;
@@ -619,7 +620,7 @@ entity_process(struct ibuf *b, struct st
switch (cert->purpose) {
case CERT_PURPOSE_TA:
case CERT_PURPOSE_CA:
- queue_add_from_cert(cert);
+ queue_add_from_cert(cert, ncatree);
break;
case CERT_PURPOSE_BGPSEC_ROUTER:
cert_insert_brks(brktree, cert);
@@ -641,6 +642,7 @@ entity_process(struct ibuf *b, struct st
if (mft->seqnum_gap)
repo_stat_inc(rp, talid, type, STYPE_SEQNUM_GAP);
queue_add_from_mft(mft);
+ cert_remove_nca(ncatree, mft->certid);
mft_free(mft);
break;
case RTYPE_CRL:
@@ -987,6 +989,7 @@ main(int argc, char *argv[])
struct vsp_tree vsps = RB_INITIALIZER(&vsps);
struct brk_tree brks = RB_INITIALIZER(&brks);
struct vap_tree vaps = RB_INITIALIZER(&vaps);
+ struct nca_tree ncas = RB_INITIALIZER(&ncas);
struct rusage ru;
struct timespec start_time, now_time;
@@ -1403,7 +1406,7 @@ main(int argc, char *argv[])
}
while ((b = io_buf_get(queues[0])) != NULL) {
entity_process(b, &stats, &vrps, &brks, &vaps,
- &vsps);
+ &vsps, &ncas);
ibuf_free(b);
}
}
@@ -1496,7 +1499,7 @@ main(int argc, char *argv[])
}
repo_stats_collect(sum_repostats, &stats.repo_stats);
- if (outputfiles(&vrps, &brks, &vaps, &vsps, &stats))
+ if (outputfiles(&vrps, &brks, &vaps, &vsps, &ncas, &stats))
rc = 1;
printf("Processing time %lld seconds "
Index: output-bgpd.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/output-bgpd.c,v
diff -u -p -r1.32 output-bgpd.c
--- output-bgpd.c 13 Nov 2024 12:51:04 -0000 1.32
+++ output-bgpd.c 11 Mar 2025 18:36:39 -0000
@@ -21,7 +21,8 @@
int
output_bgpd(FILE *out, struct vrp_tree *vrps, struct brk_tree *brks,
- struct vap_tree *vaps, struct vsp_tree *vsps, struct stats *st)
+ struct vap_tree *vaps, struct vsp_tree *vsps, struct nca_tree *ncas,
+ struct stats *st)
{
struct vrp *vrp;
struct vap *vap;
Index: output-bird.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/output-bird.c,v
diff -u -p -r1.22 output-bird.c
--- output-bird.c 3 Jan 2025 10:32:21 -0000 1.22
+++ output-bird.c 11 Mar 2025 18:36:39 -0000
@@ -22,7 +22,8 @@
int
output_bird(FILE *out, struct vrp_tree *vrps, struct brk_tree *brks,
- struct vap_tree *vaps, struct vsp_tree *vsps, struct stats *st)
+ struct vap_tree *vaps, struct vsp_tree *vsps, struct nca_tree *ncas,
+ struct stats *st)
{
struct vrp *v;
struct vap *vap;
Index: output-csv.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/output-csv.c,v
diff -u -p -r1.14 output-csv.c
--- output-csv.c 22 Feb 2024 12:49:42 -0000 1.14
+++ output-csv.c 11 Mar 2025 18:36:39 -0000
@@ -21,7 +21,8 @@
int
output_csv(FILE *out, struct vrp_tree *vrps, struct brk_tree *brks,
- struct vap_tree *vaps, struct vsp_tree *vsps, struct stats *st)
+ struct vap_tree *vaps, struct vsp_tree *vsps, struct nca_tree *ncas,
+ struct stats *st)
{
struct vrp *v;
Index: output-json.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/output-json.c,v
diff -u -p -r1.51 output-json.c
--- output-json.c 13 Nov 2024 12:51:04 -0000 1.51
+++ output-json.c 26 Mar 2025 10:13:21 -0000
@@ -145,11 +145,13 @@ output_spl(struct vsp_tree *vsps)
int
output_json(FILE *out, struct vrp_tree *vrps, struct brk_tree *brks,
- struct vap_tree *vaps, struct vsp_tree *vsps, struct stats *st)
+ struct vap_tree *vaps, struct vsp_tree *vsps, struct nca_tree *ncas,
+ struct stats *st)
{
- char buf[64];
- struct vrp *v;
- struct brk *b;
+ char buf[64];
+ struct vrp *v;
+ struct brk *b;
+ struct nonfunc_ca *nca;
json_do_start(out);
outputheader_json(st);
@@ -176,6 +178,18 @@ output_json(FILE *out, struct vrp_tree *
json_do_string("pubkey", b->pubkey);
json_do_string("ta", taldescs[b->talid]);
json_do_int("expires", b->expires);
+ json_do_end();
+ }
+ json_do_end();
+
+ json_do_array("nonfunc_cas");
+ RB_FOREACH(nca, nca_tree, ncas) {
+ json_do_object("nca", 1);
+ json_do_string("location", nca->location);
+ json_do_string("ta", taldescs[nca->talid]);
+ json_do_string("caRepository", nca->carepo);
+ json_do_string("rpkiManifest", nca->mfturi);
+ json_do_string("ski", nca->ski);
json_do_end();
}
json_do_end();
Index: output-ometric.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/output-ometric.c,v
diff -u -p -r1.12 output-ometric.c
--- output-ometric.c 2 Nov 2024 12:30:28 -0000 1.12
+++ output-ometric.c 11 Mar 2025 18:36:39 -0000
@@ -166,7 +166,8 @@ repo_stats(const struct repo *rp, const
int
output_ometric(FILE *out, struct vrp_tree *vrps, struct brk_tree *brks,
- struct vap_tree *vaps, struct vsp_tree *vsps, struct stats *st)
+ struct vap_tree *vaps, struct vsp_tree *vsps, struct nca_tree *ncas,
+ struct stats *st)
{
struct olabels *ol;
const char *keys[4] = { "nodename", "domainname", "release", NULL };
Index: output.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/output.c,v
diff -u -p -r1.38 output.c
--- output.c 3 Jan 2025 10:14:32 -0000 1.38
+++ output.c 11 Mar 2025 18:36:39 -0000
@@ -64,7 +64,8 @@ static const struct outputs {
int format;
char *name;
int (*fn)(FILE *, struct vrp_tree *, struct brk_tree *,
- struct vap_tree *, struct vsp_tree *, struct stats *);
+ struct vap_tree *, struct vsp_tree *, struct nca_tree *,
+ struct stats *);
} outputs[] = {
{ FORMAT_OPENBGPD, "openbgpd", output_bgpd },
{ FORMAT_BIRD, "bird", output_bird },
@@ -124,7 +125,7 @@ prune_as0_tals(struct vrp_tree *vrps)
int
outputfiles(struct vrp_tree *v, struct brk_tree *b, struct vap_tree *a,
- struct vsp_tree *p, struct stats *st)
+ struct vsp_tree *p, struct nca_tree *ncas, struct stats *st)
{
int i, rc = 0;
@@ -146,7 +147,7 @@ outputfiles(struct vrp_tree *v, struct b
rc = 1;
continue;
}
- if ((*outputs[i].fn)(fout, v, b, a, p, st) != 0) {
+ if ((*outputs[i].fn)(fout, v, b, a, p, ncas, st) != 0) {
warn("output for %s format failed", outputs[i].name);
fclose(fout);
output_cleantmp();
Index: parser.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/parser.c,v
diff -u -p -r1.150 parser.c
--- parser.c 12 Mar 2025 07:42:39 -0000 1.150
+++ parser.c 26 Mar 2025 10:20:42 -0000
@@ -585,6 +585,13 @@ proc_parser_cert(char *file, const unsig
cert->talid = a->cert->talid;
+ cert->path = parse_filepath(entp->repoid, entp->path, entp->file,
+ DIR_VALID);
+ if (cert->path == NULL) {
+ warnx("%s: failed to create file path", file);
+ goto out;
+ }
+
if (cert->purpose == CERT_PURPOSE_BGPSEC_ROUTER) {
if (!constraints_validate(file, cert))
goto out;
@@ -677,6 +684,9 @@ proc_parser_root_cert(struct entity *ent
}
if ((cmp = proc_parser_ta_cmp(cert1, cert2)) > 0) {
+ if ((cert1->path = strdup(file2)) == NULL)
+ err(1, NULL);
+
cert_free(cert2);
free(file2);
@@ -693,6 +703,8 @@ proc_parser_root_cert(struct entity *ent
if (cert2 != NULL) {
cert2->talid = entp->talid;
+ if ((cert2->path = strdup(file2)) == NULL)
+ err(1, NULL);
auth_insert(file2, &auths, cert2, NULL);
}
rpki-client: collect non-functional CAs