Download raw body.
rework rpki-client certificate discovery
On Wed, May 15, 2024 at 05:04:57PM +0200, Claudio Jeker wrote:
> In rpki-client we use the SKI / AKI to find the signing certificate and
> build the validation stack based on this data. The problem is that the SKI
> is not strictly unique and for example the NRO TAL abuses this.
>
> Instead of indexing certs by SKI we now use a unique per certificate id
> which is then passed back and forth between main process and the parser.
> This way we never lose track on the cert that signed the current object
> and can look that one up via its id. This allows for multiple validation
> paths to co-exist.
I never liked the auth lookup by SKI and I am really happy to see it go.
I've long suspected that we throw away important information and the new
approach shows that this is indeed the case. I am much happier with this.
The comparison of the aki with the issuer's ski ensures that we don't
throw away information - this is redundant with check_issuer()'s call to
X509_check_akid() in the verifier anyway.
One other thing worth mentioning is that by keeping track of the cert
depth in auth_insert() we reinstate the maximum depth check that was
lost when doing the partial chain optimization.
> Now on top of this the filepath_add() api had to be extended since we now
> allow a file to be visited more then once as long as it is reached by a
> different TAL. Inside the same TAL an object can not be revisited.
>
> Last but not least the filemode had to be adjusted, now the code keeps
> track of the various AIA uri and uses an own rb tree to look them up.
> Once there is a hit the stack can be unrolled like before and all new
> certs are added to both the auth and the uripath trees.
I think this is also an improvement.
> More cleanup needs to follow (I'm looking at you valid_aki_ski())
>
> This was developped together with tb@ at a femto-hackathon.
A nit and a bug in the beloved valid_ski_aki() below, with those fixed
ok tb
But I'd be happy to see some more testing.
> --
> :wq Claudio
>
> Index: cert.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v
> diff -u -p -r1.130 cert.c
> --- cert.c 21 Apr 2024 19:27:44 -0000 1.130
> +++ cert.c 15 May 2024 13:43:14 -0000
> @@ -34,6 +34,8 @@ extern ASN1_OBJECT *carepo_oid; /* 1.3.6
> extern ASN1_OBJECT *manifest_oid; /* 1.3.6.1.5.5.7.48.10 (rpkiManifest) */
> extern ASN1_OBJECT *notify_oid; /* 1.3.6.1.5.5.7.48.13 (rpkiNotify) */
>
> +static int certid = TALSZ_MAX;
> +
> /*
> * Append an IP address structure to our list of results.
> * This will also constrain us to having at most one inheritance
> @@ -1103,6 +1105,7 @@ cert_buffer(struct ibuf *b, const struct
> io_simple_buffer(b, &p->notafter, sizeof(p->notafter));
> io_simple_buffer(b, &p->purpose, sizeof(p->purpose));
> io_simple_buffer(b, &p->talid, sizeof(p->talid));
> + io_simple_buffer(b, &p->certid, sizeof(p->certid));
> io_simple_buffer(b, &p->repoid, sizeof(p->repoid));
> io_simple_buffer(b, &p->ipsz, sizeof(p->ipsz));
> io_simple_buffer(b, &p->asz, sizeof(p->asz));
> @@ -1136,6 +1139,7 @@ cert_read(struct ibuf *b)
> io_read_buf(b, &p->notafter, sizeof(p->notafter));
> io_read_buf(b, &p->purpose, sizeof(p->purpose));
> io_read_buf(b, &p->talid, sizeof(p->talid));
> + io_read_buf(b, &p->certid, sizeof(p->certid));
> io_read_buf(b, &p->repoid, sizeof(p->repoid));
> io_read_buf(b, &p->ipsz, sizeof(p->ipsz));
> io_read_buf(b, &p->asz, sizeof(p->asz));
> @@ -1167,7 +1171,11 @@ cert_read(struct ibuf *b)
> static inline int
> authcmp(struct auth *a, struct auth *b)
> {
> - return strcmp(a->cert->ski, b->cert->ski);
> + if (a->cert->certid > b->cert->certid)
> + return 1;
> + if (a->cert->certid < b->cert->certid)
> + return -1;
> + return 0;
> }
>
> RB_GENERATE_STATIC(auth_tree, auth, entry, authcmp);
> @@ -1185,33 +1193,48 @@ auth_tree_free(struct auth_tree *auths)
> }
>
> struct auth *
> -auth_find(struct auth_tree *auths, const char *aki)
> +auth_find(struct auth_tree *auths, int id)
> {
> struct auth a;
> struct cert c;
>
> - /* we look up the cert where the ski == aki */
> - c.ski = (char *)aki;
> + c.certid = id;
> a.cert = &c;
>
> return RB_FIND(auth_tree, auths, &a);
> }
>
> struct auth *
> -auth_insert(struct auth_tree *auths, struct cert *cert, struct auth *issuer)
> +auth_insert(const char *fn, struct auth_tree *auths, struct cert *cert,
> + struct auth *issuer)
> {
> struct auth *na;
>
> - na = malloc(sizeof(*na));
> + na = calloc(1, sizeof(*na));
> if (na == NULL)
> err(1, NULL);
>
> + if (issuer == NULL)
> + cert->certid = cert->talid;
> + else {
> + cert->certid = ++certid;
> + if (certid > CERTID_MAX)
> + err(1, "%s: too many certificates", fn);
> + na->depth = issuer->depth + 1;
> + }
> +
> + if (na->depth >= MAX_CERT_DEPTH) {
> + warnx("%s: stack depth exhausted", fn);
> + free(na);
> + return NULL;
> + }
> +
> na->issuer = issuer;
> na->cert = cert;
> na->any_inherits = x509_any_inherits(cert->x509);
>
> if (RB_INSERT(auth_tree, auths, na) != NULL)
> - err(1, "auth tree corrupted");
> + errx(1, "auth tree corrupted");
>
> return na;
> }
> Index: extern.h
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v
> diff -u -p -r1.217 extern.h
> --- extern.h 21 Apr 2024 19:27:44 -0000 1.217
> +++ extern.h 15 May 2024 14:41:35 -0000
> @@ -24,6 +24,9 @@
> #include <openssl/x509.h>
> #include <openssl/x509v3.h>
>
> +#define CTASSERT(x) extern char _ctassert[(x) ? 1 : -1 ] \
> + __attribute__((__unused__))
> +
> enum cert_as_type {
> CERT_AS_ID, /* single identifier */
> CERT_AS_INHERIT, /* inherit from issuer */
> @@ -120,6 +123,7 @@ struct cert {
> struct cert_as *as; /* list of AS numbers and ranges */
> size_t asz; /* length of "asz" */
> int talid; /* cert is covered by which TAL */
> + int certid;
> unsigned int repoid; /* repository of this cert file */
> char *repo; /* CA repository (rsync:// uri) */
> char *mft; /* manifest (rsync:// uri) */
> @@ -212,6 +216,7 @@ struct mft {
> size_t filesz; /* number of filenames */
> unsigned int repoid;
> int talid;
> + int certid;
> };
>
> /*
> @@ -495,14 +500,16 @@ struct auth {
> struct cert *cert; /* owner information */
> struct auth *issuer; /* pointer to issuer or NULL for TA cert */
> int any_inherits;
> + int depth;
> };
> /*
> * Tree of auth sorted by ski
> */
> RB_HEAD(auth_tree, auth);
>
> -struct auth *auth_find(struct auth_tree *, const char *);
> -struct auth *auth_insert(struct auth_tree *, struct cert *, struct auth *);
> +struct auth *auth_find(struct auth_tree *, int);
> +struct auth *auth_insert(const char *, struct auth_tree *, struct cert *,
> + struct auth *);
>
> enum http_result {
> HTTP_FAILED, /* anything else */
> @@ -560,6 +567,7 @@ struct entity {
> size_t datasz; /* length of optional data blob */
> unsigned int repoid; /* repository identifier */
> int talid; /* tal identifier */
> + int certid;
> enum rtype type; /* type of entity (not RTYPE_EOF) */
> enum location location; /* which directory the file lives in */
> };
> @@ -731,10 +739,8 @@ void crl_tree_free(struct crl_tree *);
>
> /* Validation of our objects. */
>
> -struct auth *valid_ski_aki(const char *, struct auth_tree *,
> +struct auth *valid_ski_aki(const char *, struct auth_tree *, int,
> const char *, const char *, const char *);
> -int valid_ta(const char *, struct auth_tree *,
> - const struct cert *);
> int valid_cert(const char *, struct auth *, const struct cert *);
> int valid_roa(const char *, struct cert *, struct roa *);
> int valid_filehash(int, const char *, size_t);
> @@ -826,7 +832,7 @@ void proc_http(char *, int) __attribut
> void proc_rrdp(int) __attribute__((noreturn));
>
> /* Repository handling */
> -int filepath_add(struct filepath_tree *, char *, time_t);
> +int filepath_add(struct filepath_tree *, char *, int, time_t);
> void rrdp_clear(unsigned int);
> void rrdp_session_save(unsigned int, struct rrdp_session *);
> void rrdp_session_free(struct rrdp_session *);
> @@ -981,6 +987,7 @@ int mkpathat(int, const char *);
>
> /* Maximum number of TAL files we'll load. */
> #define TALSZ_MAX 8
> +#define CERTID_MAX 1000000
>
> /*
> * Maximum number of elements in the sbgp-ipAddrBlock (IP) and
> Index: filemode.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/filemode.c,v
> diff -u -p -r1.41 filemode.c
> --- filemode.c 21 Apr 2024 19:27:44 -0000 1.41
> +++ filemode.c 15 May 2024 14:42:41 -0000
> @@ -47,6 +47,50 @@ static struct crl_tree crlt = RB_INITIA
>
> struct tal *talobj[TALSZ_MAX];
>
> +struct uripath {
> + RB_ENTRY(uripath) entry;
> + const char *uri;
> + struct cert *cert;
> +};
> +
> +static RB_HEAD(uripath_tree, uripath) uritree;
> +
> +static inline int
> +uripathcmp(const struct uripath *a, const struct uripath *b)
> +{
> + return strcmp(a->uri, b->uri);
> +}
> +
> +RB_PROTOTYPE(uripath_tree, uripath, entry, uripathcmp);
> +
> +static void
> +uripath_add(const char *uri, struct cert *cert)
> +{
> + struct uripath *up;
> +
> + if ((up = calloc(1, sizeof(*up))) == NULL)
> + err(1, NULL);
> + if ((up->uri = strdup(uri)) == NULL)
> + err(1, NULL);
> + up->cert = cert;
> + if (RB_INSERT(uripath_tree, &uritree, up) != NULL)
> + errx(1, "corrupt AIA lookup tree");
> +}
> +
> +static struct cert *
> +uripath_lookup(const char *uri)
> +{
> + struct uripath needle = { .uri = uri };
> + struct uripath *up;
> +
> + up = RB_FIND(uripath_tree, &uritree, &needle);
> + if (up == NULL)
> + return NULL;
> + return up->cert;
> +}
> +
> +RB_GENERATE(uripath_tree, uripath, entry, uripathcmp);
> +
> /*
> * Use the X509 CRL Distribution Points to locate the CRL needed for
> * verification.
> @@ -132,7 +176,7 @@ parse_load_cert(char *uri)
> * tree. Once the TA is located in the chain the chain is validated in
> * reverse order.
> */
> -static void
> +static struct auth *
> parse_load_certchain(char *uri)
> {
> struct cert *stack[MAX_CERT_DEPTH] = { 0 };
> @@ -144,18 +188,16 @@ parse_load_certchain(char *uri)
> int i;
>
> for (i = 0; i < MAX_CERT_DEPTH; i++) {
> + if ((cert = uripath_lookup(uri)) != NULL) {
> + a = auth_find(&auths, cert->certid);
> + break;
> + }
> filestack[i] = uri;
> stack[i] = cert = parse_load_cert(uri);
> if (cert == NULL || cert->purpose != CERT_PURPOSE_CA) {
> - warnx("failed to build authority chain");
> - goto fail;
> - }
> - if (auth_find(&auths, cert->ski) != NULL) {
> - assert(i == 0);
> + warnx("failed to build authority chain: %s", uri);
> goto fail;
> }
> - if ((a = auth_find(&auths, cert->aki)) != NULL)
> - break; /* found chain to TA */
> uri = cert->aia;
> }
>
> @@ -166,9 +208,9 @@ parse_load_certchain(char *uri)
> }
>
> /* TA found play back the stack and add all certs */
> - for (; i >= 0; i--) {
> - cert = stack[i];
> - uri = filestack[i];
> + for (; i > 0; i--) {
> + cert = stack[i - 1];
> + uri = filestack[i - 1];
>
> crl = crl_get(&crlt, a);
> if (!valid_x509(uri, ctx, cert->x509, a, crl, &errstr) ||
> @@ -178,14 +220,16 @@ parse_load_certchain(char *uri)
> goto fail;
> }
> cert->talid = a->cert->talid;
> - a = auth_insert(&auths, cert, a);
> + a = auth_insert(uri, &auths, cert, a);
> + uripath_add(uri, cert);
> stack[i] = NULL;
> }
>
> - return;
> + return a;
> fail:
> for (i = 0; i < MAX_CERT_DEPTH; i++)
> cert_free(stack[i]);
> + return NULL;
> }
>
> static void
> @@ -195,7 +239,7 @@ parse_load_ta(struct tal *tal)
> struct cert *cert;
> unsigned char *f = NULL;
> char *file;
> - size_t flen;
> + size_t flen, i;
>
> /* does not matter which URI, all end with same filename */
> filename = strrchr(tal->uri[0], '/');
> @@ -217,11 +261,14 @@ parse_load_ta(struct tal *tal)
> goto out;
>
> cert->talid = tal->id;
> + auth_insert(file, &auths, cert, NULL);
> + for (i = 0; i < tal->urisz; i++) {
> + if (strncasecmp(tal->uri[i], RSYNC_PROTO, RSYNC_PROTO_LEN) != 0)
> + continue;
> + /* Add all rsync uri since any of them could be use as AIA. */
my typo: s/use/used
> + uripath_add(tal->uri[i], cert);
> + }
>
> - if (!valid_ta(file, &auths, cert))
> - cert_free(cert);
> - else
> - auth_insert(&auths, cert, NULL);
> out:
> free(file);
> free(f);
> @@ -297,7 +344,7 @@ proc_parser_file(char *file, unsigned ch
> struct spl *spl = NULL;
> struct tak *tak = NULL;
> struct tal *tal = NULL;
> - char *aia = NULL, *aki = NULL;
> + char *aia = NULL;
> char *crl_uri = NULL;
> time_t *expires = NULL, *notafter = NULL;
> struct auth *a;
> @@ -349,7 +396,6 @@ proc_parser_file(char *file, unsigned ch
> if (aspa == NULL)
> break;
> aia = aspa->aia;
> - aki = aspa->aki;
> expires = &aspa->expires;
> notafter = &aspa->notafter;
> break;
> @@ -363,7 +409,6 @@ proc_parser_file(char *file, unsigned ch
> if (cert == NULL)
> break;
> aia = cert->aia;
> - aki = cert->aki;
> x509 = cert->x509;
> if (X509_up_ref(x509) == 0)
> errx(1, "%s: X509_up_ref failed", __func__);
> @@ -381,7 +426,6 @@ proc_parser_file(char *file, unsigned ch
> if (mft == NULL)
> break;
> aia = mft->aia;
> - aki = mft->aki;
> expires = &mft->expires;
> notafter = &mft->nextupdate;
> break;
> @@ -390,7 +434,6 @@ proc_parser_file(char *file, unsigned ch
> if (gbr == NULL)
> break;
> aia = gbr->aia;
> - aki = gbr->aki;
> expires = &gbr->expires;
> notafter = &gbr->notafter;
> break;
> @@ -399,7 +442,6 @@ proc_parser_file(char *file, unsigned ch
> if (geofeed == NULL)
> break;
> aia = geofeed->aia;
> - aki = geofeed->aki;
> expires = &geofeed->expires;
> notafter = &geofeed->notafter;
> break;
> @@ -408,7 +450,6 @@ proc_parser_file(char *file, unsigned ch
> if (roa == NULL)
> break;
> aia = roa->aia;
> - aki = roa->aki;
> expires = &roa->expires;
> notafter = &roa->notafter;
> break;
> @@ -417,7 +458,6 @@ proc_parser_file(char *file, unsigned ch
> if (rsc == NULL)
> break;
> aia = rsc->aia;
> - aki = rsc->aki;
> expires = &rsc->expires;
> notafter = &rsc->notafter;
> break;
> @@ -426,7 +466,6 @@ proc_parser_file(char *file, unsigned ch
> if (spl == NULL)
> break;
> aia = spl->aia;
> - aki = spl->aki;
> expires = &spl->expires;
> notafter = &spl->notafter;
> break;
> @@ -435,7 +474,6 @@ proc_parser_file(char *file, unsigned ch
> if (tak == NULL)
> break;
> aia = tak->aia;
> - aki = tak->aki;
> expires = &tak->expires;
> notafter = &tak->notafter;
> break;
> @@ -453,9 +491,7 @@ proc_parser_file(char *file, unsigned ch
> if (aia != NULL) {
> x509_get_crl(x509, file, &crl_uri);
> parse_load_crl(crl_uri);
> - if (auth_find(&auths, aki) == NULL)
> - parse_load_certchain(aia);
> - a = auth_find(&auths, aki);
> + a = parse_load_certchain(aia);
> c = crl_get(&crlt, a);
>
> if ((status = valid_x509(file, ctx, x509, a, c, &errstr))) {
> Index: main.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/main.c,v
> diff -u -p -r1.257 main.c
> --- main.c 8 Apr 2024 14:02:13 -0000 1.257
> +++ main.c 14 May 2024 14:58:48 -0000
> @@ -156,6 +156,7 @@ entity_read_req(struct ibuf *b, struct e
> io_read_buf(b, &ent->location, sizeof(ent->location));
> io_read_buf(b, &ent->repoid, sizeof(ent->repoid));
> io_read_buf(b, &ent->talid, sizeof(ent->talid));
> + io_read_buf(b, &ent->certid, sizeof(ent->certid));
> io_read_str(b, &ent->path);
> io_read_str(b, &ent->file);
> io_read_str(b, &ent->mftaki);
> @@ -176,6 +177,7 @@ entity_write_req(const struct entity *en
> io_simple_buffer(b, &ent->location, sizeof(ent->location));
> io_simple_buffer(b, &ent->repoid, sizeof(ent->repoid));
> io_simple_buffer(b, &ent->talid, sizeof(ent->talid));
> + io_simple_buffer(b, &ent->certid, sizeof(ent->certid));
> io_str_buffer(b, ent->path);
> io_str_buffer(b, ent->file);
> io_str_buffer(b, ent->mftaki);
> @@ -191,7 +193,7 @@ entity_write_repo(const struct repo *rp)
> enum location loc = DIR_UNKNOWN;
> unsigned int repoid;
> char *path, *altpath;
> - int talid = 0;
> + int talid = 0, certid = 0;
>
> repoid = repo_id(rp);
> path = repo_basedir(rp, 0);
> @@ -201,6 +203,7 @@ entity_write_repo(const struct repo *rp)
> io_simple_buffer(b, &loc, sizeof(loc));
> io_simple_buffer(b, &repoid, sizeof(repoid));
> io_simple_buffer(b, &talid, sizeof(talid));
> + io_simple_buffer(b, &certid, sizeof(certid));
> io_str_buffer(b, path);
> io_str_buffer(b, altpath);
> io_buf_buffer(b, NULL, 0); /* ent->mftaki */
> @@ -233,7 +236,7 @@ entityq_flush(struct entityq *q, struct
> */
> static void
> entityq_add(char *path, char *file, enum rtype type, enum location loc,
> - struct repo *rp, unsigned char *data, size_t datasz, int talid,
> + struct repo *rp, unsigned char *data, size_t datasz, int talid, int certid,
> char *mftaki)
> {
> struct entity *p;
> @@ -244,6 +247,7 @@ entityq_add(char *path, char *file, enum
> p->type = type;
> p->location = loc;
> p->talid = talid;
> + p->certid = certid;
> p->mftaki = mftaki;
> p->path = path;
> if (rp != NULL)
> @@ -419,7 +423,7 @@ queue_add_from_mft(const struct mft *mft
> if ((mftaki = strdup(mft->aki)) == NULL)
> err(1, NULL);
> entityq_add(npath, nfile, f->type, f->location, rp, NULL, 0,
> - mft->talid, mftaki);
> + mft->talid, mft->certid, mftaki);
> }
> }
>
> @@ -442,7 +446,7 @@ queue_add_file(const char *file, enum rt
> if ((nfile = strdup(file)) == NULL)
> err(1, NULL);
> /* Not in a repository, so directly add to queue. */
> - entityq_add(NULL, nfile, type, DIR_UNKNOWN, NULL, buf, len, talid,
> + entityq_add(NULL, nfile, type, DIR_UNKNOWN, NULL, buf, len, talid, 0,
> NULL);
> }
>
> @@ -478,7 +482,7 @@ queue_add_from_tal(struct tal *tal)
> data = tal->pkey;
> tal->pkey = NULL;
> entityq_add(NULL, nfile, RTYPE_CER, DIR_VALID, repo, data,
> - tal->pkeysz, tal->id, NULL);
> + tal->pkeysz, tal->id, tal->id, NULL);
> }
>
> /*
> @@ -547,7 +551,7 @@ queue_add_from_cert(const struct cert *c
> }
>
> entityq_add(npath, nfile, RTYPE_MFT, DIR_UNKNOWN, repo, NULL, 0,
> - cert->talid, NULL);
> + cert->talid, cert->certid, NULL);
> }
>
> /*
> @@ -591,7 +595,7 @@ entity_process(struct ibuf *b, struct st
> if (filemode)
> goto done;
>
> - if (filepath_add(&fpt, file, mtime) == 0) {
> + if (filepath_add(&fpt, file, talid, mtime) == 0) {
> warnx("%s: File already visited", file);
> goto done;
> }
> Index: mft.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/mft.c,v
> diff -u -p -r1.113 mft.c
> --- mft.c 20 Apr 2024 15:45:41 -0000 1.113
> +++ mft.c 14 May 2024 13:47:40 -0000
> @@ -537,6 +537,7 @@ mft_buffer(struct ibuf *b, const struct
>
> io_simple_buffer(b, &p->repoid, sizeof(p->repoid));
> io_simple_buffer(b, &p->talid, sizeof(p->talid));
> + io_simple_buffer(b, &p->certid, sizeof(p->certid));
> io_str_buffer(b, p->path);
>
> io_str_buffer(b, p->aia);
> @@ -569,6 +570,7 @@ mft_read(struct ibuf *b)
>
> io_read_buf(b, &p->repoid, sizeof(p->repoid));
> io_read_buf(b, &p->talid, sizeof(p->talid));
> + io_read_buf(b, &p->certid, sizeof(p->certid));
> io_read_str(b, &p->path);
>
> io_read_str(b, &p->aia);
> Index: parser.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/parser.c,v
> diff -u -p -r1.135 parser.c
> --- parser.c 21 Apr 2024 19:27:44 -0000 1.135
> +++ parser.c 15 May 2024 10:23:45 -0000
> @@ -137,7 +137,8 @@ proc_parser_roa(char *file, const unsign
> if ((roa = roa_parse(&x509, file, entp->talid, der, len)) == NULL)
> return NULL;
>
> - a = valid_ski_aki(file, &auths, roa->ski, roa->aki, entp->mftaki);
> + a = valid_ski_aki(file, &auths, entp->certid, roa->ski, roa->aki,
> + entp->mftaki);
> crl = crl_get(&crlt, a);
>
> if (!valid_x509(file, ctx, x509, a, crl, &errstr)) {
> @@ -172,7 +173,8 @@ proc_parser_spl(char *file, const unsign
> if ((spl = spl_parse(&x509, file, entp->talid, der, len)) == NULL)
> return NULL;
>
> - a = valid_ski_aki(file, &auths, spl->ski, spl->aki, entp->mftaki);
> + a = valid_ski_aki(file, &auths, entp->certid, spl->ski, spl->aki,
> + entp->mftaki);
> crl = crl_get(&crlt, a);
>
> if (!valid_x509(file, ctx, x509, a, crl, &errstr)) {
> @@ -336,7 +338,7 @@ proc_parser_mft_pre(struct entity *entp,
> if (*crl == NULL)
> *crl = parse_load_crl_from_mft(entp, mft, DIR_VALID, crlfile);
>
> - a = valid_ski_aki(file, &auths, mft->ski, mft->aki, NULL);
> + a = valid_ski_aki(file, &auths, entp->certid, mft->ski, mft->aki, NULL);
> if (!valid_x509(file, ctx, x509, a, *crl, errstr))
> goto err;
> X509_free(x509);
> @@ -344,6 +346,7 @@ proc_parser_mft_pre(struct entity *entp,
>
> mft->repoid = entp->repoid;
> mft->talid = a->cert->talid;
> + mft->certid = entp->certid;
>
> now = get_current_time();
> /* check that now is not before from */
> @@ -493,7 +496,7 @@ proc_parser_mft(struct entity *entp, str
> */
> static struct cert *
> proc_parser_cert(char *file, const unsigned char *der, size_t len,
> - const char *mftaki)
> + const struct entity *entp)
> {
> struct cert *cert;
> struct crl *crl;
> @@ -507,7 +510,8 @@ proc_parser_cert(char *file, const unsig
> if (cert == NULL)
> return NULL;
>
> - a = valid_ski_aki(file, &auths, cert->ski, cert->aki, mftaki);
> + a = valid_ski_aki(file, &auths, entp->certid, cert->ski, cert->aki,
> + entp->mftaki);
> crl = crl_get(&crlt, a);
>
> if (!valid_x509(file, ctx, cert->x509, a, crl, &errstr) ||
> @@ -531,7 +535,7 @@ proc_parser_cert(char *file, const unsig
> * Add validated CA certs to the RPKI auth tree.
> */
> if (cert->purpose == CERT_PURPOSE_CA)
> - auth_insert(&auths, cert, a);
> + auth_insert(file, &auths, cert, a);
>
> return cert;
> }
> @@ -557,19 +561,12 @@ proc_parser_root_cert(char *file, const
> cert = ta_parse(file, cert, pkey, pkeysz);
> if (cert == NULL)
> return NULL;
> -
> - if (!valid_ta(file, &auths, cert)) {
> - warnx("%s: certificate not a valid ta", file);
> - cert_free(cert);
> - return NULL;
> - }
> -
> cert->talid = talid;
>
> /*
> * Add valid roots to the RPKI auth tree.
> */
> - auth_insert(&auths, cert, NULL);
> + auth_insert(file, &auths, cert, NULL);
>
> return cert;
> }
> @@ -590,7 +587,8 @@ proc_parser_gbr(char *file, const unsign
> if ((gbr = gbr_parse(&x509, file, entp->talid, der, len)) == NULL)
> return NULL;
>
> - a = valid_ski_aki(file, &auths, gbr->ski, gbr->aki, entp->mftaki);
> + a = valid_ski_aki(file, &auths, entp->certid, gbr->ski, gbr->aki,
> + entp->mftaki);
> crl = crl_get(&crlt, a);
>
> /* return value can be ignored since nothing happens here */
> @@ -623,7 +621,8 @@ proc_parser_aspa(char *file, const unsig
> if ((aspa = aspa_parse(&x509, file, entp->talid, der, len)) == NULL)
> return NULL;
>
> - a = valid_ski_aki(file, &auths, aspa->ski, aspa->aki, entp->mftaki);
> + a = valid_ski_aki(file, &auths, entp->certid, aspa->ski, aspa->aki,
> + entp->mftaki);
> crl = crl_get(&crlt, a);
>
> if (!valid_x509(file, ctx, x509, a, crl, &errstr)) {
> @@ -658,7 +657,8 @@ proc_parser_tak(char *file, const unsign
> if ((tak = tak_parse(&x509, file, entp->talid, der, len)) == NULL)
> return NULL;
>
> - a = valid_ski_aki(file, &auths, tak->ski, tak->aki, entp->mftaki);
> + a = valid_ski_aki(file, &auths, entp->certid, tak->ski, tak->aki,
> + entp->mftaki);
> crl = crl_get(&crlt, a);
>
> if (!valid_x509(file, ctx, x509, a, crl, &errstr)) {
> @@ -764,8 +764,7 @@ parse_entity(struct entityq *q, struct m
> f, flen, entp->data, entp->datasz,
> entp->talid);
> else
> - cert = proc_parser_cert(file, f, flen,
> - entp->mftaki);
> + cert = proc_parser_cert(file, f, flen, entp);
> if (cert != NULL)
> mtime = cert->notbefore;
> io_simple_buffer(b, &mtime, sizeof(mtime));
> Index: repo.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/repo.c,v
> diff -u -p -r1.57 repo.c
> --- repo.c 21 Apr 2024 19:27:44 -0000 1.57
> +++ repo.c 15 May 2024 14:41:59 -0000
> @@ -116,13 +116,14 @@ static void remove_contents(char *);
> * Database of all file path accessed during a run.
> */
> struct filepath {
> - RB_ENTRY(filepath) entry;
> + RB_ENTRY(filepath) entry;
> char *file;
> time_t mtime;
> + unsigned int talmask;
> };
>
> static inline int
> -filepathcmp(struct filepath *a, struct filepath *b)
> +filepathcmp(const struct filepath *a, const struct filepath *b)
> {
> return strcmp(a->file, b->file);
> }
> @@ -133,22 +134,28 @@ RB_PROTOTYPE(filepath_tree, filepath, en
> * Functions to lookup which files have been accessed during computation.
> */
> int
> -filepath_add(struct filepath_tree *tree, char *file, time_t mtime)
> +filepath_add(struct filepath_tree *tree, char *file, int id, time_t mtime)
> {
> - struct filepath *fp;
> + struct filepath *fp, *rfp;
> +
> + CTASSERT(TALSZ_MAX < 8 * sizeof(fp->talmask));
> + assert(id >= 0 && id < 8 * (int)sizeof(fp->talmask));
>
> - if ((fp = malloc(sizeof(*fp))) == NULL)
> + if ((fp = calloc(1, sizeof(*fp))) == NULL)
> err(1, NULL);
> - fp->mtime = mtime;
> if ((fp->file = strdup(file)) == NULL)
> err(1, NULL);
> + fp->mtime = mtime;
>
> - if (RB_INSERT(filepath_tree, tree, fp) != NULL) {
> + if ((rfp = RB_INSERT(filepath_tree, tree, fp)) != NULL) {
> /* already in the tree */
> free(fp->file);
> free(fp);
> - return 0;
> + if (rfp->talmask & (1 << id))
> + return 0;
> + fp = rfp;
> }
> + fp->talmask |= (1 << id);
>
> return 1;
> }
> @@ -906,7 +913,7 @@ rrdp_handle_file(unsigned int id, enum p
>
> /* write new content or mark uri as deleted. */
> if (pt == PUB_DEL) {
> - filepath_add(&rr->deleted, uri, 0);
> + filepath_add(&rr->deleted, uri, 0, 0);
> } else {
> fp = filepath_find(&rr->deleted, uri);
> if (fp != NULL) {
> Index: validate.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/validate.c,v
> diff -u -p -r1.73 validate.c
> --- validate.c 19 Mar 2024 05:04:13 -0000 1.73
> +++ validate.c 15 May 2024 12:36:15 -0000
> @@ -85,11 +85,12 @@ valid_ip(struct auth *a, enum afi afi,
> * Return the issuer by its AKI, or NULL on failure.
> */
> struct auth *
> -valid_ski_aki(const char *fn, struct auth_tree *auths,
> +valid_ski_aki(const char *fn, struct auth_tree *auths, int id,
> const char *ski, const char *aki, const char *mftaki)
> {
> struct auth *a;
>
> + /* XXX - ski is now unused. Inline these functions in parser.c? */
> if (mftaki != NULL) {
> if (strcmp(aki, mftaki) != 0) {
> warnx("%s: AKI doesn't match Manifest AKI", fn);
> @@ -97,32 +98,16 @@ valid_ski_aki(const char *fn, struct aut
> }
> }
>
> - if (auth_find(auths, ski) != NULL) {
> - warnx("%s: RFC 6487: duplicate SKI", fn);
> - return NULL;
> - }
> -
> - a = auth_find(auths, aki);
> + a = auth_find(auths, id);
> if (a == NULL)
> - warnx("%s: RFC 6487: unknown AKI", fn);
> -
> - return a;
> -}
> + warnx("%s: RFC 6487: unknown cert", fn);
I think we want to do
if (a == NULL) {
warnx("%s: RFC 6487: unknown cert", fn);
return NULL;
}
because...
>
> -/*
> - * Validate a trust anchor by making sure that the SKI is unique.
> - * Returns 1 if valid, 0 otherwise.
> - */
> -int
> -valid_ta(const char *fn, struct auth_tree *auths, const struct cert *cert)
> -{
> - /* SKI must not be a dupe. */
> - if (auth_find(auths, cert->ski) != NULL) {
> - warnx("%s: RFC 6487: duplicate SKI", fn);
> - return 0;
> + if (strcmp(aki, a->cert->ski) != 0) {
... at this point we don't know that a != NULL. I think it's nicer than:
else if (strcmp(aki, a->cert->ski) != 0) {
> + warnx("%s: AKI doesn't match with cert SKI", fn);
> + return NULL;
> }
>
> - return 1;
> + return a;
> }
>
> /*
>
rework rpki-client certificate discovery