Index | Thread | Search

From:
Theo Buehler <tb@theobuehler.org>
Subject:
rpki-client: parse SIA also for EE certs
To:
tech@openbsd.org
Date:
Mon, 23 Jun 2025 09:16:06 +0200

Download raw body.

Thread
This depends on "rpki-client: cert_check_purpose tweaks"
https://marc.info/?l=openbsd-tech&m=175033387518906&w=2

Parse SIA extensions for EE certificates. There should only be (perhaps
multiple) id-ad-signed-Object access methods, but unfortunately the
ecosystem is polluted with currently roughly 50k EE certs containing
an rpkiNotify access method. So we need to continue to tolerate that.

Rename sbgp_sia() (whose name doesn't make much sense) to cert_ca_sia().
Add cert_ee_sia() which is similar to cert_ca_sia() and x509_get_sia().
There's duplication of code and work because of the latter. I am going
to remove x509_get_sia() further down the road, which will make use of
the new cert->signedobj member. Since it's only for EE certs, there's
no need to transfer it over the pipes.

Likewise, cert_parse_ee_cert() and cert_parse_pre() will see quite a bit
of deduplication.

diff --git a/usr.sbin/rpki-client/cert.c b/usr.sbin/rpki-client/cert.c
index af9c34e26fc..c4c55d505d0 100644
--- a/usr.sbin/rpki-client/cert.c
+++ b/usr.sbin/rpki-client/cert.c
@@ -34,6 +34,7 @@ extern ASN1_OBJECT	*bgpsec_oid;	/* id-kp-bgpsec-router Key Purpose */
 extern ASN1_OBJECT	*certpol_oid;	/* id-cp-ipAddr-asNumber cert policy */
 extern ASN1_OBJECT	*carepo_oid;	/* 1.3.6.1.5.5.7.48.5 (caRepository) */
 extern ASN1_OBJECT	*manifest_oid;	/* 1.3.6.1.5.5.7.48.10 (rpkiManifest) */
+extern ASN1_OBJECT	*signedobj_oid;	/* 1.3.6.1.5.5.7.48.11 (signedObject) */
 extern ASN1_OBJECT	*notify_oid;	/* 1.3.6.1.5.5.7.48.13 (rpkiNotify) */
 
 int certid = TALSZ_MAX;
@@ -505,7 +506,7 @@ sbgp_ipaddrblk(const char *fn, struct cert *cert, X509_EXTENSION *ext)
  * Returns zero on failure, non-zero on success.
  */
 static int
-sbgp_sia(const char *fn, struct cert *cert, X509_EXTENSION *ext)
+cert_ca_sia(const char *fn, struct cert *cert, X509_EXTENSION *ext)
 {
 	AUTHORITY_INFO_ACCESS	*sia = NULL;
 	ACCESS_DESCRIPTION	*ad;
@@ -638,6 +639,125 @@ sbgp_sia(const char *fn, struct cert *cert, X509_EXTENSION *ext)
 	return rc;
 }
 
+/*
+ * Parse "Subject Information Access" extension for an EE cert,
+ * RFC 6487, section 4.8.8.2 and RFC 8182, section 3.2.
+ * Returns zero on failure, non-zero on success.
+ */
+static int
+cert_ee_sia(const char *fn, struct cert *cert, X509_EXTENSION *ext)
+{
+	AUTHORITY_INFO_ACCESS	*sia = NULL;
+	ACCESS_DESCRIPTION	*ad;
+	ASN1_OBJECT		*oid;
+	char			*signedobj = NULL;
+	int			 i, rc = 0;
+
+	assert(cert->signedobj == NULL);
+
+	if (X509_EXTENSION_get_critical(ext)) {
+		warnx("%s: RFC 6487 section 4.8.8: SIA: "
+		    "extension not non-critical", fn);
+		goto out;
+	}
+
+	if ((sia = X509V3_EXT_d2i(ext)) == NULL) {
+		warnx("%s: RFC 6487 section 4.8.8: SIA: failed extension parse",
+		    fn);
+		goto out;
+	}
+
+	for (i = 0; i < sk_ACCESS_DESCRIPTION_num(sia); i++) {
+		ad = sk_ACCESS_DESCRIPTION_value(sia, i);
+
+		oid = ad->method;
+
+		/*
+		 * XXX: RFC 6487 4.8.8.2 states that the accessMethod MUST be
+		 * signedObject. However, rpkiNotify accessMethods currently
+		 * exist in the wild. Consider removing this special case.
+		 * See also https://www.rfc-editor.org/errata/eid7239.
+		 */
+		if (OBJ_cmp(oid, notify_oid) == 0) {
+			if (verbose > 1)
+				warnx("%s: RFC 6487 section 4.8.8.2: SIA should"
+				    " not contain rpkiNotify accessMethod", fn);
+			continue;
+		} else if (OBJ_cmp(oid, signedobj_oid) == 0) {
+			if (!x509_location(fn, "SIA: signedObject",
+			    ad->location, &signedobj))
+				goto out;
+			if (cert->signedobj == NULL && strncasecmp(signedobj,
+			    RSYNC_PROTO, RSYNC_PROTO_LEN) == 0) {
+				cert->signedobj = signedobj;
+				signedobj = NULL;
+				continue;
+			}
+			if (verbose)
+				warnx("%s: RFC 6487 section 4.8.8: SIA: "
+				    "ignoring location %s", fn, signedobj);
+			free(signedobj);
+			signedobj = NULL;
+		} else {
+			char buf[128];
+
+			OBJ_obj2txt(buf, sizeof(buf), oid, 0);
+			warnx("%s: RFC 6487 section 4.8.8.1: unexpected"
+			    " accessMethod: %s", fn, buf);
+			goto out;
+		}
+	}
+
+	if (cert->signedobj == NULL) {
+		warnx("%s: RFC 6487 section 4.8.8: SIA: no signedObject", fn);
+		goto out;
+	}
+
+	if (filemode) {
+		if (rtype_from_file_extension(cert->signedobj) !=
+		    rtype_from_file_extension(fn)) {
+			warnx("%s: SIA signedObject contains unexpected "
+			    "filename extension", fn);
+			goto out;
+		}
+	} else {
+		const char *p = cert->signedobj + RSYNC_PROTO_LEN;
+		size_t fnlen, plen;
+
+		fnlen = strlen(fn);
+		plen = strlen(p);
+
+		if (fnlen < plen || strcmp(p, fn + fnlen - plen) != 0) {
+			warnx("%s: mismatch between pathname and SIA (%s)",
+			    fn, cert->signedobj);
+			goto out;
+		}
+	}
+
+	rc = 1;
+ out:
+	AUTHORITY_INFO_ACCESS_free(sia);
+	return rc;
+}
+
+static int
+cert_sia(const char *fn, struct cert *cert, X509_EXTENSION *ext)
+{
+	switch (cert->purpose) {
+	case CERT_PURPOSE_TA:
+	case CERT_PURPOSE_CA:
+		return cert_ca_sia(fn, cert, ext);
+	case CERT_PURPOSE_EE:
+		return cert_ee_sia(fn, cert, ext);
+	case CERT_PURPOSE_BGPSEC_ROUTER:
+		warnx("%s: RFC 8209, 3.1.3.3, SIA MUST be omitted from "
+		    "BGPSec router certs", fn);
+		return 0;
+	default:
+		abort();
+	}
+}
+
 /*
  * Parse the certificate policies extension and check that it follows RFC 7318.
  * Returns zero on failure, non-zero on success.
@@ -922,6 +1042,12 @@ cert_parse_ee_cert(const char *fn, int talid, X509 *x)
 		goto out;
 	}
 
+	index = X509_get_ext_by_NID(x, NID_sinfo_access, -1);
+	if ((ext = X509_get_ext(x, index)) != NULL) {
+		if (!cert_sia(fn, cert, ext))
+			goto out;
+	}
+
 	index = X509_get_ext_by_NID(x, NID_sbgp_ipAddrBlock, -1);
 	if ((ext = X509_get_ext(x, index)) != NULL) {
 		if (!sbgp_ipaddrblk(fn, cert, ext))
@@ -1074,11 +1200,7 @@ cert_parse_pre(const char *fn, const unsigned char *der, size_t len)
 		case NID_sinfo_access:
 			if (sia++ > 0)
 				goto dup;
-			/*
-			 * This will fail for BGPsec certs, but they must omit
-			 * this extension anyway (RFC 8209, section 3.1.3.3).
-			 */
-			if (!sbgp_sia(fn, cert, ext))
+			if (!cert_sia(fn, cert, ext))
 				goto out;
 			break;
 		case NID_certificate_policies:
@@ -1162,11 +1284,6 @@ cert_parse_pre(const char *fn, const unsigned char *der, size_t len)
 				goto out;
 			}
 		}
-		if (sia) {
-			warnx("%s: unexpected SIA extension in BGPsec cert",
-			    fn);
-			goto out;
-		}
 		break;
 	case CERT_PURPOSE_EE:
 		warnx("%s: unexpected EE cert", fn);
@@ -1316,6 +1433,7 @@ cert_free(struct cert *p)
 	free(p->path);
 	free(p->mft);
 	free(p->notify);
+	free(p->signedobj);
 	free(p->ips);
 	free(p->ases);
 	free(p->aia);
diff --git a/usr.sbin/rpki-client/extern.h b/usr.sbin/rpki-client/extern.h
index 23edd1fc602..9e2c64c4e6d 100644
--- a/usr.sbin/rpki-client/extern.h
+++ b/usr.sbin/rpki-client/extern.h
@@ -134,6 +134,7 @@ struct cert {
 	char		*mft; /* manifest (rsync:// uri) */
 	char		*notify; /* RRDP notify (https:// uri) */
 	char		*crl; /* CRL location (rsync:// or NULL) */
+	char		*signedobj; /* rsync access location for EE certs. */
 	char		*aia; /* AIA (or NULL, for trust anchor) */
 	char		*aki; /* AKI (or NULL, for trust anchor) */
 	char		*ski; /* SKI */