From 2b470c611106a01b62bdfe1890ced529f0879992 Mon Sep 17 00:00:00 2001 From: tb Date: Fri, 16 Feb 2024 14:48:47 +0000 Subject: [PATCH] Remove struct parse from cert.c This is one of those weird things that metastasized throughout the code base. job is about to introduce the 9th incompatible copy of it. Enough is enough. It doesn't help anything. looks good to claudio ok clang --- usr.sbin/rpki-client/cert.c | 191 ++++++++++++++++-------------------- 1 file changed, 87 insertions(+), 104 deletions(-) diff --git a/usr.sbin/rpki-client/cert.c b/usr.sbin/rpki-client/cert.c index f9d6037d2c7..3c71238a81c 100644 --- a/usr.sbin/rpki-client/cert.c +++ b/usr.sbin/rpki-client/cert.c @@ -1,4 +1,4 @@ -/* $OpenBSD: cert.c,v 1.126 2024/02/16 11:55:42 tb Exp $ */ +/* $OpenBSD: cert.c,v 1.127 2024/02/16 14:48:47 tb Exp $ */ /* * Copyright (c) 2022 Theo Buehler * Copyright (c) 2021 Job Snijders @@ -29,14 +29,6 @@ #include "extern.h" -/* - * A parsing sequence of a file (which may just be ). - */ -struct parse { - struct cert *res; /* result */ - const char *fn; /* currently-parsed file */ -}; - 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) */ @@ -245,25 +237,24 @@ sbgp_parse_assysnum(const char *fn, const ASIdentifiers *asidentifiers, * Returns zero on failure, non-zero on success. */ static int -sbgp_assysnum(struct parse *p, X509_EXTENSION *ext) +sbgp_assysnum(const char *fn, struct cert *cert, X509_EXTENSION *ext) { ASIdentifiers *asidentifiers = NULL; int rc = 0; if (!X509_EXTENSION_get_critical(ext)) { warnx("%s: RFC 6487 section 4.8.11: autonomousSysNum: " - "extension not critical", p->fn); + "extension not critical", fn); goto out; } if ((asidentifiers = X509V3_EXT_d2i(ext)) == NULL) { warnx("%s: RFC 6487 section 4.8.11: autonomousSysNum: " - "failed extension parse", p->fn); + "failed extension parse", fn); goto out; } - if (!sbgp_parse_assysnum(p->fn, asidentifiers, - &p->res->as, &p->res->asz)) + if (!sbgp_parse_assysnum(fn, asidentifiers, &cert->as, &cert->asz)) goto out; rc = 1; @@ -470,28 +461,28 @@ sbgp_parse_ipaddrblk(const char *fn, const IPAddrBlocks *addrblk, * Returns zero on failure, non-zero on success. */ static int -sbgp_ipaddrblk(struct parse *p, X509_EXTENSION *ext) +sbgp_ipaddrblk(const char *fn, struct cert *cert, X509_EXTENSION *ext) { IPAddrBlocks *addrblk = NULL; int rc = 0; if (!X509_EXTENSION_get_critical(ext)) { warnx("%s: RFC 6487 section 4.8.10: sbgp-ipAddrBlock: " - "extension not critical", p->fn); + "extension not critical", fn); goto out; } if ((addrblk = X509V3_EXT_d2i(ext)) == NULL) { warnx("%s: RFC 6487 section 4.8.10: sbgp-ipAddrBlock: " - "failed extension parse", p->fn); + "failed extension parse", fn); goto out; } - if (!sbgp_parse_ipaddrblk(p->fn, addrblk, &p->res->ips, &p->res->ipsz)) + if (!sbgp_parse_ipaddrblk(fn, addrblk, &cert->ips, &cert->ipsz)) goto out; - if (p->res->ipsz == 0) { - warnx("%s: RFC 6487 section 4.8.10: empty ipAddrBlock", p->fn); + if (cert->ipsz == 0) { + warnx("%s: RFC 6487 section 4.8.10: empty ipAddrBlock", fn); goto out; } @@ -506,7 +497,7 @@ sbgp_ipaddrblk(struct parse *p, X509_EXTENSION *ext) * Returns zero on failure, non-zero on success. */ static int -sbgp_sia(struct parse *p, X509_EXTENSION *ext) +sbgp_sia(const char *fn, struct cert *cert, X509_EXTENSION *ext) { AUTHORITY_INFO_ACCESS *sia = NULL; ACCESS_DESCRIPTION *ad; @@ -516,13 +507,13 @@ sbgp_sia(struct parse *p, X509_EXTENSION *ext) if (X509_EXTENSION_get_critical(ext)) { warnx("%s: RFC 6487 section 4.8.8: SIA: " - "extension not non-critical", p->fn); + "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", - p->fn); + fn); goto out; } @@ -532,48 +523,46 @@ sbgp_sia(struct parse *p, X509_EXTENSION *ext) oid = ad->method; if (OBJ_cmp(oid, carepo_oid) == 0) { - if (!x509_location(p->fn, "SIA: caRepository", - "rsync://", ad->location, &p->res->repo)) + if (!x509_location(fn, "SIA: caRepository", + "rsync://", ad->location, &cert->repo)) goto out; } else if (OBJ_cmp(oid, manifest_oid) == 0) { - if (!x509_location(p->fn, "SIA: rpkiManifest", - "rsync://", ad->location, &p->res->mft)) + if (!x509_location(fn, "SIA: rpkiManifest", + "rsync://", ad->location, &cert->mft)) goto out; } else if (OBJ_cmp(oid, notify_oid) == 0) { - if (!x509_location(p->fn, "SIA: rpkiNotify", - "https://", ad->location, &p->res->notify)) + if (!x509_location(fn, "SIA: rpkiNotify", + "https://", ad->location, &cert->notify)) goto out; } } - if (p->res->mft == NULL || p->res->repo == NULL) { + if (cert->mft == NULL || cert->repo == NULL) { warnx("%s: RFC 6487 section 4.8.8: SIA: missing caRepository " - "or rpkiManifest", p->fn); + "or rpkiManifest", fn); goto out; } - mftfilename = strrchr(p->res->mft, '/'); + mftfilename = strrchr(cert->mft, '/'); if (mftfilename == NULL) { - warnx("%s: SIA: invalid rpkiManifest entry", p->fn); + warnx("%s: SIA: invalid rpkiManifest entry", fn); goto out; } mftfilename++; if (!valid_filename(mftfilename, strlen(mftfilename))) { warnx("%s: SIA: rpkiManifest filename contains invalid " - "characters", p->fn); + "characters", fn); goto out; } - if (strstr(p->res->mft, p->res->repo) != p->res->mft) { + if (strstr(cert->mft, cert->repo) != cert->mft) { warnx("%s: RFC 6487 section 4.8.8: SIA: " - "conflicting URIs for caRepository and rpkiManifest", - p->fn); + "conflicting URIs for caRepository and rpkiManifest", fn); goto out; } - if (rtype_from_file_extension(p->res->mft) != RTYPE_MFT) { - warnx("%s: RFC 6487 section 4.8.8: SIA: " - "not an MFT file", p->fn); + if (rtype_from_file_extension(cert->mft) != RTYPE_MFT) { + warnx("%s: RFC 6487 section 4.8.8: SIA: not an MFT file", fn); goto out; } @@ -588,7 +577,7 @@ sbgp_sia(struct parse *p, X509_EXTENSION *ext) * Returns zero on failure, non-zero on success. */ static int -certificate_policies(struct parse *p, X509_EXTENSION *ext) +certificate_policies(const char *fn, struct cert *cert, X509_EXTENSION *ext) { STACK_OF(POLICYINFO) *policies = NULL; POLICYINFO *policy; @@ -599,20 +588,19 @@ certificate_policies(struct parse *p, X509_EXTENSION *ext) if (!X509_EXTENSION_get_critical(ext)) { warnx("%s: RFC 6487 section 4.8.9: certificatePolicies: " - "extension not critical", p->fn); + "extension not critical", fn); goto out; } if ((policies = X509V3_EXT_d2i(ext)) == NULL) { warnx("%s: RFC 6487 section 4.8.9: certificatePolicies: " - "failed extension parse", p->fn); + "failed extension parse", fn); goto out; } if (sk_POLICYINFO_num(policies) != 1) { warnx("%s: RFC 6487 section 4.8.9: certificatePolicies: " - "want 1 policy, got %d", p->fn, - sk_POLICYINFO_num(policies)); + "want 1 policy, got %d", fn, sk_POLICYINFO_num(policies)); goto out; } @@ -625,7 +613,7 @@ certificate_policies(struct parse *p, X509_EXTENSION *ext) OBJ_obj2txt(pbuf, sizeof(pbuf), policy->policyid, 1); OBJ_obj2txt(cbuf, sizeof(cbuf), certpol_oid, 1); warnx("%s: RFC 7318 section 2: certificatePolicies: " - "unexpected OID: %s, want %s", p->fn, pbuf, cbuf); + "unexpected OID: %s, want %s", fn, pbuf, cbuf); goto out; } @@ -637,7 +625,7 @@ certificate_policies(struct parse *p, X509_EXTENSION *ext) if (sk_POLICYQUALINFO_num(qualifiers) != 1) { warnx("%s: RFC 7318 section 2: certificatePolicies: " - "want 1 policy qualifier, got %d", p->fn, + "want 1 policy qualifier, got %d", fn, sk_POLICYQUALINFO_num(qualifiers)); goto out; } @@ -647,12 +635,12 @@ certificate_policies(struct parse *p, X509_EXTENSION *ext) if ((nid = OBJ_obj2nid(qualifier->pqualid)) != NID_id_qt_cps) { warnx("%s: RFC 7318 section 2: certificatePolicies: " - "want CPS, got %s", p->fn, nid2str(nid)); + "want CPS, got %s", fn, nid2str(nid)); goto out; } if (verbose > 1 && !filemode) - warnx("%s: CPS %.*s", p->fn, qualifier->d.cpsuri->length, + warnx("%s: CPS %.*s", fn, qualifier->d.cpsuri->length, qualifier->d.cpsuri->data); rc = 1; @@ -669,13 +657,11 @@ certificate_policies(struct parse *p, X509_EXTENSION *ext) struct cert * cert_parse_ee_cert(const char *fn, int talid, X509 *x) { - struct parse p; + struct cert *cert; X509_EXTENSION *ext; int index; - memset(&p, 0, sizeof(struct parse)); - p.fn = fn; - if ((p.res = calloc(1, sizeof(struct cert))) == NULL) + if ((cert = calloc(1, sizeof(struct cert))) == NULL) err(1, NULL); if (X509_get_version(x) != 2) { @@ -700,13 +686,13 @@ cert_parse_ee_cert(const char *fn, int talid, X509 *x) index = X509_get_ext_by_NID(x, NID_sbgp_ipAddrBlock, -1); if ((ext = X509_get_ext(x, index)) != NULL) { - if (!sbgp_ipaddrblk(&p, ext)) + if (!sbgp_ipaddrblk(fn, cert, ext)) goto out; } index = X509_get_ext_by_NID(x, NID_sbgp_autonomousSysNum, -1); if ((ext = X509_get_ext(x, index)) != NULL) { - if (!sbgp_assysnum(&p, ext)) + if (!sbgp_assysnum(fn, cert, ext)) goto out; } @@ -715,16 +701,16 @@ cert_parse_ee_cert(const char *fn, int talid, X509 *x) goto out; } - p.res->x509 = x; - p.res->talid = talid; + cert->x509 = x; + cert->talid = talid; - if (!constraints_validate(fn, p.res)) + if (!constraints_validate(fn, cert)) goto out; - return p.res; + return cert; out: - cert_free(p.res); + cert_free(cert); return NULL; } @@ -736,6 +722,7 @@ cert_parse_ee_cert(const char *fn, int talid, X509 *x) struct cert * cert_parse_pre(const char *fn, const unsigned char *der, size_t len) { + struct cert *cert; const unsigned char *oder; size_t j; int i, extsz; @@ -746,7 +733,6 @@ cert_parse_pre(const char *fn, const unsigned char *der, size_t len) const ASN1_OBJECT *cobj; ASN1_OBJECT *obj; EVP_PKEY *pkey; - struct parse p; int nid, ip, as, sia, cp, crldp, aia, aki, ski, eku, bc, ku; @@ -756,14 +742,12 @@ cert_parse_pre(const char *fn, const unsigned char *der, size_t len) if (der == NULL) return NULL; - memset(&p, 0, sizeof(struct parse)); - p.fn = fn; - if ((p.res = calloc(1, sizeof(struct cert))) == NULL) + if ((cert = calloc(1, sizeof(struct cert))) == NULL) err(1, NULL); oder = der; if ((x = d2i_X509(NULL, &der, len)) == NULL) { - warnx("%s: d2i_X509", p.fn); + warnx("%s: d2i_X509", fn); goto out; } if (der != oder + len) { @@ -773,7 +757,7 @@ cert_parse_pre(const char *fn, const unsigned char *der, size_t len) /* Cache X509v3 extensions, see X509_check_ca(3). */ if (X509_check_purpose(x, -1, -1) <= 0) { - warnx("%s: could not cache X509v3 extensions", p.fn); + warnx("%s: could not cache X509v3 extensions", fn); goto out; } @@ -784,7 +768,7 @@ cert_parse_pre(const char *fn, const unsigned char *der, size_t len) X509_get0_signature(NULL, &palg, x); if (palg == NULL) { - warnx("%s: X509_get0_signature", p.fn); + warnx("%s: X509_get0_signature", fn); goto out; } X509_ALGOR_get0(&cobj, NULL, NULL, palg); @@ -805,7 +789,7 @@ cert_parse_pre(const char *fn, const unsigned char *der, size_t len) goto out; } - if (!x509_valid_subject(p.fn, x)) + if (!x509_valid_subject(fn, x)) goto out; /* Look for X509v3 extensions. */ @@ -824,25 +808,25 @@ cert_parse_pre(const char *fn, const unsigned char *der, size_t len) case NID_sbgp_ipAddrBlock: if (ip++ > 0) goto dup; - if (!sbgp_ipaddrblk(&p, ext)) + if (!sbgp_ipaddrblk(fn, cert, ext)) goto out; break; case NID_sbgp_autonomousSysNum: if (as++ > 0) goto dup; - if (!sbgp_assysnum(&p, ext)) + if (!sbgp_assysnum(fn, cert, ext)) goto out; break; case NID_sinfo_access: if (sia++ > 0) goto dup; - if (!sbgp_sia(&p, ext)) + if (!sbgp_sia(fn, cert, ext)) goto out; break; case NID_certificate_policies: if (cp++ > 0) goto dup; - if (!certificate_policies(&p, ext)) + if (!certificate_policies(fn, cert, ext)) goto out; break; case NID_crl_distribution_points: @@ -879,40 +863,40 @@ cert_parse_pre(const char *fn, const unsigned char *der, size_t len) char objn[64]; OBJ_obj2txt(objn, sizeof(objn), obj, 0); warnx("%s: ignoring %s (NID %d)", - p.fn, objn, OBJ_obj2nid(obj)); + fn, objn, OBJ_obj2nid(obj)); } break; } } - if (!x509_get_aki(x, p.fn, &p.res->aki)) + if (!x509_get_aki(x, fn, &cert->aki)) goto out; - if (!x509_get_ski(x, p.fn, &p.res->ski)) + if (!x509_get_ski(x, fn, &cert->ski)) goto out; - if (!x509_get_aia(x, p.fn, &p.res->aia)) + if (!x509_get_aia(x, fn, &cert->aia)) goto out; - if (!x509_get_crl(x, p.fn, &p.res->crl)) + if (!x509_get_crl(x, fn, &cert->crl)) goto out; - if (!x509_get_notbefore(x, p.fn, &p.res->notbefore)) + if (!x509_get_notbefore(x, fn, &cert->notbefore)) goto out; - if (!x509_get_notafter(x, p.fn, &p.res->notafter)) + if (!x509_get_notafter(x, fn, &cert->notafter)) goto out; - p.res->purpose = x509_get_purpose(x, p.fn); + cert->purpose = x509_get_purpose(x, fn); /* Validation on required fields. */ - switch (p.res->purpose) { + switch (cert->purpose) { case CERT_PURPOSE_CA: if ((pkey = X509_get0_pubkey(x)) == NULL) { - warnx("%s: X509_get0_pubkey failed", p.fn); + warnx("%s: X509_get0_pubkey failed", fn); goto out; } - if (!valid_ca_pkey(p.fn, pkey)) + if (!valid_ca_pkey(fn, pkey)) goto out; if (X509_get_key_usage(x) != (KU_KEY_CERT_SIGN | KU_CRL_SIGN)) { warnx("%s: RFC 6487 section 4.8.4: key usage violation", - p.fn); + fn); goto out; } @@ -923,57 +907,56 @@ cert_parse_pre(const char *fn, const unsigned char *der, size_t len) goto out; } - if (p.res->mft == NULL) { - warnx("%s: RFC 6487 section 4.8.8: missing SIA", p.fn); + if (cert->mft == NULL) { + warnx("%s: RFC 6487 section 4.8.8: missing SIA", fn); goto out; } - if (p.res->asz == 0 && p.res->ipsz == 0) { - warnx("%s: missing IP or AS resources", p.fn); + if (cert->asz == 0 && cert->ipsz == 0) { + warnx("%s: missing IP or AS resources", fn); goto out; } break; case CERT_PURPOSE_BGPSEC_ROUTER: - p.res->pubkey = x509_get_pubkey(x, p.fn); - if (p.res->pubkey == NULL) { - warnx("%s: x509_get_pubkey failed", p.fn); + cert->pubkey = x509_get_pubkey(x, fn); + if (cert->pubkey == NULL) { + warnx("%s: x509_get_pubkey failed", fn); goto out; } - if (p.res->ipsz > 0) { - warnx("%s: unexpected IP resources in BGPsec cert", - p.fn); + if (cert->ipsz > 0) { + warnx("%s: unexpected IP resources in BGPsec cert", fn); goto out; } - for (j = 0; j < p.res->asz; j++) { - if (p.res->as[j].type == CERT_AS_INHERIT) { + for (j = 0; j < cert->asz; j++) { + if (cert->as[j].type == CERT_AS_INHERIT) { warnx("%s: inherit elements not allowed in EE" - " cert", p.fn); + " cert", fn); goto out; } } if (sia) { warnx("%s: unexpected SIA extension in BGPsec cert", - p.fn); + fn); goto out; } break; default: - warnx("%s: x509_get_purpose failed in %s", p.fn, __func__); + warnx("%s: x509_get_purpose failed in %s", fn, __func__); goto out; } - if (p.res->ski == NULL) { - warnx("%s: RFC 6487 section 8.4.2: missing SKI", p.fn); + if (cert->ski == NULL) { + warnx("%s: RFC 6487 section 8.4.2: missing SKI", fn); goto out; } - p.res->x509 = x; - return p.res; + cert->x509 = x; + return cert; dup: warnx("%s: RFC 5280 section 4.2: duplicate extension: %s", fn, nid2str(nid)); out: - cert_free(p.res); + cert_free(cert); X509_free(x); return NULL; } -- 2.20.1