From: Claudio Jeker Subject: Re: rework rpki-client certificate discovery To: Theo Buehler , tech@openbsd.org Date: Thu, 16 May 2024 11:44:25 +0200 On Wed, May 15, 2024 at 08:49:02PM +0200, Claudio Jeker wrote: > On Wed, May 15, 2024 at 08:04:22PM +0200, Theo Buehler wrote: > > 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. > > Yeah, valid_aki_ski() feels like duplicated work now. We'll figure out how > and what we want to do. > > > 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. > > :) forgot about that bit. > > > > 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. > > Me too. > > > > 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 > > Fixed in my tree. Thanks for spotting the valid_ski_aki() problem. Here is an updated version. I renamed valid_ski_aki() to valid_auth() and removed the ski argument. Still not super duper happy with the name but it is better I think. Also I print the AKI and SKI hashes in valid_auth() now. At least they should give an extra hint if something hits the fan. -- :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 16 May 2024 09:12:52 -0000 @@ -24,6 +24,9 @@ #include #include +#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 *, - const char *, const char *, const char *); -int valid_ta(const char *, struct auth_tree *, - const struct cert *); +struct auth *valid_auth(const char *, struct auth_tree *, int, + const char *, const char *); 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 18:26:46 -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 used as AIA. */ + 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 16 May 2024 09:14:20 -0000 @@ -137,7 +137,7 @@ 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_auth(file, &auths, entp->certid, roa->aki, entp->mftaki); crl = crl_get(&crlt, a); if (!valid_x509(file, ctx, x509, a, crl, &errstr)) { @@ -172,7 +172,7 @@ 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_auth(file, &auths, entp->certid, spl->aki, entp->mftaki); crl = crl_get(&crlt, a); if (!valid_x509(file, ctx, x509, a, crl, &errstr)) { @@ -336,7 +336,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_auth(file, &auths, entp->certid, mft->aki, NULL); if (!valid_x509(file, ctx, x509, a, *crl, errstr)) goto err; X509_free(x509); @@ -344,6 +344,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 +494,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 +508,7 @@ 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_auth(file, &auths, entp->certid, cert->aki, entp->mftaki); crl = crl_get(&crlt, a); if (!valid_x509(file, ctx, cert->x509, a, crl, &errstr) || @@ -531,7 +532,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 +558,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 +584,7 @@ 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_auth(file, &auths, entp->certid, gbr->aki, entp->mftaki); crl = crl_get(&crlt, a); /* return value can be ignored since nothing happens here */ @@ -623,7 +617,7 @@ 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_auth(file, &auths, entp->certid, aspa->aki, entp->mftaki); crl = crl_get(&crlt, a); if (!valid_x509(file, ctx, x509, a, crl, &errstr)) { @@ -658,7 +652,7 @@ 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_auth(file, &auths, entp->certid, tak->aki, entp->mftaki); crl = crl_get(&crlt, a); if (!valid_x509(file, ctx, x509, a, crl, &errstr)) { @@ -764,8 +758,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 16 May 2024 09:14:55 -0000 @@ -81,48 +81,36 @@ valid_ip(struct auth *a, enum afi afi, /* * Make sure the AKI is the same as the AKI listed on the Manifest, - * and that the SKI doesn't already exist. - * Return the issuer by its AKI, or NULL on failure. + * and that the SKI of the cert matches with the AKI. + * Return the issuer by its certificate id, or NULL on failure. */ struct auth * -valid_ski_aki(const char *fn, struct auth_tree *auths, - const char *ski, const char *aki, const char *mftaki) +valid_auth(const char *fn, struct auth_tree *auths, int id, + const char *aki, const char *mftaki) { struct auth *a; + a = auth_find(auths, id); + if (a == NULL) { + warnx("%s: RFC 6487: unknown cert with SKI %s", fn, aki); + return NULL; + } + if (mftaki != NULL) { if (strcmp(aki, mftaki) != 0) { - warnx("%s: AKI doesn't match Manifest AKI", fn); + warnx("%s: AKI %s doesn't match Manifest AKI %s", fn, + aki, mftaki); return NULL; } } - if (auth_find(auths, ski) != NULL) { - warnx("%s: RFC 6487: duplicate SKI", fn); + if (strcmp(aki, a->cert->ski) != 0) { + warnx("%s: AKI %s doesn't match with cert SKI %s", fn, + aki, a->cert->ski); return NULL; } - a = auth_find(auths, aki); - if (a == NULL) - warnx("%s: RFC 6487: unknown AKI", fn); - return a; -} - -/* - * 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; - } - - return 1; } /*