Index | Thread | Search

From:
Theo Buehler <tb@theobuehler.org>
Subject:
rpki-client: clean up aspa handling
To:
tech@openbsd.org
Date:
Fri, 18 Jul 2025 16:26:55 +0200

Download raw body.

Thread
To make the big cert ** diff easier to review I took a bit of a shortcut
and left some ugliness in the parsing functions - which job of course
noticed and complained about. This can be cleaned up for all signed
object handlers (except mfts) similarly to the below diff for aspa.

I'm sending this one out to get bikesheds out of the way and then align
all the others with what we agree on.

Again the point is that cert_parse_ee_cert() parses all this stuff
already and ensures that the required fields are present, so there's no
reason to copy it in aspa_parse() and even transfer all this info across
the pipes in normal mode, since we only care about this information in
file mode. It probably doesn't matter a lot since signed objects are
rare, except of course roas and manifests.

Most of this should be uncontroversial except for the aspa_print()
function where I used 'struct cert *c' rather than 'struct cert *cert'
to avoid a lot of wrapping noise and make the diff more mechanical.
I think it is fine but I admit that I have no real preference here since
all this printing code is in dire need of being rewritten (especially
deduplicated).

There are probably other members of struct aspa & co that can be
reconsidered, but let's get these six out of the way first.

Index: usr.sbin/rpki-client/aspa.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/aspa.c,v
diff -u -p -r1.33 aspa.c
--- usr.sbin/rpki-client/aspa.c	18 Jul 2025 12:20:32 -0000	1.33
+++ usr.sbin/rpki-client/aspa.c	18 Jul 2025 14:03:40 -0000
@@ -183,17 +183,6 @@ aspa_parse(struct cert **out_cert, const
 
 	aspa->signtime = signtime;
 
-	aspa->aia = strdup(cert->aia);
-	aspa->aki = strdup(cert->aki);
-	aspa->sia = strdup(cert->signedobj);
-	aspa->ski = strdup(cert->ski);
-	if (aspa->aia == NULL || aspa->aki == NULL || aspa->sia == NULL ||
-	    aspa->ski == NULL)
-		err(1, NULL);
-
-	aspa->notbefore = cert->notbefore;
-	aspa->notafter = cert->notafter;
-
 	if (cert->num_ips > 0) {
 		warnx("%s: superfluous IP Resources extension present", fn);
 		goto out;
@@ -233,10 +222,6 @@ aspa_free(struct aspa *p)
 	if (p == NULL)
 		return;
 
-	free(p->aia);
-	free(p->aki);
-	free(p->sia);
-	free(p->ski);
 	free(p->providers);
 	free(p);
 }
@@ -256,10 +241,6 @@ aspa_buffer(struct ibuf *b, const struct
 	io_simple_buffer(b, &p->num_providers, sizeof(size_t));
 	io_simple_buffer(b, p->providers,
 	    p->num_providers * sizeof(p->providers[0]));
-
-	io_str_buffer(b, p->aia);
-	io_str_buffer(b, p->aki);
-	io_str_buffer(b, p->ski);
 }
 
 /*
@@ -289,11 +270,6 @@ aspa_read(struct ibuf *b)
 		io_read_buf(b, p->providers,
 		    p->num_providers * sizeof(p->providers[0]));
 	}
-
-	io_read_str(b, &p->aia);
-	io_read_str(b, &p->aki);
-	io_read_str(b, &p->ski);
-	assert(p->aia && p->aki && p->ski);
 
 	return p;
 }
Index: usr.sbin/rpki-client/parser.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/parser.c,v
diff -u -p -r1.164 parser.c
--- usr.sbin/rpki-client/parser.c	18 Jul 2025 12:20:32 -0000	1.164
+++ usr.sbin/rpki-client/parser.c	18 Jul 2025 14:03:41 -0000
@@ -790,7 +790,7 @@ proc_parser_aspa(char *file, const unsig
 	if ((aspa = aspa_parse(&cert, file, entp->talid, der, len)) == NULL)
 		goto out;
 
-	a = find_issuer(file, entp->certid, aspa->aki, entp->mftaki);
+	a = find_issuer(file, entp->certid, cert->aki, entp->mftaki);
 	if (a == NULL)
 		goto out;
 	crl = crl_get(&crls, a);
@@ -799,12 +799,13 @@ proc_parser_aspa(char *file, const unsig
 		warnx("%s: %s", file, errstr);
 		goto out;
 	}
-	cert_free(cert);
-	cert = NULL;
 
 	aspa->talid = a->cert->talid;
 
-	aspa->expires = x509_find_expires(aspa->notafter, a, &crls);
+	aspa->expires = x509_find_expires(cert->notafter, a, &crls);
+
+	cert_free(cert);
+	cert = NULL;
 
 	return aspa;
 
Index: usr.sbin/rpki-client/extern.h
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v
diff -u -p -r1.250 extern.h
--- usr.sbin/rpki-client/extern.h	18 Jul 2025 13:19:59 -0000	1.250
+++ usr.sbin/rpki-client/extern.h	18 Jul 2025 14:03:41 -0000
@@ -411,16 +411,10 @@ struct gbr {
 struct aspa {
 	int			 valid; /* contained in issuer auth */
 	int			 talid; /* TAL the ASPA is chained up to */
-	char			*aia; /* AIA */
-	char			*aki; /* AKI */
-	char			*sia; /* SIA signedObject */
-	char			*ski; /* SKI */
 	uint32_t		 custasid; /* the customerASID */
 	uint32_t		*providers; /* the providers */
 	size_t			 num_providers;
 	time_t			 signtime; /* CMS signing-time attribute */
-	time_t			 notbefore; /* EE cert's Not Before */
-	time_t			 notafter; /* notAfter of the ASPA EE cert */
 	time_t			 expires; /* when the signature path expires */
 };
 
@@ -977,7 +971,7 @@ void		 mft_print(const X509 *, const str
 void		 roa_print(const X509 *, const struct roa *);
 void		 gbr_print(const X509 *, const struct gbr *);
 void		 rsc_print(const X509 *, const struct rsc *);
-void		 aspa_print(const X509 *, const struct aspa *);
+void		 aspa_print(const struct cert *, const struct aspa *);
 void		 tak_print(const X509 *, const struct tak *);
 void		 geofeed_print(const X509 *, const struct geofeed *);
 void		 spl_print(const X509 *, const struct spl *);
Index: usr.sbin/rpki-client/filemode.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/filemode.c,v
diff -u -p -r1.64 filemode.c
--- usr.sbin/rpki-client/filemode.c	18 Jul 2025 12:20:32 -0000	1.64
+++ usr.sbin/rpki-client/filemode.c	18 Jul 2025 14:03:41 -0000
@@ -400,10 +400,10 @@ proc_parser_file(char *file, unsigned ch
 		aspa = aspa_parse(&cert, file, -1, buf, len);
 		if (aspa == NULL)
 			break;
-		aia = aspa->aia;
+		aia = cert->aia;
 		expires = &aspa->expires;
-		notbefore = &aspa->notbefore;
-		notafter = &aspa->notafter;
+		notbefore = &cert->notbefore;
+		notafter = &cert->notafter;
 		break;
 	case RTYPE_CER:
 		cert = cert_parse(file, buf, len);
@@ -553,7 +553,7 @@ proc_parser_file(char *file, unsigned ch
 
 		switch (type) {
 		case RTYPE_ASPA:
-			aspa_print(cert->x509, aspa);
+			aspa_print(cert, aspa);
 			break;
 		case RTYPE_CER:
 			cert_print(cert);
Index: usr.sbin/rpki-client/print.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/print.c,v
diff -u -p -r1.61 print.c
--- usr.sbin/rpki-client/print.c	16 Jun 2025 14:50:56 -0000	1.61
+++ usr.sbin/rpki-client/print.c	18 Jul 2025 14:03:42 -0000
@@ -720,37 +720,37 @@ rsc_print(const X509 *x, const struct rs
 }
 
 void
-aspa_print(const X509 *x, const struct aspa *p)
+aspa_print(const struct cert *c, const struct aspa *p)
 {
 	size_t	i;
 
 	if (outformats & FORMAT_JSON) {
 		json_do_string("type", "aspa");
-		json_do_string("ski", p->ski);
-		x509_print(x);
-		json_do_string("aki", p->aki);
-		json_do_string("aia", p->aia);
-		json_do_string("sia", p->sia);
+		json_do_string("ski", c->ski);
+		x509_print(c->x509);
+		json_do_string("aki", c->aki);
+		json_do_string("aia", c->aia);
+		json_do_string("sia", c->signedobj);
 		if (p->signtime != 0)
 			json_do_int("signing_time", p->signtime);
-		json_do_int("valid_since", p->notbefore);
-		json_do_int("valid_until", p->notafter);
+		json_do_int("valid_since", c->notbefore);
+		json_do_int("valid_until", c->notafter);
 		if (p->expires)
 			json_do_int("expires", p->expires);
 		json_do_uint("customer_asid", p->custasid);
 		json_do_array("providers");
 	} else {
-		printf("Subject key identifier:   %s\n", pretty_key_id(p->ski));
-		x509_print(x);
-		printf("Authority key identifier: %s\n", pretty_key_id(p->aki));
-		printf("Authority info access:    %s\n", p->aia);
-		printf("Subject info access:      %s\n", p->sia);
+		printf("Subject key identifier:   %s\n", pretty_key_id(c->ski));
+		x509_print(c->x509);
+		printf("Authority key identifier: %s\n", pretty_key_id(c->aki));
+		printf("Authority info access:    %s\n", c->aia);
+		printf("Subject info access:      %s\n", c->signedobj);
 		if (p->signtime != 0)
 			printf("Signing time:             %s\n",
 			    time2str(p->signtime));
 		printf("ASPA not before:          %s\n",
-		    time2str(p->notbefore));
-		printf("ASPA not after:           %s\n", time2str(p->notafter));
+		    time2str(c->notbefore));
+		printf("ASPA not after:           %s\n", time2str(c->notafter));
 		printf("Customer ASID:            %u\n", p->custasid);
 		printf("Providers:                ");
 	}
Index: regress/usr.sbin/rpki-client/test-aspa.c
===================================================================
RCS file: /cvs/src/regress/usr.sbin/rpki-client/test-aspa.c,v
diff -u -p -r1.9 test-aspa.c
--- regress/usr.sbin/rpki-client/test-aspa.c	18 Jul 2025 12:22:07 -0000	1.9
+++ regress/usr.sbin/rpki-client/test-aspa.c	18 Jul 2025 14:03:42 -0000
@@ -77,7 +77,7 @@ main(int argc, char *argv[])
 			break;
 		}
 		if (verb)
-			aspa_print(cert->x509, p);
+			aspa_print(cert, p);
 		if (ppem) {
 			if (!PEM_write_X509(stdout, cert->x509))
 				errx(1, "PEM_write_X509: unable to write cert");