More fixing. Previous revert was incomplete.

2 files changed, 177 insertions, 160 deletions

diff --git a/usr.sbin/rpki-client/parser.c b/usr.sbin/rpki-client/parser.c
index 1ac396a4a22..9f5d6a32979 100644
--- a/usr.sbin/rpki-client/parser.c
+++ b/usr.sbin/rpki-client/parser.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: parser.c,v 1.41 2022/01/18 16:24:55 claudio Exp $ */
+/*	$OpenBSD: parser.c,v 1.42 2022/01/18 16:29:06 claudio Exp $ */
 /*
  * Copyright (c) 2019 Claudio Jeker <claudio@openbsd.org>
  * Copyright (c) 2019 Kristaps Dzonsons <kristaps@bsd.lv>
@@ -50,6 +50,7 @@ static struct crl_tree	 crlt = RB_INITIALIZER(&crlt);
 struct parse_repo {
 	RB_ENTRY(parse_repo)	 entry;
 	char			*path;
+	char			*validpath;
 	unsigned int		 id;
 };
 
@@ -72,20 +73,75 @@ repo_get(unsigned int id)
 }
 
 static void
-repo_add(unsigned int id, char *path)
+repo_add(unsigned int id, char *path, char *validpath)
 {
 	struct parse_repo *rp;
 
-	if ((rp = malloc(sizeof(*rp))) == NULL)
+	if ((rp = calloc(1, sizeof(*rp))) == NULL)
 		err(1, NULL);
 	rp->id = id;
-	if ((rp->path = strdup(path)) == NULL)
-		err(1, NULL);
+	if (path != NULL)
+		if ((rp->path = strdup(path)) == NULL)
+			err(1, NULL);
+	if (validpath != NULL)
+		if ((rp->validpath = strdup(validpath)) == NULL)
+			err(1, NULL);
 
 	if (RB_INSERT(repo_tree, &repos, rp) != NULL)
 		errx(1, "repository already added: id %d, %s", id, path);
 }
 
+/*
+ * Build access path to file based on repoid, path and file values.
+ * If wantalt == 1 the function can return NULL, if wantalt == 0 it
+ * can not fail.
+ */
+static char *
+parse_filepath(unsigned int repoid, const char *path, const char *file,
+    int wantalt)
+{
+	struct parse_repo	*rp;
+	char			*fn, *repopath;
+
+	/* build file path based on repoid, entity path and filename */
+	rp = repo_get(repoid);
+	if (rp == NULL) {
+		/* no repo so no alternative path. */
+		if (wantalt)
+			return NULL;
+
+		if (path == NULL) {
+			if ((fn = strdup(file)) == NULL)
+				err(1, NULL);
+		} else {
+			if (asprintf(&fn, "%s/%s", path, file) == -1)
+				err(1, NULL);
+		}
+	} else {
+		if (wantalt || rp->path == NULL)
+			repopath = rp->validpath;
+		else
+			repopath = rp->path;
+
+		if (repopath == NULL)
+			return NULL;
+
+		if (path == NULL) {
+			if (asprintf(&fn, "%s/%s", repopath, file) == -1)
+				err(1, NULL);
+		} else {
+			if (asprintf(&fn, "%s/%s/%s", repopath, path,
+			    file) == -1)
+				err(1, NULL);
+		}
+	}
+	return fn;
+}
+
+/*
+ * Callback for X509_verify_cert() to handle critical extensions in old
+ * LibreSSL libraries or OpenSSL libs without RFC3779 support.
+ */
 static int
 verify_cb(int ok, X509_STORE_CTX *store_ctx)
 {
@@ -144,52 +200,74 @@ verify_cb(int ok, X509_STORE_CTX *store_ctx)
 }
 
 /*
- * Parse and validate a ROA.
- * This is standard stuff.
- * Returns the roa on success, NULL on failure.
+ * Validate the X509 certificate.  If crl is NULL don't check CRL.
+ * Returns 1 for valid certificates, returns 0 if there is a verify error
  */
-static struct roa *
-proc_parser_roa(char *file, const unsigned char *der, size_t len)
+static int
+valid_x509(char *file, X509 *x509, struct auth *a, struct crl *crl)
 {
-	struct roa		*roa;
-	X509			*x509;
-	int			 c;
-	struct auth		*a;
 	STACK_OF(X509)		*chain;
-	STACK_OF(X509_CRL)	*crls;
-	struct crl		*crl;
-
-	if ((roa = roa_parse(&x509, file, der, len)) == NULL)
-		return NULL;
+	STACK_OF(X509_CRL)	*crls = NULL;
+	int			 c;
 
-	a = valid_ski_aki(file, &auths, roa->ski, roa->aki);
 	build_chain(a, &chain);
-	crl = get_crl(a);
-	build_crls(crl, &crls);
+	if (crl != NULL)
+		build_crls(crl, &crls);
 
 	assert(x509 != NULL);
 	if (!X509_STORE_CTX_init(ctx, NULL, x509, NULL))
 		cryptoerrx("X509_STORE_CTX_init");
+
 	X509_STORE_CTX_set_verify_cb(ctx, verify_cb);
 	if (!X509_STORE_CTX_set_app_data(ctx, file))
 		cryptoerrx("X509_STORE_CTX_set_app_data");
-	X509_STORE_CTX_set_flags(ctx, X509_V_FLAG_CRL_CHECK);
+	if (crl != NULL)
+		X509_STORE_CTX_set_flags(ctx, X509_V_FLAG_CRL_CHECK);
 	X509_STORE_CTX_set_depth(ctx, MAX_CERT_DEPTH);
 	X509_STORE_CTX_set0_trusted_stack(ctx, chain);
-	X509_STORE_CTX_set0_crls(ctx, crls);
+	if (crl != NULL)
+		X509_STORE_CTX_set0_crls(ctx, crls);
 
 	if (X509_verify_cert(ctx) <= 0) {
 		c = X509_STORE_CTX_get_error(ctx);
+		warnx("%s: %s", file, X509_verify_cert_error_string(c));
 		X509_STORE_CTX_cleanup(ctx);
-		if (verbose > 0 || c != X509_V_ERR_UNABLE_TO_GET_CRL)
-			warnx("%s: %s", file, X509_verify_cert_error_string(c));
-		X509_free(x509);
-		roa_free(roa);
 		sk_X509_free(chain);
 		sk_X509_CRL_free(crls);
-		return NULL;
+		return 0;
 	}
+
 	X509_STORE_CTX_cleanup(ctx);
+	sk_X509_free(chain);
+	sk_X509_CRL_free(crls);
+	return 1;
+}
+
+/*
+ * Parse and validate a ROA.
+ * This is standard stuff.
+ * Returns the roa on success, NULL on failure.
+ */
+static struct roa *
+proc_parser_roa(char *file, const unsigned char *der, size_t len)
+{
+	struct roa		*roa;
+	struct crl		*crl;
+	struct auth		*a;
+	X509			*x509;
+
+	if ((roa = roa_parse(&x509, file, der, len)) == NULL)
+		return NULL;
+
+	a = valid_ski_aki(file, &auths, roa->ski, roa->aki);
+	crl = get_crl(a);
+
+	if (!valid_x509(file, x509, a, crl)) {
+		X509_free(x509);
+		roa_free(roa);
+		return NULL;
+	}
+	X509_free(x509);
 
 	/*
 	 * Check CRL to figure out the soonest transitive expiry moment
@@ -214,10 +292,6 @@ proc_parser_roa(char *file, const unsigned char *der, size_t len)
 	if (valid_roa(file, &auths, roa))
 		roa->valid = 1;
 
-	sk_X509_free(chain);
-	sk_X509_CRL_free(crls);
-	X509_free(x509);
-
 	return roa;
 }
 
@@ -229,13 +303,8 @@ int
 mft_check(const char *fn, struct mft *p)
 {
 	size_t	i;
-	int	fd, rc = 1;
-	char	*cp, *h, *path = NULL;
-
-	/* Check hash of file now, but first build path for it */
-	cp = strrchr(fn, '/');
-	assert(cp != NULL);
-	assert(cp - fn < INT_MAX);
+	int	fd, try, rc = 1;
+	char	*h, *path;
 
 	for (i = 0; i < p->filesz; i++) {
 		const struct mftfile *m = &p->files[i];
@@ -246,15 +315,24 @@ mft_check(const char *fn, struct mft *p)
 			free(h);
 			continue;
 		}
-		if (asprintf(&path, "%.*s/%s", (int)(cp - fn), fn,
-		    m->file) == -1)
-			err(1, NULL);
-		fd = open(path, O_RDONLY);
+
+		fd = -1;
+		try = 0;
+		path = NULL;
+		do {
+			free(path);
+			if ((path = parse_filepath(p->repoid, p->path, m->file,
+			    try++)) == NULL)
+				break;
+			fd = open(path, O_RDONLY);
+		} while (fd == -1 && try < 2);
+
+		free(path);
+
 		if (!valid_filehash(fd, m->hash, sizeof(m->hash))) {
 			warnx("%s: bad message digest for %s", fn, m->file);
 			rc = 0;
 		}
-		free(path);
 	}
 
 	return rc;
@@ -276,49 +354,31 @@ proc_parser_mft(char *file, const unsigned char *der, size_t len,
 {
 	struct mft		*mft;
 	X509			*x509;
-	int			 c;
 	struct auth		*a;
-	STACK_OF(X509)		*chain;
 
 	if ((mft = mft_parse(&x509, file, der, len)) == NULL)
 		return NULL;
 
 	a = valid_ski_aki(file, &auths, mft->ski, mft->aki);
-	build_chain(a, &chain);
-
-	if (!X509_STORE_CTX_init(ctx, NULL, x509, NULL))
-		cryptoerrx("X509_STORE_CTX_init");
-
-	/* CRL checks disabled here because CRL is referenced from mft */
-	X509_STORE_CTX_set_verify_cb(ctx, verify_cb);
-	if (!X509_STORE_CTX_set_app_data(ctx, file))
-		cryptoerrx("X509_STORE_CTX_set_app_data");
-	X509_STORE_CTX_set_depth(ctx, MAX_CERT_DEPTH);
-	X509_STORE_CTX_set0_trusted_stack(ctx, chain);
 
-	if (X509_verify_cert(ctx) <= 0) {
-		c = X509_STORE_CTX_get_error(ctx);
-		X509_STORE_CTX_cleanup(ctx);
-		warnx("%s: %s", file, X509_verify_cert_error_string(c));
+	if (!valid_x509(file, x509, a, NULL)) {
 		mft_free(mft);
 		X509_free(x509);
-		sk_X509_free(chain);
 		return NULL;
 	}
-
-	X509_STORE_CTX_cleanup(ctx);
-	sk_X509_free(chain);
 	X509_free(x509);
 
-	if (!mft_check(file, mft)) {
-		mft_free(mft);
-		return NULL;
-	}
-
+	mft->repoid = repoid;
 	if (path != NULL)
 		if ((mft->path = strdup(path)) == NULL)
 			err(1, NULL);
-	mft->repoid = repoid;
+
+	if (!mft->stale)
+		if (!mft_check(file, mft)) {
+			mft_free(mft);
+			return NULL;
+		}
+
 	return mft;
 }
 
@@ -334,10 +394,8 @@ proc_parser_cert(char *file, const unsigned char *der, size_t len)
 {
 	struct cert		*cert;
 	X509			*x509;
-	int			 c;
-	struct auth		*a = NULL;
-	STACK_OF(X509)		*chain;
-	STACK_OF(X509_CRL)	*crls;
+	struct auth		*a;
+	struct crl		*crl;
 
 	/* Extract certificate data and X509. */
 
@@ -346,35 +404,13 @@ proc_parser_cert(char *file, const unsigned char *der, size_t len)
 		return NULL;
 
 	a = valid_ski_aki(file, &auths, cert->ski, cert->aki);
-	build_chain(a, &chain);
-	build_crls(get_crl(a), &crls);
-
-	assert(x509 != NULL);
-	if (!X509_STORE_CTX_init(ctx, NULL, x509, NULL))
-		cryptoerrx("X509_STORE_CTX_init");
-
-	X509_STORE_CTX_set_verify_cb(ctx, verify_cb);
-	if (!X509_STORE_CTX_set_app_data(ctx, file))
-		cryptoerrx("X509_STORE_CTX_set_app_data");
-	X509_STORE_CTX_set_flags(ctx, X509_V_FLAG_CRL_CHECK);
-	X509_STORE_CTX_set_depth(ctx, MAX_CERT_DEPTH);
-	X509_STORE_CTX_set0_trusted_stack(ctx, chain);
-	X509_STORE_CTX_set0_crls(ctx, crls);
+	crl = get_crl(a);
 
-	if (X509_verify_cert(ctx) <= 0) {
-		c = X509_STORE_CTX_get_error(ctx);
-		warnx("%s: %s", file, X509_verify_cert_error_string(c));
-		X509_STORE_CTX_cleanup(ctx);
+	if (!valid_x509(file, x509, a, crl)) {
 		cert_free(cert);
-		sk_X509_free(chain);
-		sk_X509_CRL_free(crls);
 		X509_free(x509);
 		return NULL;
 	}
-
-	X509_STORE_CTX_cleanup(ctx);
-	sk_X509_free(chain);
-	sk_X509_CRL_free(crls);
 	X509_free(x509);
 
 	cert->talid = a->cert->talid;
@@ -535,39 +571,18 @@ proc_parser_gbr(char *file, const unsigned char *der, size_t len)
 {
 	struct gbr		*gbr;
 	X509			*x509;
-	int			 c;
 	struct auth		*a;
-	STACK_OF(X509)		*chain;
-	STACK_OF(X509_CRL)	*crls;
+	struct crl		*crl;
 
 	if ((gbr = gbr_parse(&x509, file, der, len)) == NULL)
 		return;
 
 	a = valid_ski_aki(file, &auths, gbr->ski, gbr->aki);
+	crl = get_crl(a);
 
-	build_chain(a, &chain);
-	build_crls(get_crl(a), &crls);
-
-	assert(x509 != NULL);
-	if (!X509_STORE_CTX_init(ctx, NULL, x509, NULL))
-		cryptoerrx("X509_STORE_CTX_init");
-	X509_STORE_CTX_set_verify_cb(ctx, verify_cb);
-	if (!X509_STORE_CTX_set_app_data(ctx, file))
-		cryptoerrx("X509_STORE_CTX_set_app_data");
-	X509_STORE_CTX_set_flags(ctx, X509_V_FLAG_CRL_CHECK);
-	X509_STORE_CTX_set_depth(ctx, MAX_CERT_DEPTH);
-	X509_STORE_CTX_set0_trusted_stack(ctx, chain);
-	X509_STORE_CTX_set0_crls(ctx, crls);
-
-	if (X509_verify_cert(ctx) <= 0) {
-		c = X509_STORE_CTX_get_error(ctx);
-		if (verbose > 0 || c != X509_V_ERR_UNABLE_TO_GET_CRL)
-			warnx("%s: %s", file, X509_verify_cert_error_string(c));
-	}
+	/* return value can be ignored since nothing happens here */
+	valid_x509(file, x509, a, crl);
 
-	X509_STORE_CTX_cleanup(ctx);
-	sk_X509_free(chain);
-	sk_X509_CRL_free(crls);
 	X509_free(x509);
 	gbr_free(gbr);
 }
@@ -629,33 +644,40 @@ build_crls(const struct crl *crl, STACK_OF(X509_CRL) **crls)
 }
 
 static char *
-parse_filepath(struct entity *entp)
+parse_load_file(struct entity *entp, unsigned char **f, size_t *flen)
 {
-	struct parse_repo	*rp;
-	char			*file;
+	char *file, *nfile;
 
-	/* build file path based on repoid, entity path and filename */
-	rp = repo_get(entp->repoid);
-	if (rp == NULL) {
-		if (entp->path == NULL) {
-			if ((file = strdup(entp->file)) == NULL)
-					err(1, NULL);
-		} else {
-			if (asprintf(&file, "%s/%s", entp->path,
-			    entp->file) == -1)
-				err(1, NULL);
-		}
-	} else {
-		if (entp->path == NULL) {
-			if (asprintf(&file, "%s/%s", rp->path,
-			    entp->file) == -1)
-				err(1, NULL);
-		} else {
-			if (asprintf(&file, "%s/%s/%s", rp->path,
-			    entp->path, entp->file) == -1)
-				err(1, NULL);
-		}
+	file = parse_filepath(entp->repoid, entp->path, entp->file, 0);
+
+	/* TAL files include the data already */
+	if (entp->type == RTYPE_TAL) {
+		*f = NULL;
+		*flen = 0;
+		return file;
 	}
+
+	*f = load_file(file, flen);
+	if (*f != NULL)
+		return file;
+
+	if (errno != ENOENT)
+		goto fail;
+
+	/* try alternate file location */
+	nfile = parse_filepath(entp->repoid, entp->path, entp->file, 1);
+	if (nfile == NULL)
+		goto fail;
+
+	free(file);
+	file = nfile;
+
+	*f = load_file(file, flen);
+	if (*f != NULL)
+		return file;
+
+fail:
+	warn("parse file %s", file);
 	return file;
 }
 
@@ -678,20 +700,14 @@ parse_entity(struct entityq *q, struct msgbuf *msgq)
 
 		/* handle RTYPE_REPO first */
 		if (entp->type == RTYPE_REPO) {
-			repo_add(entp->repoid, entp->path);
+			repo_add(entp->repoid, entp->path, entp->file);
 			entity_free(entp);
 			continue;
 		}
 
-		f = NULL;
-		file = parse_filepath(entp);
-		if (entp->type != RTYPE_TAL) {
-			f = load_file(file, &flen);
-			if (f == NULL)
-				warn("%s", file);
-		}
+		file = parse_load_file(entp, &f, &flen);
 
-		/* pass back at least type and filename */
+		/* pass back at least type, repoid and filename */
 		b = io_new_buffer();
 		io_simple_buffer(b, &entp->type, sizeof(entp->type));
 		io_str_buffer(b, file);
@@ -776,6 +792,7 @@ proc_parser(int fd)
 	ERR_load_crypto_strings();
 	OpenSSL_add_all_ciphers();
 	OpenSSL_add_all_digests();
+	x509_init_oid();
 
 	if ((ctx = X509_STORE_CTX_new()) == NULL)
 		cryptoerrx("X509_STORE_CTX_new");
diff --git a/usr.sbin/rpki-client/roa.c b/usr.sbin/rpki-client/roa.c
index 21cebd0d310..a67e6ff322d 100644
--- a/usr.sbin/rpki-client/roa.c
+++ b/usr.sbin/rpki-client/roa.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: roa.c,v 1.36 2022/01/18 16:18:22 claudio Exp $ */
+/*	$OpenBSD: roa.c,v 1.37 2022/01/18 16:29:06 claudio Exp $ */
 /*
  * Copyright (c) 2019 Kristaps Dzonsons <kristaps@bsd.lv>
  *
@@ -346,7 +346,7 @@ roa_parse(X509 **x509, const char *fn, const unsigned char *der, size_t len)
 	memset(&p, 0, sizeof(struct parse));
 	p.fn = fn;
 
-	cms = cms_parse_validate(x509, fn, der, len, roa_oid, &cmsz, 0);
+	cms = cms_parse_validate(x509, fn, der, len, roa_oid, &cmsz);
 	if (cms == NULL)
 		return NULL;