From: Theo Buehler <tb@theobuehler.org>
Subject: rpki-client: handle SKI and AKI in cert_parse_extensions()
To: tech@openbsd.org
Date: Wed, 2 Jul 2025 13:46:05 +0200

This completes the parsing switch by adding versions of x509_get_ski() and
x509_get_aki(). There's not much to say. I think the new implementations
are slightly less tricky and cert_aki() benefits from removing superfluous
variables. The old versions will be promoted to the attic soon.

With this diff all cert types finally inspect all the extensions relevant
to the RPKI.

Index: cert.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v
diff -u -p -r1.180 cert.c
--- cert.c	2 Jul 2025 11:23:25 -0000	1.180
+++ cert.c	2 Jul 2025 11:25:01 -0000
@@ -501,6 +501,96 @@ sbgp_ipaddrblk(const char *fn, struct ce
 	return rc;
 }
 
+static int
+cert_ski(const char *fn, struct cert *cert, X509_EXTENSION *ext)
+{
+	ASN1_OCTET_STRING	*os = NULL;
+	unsigned char		 md[EVP_MAX_MD_SIZE];
+	unsigned int		 md_len = EVP_MAX_MD_SIZE;
+	int			 rc = 0;
+
+	assert(cert->ski == NULL);
+
+	if (X509_EXTENSION_get_critical(ext)) {
+		warnx("%s: RFC 6487 section 4.8.2: "
+		    "SKI: extension not non-critical", fn);
+		goto out;
+	}
+
+	if ((os = X509V3_EXT_d2i(ext)) == NULL) {
+		warnx("%s: RFC 6487 section 4.8.2: error parsing SKI", fn);
+		goto out;
+	}
+
+	if (!X509_pubkey_digest(cert->x509, EVP_sha1(), md, &md_len)) {
+		warnx("%s: X509_pubkey_digest", fn);
+		goto out;
+	}
+
+	if (os->length < 0 || md_len != (unsigned int)os->length) {
+		warnx("%s: RFC 6487 section 4.8.2: SKI: "
+		    "want %u bytes SHA1 hash, have %d bytes",
+		    fn, md_len, os->length);
+		goto out;
+	}
+
+	if (memcmp(os->data, md, md_len) != 0) {
+		warnx("%s: SKI does not match SHA1 hash of SPK", fn);
+		goto out;
+	}
+
+	cert->ski = hex_encode(md, md_len);
+
+	rc = 1;
+ out:
+	ASN1_OCTET_STRING_free(os);
+	return rc;
+}
+
+static int
+cert_aki(const char *fn, struct cert *cert, X509_EXTENSION *ext)
+{
+	AUTHORITY_KEYID	*akid = NULL;
+	int		 rc = 0;
+
+	assert(cert->aki == NULL);
+
+	if (X509_EXTENSION_get_critical(ext)) {
+		warnx("%s: RFC 6487 section 4.8.3: "
+		    "AKI extension not non-critical", fn);
+		goto out;
+	}
+
+	if ((akid = X509V3_EXT_d2i(ext)) == NULL) {
+		warnx("%s: RFC 6487 section 4.8.3: error parsing AKI", fn);
+		goto out;
+	}
+	if (akid->issuer != NULL || akid->serial != NULL) {
+		warnx("%s: RFC 6487 section 4.8.3: AKI: authorityCertIssuer or "
+		    "authorityCertSerialNumber present", fn);
+		goto out;
+	}
+
+	if (akid->keyid == NULL || akid->keyid->data == NULL) {
+		warnx("%s: RFC 6487 section 4.8.3: AKI: Key Identifier missing",
+		    fn);
+		goto out;
+	}
+	if (akid->keyid->length != SHA_DIGEST_LENGTH) {
+		warnx("%s: RFC 6487 section 4.8.3: AKI: "
+		    "want %d bytes SHA1 hash, have %d bytes",
+		    fn, SHA_DIGEST_LENGTH, akid->keyid->length);
+		goto out;
+	}
+
+	cert->aki = hex_encode(akid->keyid->data, akid->keyid->length);
+
+	rc = 1;
+ out:
+	AUTHORITY_KEYID_free(akid);
+	return rc;
+}
+
 /*
  * Parse CRL distribution point per RFC 6487, section 4.8.6.
  */
@@ -1237,10 +1327,14 @@ cert_parse_extensions(const char *fn, st
 		case NID_subject_key_identifier:
 			if (ski++ > 0)
 				goto dup;
+			if (!cert_ski(fn, cert, ext))
+				goto out;
 			break;
 		case NID_authority_key_identifier:
 			if (aki++ > 0)
 				goto dup;
+			if (!cert_aki(fn, cert, ext))
+				goto out;
 			break;
 		case NID_key_usage:
 			if (ku++ > 0)
@@ -1440,10 +1534,6 @@ cert_parse_pre(const char *fn, const uns
 	if (!cert_parse_extensions(fn, cert))
 		goto out;
 
-	if (!x509_get_aki(x, fn, &cert->aki))
-		goto out;
-	if (!x509_get_ski(x, fn, &cert->ski))
-		goto out;
 	if (!x509_get_notbefore(x, fn, &cert->notbefore))
 		goto out;
 	if (!x509_get_notafter(x, fn, &cert->notafter))