From: Theo Buehler <tb@theobuehler.org>
Subject: rpki-client: unify proc_parser_foo a bit
To: tech@openbsd.org
Date: Thu, 13 Jun 2024 05:36:30 +0200

There is a bit more variety in here than I would like. The below makes
them look more alike. The initialization of x509 and its resetting to
NULL before the successful exit aren't strictly necessary, but I prefer
being explicit here.

Index: parser.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/parser.c,v
diff -u -p -r1.141 parser.c
--- parser.c	12 Jun 2024 10:03:09 -0000	1.141
+++ parser.c	13 Jun 2024 03:33:29 -0000
@@ -165,36 +165,38 @@ static struct roa *
 proc_parser_roa(char *file, const unsigned char *der, size_t len,
     const struct entity *entp)
 {
-	struct roa		*roa;
+	struct roa		*roa = NULL;
+	X509			*x509 = NULL;
 	struct auth		*a;
 	struct crl		*crl;
-	X509			*x509;
 	const char		*errstr;
 
 	if ((roa = roa_parse(&x509, file, entp->talid, der, len)) == NULL)
-		return NULL;
+		goto out;
 
 	a = find_issuer(file, entp->certid, roa->aki, entp->mftaki);
-	if (a == NULL) {
-		X509_free(x509);
-		roa_free(roa);
-		return NULL;
-	}
+	if (a == NULL)
+		goto out;
 	crl = crl_get(&crlt, a);
 
 	if (!valid_x509(file, ctx, x509, a, crl, &errstr)) {
 		warnx("%s: %s", file, errstr);
-		X509_free(x509);
-		roa_free(roa);
-		return NULL;
+		goto out;
 	}
 	X509_free(x509);
+	x509 = NULL;
 
 	roa->talid = a->cert->talid;
 
 	roa->expires = x509_find_expires(roa->notafter, a, &crlt);
 
 	return roa;
+
+ out:
+	roa_free(roa);
+	X509_free(x509);
+
+	return NULL;
 }
 
 /*
@@ -205,36 +207,38 @@ static struct spl *
 proc_parser_spl(char *file, const unsigned char *der, size_t len,
     const struct entity *entp)
 {
-	struct spl		*spl;
+	struct spl		*spl = NULL;
+	X509			*x509 = NULL;
 	struct auth		*a;
 	struct crl		*crl;
-	X509			*x509;
 	const char		*errstr;
 
 	if ((spl = spl_parse(&x509, file, entp->talid, der, len)) == NULL)
-		return NULL;
+		goto out;
 
 	a = find_issuer(file, entp->certid, spl->aki, entp->mftaki);
-	if (a == NULL) {
-		X509_free(x509);
-		spl_free(spl);
-		return NULL;
-	}
+	if (a == NULL)
+		goto out;
 	crl = crl_get(&crlt, a);
 
 	if (!valid_x509(file, ctx, x509, a, crl, &errstr)) {
 		warnx("%s: %s", file, errstr);
-		X509_free(x509);
-		spl_free(spl);
-		return NULL;
+		goto out;
 	}
 	X509_free(x509);
+	x509 = NULL;
 
 	spl->talid = a->cert->talid;
 
 	spl->expires = x509_find_expires(spl->notafter, a, &crlt);
 
 	return spl;
+
+ out:
+	spl_free(spl);
+	X509_free(x509);
+
+	return NULL;
 }
 
 /*
@@ -546,7 +550,7 @@ static struct cert *
 proc_parser_cert(char *file, const unsigned char *der, size_t len,
     const struct entity *entp)
 {
-	struct cert	*cert;
+	struct cert	*cert = NULL;
 	struct crl	*crl;
 	struct auth	*a;
 	const char	*errstr = NULL;
@@ -556,30 +560,25 @@ proc_parser_cert(char *file, const unsig
 	cert = cert_parse_pre(file, der, len);
 	cert = cert_parse(file, cert);
 	if (cert == NULL)
-		return NULL;
+		goto out;
 
 	a = find_issuer(file, entp->certid, cert->aki, entp->mftaki);
-	if (a == NULL) {
-		cert_free(cert);
-		return NULL;
-	}
+	if (a == NULL)
+		goto out;
 	crl = crl_get(&crlt, a);
 
 	if (!valid_x509(file, ctx, cert->x509, a, crl, &errstr) ||
 	    !valid_cert(file, a, cert)) {
 		if (errstr != NULL)
 			warnx("%s: %s", file, errstr);
-		cert_free(cert);
-		return NULL;
+		goto out;
 	}
 
 	cert->talid = a->cert->talid;
 
 	if (cert->purpose == CERT_PURPOSE_BGPSEC_ROUTER) {
-		if (!constraints_validate(file, cert)) {
-			cert_free(cert);
-			return NULL;
-		}
+		if (!constraints_validate(file, cert))
+			goto out;
 	}
 
 	/*
@@ -589,6 +588,11 @@ proc_parser_cert(char *file, const unsig
 		auth_insert(file, &auths, cert, a);
 
 	return cert;
+
+ out:
+	cert_free(cert);
+
+	return NULL;
 }
 
 static int
@@ -695,34 +699,36 @@ static struct gbr *
 proc_parser_gbr(char *file, const unsigned char *der, size_t len,
     const struct entity *entp)
 {
-	struct gbr	*gbr;
-	X509		*x509;
+	struct gbr	*gbr = NULL;
+	X509		*x509 = NULL;
 	struct crl	*crl;
 	struct auth	*a;
 	const char	*errstr;
 
 	if ((gbr = gbr_parse(&x509, file, entp->talid, der, len)) == NULL)
-		return NULL;
+		goto out;
 
 	a = find_issuer(file, entp->certid, gbr->aki, entp->mftaki);
-	if (a == NULL) {
-		X509_free(x509);
-		gbr_free(gbr);
-		return NULL;
-	}
+	if (a == NULL)
+		goto out;
 	crl = crl_get(&crlt, a);
 
 	if (!valid_x509(file, ctx, x509, a, crl, &errstr)) {
 		warnx("%s: %s", file, errstr);
-		X509_free(x509);
-		gbr_free(gbr);
-		return NULL;
+		goto out;
 	}
 	X509_free(x509);
+	x509 = NULL;
 
 	gbr->talid = a->cert->talid;
 
 	return gbr;
+
+ out:
+	gbr_free(gbr);
+	X509_free(x509);
+
+	return NULL;
 }
 
 /*
@@ -732,36 +738,38 @@ static struct aspa *
 proc_parser_aspa(char *file, const unsigned char *der, size_t len,
     const struct entity *entp)
 {
-	struct aspa	*aspa;
+	struct aspa	*aspa = NULL;
+	X509		*x509 = NULL;
 	struct auth	*a;
 	struct crl	*crl;
-	X509		*x509;
 	const char	*errstr;
 
 	if ((aspa = aspa_parse(&x509, file, entp->talid, der, len)) == NULL)
-		return NULL;
+		goto out;
 
 	a = find_issuer(file, entp->certid, aspa->aki, entp->mftaki);
-	if (a == NULL) {
-		X509_free(x509);
-		aspa_free(aspa);
-		return NULL;
-	}
+	if (a == NULL)
+		goto out;
 	crl = crl_get(&crlt, a);
 
 	if (!valid_x509(file, ctx, x509, a, crl, &errstr)) {
 		warnx("%s: %s", file, errstr);
-		X509_free(x509);
-		aspa_free(aspa);
-		return NULL;
+		goto out;
 	}
 	X509_free(x509);
+	x509 = NULL;
 
 	aspa->talid = a->cert->talid;
 
 	aspa->expires = x509_find_expires(aspa->notafter, a, &crlt);
 
 	return aspa;
+
+ out:
+	aspa_free(aspa);
+	X509_free(x509);
+
+	return NULL;
 }
 
 /*
@@ -771,15 +779,14 @@ static struct tak *
 proc_parser_tak(char *file, const unsigned char *der, size_t len,
     const struct entity *entp)
 {
-	struct tak	*tak;
-	X509		*x509;
+	struct tak	*tak = NULL;
+	X509		*x509 = NULL;
 	struct crl	*crl;
 	struct auth	*a;
 	const char	*errstr;
-	int		 rc = 0;
 
 	if ((tak = tak_parse(&x509, file, entp->talid, der, len)) == NULL)
-		return NULL;
+		goto out;
 
 	a = find_issuer(file, entp->certid, tak->aki, entp->mftaki);
 	if (a == NULL)
@@ -790,20 +797,22 @@ proc_parser_tak(char *file, const unsign
 		warnx("%s: %s", file, errstr);
 		goto out;
 	}
+	X509_free(x509);
+	x509 = NULL;
 
 	/* TAK EE must be signed by self-signed CA */
 	if (a->issuer != NULL)
 		goto out;
 
 	tak->talid = a->cert->talid;
-	rc = 1;
+
+	return tak;
+
  out:
-	if (rc == 0) {
-		tak_free(tak);
-		tak = NULL;
-	}
+	tak_free(tak);
 	X509_free(x509);
-	return tak;
+
+	return NULL;
 }
 
 /*