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 66f8366ac6b40a5462858b98012bff5d168dae0f..19283252ae2986f010d19af552a70eaa47de5fee 100644 (file)
--- 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 e97e2b6ee21355a9d2f44f13c47df08f1b3347c2..91adbd423cf6ffb7c8f4553cc55f841dfef1252d 100644 (file)
--- 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 58a6359c55d2d07b4842cbd0c81727b4e0f5ab80..5c83ac6623c2a230fa20495bef0f174ce950da05 100644 (file)
--- 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 2cd0de3ba4000d946c1c910b992efb36852b4d68..aa0b3ba52954ed61661809daf7dc7c26ef3cd9cd 100644 (file)
--- 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 },