Index | Thread | Search

From:
Theo Buehler <tb@theobuehler.org>
Subject:
rpki-client: add an X509 reference to cert early on
To:
tech@openbsd.org
Date:
Wed, 2 Jul 2025 07:57:45 +0200

Download raw body.

Thread
In order to deal with SKI and AKI, it's cleaner to have the libcrypto
cert available in struct cert when parsing them so that the extension
handlers can all have the same signature. Hoisting the assignment up
in cert_parse_ee_cert() for this is very simple.

In cert_parse_pre() we own the X509 from the start, so we take an extra
reference which we must release before exit. In the error path there's
an X509_free() and cert_free() releases the extra reference.

Again this will become a bit simpler in a few more steps.

diff --git a/usr.sbin/rpki-client/cert.c b/usr.sbin/rpki-client/cert.c
index 388e9099795..3c2693d5af9 100644
--- a/usr.sbin/rpki-client/cert.c
+++ b/usr.sbin/rpki-client/cert.c
@@ -1198,8 +1198,9 @@ cert_check_purpose(const char *fn, X509 *x)
  */
 
 static int
-cert_parse_extensions(const char *fn, struct cert *cert, X509 *x)
+cert_parse_extensions(const char *fn, struct cert *cert)
 {
+	X509		*x = cert->x509;
 	X509_EXTENSION	*ext;
 	ASN1_OBJECT	*obj;
 	int		 extsz, i, nid;
@@ -1327,6 +1328,14 @@ cert_parse_ee_cert(const char *fn, int talid, X509 *x)
 	if ((cert = calloc(1, sizeof(struct cert))) == NULL)
 		err(1, NULL);
 
+	if (!X509_up_ref(x)) {
+		warnx("%s: X509_up_ref failed", fn);
+		goto out;
+	}
+
+	cert->x509 = x;
+	cert->talid = talid;
+
 	if (X509_get_version(x) != 2) {
 		warnx("%s: RFC 6487 4.1: X.509 version must be v3", fn);
 		goto out;
@@ -1335,7 +1344,7 @@ cert_parse_ee_cert(const char *fn, int talid, X509 *x)
 	if (!cert_check_subject_and_issuer(fn, x))
 		goto out;
 
-	if (!cert_parse_extensions(fn, cert, x))
+	if (!cert_parse_extensions(fn, cert))
 		goto out;
 
 	if (cert->purpose != CERT_PURPOSE_EE) {
@@ -1344,14 +1353,6 @@ cert_parse_ee_cert(const char *fn, int talid, X509 *x)
 		goto out;
 	}
 
-	if (!X509_up_ref(x)) {
-		warnx("%s: X509_up_ref failed", fn);
-		goto out;
-	}
-
-	cert->x509 = x;
-	cert->talid = talid;
-
 	if (!constraints_validate(fn, cert))
 		goto out;
 
@@ -1395,6 +1396,12 @@ cert_parse_pre(const char *fn, const unsigned char *der, size_t len)
 		goto out;
 	}
 
+	if (!X509_up_ref(x)) {
+		warnx("%s: X509_up_ref failed", fn);
+		goto out;
+	}
+	cert->x509 = x;
+
 	if (!x509_cache_extensions(x, fn))
 		goto out;
 
@@ -1430,7 +1437,7 @@ cert_parse_pre(const char *fn, const unsigned char *der, size_t len)
 	if (!cert_check_subject_and_issuer(fn, x))
 		goto out;
 
-	if (!cert_parse_extensions(fn, cert, x))
+	if (!cert_parse_extensions(fn, cert))
 		goto out;
 
 	if (!x509_get_aki(x, fn, &cert->aki))
@@ -1494,7 +1501,7 @@ cert_parse_pre(const char *fn, const unsigned char *der, size_t len)
 		goto out;
 	}
 
-	cert->x509 = x;
+	X509_free(x);
 	return cert;
 
  out: