From: Job Snijders <job@openbsd.org>
Subject: rpki-client: refactor manifest thisUpdate / nextUpdate parsing
To: tech@openbsd.org
Date: Sat, 3 Feb 2024 22:49:25 +0000

When d2i_Manifest() was introduced in in 2022, I think we got the check
whether the thisUpdate/nextUpdate fields are ASN1_GENERALIZEDTIME for
free with it.

So it seems that instead of the bespoke mft_parse_time() +
generalizedtime_to_tm() dance, x509_get_time() can just be used.

OK?

Index: mft.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/mft.c,v
diff -u -p -r1.104 mft.c
--- mft.c	3 Feb 2024 14:30:47 -0000	1.104
+++ mft.c	3 Feb 2024 22:48:24 -0000
@@ -87,60 +87,6 @@ ASN1_SEQUENCE(Manifest) = {
 DECLARE_ASN1_FUNCTIONS(Manifest);
 IMPLEMENT_ASN1_FUNCTIONS(Manifest);
 
-#define GENTIME_LENGTH 15
-
-/*
- * Convert an ASN1_GENERALIZEDTIME to a struct tm.
- * Returns 1 on success, 0 on failure.
- */
-static int
-generalizedtime_to_tm(const ASN1_GENERALIZEDTIME *gtime, struct tm *tm)
-{
-	/*
-	 * ASN1_GENERALIZEDTIME is another name for ASN1_STRING. Check type and
-	 * length, so we don't accidentally accept a UTCTime. Punt on checking
-	 * Zulu time for OpenSSL: we don't want to mess about with silly flags.
-	 */
-	if (ASN1_STRING_type(gtime) != V_ASN1_GENERALIZEDTIME)
-		return 0;
-	if (ASN1_STRING_length(gtime) != GENTIME_LENGTH)
-		return 0;
-
-	memset(tm, 0, sizeof(*tm));
-	return ASN1_TIME_to_tm(gtime, tm);
-}
-
-/*
- * Validate and verify the time validity of the mft.
- * Returns 1 if all is good and for any other case 0.
- */
-static int
-mft_parse_time(const ASN1_GENERALIZEDTIME *from,
-    const ASN1_GENERALIZEDTIME *until, struct parse *p)
-{
-	struct tm tm_from, tm_until;
-
-	if (!generalizedtime_to_tm(from, &tm_from)) {
-		warnx("%s: embedded from time format invalid", p->fn);
-		return 0;
-	}
-	if (!generalizedtime_to_tm(until, &tm_until)) {
-		warnx("%s: embedded until time format invalid", p->fn);
-		return 0;
-	}
-
-	if ((p->res->thisupdate = timegm(&tm_from)) == -1 ||
-	    (p->res->nextupdate = timegm(&tm_until)) == -1)
-		errx(1, "%s: timegm failed", p->fn);
-
-	if (p->res->thisupdate > p->res->nextupdate) {
-		warnx("%s: bad update interval", p->fn);
-		return 0;
-	}
-
-	return 1;
-}
-
 /*
  * Determine rtype corresponding to file extension. Returns RTYPE_INVALID
  * on error or unkown extension.
@@ -301,8 +247,15 @@ mft_parse_econtent(const unsigned char *
 	if (p->res->seqnum == NULL)
 		goto out;
 
-	if (!mft_parse_time(mft->thisUpdate, mft->nextUpdate, p))
+	if (!x509_get_time(mft->thisUpdate, &p->res->thisupdate)) {
+		warn("%s: parsing manifest thisUpdate failed", p->fn);
 		goto out;
+	}
+
+	if (!x509_get_time(mft->nextUpdate, &p->res->nextupdate)) {
+		warn("%s: parsing manifest nextUpdate failed", p->fn);
+		goto out;
+	}
 
 	if (OBJ_obj2nid(mft->fileHashAlg) != NID_sha256) {
 		warnx("%s: RFC 6486 section 4.2.1: fileHashAlg: "
Index: parser.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/parser.c,v
diff -u -p -r1.128 parser.c
--- parser.c	3 Feb 2024 14:30:47 -0000	1.128
+++ parser.c	3 Feb 2024 22:48:24 -0000
@@ -309,6 +309,11 @@ proc_parser_mft_pre(struct entity *entp,
 	mft->repoid = entp->repoid;
 	mft->talid = a->cert->talid;
 
+	if (mft->thisupdate >= mft->nextupdate) {
+		warnx("%s: bad update interval", file);
+		goto err;
+	}
+
 	now = get_current_time();
 	/* check that now is not before from */
 	if (now < mft->thisupdate) {