From: Alberto Leiva Popper Date: Wed, 15 May 2024 23:23:46 +0000 (-0600) Subject: Fix TODO: Use extension_metadata.destructor more X-Git-Tag: 1.6.2~13 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=49025d359bbed82d490db6ccd38f304735cc98d7;p=thirdparty%2FFORT-validator.git Fix TODO: Use extension_metadata.destructor more This frees the extension callbacks from having to decode and free the extensions themselves. --- diff --git a/src/extension.c b/src/extension.c index 66f8366a..19283252 100644 --- a/src/extension.c +++ b/src/extension.c @@ -840,8 +840,10 @@ ext_metadatas(void) static int handle_extension(struct extension_handler *handlers, X509_EXTENSION *ext) { - struct extension_handler *handler; int nid; + struct extension_handler *handler; + void *decoded; + int error; nid = OBJ_obj2nid(X509_EXTENSION_get_object(ext)); @@ -859,7 +861,17 @@ handle_extension(struct extension_handler *handlers, X509_EXTENSION *ext) goto critical; } - return handler->cb(ext, handler->arg); + if (handler->cb == NULL) + return 0; /* Nothing to validate, for now. */ + + decoded = X509V3_EXT_d2i(ext); + if (decoded == NULL) + return cannot_decode(handler->meta); + + error = handler->cb(decoded, handler->arg); + + handler->meta->destructor(decoded); + return error; } } @@ -988,38 +1000,23 @@ validate_public_key_hash(X509 *cert, ASN1_OCTET_STRING *hash) } int -handle_aki(X509_EXTENSION *ext, void *arg) +handle_aki(void *ext, void *arg) { - AUTHORITY_KEYID *aki; - struct validation *state; + AUTHORITY_KEYID *aki = ext; X509 *parent; - int error; - - aki = X509V3_EXT_d2i(ext); - if (aki == NULL) - return cannot_decode(ext_aki()); if (aki->issuer != NULL) { - error = pr_val_err("%s extension contains an authorityCertIssuer.", + return pr_val_err("%s extension contains an authorityCertIssuer.", ext_aki()->name); - goto end; } if (aki->serial != NULL) { - error = pr_val_err("%s extension contains an authorityCertSerialNumber.", + return pr_val_err("%s extension contains an authorityCertSerialNumber.", ext_aki()->name); - goto end; - } - - state = state_retrieve(); - parent = x509stack_peek(validation_certstack(state)); - if (parent == NULL) { - error = pr_val_err("Certificate has no parent."); - goto end; } - error = validate_public_key_hash(parent, aki->keyid); + parent = x509stack_peek(validation_certstack(state_retrieve())); + if (parent == NULL) + return pr_val_err("Certificate has no parent."); -end: - AUTHORITY_KEYID_free(aki); - return error; + return validate_public_key_hash(parent, aki->keyid); } diff --git a/src/extension.h b/src/extension.h index e97e2b6e..91adbd42 100644 --- a/src/extension.h +++ b/src/extension.h @@ -11,14 +11,14 @@ struct extension_metadata { int nid; bool critical; json_t *(*to_json)(void const *); - void (*destructor)(void *); /* TODO use this more */ + void (*destructor)(void *); }; struct extension_handler { struct extension_metadata const *meta; bool mandatory; - int (*cb)(X509_EXTENSION *, void *); + int (*cb)(void *, void *); void *arg; /* For internal use */ @@ -49,6 +49,6 @@ int handle_extensions(struct extension_handler *, int cannot_decode(struct extension_metadata const *); int validate_public_key_hash(X509 *, ASN1_OCTET_STRING *); -int handle_aki(X509_EXTENSION *, void *); +int handle_aki(void *, void *); #endif /* SRC_EXTENSION_H_ */ diff --git a/src/object/certificate.c b/src/object/certificate.c index 58a6359c..5c83ac66 100644 --- a/src/object/certificate.c +++ b/src/object/certificate.c @@ -1221,14 +1221,9 @@ handle_signedObject(struct rpki_uri *uri, void *arg) } static int -handle_bc(X509_EXTENSION *ext, void *arg) +handle_bc(void *ext, void *arg) { - BASIC_CONSTRAINTS *bc; - int error; - - bc = X509V3_EXT_d2i(ext); - if (bc == NULL) - return cannot_decode(ext_bc()); + BASIC_CONSTRAINTS *bc = ext; /* * 'The issuer determines whether the "cA" boolean is set.' @@ -1236,102 +1231,70 @@ handle_bc(X509_EXTENSION *ext, void *arg) * Well, libcrypto should do the RFC 5280 thing with it anyway. */ - error = (bc->pathlen == NULL) + return (bc->pathlen == NULL) ? 0 : pr_val_err("%s extension contains a Path Length Constraint.", ext_bc()->name); - - BASIC_CONSTRAINTS_free(bc); - return error; } static int -handle_ski_ca(X509_EXTENSION *ext, void *arg) +handle_ski_ca(void *ext, void *arg) { - ASN1_OCTET_STRING *ski; - int error; - - ski = X509V3_EXT_d2i(ext); - if (ski == NULL) - return cannot_decode(ext_ski()); - - error = validate_public_key_hash(arg, ski); - - ASN1_OCTET_STRING_free(ski); - return error; + return validate_public_key_hash(arg, ext); } static int -handle_ski_ee(X509_EXTENSION *ext, void *arg) +handle_ski_ee(void *ext, void *arg) { - struct ski_arguments *args; - ASN1_OCTET_STRING *ski; + ASN1_OCTET_STRING *ski = ext; + struct ski_arguments *args = arg; OCTET_STRING_t *sid; int error; - ski = X509V3_EXT_d2i(ext); - if (ski == NULL) - return cannot_decode(ext_ski()); - - args = arg; error = validate_public_key_hash(args->cert, ski); if (error) - goto end; + return error; /* rfc6488#section-2.1.6.2 */ /* rfc6488#section-3.1.c 2/2 */ sid = args->sid; if (ski->length != sid->size || memcmp(ski->data, sid->buf, sid->size) != 0) { - error = pr_val_err("The EE certificate's subjectKeyIdentifier does not equal the Signed Object's sid."); + return pr_val_err("The EE certificate's subjectKeyIdentifier does not equal the Signed Object's sid."); } -end: - ASN1_OCTET_STRING_free(ski); - return error; + return 0; } static int -handle_aki_ta(X509_EXTENSION *ext, void *arg) +handle_aki_ta(void *ext, void *arg) { - struct AUTHORITY_KEYID_st *aki; + struct AUTHORITY_KEYID_st *aki = ext; ASN1_OCTET_STRING *ski; int error; - aki = X509V3_EXT_d2i(ext); - if (aki == NULL) - return cannot_decode(ext_aki()); if (aki->keyid == NULL) { - error = pr_val_err("The '%s' extension lacks a keyIdentifier.", + return pr_val_err("The '%s' extension lacks a keyIdentifier.", ext_aki()->name); - goto revert_aki; } ski = X509_get_ext_d2i(arg, NID_subject_key_identifier, NULL, NULL); if (ski == NULL) { pr_val_err("Certificate lacks the '%s' extension.", ext_ski()->name); - error = -ESRCH; - goto revert_aki; - } - - if (ASN1_OCTET_STRING_cmp(aki->keyid, ski) != 0) { - error = pr_val_err("The '%s' does not equal the '%s'.", - ext_aki()->name, ext_ski()->name); - goto revert_ski; + return -ESRCH; } - error = 0; + error = (ASN1_OCTET_STRING_cmp(aki->keyid, ski) != 0) + ? pr_val_err("The '%s' does not equal the '%s'.", ext_aki()->name, ext_ski()->name) + : 0; -revert_ski: ASN1_BIT_STRING_free(ski); -revert_aki: - AUTHORITY_KEYID_free(aki); return error; } static int -handle_ku(X509_EXTENSION *ext, unsigned char byte1) +handle_ku(ASN1_BIT_STRING *ku, unsigned char byte1) { /* * About the key usage string: At time of writing, it's 9 bits long. @@ -1339,72 +1302,56 @@ handle_ku(X509_EXTENSION *ext, unsigned char byte1) * This implementation assumes that the ninth bit should always be zero. */ - ASN1_BIT_STRING *ku; unsigned char data[2]; - int error = 0; - - ku = X509V3_EXT_d2i(ext); - if (ku == NULL) - return cannot_decode(ext_ku()); if (ku->length == 0) { - error = pr_val_err("%s bit string has no enabled bits.", + return pr_val_err("%s bit string has no enabled bits.", ext_ku()->name); - goto end; } memset(data, 0, sizeof(data)); memcpy(data, ku->data, ku->length); if (ku->data[0] != byte1) { - error = pr_val_err("Illegal key usage flag string: %d%d%d%d%d%d%d%d%d", + return pr_val_err("Illegal key usage flag string: %d%d%d%d%d%d%d%d%d", !!(ku->data[0] & 0x80u), !!(ku->data[0] & 0x40u), !!(ku->data[0] & 0x20u), !!(ku->data[0] & 0x10u), !!(ku->data[0] & 0x08u), !!(ku->data[0] & 0x04u), !!(ku->data[0] & 0x02u), !!(ku->data[0] & 0x01u), !!(ku->data[1] & 0x80u)); - goto end; } -end: - ASN1_BIT_STRING_free(ku); - return error; + return 0; } static int -handle_ku_ca(X509_EXTENSION *ext, void *arg) +handle_ku_ca(void *ext, void *arg) { return handle_ku(ext, 0x06); } static int -handle_ku_ee(X509_EXTENSION *ext, void *arg) +handle_ku_ee(void *ext, void *arg) { return handle_ku(ext, 0x80); } static int -handle_cdp(X509_EXTENSION *ext, void *arg) +handle_cdp(void *ext, void *arg) { + STACK_OF(DIST_POINT) *crldp = ext; struct certificate_refs *refs = arg; - STACK_OF(DIST_POINT) *crldp; DIST_POINT *dp; GENERAL_NAMES *names; GENERAL_NAME *name; ASN1_IA5STRING *str; int i; int type; - int error; char const *error_msg; - crldp = X509V3_EXT_d2i(ext); - if (crldp == NULL) - return cannot_decode(ext_cdp()); - if (sk_DIST_POINT_num(crldp) != 1) { - error = pr_val_err("The %s extension has %d distribution points. (1 expected)", + return pr_val_err("The %s extension has %d distribution points. (1 expected)", ext_cdp()->name, sk_DIST_POINT_num(crldp)); - goto end; } dp = sk_DIST_POINT_value(crldp, 0); @@ -1454,20 +1401,15 @@ handle_cdp(X509_EXTENSION *ext, void *arg) * So we will store the URI in @refs, and validate it * later. */ - error = ia5s2string(str, &refs->crldp); - goto end; + return ia5s2string(str, &refs->crldp); } } error_msg = "lacks an RSYNC URI"; dist_point_error: - error = pr_val_err("The %s extension's distribution point %s.", + return pr_val_err("The %s extension's distribution point %s.", ext_cdp()->name, error_msg); - -end: - sk_DIST_POINT_pop_free(crldp, DIST_POINT_free); - return error; } /* @@ -1607,89 +1549,55 @@ handle_caIssuers(struct rpki_uri *uri, void *arg) } static int -handle_aia(X509_EXTENSION *ext, void *arg) +handle_aia(void *ext, void *arg) { - AUTHORITY_INFO_ACCESS *aia; - int error; - - aia = X509V3_EXT_d2i(ext); - if (aia == NULL) - return cannot_decode(ext_aia()); - - error = handle_ad(NID_ad_ca_issuers, &CA_ISSUERS, aia, handle_caIssuers, - arg); - - AUTHORITY_INFO_ACCESS_free(aia); - return error; + return handle_ad(NID_ad_ca_issuers, &CA_ISSUERS, ext, + handle_caIssuers, arg); } static int -handle_sia_ca(X509_EXTENSION *ext, void *arg) +handle_sia_ca(void *ext, void *arg) { - SIGNATURE_INFO_ACCESS *sia; + SIGNATURE_INFO_ACCESS *sia = ext; struct sia_uris *uris = arg; int error; - sia = X509V3_EXT_d2i(ext); - if (sia == NULL) - return cannot_decode(ext_sia()); - /* rsync */ error = handle_ad(NID_caRepository, &CA_REPOSITORY, sia, handle_caRepository, uris); if (error) - goto end; + return error; /* RRDP */ error = handle_ad(nid_ad_notify(), &RPKI_NOTIFY, sia, handle_rpkiNotify, uris); if (error) - goto end; + return error; /* Manifest */ - error = handle_ad(nid_ad_mft(), &RPKI_MANIFEST, sia, + return handle_ad(nid_ad_mft(), &RPKI_MANIFEST, sia, handle_rpkiManifest, uris); - -end: - AUTHORITY_INFO_ACCESS_free(sia); - return error; } static int -handle_sia_ee(X509_EXTENSION *ext, void *arg) +handle_sia_ee(void *ext, void *arg) { - SIGNATURE_INFO_ACCESS *sia; - int error; - - sia = X509V3_EXT_d2i(ext); - if (sia == NULL) - return cannot_decode(ext_sia()); - - error = handle_ad(nid_ad_so(), &SIGNED_OBJECT, sia, + return handle_ad(nid_ad_so(), &SIGNED_OBJECT, ext, handle_signedObject, arg); - - AUTHORITY_INFO_ACCESS_free(sia); - return error; } static int -handle_cp(X509_EXTENSION *ext, void *arg) +handle_cp(void *ext, void *arg) { + CERTIFICATEPOLICIES *cp = ext; enum rpki_policy *policy = arg; - CERTIFICATEPOLICIES *cp; POLICYINFO *pi; POLICYQUALINFO *pqi; - int error, nid_cp, nid_qt_cps, pqi_num; - - error = 0; - cp = X509V3_EXT_d2i(ext); - if (cp == NULL) - return cannot_decode(ext_cp()); + int nid_cp, nid_qt_cps, pqi_num; if (sk_POLICYINFO_num(cp) != 1) { - error = pr_val_err("The %s extension has %d policy information's. (1 expected)", + return pr_val_err("The %s extension has %d policy information's. (1 expected)", ext_cp()->name, sk_POLICYINFO_num(cp)); - goto end; } /* rfc7318#section-2 and consider rfc8360#section-4.2.1 */ @@ -1703,44 +1611,27 @@ handle_cp(X509_EXTENSION *ext, void *arg) if (policy != NULL) *policy = RPKI_POLICY_RFC8360; } else { - error = pr_val_err("Invalid certificate policy OID, isn't 'id-cp-ipAddr-asNumber' nor 'id-cp-ipAddr-asNumber-v2'"); - goto end; + return pr_val_err("Invalid certificate policy OID, isn't 'id-cp-ipAddr-asNumber' nor 'id-cp-ipAddr-asNumber-v2'"); } /* Exactly one policy qualifier MAY be included (so none is also valid) */ if (pi->qualifiers == NULL) - goto end; + return 0; pqi_num = sk_POLICYQUALINFO_num(pi->qualifiers); if (pqi_num == 0) - goto end; + return 0; if (pqi_num != 1) { - error = pr_val_err("The %s extension has %d policy qualifiers. (none or only 1 expected)", + return pr_val_err("The %s extension has %d policy qualifiers. (none or only 1 expected)", ext_cp()->name, pqi_num); - goto end; } pqi = sk_POLICYQUALINFO_value(pi->qualifiers, 0); nid_qt_cps = OBJ_obj2nid(pqi->pqualid); - if (nid_qt_cps != NID_id_qt_cps) { - error = pr_val_err("Policy qualifier ID isn't Certification Practice Statement (CPS)"); - goto end; - } -end: - CERTIFICATEPOLICIES_free(cp); - return error; -} + if (nid_qt_cps != NID_id_qt_cps) + return pr_val_err("Policy qualifier ID isn't Certification Practice Statement (CPS)"); -static int -handle_ir(X509_EXTENSION *ext, void *arg) -{ - return 0; /* Handled in certificate_get_resources(). */ -} - -static int -handle_ar(X509_EXTENSION *ext, void *arg) -{ - return 0; /* Handled in certificate_get_resources(). */ + return 0; } /** @@ -1761,10 +1652,11 @@ certificate_validate_extensions_ta(X509 *cert, struct sia_uris *sia_uris, { ext_ku(), true, handle_ku_ca, }, { ext_sia(), true, handle_sia_ca, sia_uris }, { ext_cp(), true, handle_cp, policy }, - { ext_ir(), false, handle_ir, }, - { ext_ar(), false, handle_ar, }, - { ext_ir2(), false, handle_ir, }, - { ext_ar2(), false, handle_ar, }, + /* These are handled by certificate_get_resources(). */ + { ext_ir(), false, }, + { ext_ar(), false, }, + { ext_ir2(), false, }, + { ext_ar2(), false, }, { NULL }, }; @@ -1795,10 +1687,10 @@ certificate_validate_extensions_ca(X509 *cert, struct sia_uris *sia_uris, { ext_aia(), true, handle_aia, &refs }, { ext_sia(), true, handle_sia_ca, sia_uris }, { ext_cp(), true, handle_cp, policy }, - { ext_ir(), false, handle_ir, }, - { ext_ar(), false, handle_ar, }, - { ext_ir2(), false, handle_ir, }, - { ext_ar2(), false, handle_ar, }, + { ext_ir(), false, }, + { ext_ar(), false, }, + { ext_ir2(), false, }, + { ext_ar2(), false, }, { NULL }, }; int error; @@ -1830,10 +1722,10 @@ certificate_validate_extensions_ee(X509 *cert, OCTET_STRING_t *sid, { ext_aia(), true, handle_aia, refs }, { ext_sia(), true, handle_sia_ee, refs }, { ext_cp(), true, handle_cp, policy }, - { ext_ir(), false, handle_ir, }, - { ext_ar(), false, handle_ar, }, - { ext_ir2(), false, handle_ir, }, - { ext_ar2(), false, handle_ar, }, + { ext_ir(), false, }, + { ext_ar(), false, }, + { ext_ir2(), false, }, + { ext_ar2(), false, }, { NULL }, }; diff --git a/src/object/crl.c b/src/object/crl.c index 2cd0de3b..aa0b3ba5 100644 --- a/src/object/crl.c +++ b/src/object/crl.c @@ -102,7 +102,7 @@ validate_revoked(X509_CRL *crl) } static int -handle_crlnum(X509_EXTENSION *ext, void *arg) +handle_crlnum(void *ext, void *arg) { /* * We're allowing only one CRL per RPP, so there's nothing to do here I @@ -115,7 +115,7 @@ static int validate_extensions(X509_CRL *crl) { struct extension_handler handlers[] = { - /* ext reqd handler arg */ + /* ext reqd handler arg */ { ext_aki(), true, handle_aki, }, { ext_cn(), true, handle_crlnum, }, { NULL },