On Thu, May 16, 2024 at 12:07:21PM +0200, Theo Buehler wrote:
> On Thu, May 16, 2024 at 11:44:25AM +0200, Claudio Jeker wrote:
> > 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.
> 
> Makes sense to me. The AKI is slightly redundant since we already have
> the file name, but it doesn't hurt.
> 
> I can live with valid_auth(), but what it really does is returning the
> issuing CA cert after looking it up by its internal id and seeing if
> AKI/SKI match expectations. Maybe find_issuer()?

That is better. I will also move the code to parser.c since it is the only
consumer of this function and it is misplaced in validate.c
 
> > +		warnx("%s: AKI %s doesn't match with cert SKI %s", fn,
> > +		    aki, a->cert->ski);
> 
> If you go with find_issuer() this could become
> 
> 		warnx("%s: AKI %s doesn't match issuer SKI %s", fn,
> 		    aki, a->cert->ski);
> 
> Other than that, the diff works well in the testing and now that NRO
> fixed their new*.cer's AIA filemode proves to work nicely as well.
> 
> ok with either valid_auth() or find_issuer().
 
So this is now my version with find_issuer() in parser.c
-- 
: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	16 May 2024 09:49:50 -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 10:47:24 -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,6 @@ 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 *);
 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 +830,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 +985,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 10:50:31 -0000
@@ -87,6 +87,39 @@ repo_add(unsigned int id, char *path, ch
 }
 
 /*
+ * Return the issuer by its certificate id, or NULL on failure.
+ * Make sure the AKI is the same as the AKI listed on the Manifest,
+ * and that the SKI of the cert matches with the AKI.
+ */
+static struct auth *
+find_issuer(const char *fn, 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 %s doesn't match Manifest AKI %s", fn,
+			    aki, mftaki);
+			return NULL;
+		}
+	}
+
+	if (strcmp(aki, a->cert->ski) != 0) {
+		warnx("%s: AKI %s doesn't match issuer SKI %s", fn,
+		    aki, a->cert->ski);
+		return NULL;
+	}
+
+	return a;
+}
+
+/*
  * Build access path to file based on repoid, path, location and file values.
  */
 static char *
@@ -137,7 +170,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 = find_issuer(file, entp->certid, roa->aki, entp->mftaki);
 	crl = crl_get(&crlt, a);
 
 	if (!valid_x509(file, ctx, x509, a, crl, &errstr)) {
@@ -172,7 +205,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 = find_issuer(file, entp->certid, spl->aki, entp->mftaki);
 	crl = crl_get(&crlt, a);
 
 	if (!valid_x509(file, ctx, x509, a, crl, &errstr)) {
@@ -336,7 +369,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 = find_issuer(file, entp->certid, mft->aki, NULL);
 	if (!valid_x509(file, ctx, x509, a, *crl, errstr))
 		goto err;
 	X509_free(x509);
@@ -344,6 +377,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 +527,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 +541,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 = find_issuer(file, entp->certid, cert->aki, entp->mftaki);
 	crl = crl_get(&crlt, a);
 
 	if (!valid_x509(file, ctx, cert->x509, a, crl, &errstr) ||
@@ -531,7 +565,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 +591,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 +617,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 = find_issuer(file, entp->certid, gbr->aki, entp->mftaki);
 	crl = crl_get(&crlt, a);
 
 	/* return value can be ignored since nothing happens here */
@@ -623,7 +650,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 = find_issuer(file, entp->certid, aspa->aki, entp->mftaki);
 	crl = crl_get(&crlt, a);
 
 	if (!valid_x509(file, ctx, x509, a, crl, &errstr)) {
@@ -658,7 +685,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 = find_issuer(file, entp->certid, tak->aki, entp->mftaki);
 	crl = crl_get(&crlt, a);
 
 	if (!valid_x509(file, ctx, x509, a, crl, &errstr)) {
@@ -764,8 +791,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 10:46:58 -0000
@@ -80,52 +80,6 @@ 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.
- */
-struct auth *
-valid_ski_aki(const char *fn, struct auth_tree *auths,
-    const char *ski, const char *aki, const char *mftaki)
-{
-	struct auth *a;
-
-	if (mftaki != NULL) {
-		if (strcmp(aki, mftaki) != 0) {
-			warnx("%s: AKI doesn't match Manifest AKI", fn);
-			return NULL;
-		}
-	}
-
-	if (auth_find(auths, ski) != NULL) {
-		warnx("%s: RFC 6487: duplicate SKI", fn);
-		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;
-}
-
-/*
  * Validate a non-TA certificate: make sure its IP and AS resources are
  * fully covered by those in the authority key (which must exist).
  * Returns 1 if valid, 0 otherwise.