From: Alberto Leiva Popper Date: Fri, 7 Jul 2023 21:28:19 +0000 (-0600) Subject: Partial refactor of certificate_traverse() X-Git-Tag: 1.6.0~72^2 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=418f1bd0732154b2bfd48cd8ce3e261baf4a49d5;p=thirdparty%2FFORT-validator.git Partial refactor of certificate_traverse() Simplifies it by moving some of its clutter to helper functions, seemingly in preparation for #58. Part of a series of patches meant to manually rebase the issue58-proper branch. --- diff --git a/src/certificate_refs.c b/src/certificate_refs.c index 7fb617cf..61bc698c 100644 --- a/src/certificate_refs.c +++ b/src/certificate_refs.c @@ -58,7 +58,7 @@ validate_signedObject(struct certificate_refs *refs, /** * Ensures the @refs URIs match the parent Manifest's URIs. Assumes @refs came - * from a CA certificate. + * from a (non-TA) CA certificate. * * @refs: References you want validated. * @pp: Repository Publication Point, as described by the parent Manifest. @@ -68,9 +68,6 @@ refs_validate_ca(struct certificate_refs *refs, struct rpp const *pp) { int error; - if (pp == NULL) - return 0; /* This CA is the TA, and therefore lacks a parent. */ - error = validate_cdp(refs, pp); if (error) return error; diff --git a/src/object/bgpsec.c b/src/object/bgpsec.c index 104b9d1a..7da60091 100644 --- a/src/object/bgpsec.c +++ b/src/object/bgpsec.c @@ -1,8 +1,9 @@ -#include "bgpsec.h" +#include "object/bgpsec.h" #include "alloc.h" #include "log.h" #include "validation_handler.h" +#include "object/certificate.h" struct resource_params { unsigned char const *ski; @@ -23,31 +24,60 @@ asn_cb(struct asn_range const *range, void *arg) } int -handle_bgpsec(X509 *cert, unsigned char const *ski, struct resources *resources) +handle_bgpsec(X509 *cert, struct resources *parent_resources, struct rpp *pp) { - struct resource_params res_params; + unsigned char *ski; + enum rpki_policy policy; + struct resources *resources; X509_PUBKEY *pub_key; unsigned char *cert_spk, *tmp; int cert_spk_len; + struct resource_params res_params; int error; + error = certificate_validate_rfc6487(cert, CERTYPE_BGPSEC); + if (error) + return error; + error = certificate_validate_extensions_bgpsec(cert, &ski, &policy, pp); + if (error) + return error; + + resources = resources_create(policy, false); + if (resources == NULL) + goto revert_ski; + error = certificate_get_resources(cert, resources, CERTYPE_BGPSEC); + if (error) + goto revert_resources; + pub_key = X509_get_X509_PUBKEY(cert); - if (pub_key == NULL) - return val_crypto_err("X509_get_X509_PUBKEY() returned NULL at BGPsec"); + if (pub_key == NULL) { + error = val_crypto_err("X509_get_X509_PUBKEY() returned NULL at BGPsec"); + goto revert_resources; + } cert_spk = pmalloc(RK_SPKI_LEN); /* Use a temporal pointer, since i2d_X509_PUBKEY moves it */ tmp = cert_spk; cert_spk_len = i2d_X509_PUBKEY(pub_key, &tmp); - if(cert_spk_len < 0) - return val_crypto_err("i2d_X509_PUBKEY() returned error"); + if (cert_spk_len != RK_SPKI_LEN) { + error = val_crypto_err("i2d_X509_PUBKEY() returned %d", + cert_spk_len); + goto revert_spk; + } res_params.spk = cert_spk; res_params.ski = ski; res_params.parent_resources = resources; error = resources_foreach_asn(resources, asn_cb, &res_params); + /* Fall through */ + +revert_spk: free(cert_spk); +revert_resources: + resources_destroy(resources); +revert_ski: + free(ski); return error; } diff --git a/src/object/bgpsec.h b/src/object/bgpsec.h index e1df2126..62456a32 100644 --- a/src/object/bgpsec.h +++ b/src/object/bgpsec.h @@ -3,7 +3,8 @@ #include #include "resource.h" +#include "rpp.h" -int handle_bgpsec(X509 *, unsigned char const *, struct resources *); +int handle_bgpsec(X509 *, struct resources *, struct rpp *); #endif /* SRC_OBJECT_BGPSEC_H_ */ diff --git a/src/object/certificate.c b/src/object/certificate.c index 80d630bd..50f6a977 100644 --- a/src/object/certificate.c +++ b/src/object/certificate.c @@ -1715,16 +1715,17 @@ certificate_validate_extensions_ta(X509 *cert, struct sia_ca_uris *sia_uris, */ static int certificate_validate_extensions_ca(X509 *cert, struct sia_ca_uris *sia_uris, - struct certificate_refs *refs, enum rpki_policy *policy) + enum rpki_policy *policy, struct rpp *rpp_parent) { + struct certificate_refs refs = { 0 }; struct extension_handler handlers[] = { /* ext reqd handler arg */ { ext_bc(), true, handle_bc, }, { ext_ski(), true, handle_ski_ca, cert }, { ext_aki(), true, handle_aki, }, { ext_ku(), true, handle_ku_ca, }, - { ext_cdp(), true, handle_cdp, refs }, - { ext_aia(), true, handle_aia, refs }, + { ext_cdp(), true, handle_cdp, &refs }, + { 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, }, @@ -1733,8 +1734,19 @@ certificate_validate_extensions_ca(X509 *cert, struct sia_ca_uris *sia_uris, { ext_ar2(), false, handle_ar, }, { NULL }, }; + int error; - return handle_extensions(handlers, X509_get0_extensions(cert)); + error = handle_extensions(handlers, X509_get0_extensions(cert)); + if (error) + goto end; + error = certificate_validate_aia(refs.caIssuers, cert); + if (error) + goto end; + error = refs_validate_ca(&refs, rpp_parent); + +end: + refs_cleanup(&refs); + return error; } int @@ -1764,6 +1776,13 @@ certificate_validate_extensions_ee(X509 *cert, OCTET_STRING_t *sid, return handle_extensions(handlers, X509_get0_extensions(cert)); } +int +certificate_validate_extensions_bgpsec(X509 *cert, unsigned char **ski, + enum rpki_policy *policy, struct rpp *pp) +{ + return 0; /* TODO (#58) */ +} + static bool has_bgpsec_router_eku(X509 *cert) { @@ -2069,17 +2088,13 @@ verify_mft: int certificate_traverse(struct rpp *rpp_parent, struct rpki_uri *cert_uri) { -/** Is the CA certificate the TA certificate? */ -#define IS_TA (rpp_parent == NULL) - struct validation *state; int total_parents; STACK_OF(X509_CRL) *rpp_parent_crl; X509 *cert; struct sia_ca_uris sia_uris; - struct certificate_refs refs; enum rpki_policy policy; - enum cert_type type; + enum cert_type certype; struct rpp *pp; bool repo_retry; int error; @@ -2091,7 +2106,7 @@ certificate_traverse(struct rpp *rpp_parent, struct rpki_uri *cert_uri) return pr_val_err("Certificate chain maximum depth exceeded."); /* Debug cert type */ - if (IS_TA) + if (rpp_parent == NULL) pr_val_debug("TA Certificate '%s' {", uri_val_get_printable(cert_uri)); else @@ -2112,12 +2127,12 @@ certificate_traverse(struct rpp *rpp_parent, struct rpki_uri *cert_uri) if (error) goto revert_cert; - error = get_certificate_type(cert, IS_TA, &type); + error = get_certificate_type(cert, rpp_parent == NULL, &certype); if (error) goto revert_cert; /* Debug cert type */ - switch (type) { + switch (certype) { case CERTYPE_TA: break; case CERTYPE_CA: @@ -2125,40 +2140,22 @@ certificate_traverse(struct rpp *rpp_parent, struct rpki_uri *cert_uri) break; case CERTYPE_BGPSEC: pr_val_debug("Type: BGPsec EE. Ignoring..."); +// error = handle_bgpsec(cert, x509stack_peek_resources( +// validation_certstack(state)), rpp_parent); + goto revert_cert; + default: + pr_val_debug("Type: Unknown. Ignoring..."); goto revert_cert; - case CERTYPE_EE: - pr_val_debug("Type: unexpected, validated as CA"); - break; } - error = certificate_validate_rfc6487(cert, type); + error = certificate_validate_rfc6487(cert, certype); if (error) goto revert_cert; sia_ca_uris_init(&sia_uris); - memset(&refs, 0, sizeof(refs)); - - switch (type) { - case CERTYPE_TA: - error = certificate_validate_extensions_ta(cert, &sia_uris, - &policy); - break; - default: - /* Validate as a CA */ - error = certificate_validate_extensions_ca(cert, &sia_uris, - &refs, &policy); - break; - } - if (error) - goto revert_uris; - - if (!IS_TA) { - error = certificate_validate_aia(refs.caIssuers, cert); - if (error) - goto revert_uris; - } - - error = refs_validate_ca(&refs, rpp_parent); + error = (certype == CERTYPE_TA) + ? certificate_validate_extensions_ta(cert, &sia_uris, &policy) + : certificate_validate_extensions_ca(cert, &sia_uris, &policy, rpp_parent); if (error) goto revert_uris; @@ -2182,7 +2179,7 @@ certificate_traverse(struct rpp *rpp_parent, struct rpki_uri *cert_uri) do { /* Validate the manifest (@mft) pointed by the certificate */ error = x509stack_push(validation_certstack(state), cert_uri, - cert, policy, IS_TA); + cert, policy, certype); if (error) goto revert_uris; @@ -2225,7 +2222,6 @@ certificate_traverse(struct rpp *rpp_parent, struct rpki_uri *cert_uri) rpp_refput(pp); revert_uris: sia_ca_uris_cleanup(&sia_uris); - refs_cleanup(&refs); revert_cert: if (cert != NULL) X509_free(cert); diff --git a/src/object/certificate.h b/src/object/certificate.h index 5412e201..84ad0806 100644 --- a/src/object/certificate.h +++ b/src/object/certificate.h @@ -50,6 +50,8 @@ int certificate_get_resources(X509 *, struct resources *, enum cert_type); */ int certificate_validate_extensions_ee(X509 *, OCTET_STRING_t *, struct certificate_refs *, enum rpki_policy *); +int certificate_validate_extensions_bgpsec(X509 *, unsigned char **, + enum rpki_policy *, struct rpp *); /* * Specific validation of AIA (rfc6487#section-4.8.7) extension, public so that diff --git a/src/rtr/db/db_table.c b/src/rtr/db/db_table.c index 13f6d43e..4adb232f 100644 --- a/src/rtr/db/db_table.c +++ b/src/rtr/db/db_table.c @@ -197,8 +197,8 @@ rtrhandler_handle_roa_v6(struct db_table *table, uint32_t asn, } int -rtrhandler_handle_router_key(struct db_table *table, - unsigned char const *ski, uint32_t as, unsigned char const *spk) +rtrhandler_handle_router_key(struct db_table *table, unsigned char const *ski, + uint32_t as, unsigned char const *spk) { struct hashable_key *key; int error;