From: Job Snijders <job@openbsd.org>
Subject: Re: rpki-client: check for duplicate file names and hashes in manifests
To: Theo Buehler <tb@theobuehler.org>
Cc: tech@openbsd.org
Date: Tue, 13 Feb 2024 13:12:16 +0000

On Tue, Feb 13, 2024 at 01:59:07PM +0100, Theo Buehler wrote:
> This checks for duplicates among file names and hashes in the
> FileAndHash list of a manifest. The check is of course not entirely
> free, but I think acceptable: on a modernish machine this adds well
> below 1s overhead on the total runtime.
> 
> For the two largest manifests with roughly 20k entries, the function
> takes 15ms on my m1 mini. The vast majority (>99%) of manifests has
> fewer than 32 entries for which the check is a couple dozen us on a
> slow m1 core.

I haven't tested it yet but while there, might make sense to flip the
order of this check and the strndup()?

Index: mft.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/mft.c,v
diff -u -p -r1.106 mft.c
--- mft.c	5 Feb 2024 19:23:58 -0000	1.106
+++ mft.c	13 Feb 2024 13:11:33 -0000
@@ -187,9 +187,6 @@ mft_parse_filehash(struct parse *p, cons
 		warnx("%s: RFC 6486 section 4.2.2: bad filename", p->fn);
 		goto out;
 	}
-	fn = strndup(fh->file->data, fh->file->length);
-	if (fn == NULL)
-		err(1, NULL);
 
 	if (fh->hash->length != SHA256_DIGEST_LENGTH) {
 		warnx("%s: RFC 6486 section 4.2.1: hash: "
@@ -197,6 +194,10 @@ mft_parse_filehash(struct parse *p, cons
 		    p->fn, fh->hash->length);
 		goto out;
 	}
+
+	fn = strndup(fh->file->data, fh->file->length);
+	if (fn == NULL)
+		err(1, NULL);
 
 	type = rtype_from_mftfile(fn);
 	/* remember the filehash for the CRL in struct mft */