Cleanup mft file handling, especially the stale mft bits.

Move staleness check up into mft_parse_econtent() to simplify code.
Remove the big FIXME bits since they are no longer needed. The parent
process will only process MFTs that are not stale.
Cleanup a few other bits mainly unneccessary else if cascades and
use valid_filename() to check if the filename embedded in the mft
fileandhash is sensible.
OK tb@

2 files changed, 26 insertions, 51 deletions

diff --git a/usr.sbin/rpki-client/main.c b/usr.sbin/rpki-client/main.c
index 0160877e96b..ce7abda7d12 100644
--- a/usr.sbin/rpki-client/main.c
+++ b/usr.sbin/rpki-client/main.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: main.c,v 1.171 2022/01/04 18:41:32 claudio Exp $ */
+/*	$OpenBSD: main.c,v 1.172 2022/01/06 16:06:30 claudio Exp $ */
 /*
  * Copyright (c) 2021 Claudio Jeker <claudio@openbsd.org>
  * Copyright (c) 2019 Kristaps Dzonsons <kristaps@bsd.lv>
@@ -518,9 +518,10 @@ entity_process(struct ibuf *b, struct stats *st, struct vrp_tree *tree,
 			break;
 		}
 		mft = mft_read(b);
-		if (mft->stale)
+		if (!mft->stale)
+			queue_add_from_mft_set(mft);
+		else
 			st->mfts_stale++;
-		queue_add_from_mft_set(mft);
 		mft_free(mft);
 		break;
 	case RTYPE_CRL:
diff --git a/usr.sbin/rpki-client/mft.c b/usr.sbin/rpki-client/mft.c
index 77e16c49e9d..3c30d5f4cf9 100644
--- a/usr.sbin/rpki-client/mft.c
+++ b/usr.sbin/rpki-client/mft.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: mft.c,v 1.42 2021/10/28 13:51:42 job Exp $ */
+/*	$OpenBSD: mft.c,v 1.43 2022/01/06 16:06:30 claudio Exp $ */
 /*
  * Copyright (c) 2019 Kristaps Dzonsons <kristaps@bsd.lv>
  *
@@ -161,20 +161,8 @@ mft_parse_filehash(struct parse *p, const ASN1_OCTET_STRING *os)
 	if (fn == NULL)
 		err(1, NULL);
 
-	/*
-	 * Make sure we're just a pathname and either an ROA or CER.
-	 * I don't think that the RFC specifically mentions this, but
-	 * it's in practical use and would really screw things up
-	 * (arbitrary filenames) otherwise.
-	 */
-
-	if (strchr(fn, '/') != NULL) {
-		warnx("%s: path components disallowed in filename: %s",
-		    p->fn, fn);
-		goto out;
-	} else if (strlen(fn) <= 4) {
-		warnx("%s: filename must be large enough for suffix part: %s",
-		    p->fn, fn);
+	if (!valid_filename(fn)) {
+		warnx("%s: invalid filename: %s", p->fn, fn);
 		goto out;
 	}
 
@@ -257,7 +245,7 @@ mft_parse_flist(struct parse *p, const ASN1_OCTET_STRING *os)
 
 /*
  * Handle the eContent of the manifest object, RFC 6486 sec. 4.2.
- * Returns <0 on failure, 0 on stale, >0 on success.
+ * Returns 0 on failure and 1 on success.
  */
 static int
 mft_parse_econtent(const unsigned char *d, size_t dsz, struct parse *p)
@@ -267,7 +255,7 @@ mft_parse_econtent(const unsigned char *d, size_t dsz, struct parse *p)
 	const ASN1_GENERALIZEDTIME *from, *until;
 	long			 mft_version;
 	BIGNUM			*mft_seqnum = NULL;
-	int			 i = 0, rc = -1;
+	int			 i = 0, rc = 0;
 
 	if ((seq = d2i_ASN1_SEQUENCE_ANY(NULL, &d, dsz)) == NULL) {
 		cryptowarnx("%s: RFC 6486 section 4.2: Manifest: "
@@ -366,22 +354,26 @@ mft_parse_econtent(const unsigned char *d, size_t dsz, struct parse *p)
 	}
 	until = t->value.generalizedtime;
 
-	rc = check_validity(from, until, p->fn);
-	if (rc != 1)
+	switch (check_validity(from, until, p->fn)) {
+	case 0:
+		p->res->stale = 1;
+		/* FALLTHROUGH */
+	case 1:
+		break;
+	case -1:
 		goto out;
-
-	/* The mft is valid. Reset rc so later 'goto out' return failure. */
-	rc = -1;
+	}
 
 	/* File list algorithm. */
 
 	t = sk_ASN1_TYPE_value(seq, i++);
 	if (t->type != V_ASN1_OBJECT) {
 		warnx("%s: RFC 6486 section 4.2.1: fileHashAlg: "
-		    "want ASN.1 object time, have %s (NID %d)",
+		    "want ASN.1 object, have %s (NID %d)",
 		    p->fn, ASN1_tag2str(t->type), t->type);
 		goto out;
-	} else if (OBJ_obj2nid(t->value.object) != NID_sha256) {
+	}
+	if (OBJ_obj2nid(t->value.object) != NID_sha256) {
 		warnx("%s: RFC 6486 section 4.2.1: fileHashAlg: "
 		    "want SHA256 object, have %s (NID %d)", p->fn,
 		    ASN1_tag2str(OBJ_obj2nid(t->value.object)),
@@ -397,7 +389,9 @@ mft_parse_econtent(const unsigned char *d, size_t dsz, struct parse *p)
 		    "want ASN.1 sequence, have %s (NID %d)",
 		    p->fn, ASN1_tag2str(t->type), t->type);
 		goto out;
-	} else if (!mft_parse_flist(p, t->value.octet_string))
+	}
+
+	if (!mft_parse_flist(p, t->value.octet_string))
 		goto out;
 
 	rc = 1;
@@ -418,8 +412,8 @@ struct mft *
 mft_parse(X509 **x509, const char *fn, const unsigned char *der, size_t len)
 {
 	struct parse	 p;
-	int		 c, rc = 0;
-	size_t		 i, cmsz;
+	int		 rc = 0;
+	size_t		 cmsz;
 	unsigned char	*cms;
 
 	memset(&p, 0, sizeof(struct parse));
@@ -451,27 +445,7 @@ mft_parse(X509 **x509, const char *fn, const unsigned char *der, size_t len)
 		goto out;
 	}
 
-	/*
-	 * If we're stale, then remove all of the files that the MFT
-	 * references as well as marking it as stale.
-	 */
-
-	if ((c = mft_parse_econtent(cms, cmsz, &p)) == 0) {
-		/*
-		 * FIXME: it should suffice to just mark this as stale
-		 * and have the logic around mft_read() simply ignore
-		 * the contents of stale entries, just like it does for
-		 * invalid ROAs or certificates.
-		 */
-
-		p.res->stale = 1;
-		if (p.res->files != NULL)
-			for (i = 0; i < p.res->filesz; i++)
-				free(p.res->files[i].file);
-		free(p.res->files);
-		p.res->filesz = 0;
-		p.res->files = NULL;
-	} else if (c == -1)
+	if (mft_parse_econtent(cms, cmsz, &p) == 0)
 		goto out;
 
 	rc = 1;