From: Theo Buehler Subject: rpki-client: split extension parsing out of cert_parse_pre() To: tech@openbsd.org Date: Mon, 30 Jun 2025 13:34:33 +0200 Entirely straightforward refactor: extract a helper from cert_parse_pre() so it can be shared with cert_parse_ee_cert(). I haven't hooked this into cert_parse_ee_cert() yet because that makes the diff messy. It will be the next step. Index: cert.c =================================================================== RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v diff -u -p -r1.174 cert.c --- cert.c 30 Jun 2025 11:15:47 -0000 1.174 +++ cert.c 30 Jun 2025 11:21:09 -0000 @@ -1087,6 +1087,130 @@ cert_check_purpose(const char *fn, X509 } /* + * Parse extensions in a resource certificate following RFC 6487, section 4.8. + * + * Check issuance, basic constraints, and key usage bits are consistent and + * determine what kind of cert we were passed: annoyingly, callers can't + * distinguish BGPsec router certs (a rare kind of EE cert) from CA certs, + * as they both are .cer files in a Manifest. + * + * Then walk the list of extensions, handle each one and ensure there are no + * duplicates. Store the relevant information in cert. + */ + +static int +cert_parse_extensions(const char *fn, struct cert *cert, X509 *x) +{ + X509_EXTENSION *ext; + ASN1_OBJECT *obj; + int extsz, i, nid; + int bc, ski, aki, ku, eku, crldp, aia, sia, cp, ip, as; + + nid = bc = ski = aki = ku = eku = crldp = aia = sia = cp = ip = as = 0; + + /* + * Check issuance, basic constraints and (extended) key usage bits are + * appropriate for a resource cert. Covers RFC 6487 4.8.1, 4.8.4, 4.8.5. + */ + if ((cert->purpose = cert_check_purpose(fn, x)) == CERT_PURPOSE_INVALID) + goto out; + + /* Look for X509v3 extensions. */ + if ((extsz = X509_get_ext_count(x)) <= 0) { + warnx("%s: certificate without X.509v3 extensions", fn); + goto out; + } + + for (i = 0; i < extsz; i++) { + ext = X509_get_ext(x, i); + assert(ext != NULL); + obj = X509_EXTENSION_get_object(ext); + assert(obj != NULL); + + /* The switch is ordered following RFC 6487, section 4.8. */ + switch (nid = OBJ_obj2nid(obj)) { + case NID_basic_constraints: + if (bc++ > 0) + goto dup; + break; + case NID_subject_key_identifier: + if (ski++ > 0) + goto dup; + break; + case NID_authority_key_identifier: + if (aki++ > 0) + goto dup; + break; + case NID_key_usage: + if (ku++ > 0) + goto dup; + break; + case NID_ext_key_usage: + if (eku++ > 0) + goto dup; + break; + case NID_crl_distribution_points: + if (crldp++ > 0) + goto dup; + break; + case NID_info_access: + if (aia++ > 0) + goto dup; + if (!cert_aia(fn, cert, ext)) + goto out; + break; + case NID_sinfo_access: + if (sia++ > 0) + goto dup; + if (!cert_sia(fn, cert, ext)) + goto out; + break; + case NID_certificate_policies: + if (cp++ > 0) + goto dup; + if (!certificate_policies(fn, cert, ext)) + goto out; + break; + case NID_sbgp_ipAddrBlock: + if (ip++ > 0) + goto dup; + if (!sbgp_ipaddrblk(fn, cert, ext)) + goto out; + break; + case NID_sbgp_autonomousSysNum: + if (as++ > 0) + goto dup; + if (!sbgp_assysnum(fn, cert, ext)) + goto out; + break; + default: + /* unexpected extensions warrant investigation */ + { + char objn[64]; + + OBJ_obj2txt(objn, sizeof(objn), obj, 0); + if (X509_EXTENSION_get_critical(ext)) { + warnx("%s: unknown critical extension " + "%s (NID %d)", fn, objn, nid); + goto out; + } + warnx("%s: ignoring %s (NID %d)", + fn, objn, nid); + } + break; + } + } + + return 1; + + dup: + warnx("%s: RFC 5280 section 4.2: duplicate extension: %s", fn, + nid2str(nid)); + out: + return 0; +} + +/* * Lightweight version of cert_parse_pre() for EE certs. * Parses the two RFC 3779 extensions, and performs some sanity checks. * Returns cert on success and NULL on failure. @@ -1170,16 +1294,10 @@ cert_parse_pre(const char *fn, const uns struct cert *cert; const unsigned char *oder; size_t j; - int i, extsz; X509 *x = NULL; - X509_EXTENSION *ext = NULL; const ASN1_BIT_STRING *issuer_uid = NULL, *subject_uid = NULL; - ASN1_OBJECT *obj; EVP_PKEY *pkey; - int nid, bc, ski, aki, ku, eku, crldp, aia, sia, - cp, ip, as; - - nid = bc = ski = aki = ku = eku = crldp = aia = sia = cp = ip = as = 0; + int nid; /* just fail for empty buffers, the warning was printed elsewhere */ if (der == NULL) @@ -1233,98 +1351,8 @@ cert_parse_pre(const char *fn, const uns if (!cert_check_subject_and_issuer(fn, x)) goto out; - /* - * Check issuance, basic constraints and (extended) key usage bits are - * appropriate for a resource cert. Covers RFC 6487 4.8.1, 4.8.4, 4.8.5. - */ - if ((cert->purpose = cert_check_purpose(fn, x)) == CERT_PURPOSE_INVALID) - goto out; - - /* Look for X509v3 extensions. */ - if ((extsz = X509_get_ext_count(x)) <= 0) { - warnx("%s: certificate without X.509v3 extensions", fn); + if (!cert_parse_extensions(fn, cert, x)) goto out; - } - - for (i = 0; i < extsz; i++) { - ext = X509_get_ext(x, i); - assert(ext != NULL); - obj = X509_EXTENSION_get_object(ext); - assert(obj != NULL); - - /* The switch is ordered following RFC 6487, section 4.8. */ - switch (nid = OBJ_obj2nid(obj)) { - case NID_basic_constraints: - if (bc++ > 0) - goto dup; - break; - case NID_subject_key_identifier: - if (ski++ > 0) - goto dup; - break; - case NID_authority_key_identifier: - if (aki++ > 0) - goto dup; - break; - case NID_key_usage: - if (ku++ > 0) - goto dup; - break; - case NID_ext_key_usage: - if (eku++ > 0) - goto dup; - break; - case NID_crl_distribution_points: - if (crldp++ > 0) - goto dup; - break; - case NID_info_access: - if (aia++ > 0) - goto dup; - if (!cert_aia(fn, cert, ext)) - goto out; - break; - case NID_sinfo_access: - if (sia++ > 0) - goto dup; - if (!cert_sia(fn, cert, ext)) - goto out; - break; - case NID_certificate_policies: - if (cp++ > 0) - goto dup; - if (!certificate_policies(fn, cert, ext)) - goto out; - break; - case NID_sbgp_ipAddrBlock: - if (ip++ > 0) - goto dup; - if (!sbgp_ipaddrblk(fn, cert, ext)) - goto out; - break; - case NID_sbgp_autonomousSysNum: - if (as++ > 0) - goto dup; - if (!sbgp_assysnum(fn, cert, ext)) - goto out; - break; - default: - /* unexpected extensions warrant investigation */ - { - char objn[64]; - - OBJ_obj2txt(objn, sizeof(objn), obj, 0); - if (X509_EXTENSION_get_critical(ext)) { - warnx("%s: unknown critical extension " - "%s (NID %d)", fn, objn, nid); - goto out; - } - warnx("%s: ignoring %s (NID %d)", - fn, objn, nid); - } - break; - } - } if (!x509_get_aki(x, fn, &cert->aki)) goto out; @@ -1392,9 +1420,6 @@ cert_parse_pre(const char *fn, const uns cert->x509 = x; return cert; - dup: - warnx("%s: RFC 5280 section 4.2: duplicate extension: %s", fn, - nid2str(nid)); out: cert_free(cert); X509_free(x);