From: Alberto Leiva Popper Date: Thu, 30 May 2019 22:16:34 +0000 (-0500) Subject: Replace recursive tree traversal with iterative one X-Git-Tag: v0.0.2~12^2~3 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=361e6a8b8ac614aadad60fd79064bbf64e035fe5;p=thirdparty%2FFORT-validator.git Replace recursive tree traversal with iterative one Prevents stack overflows on tall RPKI trees. --- diff --git a/docs/doc/usage.md b/docs/doc/usage.md index 416acc62..f682bb9c 100644 --- a/docs/doc/usage.md +++ b/docs/doc/usage.md @@ -226,7 +226,13 @@ Of course, this is only relevant if the TAL lists more than one URL. ### `--maximum-certificate-depth` -> TODO; unfinished code +- **Type:** Integer +- **Availability:** `argv` and JSON +- **Default:** 32 + +Maximum allowable RPKI tree height. Meant to protect Fort from iterating infinitely due to certificate chain loops. + +Fort's tree traversal is actually iterative (not recursive), so there should be no risk of stack overflow, regardless of this value. ### `--server.address` diff --git a/src/Makefile.am b/src/Makefile.am index 40a8d9b1..d4aac56e 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -10,6 +10,7 @@ fort_SOURCES = main.c fort_SOURCES += address.h address.c fort_SOURCES += algorithm.h algorithm.c fort_SOURCES += certificate_refs.h certificate_refs.c +fort_SOURCES += cert_stack.h cert_stack.c fort_SOURCES += clients.c clients.h fort_SOURCES += common.c common.h fort_SOURCES += console_handler.h console_handler.c diff --git a/src/abbreviations.txt b/src/abbreviations.txt index 6cab74cd..65e26a48 100644 --- a/src/abbreviations.txt +++ b/src/abbreviations.txt @@ -7,10 +7,13 @@ addr: address addr4: IPv4 address addr6: IPv6 address be: Big Endian +cert: certificate +certstack: certificate stack db: database eof: end of file err: error fd: File Descriptor (see `man 2 accept`) +guri: global URI hdr: header id: identifier len: length @@ -18,10 +21,12 @@ max: maximum min: minimum msg: message pdu: Protocol Data Unit (RFC 6810) +pp: Publication Point pr: print ptr: pointer refget: reference get (+1 to reference counter) refput: reference put (-1 to reference counter) +rpp: Repository Publication Point str: string tmp: temporal uint: unsigned int diff --git a/src/asn1/content_info.c b/src/asn1/content_info.c index 8489b869..5b455100 100644 --- a/src/asn1/content_info.c +++ b/src/asn1/content_info.c @@ -48,12 +48,12 @@ decode(struct file_contents *fc, struct ContentInfo **result) } int -content_info_load(struct rpki_uri const *uri, struct ContentInfo **result) +content_info_load(struct rpki_uri *uri, struct ContentInfo **result) { struct file_contents fc; int error; - error = file_load(uri->local, &fc); + error = file_load(uri_get_local(uri), &fc); if (error) return error; diff --git a/src/asn1/content_info.h b/src/asn1/content_info.h index 11c7969a..78ad8357 100644 --- a/src/asn1/content_info.h +++ b/src/asn1/content_info.h @@ -6,7 +6,7 @@ #include #include "uri.h" -int content_info_load(struct rpki_uri const *, struct ContentInfo **); +int content_info_load(struct rpki_uri *, struct ContentInfo **); void content_info_free(struct ContentInfo *); #endif /* SRC_CONTENT_INFO_H_ */ diff --git a/src/asn1/signed_data.c b/src/asn1/signed_data.c index 42e51224..4047bb12 100644 --- a/src/asn1/signed_data.c +++ b/src/asn1/signed_data.c @@ -20,7 +20,7 @@ static const OID oid_bsta = OID_BINARY_SIGNING_TIME_ATTR; int signed_object_args_init(struct signed_object_args *args, - struct rpki_uri const *uri, + struct rpki_uri *uri, STACK_OF(X509_CRL) *crls, bool force_inherit) { @@ -61,17 +61,15 @@ static int handle_sdata_certificate(ANY_t *cert_encoded, struct signed_object_args *args, OCTET_STRING_t *sid, ANY_t *signedData, SignatureValue_t *signature) { - struct validation *state; const unsigned char *tmp; X509 *cert; enum rpki_policy policy; int error; - state = state_retrieve(); - if (state == NULL) - return -EINVAL; - if (sk_X509_num(validation_certs(state)) >= config_get_max_cert_depth()) - return pr_err("Certificate chain maximum depth exceeded."); + /* + * No need to validate certificate chain length, since we just arrived + * to a tree leaf. Loops aren't possible. + */ pr_debug_add("EE Certificate (embedded) {"); diff --git a/src/asn1/signed_data.h b/src/asn1/signed_data.h index 75ca7089..da81780a 100644 --- a/src/asn1/signed_data.h +++ b/src/asn1/signed_data.h @@ -13,7 +13,7 @@ */ struct signed_object_args { /** Location of the signed object. */ - struct rpki_uri const *uri; + struct rpki_uri *uri; /** CRL that might or might not revoke the embedded certificate. */ STACK_OF(X509_CRL) *crls; /** A copy of the resources carried by the embedded certificate. */ @@ -25,8 +25,8 @@ struct signed_object_args { struct certificate_refs refs; }; -int signed_object_args_init(struct signed_object_args *, - struct rpki_uri const *, STACK_OF(X509_CRL) *, bool); +int signed_object_args_init(struct signed_object_args *, struct rpki_uri *, + STACK_OF(X509_CRL) *, bool); void signed_object_args_cleanup(struct signed_object_args *); int signed_data_decode(ANY_t *, struct signed_object_args *args, diff --git a/src/cert_stack.c b/src/cert_stack.c new file mode 100644 index 00000000..ace70c0f --- /dev/null +++ b/src/cert_stack.c @@ -0,0 +1,569 @@ +#include "cert_stack.h" + +#include + +#include "resource.h" +#include "str.h" +#include "thread_var.h" +#include "data_structure/array_list.h" +#include "object/name.h" +#include "object/certificate.h" + +enum defer_node_type { + DNT_SEPARATOR, + DNT_CERT, +}; + +struct defer_node { + enum defer_node_type type; + + /** + * This field is only relevant if @type == PCT_CERT. + * Do not dereference members otherwise. + */ + struct deferred_cert deferred; + + /** Used by certstack. Points to the next stacked certificate. */ + SLIST_ENTRY(defer_node) next; +}; + +SLIST_HEAD(defer_stack, defer_node); + +struct serial_number { + BIGNUM *number; + char *file; /* File where this serial number was found. */ +}; + +ARRAY_LIST(serial_numbers, struct serial_number) + +struct subject_name { + struct rfc5280_name *name; + char *file; /* File where this subject name was found. */ +}; + +ARRAY_LIST(subjects, struct subject_name) + +/** + * Cached certificate data. + */ +struct metadata_node { + struct rpki_uri *uri; + struct resources *resources; + /* + * Serial numbers of the children. + * This is an unsorted array list for two reasons: Certificates usually + * don't have many children, and I'm running out of time. + */ + struct serial_numbers serials; + struct subjects subjects; + + /** Used by certstack. Points to the next stacked certificate. */ + SLIST_ENTRY(metadata_node) next; +}; + +SLIST_HEAD(metadata_stack, metadata_node); + +/** + * This is the foundation through which we pull off our iterative traversal, + * as opposed to a stack-threatening recursive one. + * + * It is a bunch of data that replaces the one that would normally be allocated + * in the function stack. + */ +struct cert_stack { + /** + * Defer stack. Certificates we haven't iterated through yet. + * + * Every time a certificate validates successfully, its children are + * stored here so they can be traversed later. + */ + struct defer_stack defers; + + /** + * x509 stack. Parents of the certificate we're currently iterating + * through. + * Formatted for immediate libcrypto consumption. + */ + STACK_OF(X509) *x509s; + + /** + * Stacked additional data to each @x509 certificate. + * + * (These two stacks should always have the same size. The reason why I + * don't combine them is because libcrypto's validation function needs + * the X509 stack, and I'm not creating it over and over again.) + * + * (This is a SLIST and not a STACK_OF because the OpenSSL stack + * implementation is different than the LibreSSL one, and the latter is + * seemingly not intended to be used outside of its library.) + */ + struct metadata_stack metas; +}; + +int +certstack_create(struct cert_stack **result) +{ + struct cert_stack *stack; + + stack = malloc(sizeof(struct cert_stack)); + if (stack == NULL) + return pr_enomem(); + + stack->x509s = sk_X509_new_null(); + if (stack->x509s == NULL) { + free(stack); + return crypto_err("sk_X509_new_null() returned NULL"); + } + + SLIST_INIT(&stack->defers); + SLIST_INIT(&stack->metas); + + *result = stack; + return 0; +} + +static void +defer_destroy(struct defer_node *defer) +{ + switch (defer->type) { + case DNT_SEPARATOR: + break; + case DNT_CERT: + uri_refput(defer->deferred.uri); + rpp_refput(defer->deferred.pp); + break; + } + + free(defer); +} + +static void +serial_cleanup(struct serial_number *serial) +{ + BN_free(serial->number); + free(serial->file); +} + +static void +subject_cleanup(struct subject_name *subject) +{ + x509_name_put(subject->name); + free(subject->file); +} + +static void +meta_destroy(struct metadata_node *meta) +{ + uri_refput(meta->uri); + resources_destroy(meta->resources); + serial_numbers_cleanup(&meta->serials, serial_cleanup); + subjects_cleanup(&meta->subjects, subject_cleanup); + free(meta); +} + +void +certstack_destroy(struct cert_stack *stack) +{ + unsigned int stack_size; + struct metadata_node *meta; + struct defer_node *post; + + stack_size = 0; + while (!SLIST_EMPTY(&stack->defers)) { + post = SLIST_FIRST(&stack->defers); + SLIST_REMOVE_HEAD(&stack->defers, next); + defer_destroy(post); + stack_size++; + } + pr_info("Deleted %u deferred certificates.", stack_size); + + pr_info("Deleting %d stacked x509s.", sk_X509_num(stack->x509s)); + sk_X509_pop_free(stack->x509s, X509_free); + + stack_size = 0; + while (!SLIST_EMPTY(&stack->metas)) { + meta = SLIST_FIRST(&stack->metas); + SLIST_REMOVE_HEAD(&stack->metas, next); + meta_destroy(meta); + stack_size++; + } + pr_info("Deleted %u metadatas.", stack_size); + + free(stack); +} + +int +deferstack_push(struct cert_stack *stack, struct deferred_cert *deferred) +{ + struct defer_node *node; + + node = malloc(sizeof(struct defer_node)); + if (node == NULL) + return pr_enomem(); + + node->type = DNT_CERT; + node->deferred = *deferred; + uri_refget(deferred->uri); + rpp_refget(deferred->pp); + SLIST_INSERT_HEAD(&stack->defers, node, next); + return 0; +} + +static int +x509stack_pop(struct cert_stack *stack) +{ + X509 *cert; + struct metadata_node *meta; + + cert = sk_X509_pop(stack->x509s); + if (cert == NULL) + return pr_crit("Attempted to pop empty X509 stack"); + X509_free(cert); + + meta = SLIST_FIRST(&stack->metas); + if (meta == NULL) + return pr_crit("Attempted to pop empty metadata stack"); + SLIST_REMOVE_HEAD(&stack->metas, next); + meta_destroy(meta); + + return 0; +} + +int +deferstack_pop(struct cert_stack *stack, struct deferred_cert *result) +{ + struct defer_node *node; + int error; + +again: node = SLIST_FIRST(&stack->defers); + if (node == NULL) + return -ENOENT; + + if (node->type == DNT_SEPARATOR) { + error = x509stack_pop(stack); + if (error) + return error; + + SLIST_REMOVE_HEAD(&stack->defers, next); + defer_destroy(node); + goto again; + } + + *result = node->deferred; + uri_refget(node->deferred.uri); + rpp_refget(node->deferred.pp); + + SLIST_REMOVE_HEAD(&stack->defers, next); + defer_destroy(node); + return 0; +} + +bool +deferstack_is_empty(struct cert_stack *stack) +{ + return SLIST_EMPTY(&stack->defers); +} + +/** Steals ownership of @x509 on success. */ +int +x509stack_push(struct cert_stack *stack, struct rpki_uri *uri, X509 *x509, + enum rpki_policy policy, bool is_ta) +{ + struct metadata_node *meta; + struct defer_node *defer_separator; + int ok; + int error; + + meta = malloc(sizeof(struct metadata_node)); + if (meta == NULL) + return pr_enomem(); + + meta->uri = uri; + uri_refget(uri); + serial_numbers_init(&meta->serials); + subjects_init(&meta->subjects); + + meta->resources = resources_create(false); + if (meta->resources == NULL) { + error = pr_enomem(); + goto end4; + } + resources_set_policy(meta->resources, policy); + error = certificate_get_resources(x509, meta->resources); + if (error) + goto end5; + + /* + * rfc7730#section-2.2 + * "The INR extension(s) of this trust anchor MUST contain a non-empty + * set of number resources." + * The "It MUST NOT use the "inherit" form of the INR extension(s)" + * part is already handled in certificate_get_resources(). + */ + if (is_ta && resources_empty(meta->resources)) { + error = pr_err("Trust Anchor certificate does not define any number resources."); + goto end5; + } + + defer_separator = malloc(sizeof(struct defer_node)); + if (defer_separator == NULL) { + error = pr_enomem(); + goto end5; + } + defer_separator->type = DNT_SEPARATOR; + + ok = sk_X509_push(stack->x509s, x509); + if (ok <= 0) { + error = crypto_err( + "Could not add certificate to trusted stack: %d", ok); + goto end5; + } + + SLIST_INSERT_HEAD(&stack->defers, defer_separator, next); + SLIST_INSERT_HEAD(&stack->metas, meta, next); + + return 0; + +end5: resources_destroy(meta->resources); +end4: subjects_cleanup(&meta->subjects, subject_cleanup); + serial_numbers_cleanup(&meta->serials, serial_cleanup); + free(meta); + return error; +} + +/** + * This one is intended to revert a recent x509 push. + * Reverts that particular push. + * + * (x509 stack elements are otherwise indirecly popped through + * deferstack_pop().) + */ +void +x509stack_cancel(struct cert_stack *stack) +{ + struct defer_node *defer_separator; + + x509stack_pop(stack); + + defer_separator = SLIST_FIRST(&stack->defers); + if (defer_separator == NULL) { + pr_crit("Attempted to pop empty defer stack"); + return; + } + SLIST_REMOVE_HEAD(&stack->defers, next); + defer_destroy(defer_separator); +} + +X509 * +x509stack_peek(struct cert_stack *stack) +{ + return sk_X509_value(stack->x509s, sk_X509_num(stack->x509s) - 1); +} + +/** Does not grab reference. */ +struct rpki_uri * +x509stack_peek_uri(struct cert_stack *stack) +{ + struct metadata_node *meta = SLIST_FIRST(&stack->metas); + return (meta != NULL) ? meta->uri : NULL; +} + +struct resources * +x509stack_peek_resources(struct cert_stack *stack) +{ + struct metadata_node *meta = SLIST_FIRST(&stack->metas); + return (meta != NULL) ? meta->resources : NULL; +} + +static int +get_current_file_name(char **_result) +{ + char const *tmp; + char *result; + + tmp = fnstack_peek(); + if (tmp == NULL) + return pr_crit("The file name stack is empty."); + + result = strdup(tmp); + if (result == NULL) + return pr_enomem(); + + *_result = result; + return 0; +} + +/** + * Intended to validate serial number uniqueness. + * "Stores" the serial number in the current relevant certificate metadata, + * and complains if there's a collision. That's all. + * + * This function will steal ownership of @number on success. + */ +int +x509stack_store_serial(struct cert_stack *stack, BIGNUM *number) +{ + struct metadata_node *meta; + struct serial_number *cursor; + array_index i; + struct serial_number duplicate; + char *string; + int error; + + /* Remember to free @number if you return 0 but don't store it. */ + + meta = SLIST_FIRST(&stack->metas); + if (meta == NULL) { + BN_free(number); + return 0; /* The TA lacks siblings, so serial is unique. */ + } + + /* + * Note: This is is reported as a warning, even though duplicate serial + * numbers are clearly a violation of the RFC and common sense. + * + * But it cannot be simply upgraded into an error because we are + * realizing the problem too late; our traversal is depth-first, so we + * already approved the other bogus certificate and its children. + * (I don't think changing to a breath-first traversal would be a good + * idea; the RAM usage would skyrocket because, since we need the entire + * certificate path to the root to validate any certificate, we would + * end up having the entire tree loaded in memory by the time we're done + * traversing.) + * + * So instead of arbitrarily approving one certificate but not the + * other, we will accept both but report a warning. + * + * Also: It's pretty odd; a significant amount of certificates seem to + * be breaking this rule. Maybe we're the only ones catching it? + * + * TODO I haven't seen this warning in a while. Review. + */ + ARRAYLIST_FOREACH(&meta->serials, cursor, i) { + if (BN_cmp(cursor->number, number) == 0) { + BN2string(number, &string); + pr_warn("Serial number '%s' is not unique. (Also found in '%s'.)", + string, cursor->file); + BN_free(number); + free(string); + return 0; + } + } + + duplicate.number = number; + error = get_current_file_name(&duplicate.file); + if (error) + return error; + + error = serial_numbers_add(&meta->serials, &duplicate); + if (error) + free(duplicate.file); + + return error; +} + +/** + * Intended to validate subject uniqueness. + * "Stores" the subject in the current relevant certificate metadata, and + * complains if there's a collision. That's all. + */ +int +x509stack_store_subject(struct cert_stack *stack, struct rfc5280_name *subject) +{ + struct metadata_node *meta; + struct subject_name *cursor; + array_index i; + struct subject_name duplicate; + int error; + + /* + * There's something that's not clicking with me: + * + * "Each distinct subordinate CA and + * EE certified by the issuer MUST be identified using a subject name + * that is unique per issuer. In this context, 'distinct' is defined as + * an entity and a given public key." + * + * Does the last sentence have any significance to us? I don't even + * understand why the requirement exists. 5280 and 6487 don't even + * define "entity." I guess it's the same meaning from "End-Entity", + * and in this case it's supposed to function as a synonym for "subject + * name." + * + * "An issuer SHOULD use a different + * subject name if the subject's key pair has changed (i.e., when the CA + * issues a certificate as part of re-keying the subject.)" + * + * It's really weird that it seems to be rewording the same requirement + * except the first version is defined as MUST and the second one is + * defined as SHOULD. + * + * Ugh. Okay. Let's use some common sense. There are four possible + * situations: + * + * - Certificates do not share name nor public key. We should accept + * this. + * - Certificates share name, but not public key. We should reject this. + * - Certificates share public key, but not name. This is basically + * impossible, but fine nonetheless. Accept. + * (But maybe issue a warning. It sounds like the two children can + * impersonate each other.) + * - Certificates share name and public key. This likely means that we + * are looking at two different versions of the same certificate, but + * we can't tell for sure right now, and so we should accept it. + * + * This is all conjecture. We should probably mail the IETF. + * + * TODO (next iteration) The code below complains when certificates + * share names, and ignores public keys. I've decided to defer the + * fixing. + */ + + meta = SLIST_FIRST(&stack->metas); + if (meta == NULL) + return 0; /* The TA lacks siblings, so subject is unique. */ + + /* See the large comment in certstack_x509_store_serial(). */ + ARRAYLIST_FOREACH(&meta->subjects, cursor, i) { + if (x509_name_equals(cursor->name, subject)) { + char const *serial = x509_name_serialNumber(subject); + pr_warn("Subject name '%s%s%s' is not unique. (Also found in '%s'.)", + x509_name_commonName(subject), + (serial != NULL) ? "/" : "", + (serial != NULL) ? serial : "", + cursor->file); + return 0; + } + } + + duplicate.name = subject; + x509_name_get(subject); + + error = get_current_file_name(&duplicate.file); + if (error) + goto revert_name; + + error = subjects_add(&meta->subjects, &duplicate); + if (error) + goto revert_file; + + return 0; + +revert_file: + free(duplicate.file); +revert_name: + x509_name_put(subject); + return error; +} + +STACK_OF(X509) * +certstack_get_x509s(struct cert_stack *stack) +{ + return stack->x509s; +} + +int +certstack_get_x509_num(struct cert_stack *stack) +{ + return sk_X509_num(stack->x509s); +} diff --git a/src/cert_stack.h b/src/cert_stack.h new file mode 100644 index 00000000..6e0533c3 --- /dev/null +++ b/src/cert_stack.h @@ -0,0 +1,55 @@ +#ifndef SRC_CERT_STACK_H_ +#define SRC_CERT_STACK_H_ + +#include +#include "resource.h" +#include "uri.h" +#include "object/name.h" + +/* + * One certificate stack is allocated per validation cycle, and it is used + * through its entirety to hold the certificates relevant to the ongoing + * validation. + * + * Keep in mind: This module deals with two different (but correlated) stack + * data structures, and they both store "certificates" (albeit in different + * representations): + * + * - Defer stack: This one stores certificates whose validation has been + * postponed during the validation cycle. (They were found in some manifest + * list, and haven't been opened yet.) + * It prevents us from having to validate the RPKI tree in a recursive manner, + * which would be prone to stack overflow. + * - x509 stack: It is a chain of certificates, ready to be validated by + * libcrypto. + * For any given certificate being validated, this stack stores all of its + * parents. + */ + +struct cert_stack; + +struct deferred_cert { + struct rpki_uri *uri; + struct rpp *pp; +}; + +int certstack_create(struct cert_stack **); +void certstack_destroy(struct cert_stack *); + +int deferstack_push(struct cert_stack *, struct deferred_cert *cert); +int deferstack_pop(struct cert_stack *, struct deferred_cert *cert); +bool deferstack_is_empty(struct cert_stack *); + +int x509stack_push(struct cert_stack *, struct rpki_uri *, X509 *, + enum rpki_policy, bool); +void x509stack_cancel(struct cert_stack *); +X509 *x509stack_peek(struct cert_stack *); +struct rpki_uri *x509stack_peek_uri(struct cert_stack *); +struct resources *x509stack_peek_resources(struct cert_stack *); +int x509stack_store_serial(struct cert_stack *, BIGNUM *); +int x509stack_store_subject(struct cert_stack *, struct rfc5280_name *); + +STACK_OF(X509) *certstack_get_x509s(struct cert_stack *); +int certstack_get_x509_num(struct cert_stack *); + +#endif /* SRC_CERT_STACK_H_ */ diff --git a/src/certificate_refs.c b/src/certificate_refs.c index f3ddf2a3..2a4fca57 100644 --- a/src/certificate_refs.c +++ b/src/certificate_refs.c @@ -14,14 +14,16 @@ void refs_cleanup(struct certificate_refs *refs) { free(refs->crldp); - free(refs->caIssuers); - free(refs->signedObject); + if (refs->caIssuers != NULL) + uri_refput(refs->caIssuers); + if (refs->signedObject != NULL) + uri_refput(refs->signedObject); } static int validate_cdp(struct certificate_refs *refs, struct rpp const *pp) { - struct rpki_uri const *pp_crl; + struct rpki_uri *pp_crl; if (refs->crldp == NULL) return pr_crit("Certificate's CRL Distribution Point was not recorded."); @@ -30,9 +32,9 @@ validate_cdp(struct certificate_refs *refs, struct rpp const *pp) if (pp_crl == NULL) return pr_crit("Manifest's CRL was not recorded."); - if (strcmp(refs->crldp, pp_crl->global) != 0) { + if (strcmp(refs->crldp, uri_get_global(pp_crl)) != 0) { return pr_err("Certificate's CRL Distribution Point ('%s') does not match manifest's CRL ('%s').", - refs->crldp, pp_crl->global); + refs->crldp, uri_get_global(pp_crl)); } return 0; @@ -42,7 +44,7 @@ static int validate_aia(struct certificate_refs *refs) { struct validation *state; - struct rpki_uri const *parent; + struct rpki_uri *parent; if (refs->caIssuers == NULL) return pr_crit("Certificate's AIA was not recorded."); @@ -50,13 +52,14 @@ validate_aia(struct certificate_refs *refs) state = state_retrieve(); if (state == NULL) return -EINVAL; - parent = validation_peek_cert_uri(state); + parent = x509stack_peek_uri(validation_certstack(state)); if (parent == NULL) return pr_crit("CA certificate has no parent."); - if (strcmp(refs->caIssuers, parent->global) != 0) { + if (!uri_equals(refs->caIssuers, parent)) { return pr_err("Certificate's AIA ('%s') does not match parent's URI ('%s').", - refs->caIssuers, parent->global); + uri_get_printable(refs->caIssuers), + uri_get_printable(parent)); } return 0; @@ -64,14 +67,15 @@ validate_aia(struct certificate_refs *refs) static int validate_signedObject(struct certificate_refs *refs, - struct rpki_uri const *signedObject_uri) + struct rpki_uri *signedObject_uri) { if (refs->signedObject == NULL) return pr_crit("Certificate's signedObject was not recorded."); - if (strcmp(refs->signedObject, signedObject_uri->global) != 0) { + if (!uri_equals(refs->signedObject, signedObject_uri)) { return pr_err("Certificate's signedObject ('%s') does not match the URI of its own signed object (%s).", - refs->signedObject, signedObject_uri->global); + uri_get_printable(refs->signedObject), + uri_get_printable(signedObject_uri)); } return 0; @@ -102,7 +106,7 @@ refs_validate_ca(struct certificate_refs *refs, struct rpp const *pp) if (refs->signedObject != NULL) { return pr_crit("CA summary has a signedObject ('%s').", - refs->signedObject); + uri_get_printable(refs->signedObject)); } return 0; @@ -118,7 +122,7 @@ refs_validate_ca(struct certificate_refs *refs, struct rpp const *pp) */ int refs_validate_ee(struct certificate_refs *refs, struct rpp const *pp, - struct rpki_uri const *uri) + struct rpki_uri *uri) { int error; diff --git a/src/certificate_refs.h b/src/certificate_refs.h index a2dc8818..17b8384f 100644 --- a/src/certificate_refs.h +++ b/src/certificate_refs.h @@ -28,18 +28,18 @@ struct certificate_refs { * AIA's caIssuers. Non-TA certificates only. * RFC 6487, section 4.8.7. */ - char *caIssuers; + struct rpki_uri *caIssuers; /** * SIA's signedObject. EE certificates only. * RFC 6487, section 4.8.8.2. */ - char *signedObject; + struct rpki_uri *signedObject; }; void refs_init(struct certificate_refs *); void refs_cleanup(struct certificate_refs *); int refs_validate_ca(struct certificate_refs *, struct rpp const *); int refs_validate_ee(struct certificate_refs *, struct rpp const *, - struct rpki_uri const *); + struct rpki_uri *); #endif /* SRC_CERTIFICATE_REFS_H_ */ diff --git a/src/common.h b/src/common.h index bc136dbb..57851697 100644 --- a/src/common.h +++ b/src/common.h @@ -19,6 +19,8 @@ /* * If you're wondering why I'm not using -abs(error), it's because abs(INT_MIN) * overflows, so gcc complains sometimes. + * + * BE CAREFUL ABOUT DOUBLE EVALUATION. */ #define ENSURE_NEGATIVE(error) (((error) < 0) ? (error) : -(error)) diff --git a/src/console_handler.c b/src/console_handler.c index 3ab71049..a4fc44b7 100644 --- a/src/console_handler.c +++ b/src/console_handler.c @@ -27,8 +27,6 @@ validate_into_console(void) { struct validation_handler handler; - handler.merge = NULL; - handler.merge_arg = NULL; handler.reset = NULL; handler.handle_roa_v4 = print_v4_roa; handler.handle_roa_v6 = print_v6_roa; diff --git a/src/crypto/hash.c b/src/crypto/hash.c index 79926870..fa5ecdf8 100644 --- a/src/crypto/hash.c +++ b/src/crypto/hash.c @@ -32,8 +32,8 @@ hash_matches(unsigned char const *expected, size_t expected_len, } static int -hash_file(char const *algorithm, struct rpki_uri const *uri, - unsigned char *result, unsigned int *result_len) +hash_file(char const *algorithm, struct rpki_uri *uri, unsigned char *result, + unsigned int *result_len) { EVP_MD const *md; FILE *file; @@ -48,7 +48,7 @@ hash_file(char const *algorithm, struct rpki_uri const *uri, if (error) return error; - error = file_open(uri->local, &file, &stat); + error = file_open(uri_get_local(uri), &file, &stat); if (error) return error; @@ -103,7 +103,7 @@ end1: * "expected" hash). Returns 0 if no errors happened and the hashes match. */ int -hash_validate_file(char const *algorithm, struct rpki_uri const *uri, +hash_validate_file(char const *algorithm, struct rpki_uri *uri, BIT_STRING_t const *expected) { unsigned char actual[EVP_MAX_MD_SIZE]; diff --git a/src/crypto/hash.h b/src/crypto/hash.h index c6693ad9..3425e923 100644 --- a/src/crypto/hash.h +++ b/src/crypto/hash.h @@ -6,7 +6,7 @@ #include #include "uri.h" -int hash_validate_file(char const *, struct rpki_uri const *uri, +int hash_validate_file(char const *, struct rpki_uri *uri, BIT_STRING_t const *); int hash_validate(char const *, unsigned char const *, size_t, unsigned char const *, size_t); diff --git a/src/extension.c b/src/extension.c index 52edb127..2baa428f 100644 --- a/src/extension.c +++ b/src/extension.c @@ -325,7 +325,7 @@ handle_aki(X509_EXTENSION *ext, void *arg) goto end; } - parent = validation_peek_cert(state); + parent = x509stack_peek(validation_certstack(state)); if (parent == NULL) { error = pr_err("Certificate has no parent."); goto end; diff --git a/src/log.c b/src/log.c index 118e1497..e2ce9491 100644 --- a/src/log.c +++ b/src/log.c @@ -4,6 +4,7 @@ #include #include "config.h" +#include "debug.h" #include "thread_var.h" #ifdef DEBUG @@ -266,6 +267,8 @@ pr_crit(const char *format, ...) va_end(args); PR_SUFFIX(STDERR); + + print_stack_trace(); return -EINVAL; } diff --git a/src/object/certificate.c b/src/object/certificate.c index 3f1cbd5a..74318b92 100644 --- a/src/object/certificate.c +++ b/src/object/certificate.c @@ -57,7 +57,7 @@ validate_serial_number(X509 *cert) fprintf(stdout, "\n"); #endif - error = validation_store_serial_number(state, number); + error = x509stack_store_serial(validation_certstack(state), number); if (error) BN_free(number); @@ -105,7 +105,7 @@ validate_subject(X509 *cert) if (error) return error; - error = validation_store_subject(state, name); + error = x509stack_store_subject(validation_certstack(state), name); x509_name_put(name); return error; @@ -585,7 +585,7 @@ end: } int -certificate_load(struct rpki_uri const *uri, X509 **result) +certificate_load(struct rpki_uri *uri, X509 **result) { X509 *cert = NULL; BIO *bio; @@ -594,7 +594,7 @@ certificate_load(struct rpki_uri const *uri, X509 **result) bio = BIO_new(BIO_s_file()); if (bio == NULL) return crypto_err("BIO_new(BIO_s_file()) returned NULL"); - if (BIO_read_filename(bio, uri->local) <= 0) { + if (BIO_read_filename(bio, uri_get_local(uri)) <= 0) { error = crypto_err("Error reading certificate"); goto end; } @@ -642,7 +642,8 @@ certificate_validate_chain(X509 *cert, STACK_OF(X509_CRL) *crls) goto abort; } - X509_STORE_CTX_trusted_stack(ctx, validation_certs(state)); + X509_STORE_CTX_trusted_stack(ctx, + certstack_get_x509s(validation_certstack(state))); X509_STORE_CTX_set0_crls(ctx, crls); /* @@ -856,10 +857,9 @@ is_rsync_uri(GENERAL_NAME *name) static int handle_rpkiManifest(struct rpki_uri *uri, void *arg) { - struct rpki_uri *mft = arg; - *mft = *uri; - uri->global = NULL; - uri->local = NULL; + struct rpki_uri **mft = arg; + *mft = uri; + uri_refget(uri); return 0; } @@ -875,8 +875,8 @@ handle_signedObject(struct rpki_uri *uri, void *arg) { struct certificate_refs *refs = arg; pr_debug("signedObject: %s", uri_get_printable(uri)); - refs->signedObject = uri->global; - uri->global = NULL; + refs->signedObject = uri; + uri_refget(uri); return 0; } @@ -1154,7 +1154,7 @@ handle_ad(char const *ia_name, SIGNATURE_INFO_ACCESS *ia, int (*cb)(struct rpki_uri *, void *), void *arg) { ACCESS_DESCRIPTION *ad; - struct rpki_uri uri; + struct rpki_uri *uri; bool found = false; int i; int error; @@ -1162,25 +1162,25 @@ handle_ad(char const *ia_name, SIGNATURE_INFO_ACCESS *ia, for (i = 0; i < sk_ACCESS_DESCRIPTION_num(ia); i++) { ad = sk_ACCESS_DESCRIPTION_value(ia, i); if (OBJ_obj2nid(ad->method) == ad_nid) { - error = uri_init_ad(&uri, ad); + error = uri_create_ad(&uri, ad); if (error == ENOTRSYNC) continue; if (error) return error; if (found) { - uri_cleanup(&uri); + uri_refput(uri); return pr_err("Extension '%s' has multiple '%s' RSYNC URIs.", ia_name, ad_name); } - error = cb(&uri, arg); + error = cb(uri, arg); if (error) { - uri_cleanup(&uri); + uri_refput(uri); return error; } - uri_cleanup(&uri); + uri_refput(uri); found = true; } } @@ -1203,8 +1203,8 @@ handle_caIssuers(struct rpki_uri *uri, void *arg) * over here is too much trouble, so do the handle_cdp() * hack. */ - refs->caIssuers = uri->global; - uri->global = NULL; + refs->caIssuers = uri; + uri_refget(uri); return 0; } @@ -1343,7 +1343,7 @@ handle_ar(X509_EXTENSION *ext, void *arg) } int -certificate_validate_extensions_ta(X509 *cert, struct rpki_uri *mft, +certificate_validate_extensions_ta(X509 *cert, struct rpki_uri **mft, enum rpki_policy *policy) { struct extension_handler handlers[] = { @@ -1365,7 +1365,7 @@ certificate_validate_extensions_ta(X509 *cert, struct rpki_uri *mft, } int -certificate_validate_extensions_ca(X509 *cert, struct rpki_uri *mft, +certificate_validate_extensions_ca(X509 *cert, struct rpki_uri **mft, struct certificate_refs *refs, enum rpki_policy *policy) { struct extension_handler handlers[] = { @@ -1415,17 +1415,17 @@ certificate_validate_extensions_ee(X509 *cert, OCTET_STRING_t *sid, return handle_extensions(handlers, X509_get0_extensions(cert)); } -/* Boilerplate code for CA certificate validation and recursive traversal. */ +/** Boilerplate code for CA certificate validation and recursive traversal. */ int -certificate_traverse(struct rpp *rpp_parent, struct rpki_uri const *cert_uri, - STACK_OF(X509_CRL) *crls) +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; X509 *cert; - struct rpki_uri mft; + struct rpki_uri *mft; struct certificate_refs refs; enum rpki_policy policy; struct rpp *pp; @@ -1434,10 +1434,11 @@ certificate_traverse(struct rpp *rpp_parent, struct rpki_uri const *cert_uri, state = state_retrieve(); if (state == NULL) return -EINVAL; - if (sk_X509_num(validation_certs(state)) >= config_get_max_cert_depth()) + total_parents = certstack_get_x509_num(validation_certstack(state)); + if (total_parents >= config_get_max_cert_depth()) return pr_err("Certificate chain maximum depth exceeded."); - pr_debug_add("%s Certificate '%s' {", is_ta ? "TA" : "CA", + pr_debug_add("%s Certificate '%s' {", IS_TA ? "TA" : "CA", uri_get_printable(cert_uri)); fnstack_push_uri(cert_uri); memset(&refs, 0, sizeof(refs)); @@ -1446,7 +1447,7 @@ certificate_traverse(struct rpp *rpp_parent, struct rpki_uri const *cert_uri, error = certificate_load(cert_uri, &cert); if (error) goto revert_fnstack_and_debug; - error = certificate_validate_chain(cert, crls); + error = certificate_validate_chain(cert, rpp_crl(rpp_parent)); if (error) goto revert_cert; error = certificate_validate_rfc6487(cert, IS_TA); @@ -1463,25 +1464,28 @@ certificate_traverse(struct rpp *rpp_parent, struct rpki_uri const *cert_uri, goto revert_uri_and_refs; /* -- Validate the manifest (@mft) pointed by the certificate -- */ - error = validation_push_cert(state, cert_uri, cert, policy, IS_TA); + error = x509stack_push(validation_certstack(state), cert_uri, cert, + policy, IS_TA); if (error) goto revert_uri_and_refs; + cert = NULL; /* Ownership stolen */ - error = handle_manifest(&mft, crls, &pp); - if (error) - goto revert_cert_push; + error = handle_manifest(mft, rpp_crl(rpp_parent), &pp); + if (error) { + x509stack_cancel(validation_certstack(state)); + goto revert_uri_and_refs; + } /* -- Validate & traverse the RPP (@pp) described by the manifest -- */ - error = rpp_traverse(pp); + rpp_traverse(pp); - rpp_destroy(pp); -revert_cert_push: - validation_pop_cert(state); /* Error code is useless. */ + rpp_refput(pp); revert_uri_and_refs: - uri_cleanup(&mft); + uri_refput(mft); refs_cleanup(&refs); revert_cert: - X509_free(cert); + if (cert != NULL) + X509_free(cert); revert_fnstack_and_debug: fnstack_pop(); pr_debug_rm("}"); diff --git a/src/object/certificate.h b/src/object/certificate.h index 2c9e4e25..c2ad3951 100644 --- a/src/object/certificate.h +++ b/src/object/certificate.h @@ -10,7 +10,7 @@ #include "rpp.h" #include "uri.h" -int certificate_load(struct rpki_uri const *, X509 **); +int certificate_load(struct rpki_uri *, X509 **); /** * Performs the basic (RFC 5280, presumably) chain validation. @@ -42,7 +42,7 @@ int certificate_get_resources(X509 *, struct resources *); * Also initializes the second argument as the URI of the rpkiManifest Access * Description from the SIA extension. */ -int certificate_validate_extensions_ta(X509 *, struct rpki_uri *, +int certificate_validate_extensions_ta(X509 *, struct rpki_uri **, enum rpki_policy *); /** * Validates the certificate extensions, (intermediate) Certificate Authority @@ -53,7 +53,7 @@ int certificate_validate_extensions_ta(X509 *, struct rpki_uri *, * Also initializes the third argument with the references found in the * extensions. */ -int certificate_validate_extensions_ca(X509 *, struct rpki_uri *, +int certificate_validate_extensions_ca(X509 *, struct rpki_uri **, struct certificate_refs *, enum rpki_policy *); /** * Validates the certificate extensions, End-Entity style. @@ -64,7 +64,6 @@ int certificate_validate_extensions_ca(X509 *, struct rpki_uri *, int certificate_validate_extensions_ee(X509 *, OCTET_STRING_t *, struct certificate_refs *, enum rpki_policy *); -int certificate_traverse(struct rpp *, struct rpki_uri const *, - STACK_OF(X509_CRL) *); +int certificate_traverse(struct rpp *, struct rpki_uri *); #endif /* SRC_OBJECT_CERTIFICATE_H_ */ diff --git a/src/object/crl.c b/src/object/crl.c index aa15a5fb..53d0d6f4 100644 --- a/src/object/crl.c +++ b/src/object/crl.c @@ -8,7 +8,7 @@ #include "object/name.h" static int -__crl_load(struct rpki_uri const *uri, X509_CRL **result) +__crl_load(struct rpki_uri *uri, X509_CRL **result) { X509_CRL *crl; BIO *bio; @@ -17,14 +17,16 @@ __crl_load(struct rpki_uri const *uri, X509_CRL **result) bio = BIO_new(BIO_s_file()); if (bio == NULL) return crypto_err("BIO_new(BIO_s_file()) returned NULL"); - if (BIO_read_filename(bio, uri->local) <= 0) { - error = crypto_err("Error reading CRL '%s'", uri->local); + if (BIO_read_filename(bio, uri_get_local(uri)) <= 0) { + error = crypto_err("Error reading CRL '%s'", + uri_get_printable(uri)); goto end; } crl = d2i_X509_CRL_bio(bio, NULL); if (crl == NULL) { - error = crypto_err("Error parsing CRL '%s'", uri->local); + error = crypto_err("Error parsing CRL '%s'", + uri_get_printable(uri)); goto end; } @@ -137,7 +139,7 @@ crl_validate(X509_CRL *crl) } int -crl_load(struct rpki_uri const *uri, X509_CRL **result) +crl_load(struct rpki_uri *uri, X509_CRL **result) { int error; pr_debug_add("CRL '%s' {", uri_get_printable(uri)); diff --git a/src/object/crl.h b/src/object/crl.h index f54a7fb0..b71fa7f8 100644 --- a/src/object/crl.h +++ b/src/object/crl.h @@ -4,6 +4,6 @@ #include #include "uri.h" -int crl_load(struct rpki_uri const *uri, X509_CRL **); +int crl_load(struct rpki_uri *uri, X509_CRL **); #endif /* SRC_OBJECT_CRL_H_ */ diff --git a/src/object/ghostbusters.c b/src/object/ghostbusters.c index 72c28515..da663604 100644 --- a/src/object/ghostbusters.c +++ b/src/object/ghostbusters.c @@ -14,18 +14,17 @@ handle_vcard(OCTET_STRING_t *vcard, void *arg) } int -ghostbusters_traverse(struct rpki_uri const *uri, struct rpp *pp, - STACK_OF(X509_CRL) *crls) +ghostbusters_traverse(struct rpki_uri *uri, struct rpp *pp) { static OID oid = OID_GHOSTBUSTERS; struct oid_arcs arcs = OID2ARCS("ghostbusters", oid); struct signed_object_args sobj_args; int error; - pr_debug_add("Ghostbusters '%s' {", uri->global); + pr_debug_add("Ghostbusters '%s' {", uri_get_printable(uri)); fnstack_push_uri(uri); - error = signed_object_args_init(&sobj_args, uri, crls, true); + error = signed_object_args_init(&sobj_args, uri, rpp_crl(pp), true); if (error) goto end1; diff --git a/src/object/ghostbusters.h b/src/object/ghostbusters.h index b4bbbd8c..e6b04666 100644 --- a/src/object/ghostbusters.h +++ b/src/object/ghostbusters.h @@ -1,11 +1,9 @@ #ifndef SRC_OBJECT_GHOSTBUSTERS_H_ #define SRC_OBJECT_GHOSTBUSTERS_H_ -#include #include "uri.h" #include "rpp.h" -int ghostbusters_traverse(struct rpki_uri const *, struct rpp *, - STACK_OF(X509_CRL) *); +int ghostbusters_traverse(struct rpki_uri *, struct rpp *); #endif /* SRC_OBJECT_GHOSTBUSTERS_H_ */ diff --git a/src/object/manifest.c b/src/object/manifest.c index 7d2d7753..b1b3244d 100644 --- a/src/object/manifest.c +++ b/src/object/manifest.c @@ -15,11 +15,6 @@ #include "object/roa.h" #include "object/signed_object.h" -struct manifest { - struct Manifest *obj; - char const *file_path; -}; - static int manifest_decode(OCTET_STRING_t *string, void *arg) { @@ -146,21 +141,22 @@ validate_manifest(struct Manifest *manifest) } static int -__handle_manifest(struct manifest *mft, struct rpp **pp) +__handle_manifest(struct Manifest *mft, struct rpki_uri *mft_uri, + struct rpp **pp) { int i; struct FileAndHash *fah; - struct rpki_uri uri; + struct rpki_uri *uri; int error; *pp = rpp_create(); if (*pp == NULL) return pr_enomem(); - for (i = 0; i < mft->obj->fileList.list.count; i++) { - fah = mft->obj->fileList.list.array[i]; + for (i = 0; i < mft->fileList.list.count; i++) { + fah = mft->fileList.list.array[i]; - error = uri_init_mft(&uri, mft->file_path, &fah->file); + error = uri_create_mft(&uri, mft_uri, &fah->file); /* * Not handling ENOTRSYNC is fine because the manifest URL * should have been RSYNC. Something went wrong if an RSYNC URL @@ -169,25 +165,25 @@ __handle_manifest(struct manifest *mft, struct rpp **pp) if (error) goto fail; - error = hash_validate_file("sha256", &uri, &fah->hash); + error = hash_validate_file("sha256", uri, &fah->hash); if (error) { - uri_cleanup(&uri); + uri_refput(uri); continue; } - if (uri_has_extension(&uri, ".cer")) - error = rpp_add_cert(*pp, &uri); - else if (uri_has_extension(&uri, ".roa")) - error = rpp_add_roa(*pp, &uri); - else if (uri_has_extension(&uri, ".crl")) - error = rpp_add_crl(*pp, &uri); - else if (uri_has_extension(&uri, ".gbr")) - error = rpp_add_ghostbusters(*pp, &uri); + if (uri_has_extension(uri, ".cer")) + error = rpp_add_cert(*pp, uri); + else if (uri_has_extension(uri, ".roa")) + error = rpp_add_roa(*pp, uri); + else if (uri_has_extension(uri, ".crl")) + error = rpp_add_crl(*pp, uri); + else if (uri_has_extension(uri, ".gbr")) + error = rpp_add_ghostbusters(*pp, uri); else - uri_cleanup(&uri); /* ignore it. */ + uri_refput(uri); /* ignore it. */ if (error) { - uri_cleanup(&uri); + uri_refput(uri); goto fail; } /* Otherwise ownership was transferred to @pp. */ } @@ -201,7 +197,7 @@ __handle_manifest(struct manifest *mft, struct rpp **pp) return 0; fail: - rpp_destroy(*pp); + rpp_refput(*pp); return error; } @@ -210,13 +206,12 @@ fail: * @pp. */ int -handle_manifest(struct rpki_uri const *uri, STACK_OF(X509_CRL) *crls, - struct rpp **pp) +handle_manifest(struct rpki_uri *uri, STACK_OF(X509_CRL) *crls, struct rpp **pp) { static OID oid = OID_MANIFEST; struct oid_arcs arcs = OID2ARCS("manifest", oid); struct signed_object_args sobj_args; - struct manifest mft; + struct Manifest *mft; int error; pr_debug_add("Manifest '%s' {", uri_get_printable(uri)); @@ -225,26 +220,24 @@ handle_manifest(struct rpki_uri const *uri, STACK_OF(X509_CRL) *crls, error = signed_object_args_init(&sobj_args, uri, crls, false); if (error) goto end1; - mft.file_path = uri->global; - error = signed_object_decode(&sobj_args, &arcs, - manifest_decode, &mft.obj); + error = signed_object_decode(&sobj_args, &arcs, manifest_decode, &mft); if (error) goto end2; - error = validate_manifest(mft.obj); + error = validate_manifest(mft); if (error) goto end3; - error = __handle_manifest(&mft, pp); + error = __handle_manifest(mft, uri, pp); if (error) goto end3; error = refs_validate_ee(&sobj_args.refs, *pp, uri); if (error) - rpp_destroy(*pp); + rpp_refput(*pp); end3: - ASN_STRUCT_FREE(asn_DEF_Manifest, mft.obj); + ASN_STRUCT_FREE(asn_DEF_Manifest, mft); end2: signed_object_args_cleanup(&sobj_args); end1: diff --git a/src/object/manifest.h b/src/object/manifest.h index 3bc894bd..ded148c0 100644 --- a/src/object/manifest.h +++ b/src/object/manifest.h @@ -5,7 +5,6 @@ #include "uri.h" #include "rpp.h" -int handle_manifest(struct rpki_uri const *, STACK_OF(X509_CRL) *, - struct rpp **); +int handle_manifest(struct rpki_uri *, STACK_OF(X509_CRL) *, struct rpp **); #endif /* SRC_OBJECT_MANIFEST_H_ */ diff --git a/src/object/name.c b/src/object/name.c index 2aed1570..d800fde2 100644 --- a/src/object/name.c +++ b/src/object/name.c @@ -161,7 +161,7 @@ validate_issuer_name(char const *container, X509_NAME *issuer) state = state_retrieve(); if (state == NULL) return -EINVAL; - parent = validation_peek_cert(state); + parent = x509stack_peek(validation_certstack(state)); if (parent == NULL) { return pr_err("%s appears to have no parent certificate.", container); @@ -184,12 +184,12 @@ validate_issuer_name(char const *container, X509_NAME *issuer) error = pr_err("%s's issuer name ('%s%s%s') does not equal issuer certificate's name ('%s%s%s').", container, - x509_name_commonName(parent_subject), - (parent_serial != NULL) ? "/" : "", - (parent_serial != NULL) ? parent_serial : "", x509_name_commonName(child_issuer), (child_serial != NULL) ? "/" : "", - (child_serial != NULL) ? child_serial : ""); + (child_serial != NULL) ? child_serial : "", + x509_name_commonName(parent_subject), + (parent_serial != NULL) ? "/" : "", + (parent_serial != NULL) ? parent_serial : ""); } x509_name_put(child_issuer); diff --git a/src/object/roa.c b/src/object/roa.c index 8e04fad7..4e563fab 100644 --- a/src/object/roa.c +++ b/src/object/roa.c @@ -185,8 +185,7 @@ family_error: } int -roa_traverse(struct rpki_uri const *uri, struct rpp *pp, - STACK_OF(X509_CRL) *crls) +roa_traverse(struct rpki_uri *uri, struct rpp *pp) { static OID oid = OID_ROA; struct oid_arcs arcs = OID2ARCS("roa", oid); @@ -197,7 +196,7 @@ roa_traverse(struct rpki_uri const *uri, struct rpp *pp, pr_debug_add("ROA '%s' {", uri_get_printable(uri)); fnstack_push_uri(uri); - error = signed_object_args_init(&sobj_args, uri, crls, false); + error = signed_object_args_init(&sobj_args, uri, rpp_crl(pp), false); if (error) goto revert_fnstack; diff --git a/src/object/roa.h b/src/object/roa.h index 961ba3f6..09183026 100644 --- a/src/object/roa.h +++ b/src/object/roa.h @@ -1,12 +1,10 @@ #ifndef SRC_OBJECT_ROA_H_ #define SRC_OBJECT_ROA_H_ -#include - #include "address.h" #include "rpp.h" #include "uri.h" -int roa_traverse(struct rpki_uri const *, struct rpp *, STACK_OF(X509_CRL) *); +int roa_traverse(struct rpki_uri *, struct rpp *); #endif /* SRC_OBJECT_ROA_H_ */ diff --git a/src/object/tal.c b/src/object/tal.c index a8e881d8..97091029 100644 --- a/src/object/tal.c +++ b/src/object/tal.c @@ -9,6 +9,7 @@ #include #include +#include "cert_stack.h" #include "common.h" #include "config.h" #include "line_file.h" @@ -215,12 +216,12 @@ void tal_destroy(struct tal *tal) int foreach_uri(struct tal *tal, foreach_uri_cb cb, void *arg) { - struct rpki_uri uri; + struct rpki_uri *uri; unsigned int i; int error; for (i = 0; i < tal->uris.count; i++) { - error = uri_init_str(&uri, tal->uris.array[i], + error = uri_create_str(&uri, tal->uris.array[i], strlen(tal->uris.array[i])); if (error == ENOTRSYNC) { /* Log level should probably be INFO. */ @@ -230,8 +231,8 @@ foreach_uri(struct tal *tal, foreach_uri_cb cb, void *arg) if (error) return error; - error = cb(tal, &uri, arg); - uri_cleanup(&uri); + error = cb(tal, uri, arg); + uri_refput(uri); if (error) return error; } @@ -276,7 +277,7 @@ tal_get_spki(struct tal *tal, unsigned char const **buffer, size_t *len) * have been extracted from a TAL. */ static int -handle_tal_uri(struct tal *tal, struct rpki_uri const *uri, void *arg) +handle_tal_uri(struct tal *tal, struct rpki_uri *uri, void *arg) { /* * Because of the way the foreach iterates, this function must return @@ -293,12 +294,14 @@ handle_tal_uri(struct tal *tal, struct rpki_uri const *uri, void *arg) */ struct validation *state; + struct cert_stack *certstack; + struct deferred_cert deferred; int error; error = download_files(uri, true); if (error) { - return pr_warn("TAL URI '%s' could not be RSYNC'd.", - uri->global); + return pr_warn("TAL '%s' could not be RSYNC'd.", + uri_get_printable(uri)); } error = validation_prepare(&state, tal, arg); @@ -312,31 +315,62 @@ handle_tal_uri(struct tal *tal, struct rpki_uri const *uri, void *arg) pr_debug_add("TAL URI '%s' {", uri_get_printable(uri)); if (!uri_is_certificate(uri)) { - pr_err("TAL file does not point to a certificate. (Expected .cer, got '%s')", + error = pr_err("TAL file does not point to a certificate. (Expected .cer, got '%s')", uri_get_printable(uri)); - error = -EINVAL; - goto end; + goto fail; } - error = certificate_traverse(NULL, uri, NULL); + /* Handle root certificate. */ + error = certificate_traverse(NULL, uri); if (error) { switch (validation_pubkey_state(state)) { case PKS_INVALID: - error = 0; - break; + error = 0; /* Try a different TAL URI. */ + goto end; case PKS_VALID: case PKS_UNTESTED: - error = ENSURE_NEGATIVE(error); - break; + goto fail; /* Reject the TAL. */ } - } else { - error = vhandler_merge(arg); - if (error) - error = ENSURE_NEGATIVE(error); - else - error = 1; + + error = pr_crit("Unknown public key state: %u", + validation_pubkey_state(state)); + goto fail; + } + + /* + * From now on, the tree should be considered valid, even if subsequent + * certificates fail. + * (the root validated successfully; subtrees are isolated problems.) + */ + + /* Handle every other certificate. */ + certstack = validation_certstack(state); + if (certstack == NULL) { + error = pr_crit("Validation state has no certificate stack"); + goto fail; } + do { + error = deferstack_pop(certstack, &deferred); + if (error == -ENOENT) { + /* No more certificates left; we're done. */ + error = 1; + goto end; + } + if (error) + goto fail; + + /* + * Ignore result code; remaining certificates are unrelated, + * so they should not be affected. + */ + certificate_traverse(deferred.pp, deferred.uri); + + uri_refput(deferred.uri); + rpp_refput(deferred.pp); + } while (true); + +fail: error = ENSURE_NEGATIVE(error); end: validation_destroy(state); pr_debug_rm("}"); return error; diff --git a/src/object/tal.h b/src/object/tal.h index 06b53263..d5564d36 100644 --- a/src/object/tal.h +++ b/src/object/tal.h @@ -12,7 +12,7 @@ struct tal; int tal_load(char const *, struct tal **); void tal_destroy(struct tal *); -typedef int (*foreach_uri_cb)(struct tal *, struct rpki_uri const *, void *); +typedef int (*foreach_uri_cb)(struct tal *, struct rpki_uri *, void *); int foreach_uri(struct tal *, foreach_uri_cb, void *); void tal_shuffle_uris(struct tal *); diff --git a/src/resource.c b/src/resource.c index d144c968..36e1f7f8 100644 --- a/src/resource.c +++ b/src/resource.c @@ -88,7 +88,9 @@ static struct resources * get_parent_resources(void) { struct validation *state = state_retrieve(); - return (state != NULL) ? validation_peek_resource(state) : NULL; + return (state != NULL) + ? x509stack_peek_resources(validation_certstack(state)) + : NULL; } static int diff --git a/src/rpp.c b/src/rpp.c index bb74f77b..640413b1 100644 --- a/src/rpp.c +++ b/src/rpp.c @@ -1,6 +1,7 @@ #include "rpp.h" #include +#include "cert_stack.h" #include "log.h" #include "thread_var.h" #include "uri.h" @@ -10,20 +11,28 @@ #include "object/ghostbusters.h" #include "object/roa.h" -ARRAY_LIST(uris, struct rpki_uri) +ARRAY_LIST(uris, struct rpki_uri *) /** A Repository Publication Point (RFC 6481), as described by some manifest. */ struct rpp { struct uris certs; /* Certificates */ - struct rpki_uri crl; /* Certificate Revocation List */ + struct rpki_uri *crl; /* Certificate Revocation List */ bool crl_set; + /* Initialized lazily. Access via rpp_crl(). */ + STACK_OF(X509_CRL) *crl_stack; /* The Manifest is not needed for now. */ struct uris roas; /* Route Origin Attestations */ struct uris ghostbusters; + + /* + * Note that the reference counting functions are not prepared for + * multithreading, because this is not atomic. + */ + unsigned int references; }; struct rpp * @@ -37,40 +46,63 @@ rpp_create(void) uris_init(&result->certs); result->crl_set = false; + result->crl_stack = NULL; uris_init(&result->roas); uris_init(&result->ghostbusters); + result->references = 1; return result; } void -rpp_destroy(struct rpp *pp) +rpp_refget(struct rpp *pp) +{ + pp->references++; +} + +void +__uri_refput(struct rpki_uri **uri) { - uris_cleanup(&pp->certs, uri_cleanup); - uri_cleanup(&pp->crl); - uris_cleanup(&pp->roas, uri_cleanup); - uris_cleanup(&pp->ghostbusters, uri_cleanup); - free(pp); + uri_refput(*uri); } +void +rpp_refput(struct rpp *pp) +{ + pp->references--; + if (pp->references == 0) { + uris_cleanup(&pp->certs, __uri_refput); + uri_refput(pp->crl); + if (pp->crl_stack != NULL) + sk_X509_CRL_pop_free(pp->crl_stack, X509_CRL_free); + uris_cleanup(&pp->roas, __uri_refput); + uris_cleanup(&pp->ghostbusters, __uri_refput); + free(pp); + } +} + +/** Steals ownership of @uri. */ int rpp_add_cert(struct rpp *pp, struct rpki_uri *uri) { - return uris_add(&pp->certs, uri); + return uris_add(&pp->certs, &uri); } +/** Steals ownership of @uri. */ int rpp_add_roa(struct rpp *pp, struct rpki_uri *uri) { - return uris_add(&pp->roas, uri); + return uris_add(&pp->roas, &uri); } +/** Steals ownership of @uri. */ int rpp_add_ghostbusters(struct rpp *pp, struct rpki_uri *uri) { - return uris_add(&pp->ghostbusters, uri); + return uris_add(&pp->ghostbusters, &uri); } +/** Steals ownership of @uri. */ int rpp_add_crl(struct rpp *pp, struct rpki_uri *uri) { @@ -78,11 +110,17 @@ rpp_add_crl(struct rpp *pp, struct rpki_uri *uri) if (pp->crl_set) return pr_err("Repository Publication Point has more than one CRL."); - pp->crl = *uri; + pp->crl = uri; pp->crl_set = true; return 0; } +struct rpki_uri * +rpp_get_crl(struct rpp const *pp) +{ + return pp->crl_set ? pp->crl : NULL; +} + static int add_crl_to_stack(struct rpp *pp, STACK_OF(X509_CRL) *crls) { @@ -90,9 +128,9 @@ add_crl_to_stack(struct rpp *pp, STACK_OF(X509_CRL) *crls) int error; int idx; - fnstack_push_uri(&pp->crl); + fnstack_push_uri(pp->crl); - error = crl_load(&pp->crl, &crl); + error = crl_load(pp->crl, &crl); if (error) goto end; @@ -108,43 +146,91 @@ end: return error; } -struct rpki_uri const * -rpp_get_crl(struct rpp const *pp) +STACK_OF(X509_CRL) * +rpp_crl(struct rpp *pp) { - return pp->crl_set ? &pp->crl : NULL; + if (pp == NULL) + return NULL; + if (!pp->crl_set) + return NULL; + if (pp->crl_stack != NULL) + return pp->crl_stack; + + pp->crl_stack = sk_X509_CRL_new_null(); + if (pp->crl_stack == NULL) + return NULL; + if (add_crl_to_stack(pp, pp->crl_stack) != 0) { + sk_X509_CRL_pop_free(pp->crl_stack, X509_CRL_free); + return NULL; + } + + return pp->crl_stack; } -int -rpp_traverse(struct rpp *pp) +static int +__cert_traverse(struct rpp *pp) { + struct validation *state; + struct cert_stack *certstack; + ssize_t i; + struct deferred_cert deferred; + int error; + + if (pp->certs.len == 0) + return 0; + + state = state_retrieve(); + if (state == NULL) + return -EINVAL; + certstack = validation_certstack(state); + + deferred.pp = pp; /* - * TODO is the stack supposed to have only the CRLs of this layer, - * or all of them? + * The for is inverted, to achieve FIFO behavior since the separator. + * Not really important; it simply makes the traversal order more + * intuitive. */ - STACK_OF(X509_CRL) *crls; - struct rpki_uri *uri; + for (i = pp->certs.len - 1; i >= 0; i--) { + deferred.uri = pp->certs.array[i]; + error = deferstack_push(certstack, &deferred); + if (error) + return error; + } + + return 0; +} + +/** + * Traverses through all of @pp's known files, validating them. + */ +void +rpp_traverse(struct rpp *pp) +{ + struct rpki_uri **uri; array_index i; - int error; - crls = sk_X509_CRL_new_null(); - if (crls == NULL) - return pr_enomem(); - error = add_crl_to_stack(pp, crls); - if (error) - goto end; + /* + * A subtree should not invalidate the rest of the tree, so error codes + * are ignored. + * (Errors log messages anyway.) + */ - /* Use CRL stack to validate certificates, and also traverse them. */ - ARRAYLIST_FOREACH(&pp->certs, uri, i) - certificate_traverse(pp, uri, crls); + /* + * Certificates cannot be validated now, because then the algorithm + * would be recursive. + * Store them in the defer stack (see cert_stack.h), will get back to + * them later. + */ + __cert_traverse(pp); - /* Use valid address ranges to print ROAs that match them. */ + /* Validate ROAs, apply validation_handler on them. */ ARRAYLIST_FOREACH(&pp->roas, uri, i) - roa_traverse(uri, pp, crls); + roa_traverse(*uri, pp); + /* + * We don't do much with the ghostbusters right now. + * Just validate them. + */ ARRAYLIST_FOREACH(&pp->ghostbusters, uri, i) - ghostbusters_traverse(uri, pp, crls); - -end: - sk_X509_CRL_pop_free(crls, X509_CRL_free); - return error; + ghostbusters_traverse(*uri, pp); } diff --git a/src/rpp.h b/src/rpp.h index 7d17dbd1..3b310c24 100644 --- a/src/rpp.h +++ b/src/rpp.h @@ -6,15 +6,17 @@ struct rpp; struct rpp *rpp_create(void); -void rpp_destroy(struct rpp *); +void rpp_refget(struct rpp *pp); +void rpp_refput(struct rpp *pp); int rpp_add_cert(struct rpp *, struct rpki_uri *); int rpp_add_crl(struct rpp *, struct rpki_uri *); int rpp_add_roa(struct rpp *, struct rpki_uri *); int rpp_add_ghostbusters(struct rpp *, struct rpki_uri *); -struct rpki_uri const *rpp_get_crl(struct rpp const *); +struct rpki_uri *rpp_get_crl(struct rpp const *); +STACK_OF(X509_CRL) *rpp_crl(struct rpp *); -int rpp_traverse(struct rpp *); +void rpp_traverse(struct rpp *); #endif /* SRC_RPP_H_ */ diff --git a/src/rsync/rsync.c b/src/rsync/rsync.c index da6e5108..d023f683 100644 --- a/src/rsync/rsync.c +++ b/src/rsync/rsync.c @@ -14,8 +14,7 @@ #include "str.h" struct uri { - char *string; - size_t len; + struct rpki_uri *uri; SLIST_ENTRY(uri) next; }; @@ -39,7 +38,7 @@ rsync_destroy(void) while (!SLIST_EMPTY(&visited_uris)) { uri = SLIST_FIRST(&visited_uris); SLIST_REMOVE_HEAD(&visited_uris, next); - free(uri->string); + uri_refput(uri->uri); free(uri); } } @@ -49,15 +48,15 @@ rsync_destroy(void) * Returns false otherwise. */ static bool -is_descendant(struct uri *ancestor, struct rpki_uri const *descendant) +is_descendant(struct rpki_uri *ancestor, struct rpki_uri *descendant) { struct string_tokenizer ancestor_tokenizer; struct string_tokenizer descendant_tokenizer; - string_tokenizer_init(&ancestor_tokenizer, ancestor->string, - ancestor->len, '/'); - string_tokenizer_init(&descendant_tokenizer, descendant->global, - descendant->global_len, '/'); + string_tokenizer_init(&ancestor_tokenizer, uri_get_global(ancestor), + uri_get_global_len(ancestor), '/'); + string_tokenizer_init(&descendant_tokenizer, uri_get_global(descendant), + uri_get_global_len(descendant), '/'); do { if (!string_tokenizer_next(&ancestor_tokenizer)) @@ -74,13 +73,13 @@ is_descendant(struct uri *ancestor, struct rpki_uri const *descendant) * run. */ static bool -is_already_downloaded(struct rpki_uri const *uri) +is_already_downloaded(struct rpki_uri *uri) { struct uri *cursor; /* TODO (next iteration) this is begging for a radix trie. */ SLIST_FOREACH(cursor, &visited_uris, next) - if (is_descendant(cursor, uri)) + if (is_descendant(cursor->uri, uri)) return true; return false; @@ -95,9 +94,8 @@ mark_as_downloaded(struct rpki_uri *uri) if (node == NULL) return pr_enomem(); - node->string = uri->global; - node->len = uri->global_len; - uri->global = NULL; /* Ownership transferred. */ + node->uri = uri; + uri_refget(uri); SLIST_INSERT_HEAD(&visited_uris, node, next); @@ -105,33 +103,42 @@ mark_as_downloaded(struct rpki_uri *uri) } static int -handle_strict_strategy(struct rpki_uri const *requested_uri, - struct rpki_uri *rsync_uri) +handle_strict_strategy(struct rpki_uri *requested_uri, + struct rpki_uri **rsync_uri) { - return uri_clone(requested_uri, rsync_uri); + *rsync_uri = requested_uri; + uri_refget(requested_uri); + return 0; } static int -handle_root_strategy(struct rpki_uri const *src, struct rpki_uri *dst) +handle_root_strategy(struct rpki_uri *src, struct rpki_uri **dst) { + char const *global; + size_t global_len; unsigned int slashes; size_t i; + global = uri_get_global(src); + global_len = uri_get_global_len(src); slashes = 0; - for (i = 0; i < src->global_len; i++) { - if (src->global[i] == '/') { + + for (i = 0; i < global_len; i++) { + if (global[i] == '/') { slashes++; if (slashes == 4) - return uri_init_str(dst, src->global, i); + return uri_create_str(dst, global, i); } } - return uri_clone(src, dst); + *dst = src; + uri_refget(src); + return 0; } static int -get_rsync_uri(struct rpki_uri const *requested_uri, bool is_ta, - struct rpki_uri *rsync_uri) +get_rsync_uri(struct rpki_uri *requested_uri, bool is_ta, + struct rpki_uri **rsync_uri) { switch (config_get_sync_strategy()) { case SYNC_ROOT: @@ -150,7 +157,7 @@ get_rsync_uri(struct rpki_uri const *requested_uri, bool is_ta, } static int -dir_exists(char *path, bool *result) +dir_exists(char const *path, bool *result) { struct stat _stat; char *last_slash; @@ -202,18 +209,22 @@ create_dir(char *path) * This function fixes that. */ static int -create_dir_recursive(char *localuri) +create_dir_recursive(struct rpki_uri *uri) { + char *localuri; int i, error; bool exist = false; - error = dir_exists(localuri, &exist); + error = dir_exists(uri_get_local(uri), &exist); if (error) return error; - if (exist) return 0; + localuri = strdup(uri_get_local(uri)); + if (localuri == NULL) + return pr_enomem(); + for (i = 1; localuri[i] != '\0'; i++) { if (localuri[i] == '/') { localuri[i] = '\0'; @@ -221,11 +232,13 @@ create_dir_recursive(char *localuri) localuri[i] = '/'; if (error) { /* error msg already printed */ + free(localuri); return error; } } } + free(localuri); return 0; } @@ -257,9 +270,11 @@ handle_child_thread(struct rpki_uri *uri, bool is_ta) for (i = 1; i < config_args->length + 1; i++) { if (strcmp(copy_args[i], "$REMOTE") == 0) - copy_args[i] = uri->global; + copy_args[i] = strdup(uri_get_global(uri)); else if (strcmp(copy_args[i], "$LOCAL") == 0) - copy_args[i] = uri->local; + copy_args[i] = strdup(uri_get_local(uri)); + if (copy_args[i] == NULL) + exit(pr_enomem()); } pr_debug("Executing RSYNC:"); @@ -271,12 +286,7 @@ handle_child_thread(struct rpki_uri *uri, bool is_ta) pr_err("Could not execute the rsync command: %s", strerror(error)); - /* - * https://stackoverflow.com/a/14493459/1735458 - * Might as well. Prrbrrrlllt. - */ - free(copy_args); - + /* https://stackoverflow.com/a/14493459/1735458 */ exit(error); } @@ -290,7 +300,7 @@ do_rsync(struct rpki_uri *uri, bool is_ta) int child_status; int error; - error = create_dir_recursive(uri->local); + error = create_dir_recursive(uri); if (error) return error; @@ -348,7 +358,7 @@ do_rsync(struct rpki_uri *uri, bool is_ta) * validated the TA's public key. */ int -download_files(struct rpki_uri const *requested_uri, bool is_ta) +download_files(struct rpki_uri *requested_uri, bool is_ta) { /** * Note: @@ -356,14 +366,15 @@ download_files(struct rpki_uri const *requested_uri, bool is_ta) * @rsync_uri is the URL we're actually going to RSYNC. * (They can differ, depending on config_get_sync_strategy().) */ - struct rpki_uri rsync_uri; + struct rpki_uri *rsync_uri; int error; if (config_get_sync_strategy() == SYNC_OFF) return 0; if (is_already_downloaded(requested_uri)) { - pr_debug("No need to redownload '%s'.", requested_uri->global); + pr_debug("No need to redownload '%s'.", + uri_get_printable(requested_uri)); return 0; } @@ -371,13 +382,12 @@ download_files(struct rpki_uri const *requested_uri, bool is_ta) if (error) return error; - pr_debug("Going to RSYNC '%s' ('%s').", rsync_uri.global, - rsync_uri.local); + pr_debug("Going to RSYNC '%s'.", uri_get_printable(rsync_uri)); - error = do_rsync(&rsync_uri, is_ta); + error = do_rsync(rsync_uri, is_ta); if (!error) - error = mark_as_downloaded(&rsync_uri); + error = mark_as_downloaded(rsync_uri); - uri_cleanup(&rsync_uri); + uri_refput(rsync_uri); return error; } diff --git a/src/rsync/rsync.h b/src/rsync/rsync.h index bb94814a..f567d9e7 100644 --- a/src/rsync/rsync.h +++ b/src/rsync/rsync.h @@ -4,7 +4,7 @@ #include #include "uri.h" -int download_files(struct rpki_uri const *, bool); +int download_files(struct rpki_uri *, bool); int rsync_init(void); void rsync_destroy(void); diff --git a/src/rtr/db/vrps.c b/src/rtr/db/vrps.c index aebc7d8e..bcbe6ede 100644 --- a/src/rtr/db/vrps.c +++ b/src/rtr/db/vrps.c @@ -88,12 +88,6 @@ vrps_destroy(void) pthread_rwlock_destroy(&lock); /* Nothing to do with error code */ } -static int -__merge(void *dst, void *src) -{ - return rtrhandler_merge(dst, src); -} - static int __reset(void *arg) { @@ -131,8 +125,6 @@ __perform_standalone_validation(struct roa_table **result) return pr_enomem(); } - validation_handler.merge = __merge; - validation_handler.merge_arg = global_roas; validation_handler.reset = __reset; validation_handler.handle_roa_v4 = __handle_roa_v4; validation_handler.handle_roa_v6 = __handle_roa_v6; diff --git a/src/state.c b/src/state.c index 675280c7..b136bdc2 100644 --- a/src/state.c +++ b/src/state.c @@ -1,46 +1,8 @@ #include "state.h" -#include #include -#include #include "log.h" -#include "str.h" #include "thread_var.h" -#include "data_structure/array_list.h" -#include "object/certificate.h" - -struct serial_number { - BIGNUM *number; - char *file; /* File where this serial number was found. */ -}; - -struct subject_name { - struct rfc5280_name *name; - char *file; /* File where this subject name was found. */ -}; - -ARRAY_LIST(serial_numbers, struct serial_number) -ARRAY_LIST(subjects, struct subject_name) - -/** - * Cached certificate data. - */ -struct certificate { - struct rpki_uri uri; - struct resources *resources; - /* - * Serial numbers of the children. - * This is an unsorted array list for two reasons: Certificates usually - * don't have many children, and I'm running out of time. - */ - struct serial_numbers serials; - struct subjects subjects; - - /** Used by certstack. Points to the next stacked certificate. */ - SLIST_ENTRY(certificate) next; -}; - -SLIST_HEAD(certstack, certificate); /** * The current state of the validation cycle. @@ -55,21 +17,7 @@ struct validation { /** https://www.openssl.org/docs/man1.1.1/man3/X509_STORE_load_locations.html */ X509_STORE *store; - /** Certificates we've already validated. */ - STACK_OF(X509) *trusted; - - /** - * Stacked additional data to each @trusted certificate. - * - * (These two stacks should always have the same size. The reason why I - * don't combine them is because libcrypto's validation function needs - * the X509 stack, and I'm not creating it over and over again.) - * - * (This is a SLIST and not a STACK_OF because the OpenSSL stack - * implementation is different than the LibreSSL one, and the latter is - * seemingly not intended to be used outside of its library.) - */ - struct certstack certs; + struct cert_stack *certstack; /* Did the TAL's public key match the root certificate's public key? */ enum pubkey_state pubkey_state; @@ -122,7 +70,10 @@ cb(int ok, X509_STORE_CTX *ctx) return (error == X509_V_ERR_UNHANDLED_CRITICAL_EXTENSION) ? 1 : ok; } -/** Creates a struct validation, puts it in thread local, and returns it. */ +/** + * Creates a struct validation, puts it in thread local, and (incidentally) + * returns it. + */ int validation_prepare(struct validation **out, struct tal *tal, struct validation_handler *validation_handler) @@ -148,13 +99,10 @@ validation_prepare(struct validation **out, struct tal *tal, X509_STORE_set_verify_cb(result->store, cb); - result->trusted = sk_X509_new_null(); - if (result->trusted == NULL) { - error = crypto_err("sk_X509_new_null() returned NULL"); + error = certstack_create(&result->certstack); + if (error) goto abort2; - } - SLIST_INIT(&result->certs); result->pubkey_state = PKS_UNTESTED; result->validation_handler = *validation_handler; @@ -168,47 +116,11 @@ abort1: return error; } -static void -serial_cleanup(struct serial_number *serial) -{ - BN_free(serial->number); - free(serial->file); -} - -static void -subject_cleanup(struct subject_name *subject) -{ - x509_name_put(subject->name); - free(subject->file); -} - void validation_destroy(struct validation *state) { - int cert_num; - struct certificate *cert; - unsigned int c; - - cert_num = sk_X509_num(state->trusted); - if (cert_num != 0) { - pr_err("Error: validation state has %d certificates. (0 expected)", - cert_num); - } - - c = 0; - while (!SLIST_EMPTY(&state->certs)) { - cert = SLIST_FIRST(&state->certs); - SLIST_REMOVE_HEAD(&state->certs, next); - resources_destroy(cert->resources); - serial_numbers_cleanup(&cert->serials, serial_cleanup); - subjects_cleanup(&cert->subjects, subject_cleanup); - free(cert); - c++; - } - pr_debug("Deleted %u certificates from the stack.", c); - - sk_X509_pop_free(state->trusted, X509_free); X509_STORE_free(state->store); + certstack_destroy(state->certstack); free(state); } @@ -224,299 +136,30 @@ validation_store(struct validation *state) return state->store; } -STACK_OF(X509) * -validation_certs(struct validation *state) +struct cert_stack * +validation_certstack(struct validation *state) { - return state->trusted; + return state->certstack; } -void validation_pubkey_valid(struct validation *state) +void +validation_pubkey_valid(struct validation *state) { state->pubkey_state = PKS_VALID; } -void validation_pubkey_invalid(struct validation *state) +void +validation_pubkey_invalid(struct validation *state) { state->pubkey_state = PKS_INVALID; } -enum pubkey_state validation_pubkey_state(struct validation *state) +enum pubkey_state +validation_pubkey_state(struct validation *state) { return state->pubkey_state; } -/** - * Ownership of @cert_uri's members will NOT be transferred to @state on - * success, and they will be expected to outlive @x509. - */ -int -validation_push_cert(struct validation *state, struct rpki_uri const *cert_uri, - X509 *x509, enum rpki_policy policy, bool is_ta) -{ - struct certificate *cert; - int ok; - int error; - - cert = malloc(sizeof(struct certificate)); - if (cert == NULL) { - error = pr_enomem(); - goto end1; - } - - cert->uri = *cert_uri; - serial_numbers_init(&cert->serials); - subjects_init(&cert->subjects); - cert->resources = resources_create(false); - if (cert->resources == NULL) { - error = pr_enomem(); - goto end4; - } - - resources_set_policy(cert->resources, policy); - error = certificate_get_resources(x509, cert->resources); - if (error) - goto end5; - - /* - * rfc7730#section-2.2 - * "The INR extension(s) of this trust anchor MUST contain a non-empty - * set of number resources." - * The "It MUST NOT use the "inherit" form of the INR extension(s)" - * part is already handled in certificate_get_resources(). - */ - if (is_ta && resources_empty(cert->resources)) { - error = pr_err("Trust Anchor certificate does not define any number resources."); - goto end5; - } - - ok = sk_X509_push(state->trusted, x509); - if (ok <= 0) { - error = crypto_err( - "Couldn't add certificate to trusted stack: %d", ok); - goto end5; - } - - SLIST_INSERT_HEAD(&state->certs, cert, next); - - return 0; - -end5: resources_destroy(cert->resources); -end4: subjects_cleanup(&cert->subjects, subject_cleanup); - serial_numbers_cleanup(&cert->serials, serial_cleanup); - free(cert); -end1: return error; -} - -int -validation_pop_cert(struct validation *state) -{ - struct certificate *cert; - - if (sk_X509_pop(state->trusted) == NULL) - return pr_crit("Attempted to pop empty certificate stack (1)"); - - cert = SLIST_FIRST(&state->certs); - if (cert == NULL) - return pr_crit("Attempted to pop empty certificate stack (2)"); - SLIST_REMOVE_HEAD(&state->certs, next); - resources_destroy(cert->resources); - serial_numbers_cleanup(&cert->serials, serial_cleanup); - subjects_cleanup(&cert->subjects, subject_cleanup); - free(cert); - - return 0; -} - -X509 * -validation_peek_cert(struct validation *state) -{ - return sk_X509_value(state->trusted, sk_X509_num(state->trusted) - 1); -} - -struct rpki_uri const * -validation_peek_cert_uri(struct validation *state) -{ - struct certificate *cert = SLIST_FIRST(&state->certs); - return (cert != NULL) ? &cert->uri : NULL; -} - -struct resources * -validation_peek_resource(struct validation *state) -{ - struct certificate *cert = SLIST_FIRST(&state->certs); - return (cert != NULL) ? cert->resources : NULL; -} - -static int -get_current_file_name(char **_result) -{ - char const *tmp; - char *result; - - tmp = fnstack_peek(); - if (tmp == NULL) - return pr_crit("The file name stack is empty."); - - result = strdup(tmp); - if (result == NULL) - return pr_enomem(); - - *_result = result; - return 0; -} - -/** - * This function will steal ownership of @number on success. - */ -int -validation_store_serial_number(struct validation *state, BIGNUM *number) -{ - struct certificate *cert; - struct serial_number *cursor; - array_index i; - struct serial_number duplicate; - char *string; - int error; - - /* Remember to free @number if you return 0 but don't store it. */ - - cert = SLIST_FIRST(&state->certs); - if (cert == NULL) { - BN_free(number); - return 0; /* The TA lacks siblings, so serial is unique. */ - } - - /* - * Note: This is is reported as a warning, even though duplicate serial - * numbers are clearly a violation of the RFC and common sense. - * - * But it cannot be simply upgraded into an error because we are - * realizing the problem too late; our traversal is depth-first, so we - * already approved the other bogus certificate and its children. - * (I don't think changing to a breath-first traversal would be a good - * idea; the RAM usage would skyrocket because, since we need the entire - * certificate path to the root to validate any certificate, we would - * end up having the entire tree loaded in memory by the time we're done - * traversing.) - * - * So instead of arbitrarily approving one certificate but not the - * other, we will accept both but report a warning. - * - * Also: It's pretty odd; a significant amount of certificates seem to - * be breaking this rule. Maybe we're the only ones catching it? - */ - ARRAYLIST_FOREACH(&cert->serials, cursor, i) { - if (BN_cmp(cursor->number, number) == 0) { - BN2string(number, &string); - pr_warn("Serial number '%s' is not unique. (Also found in '%s'.)", - string, cursor->file); - BN_free(number); - free(string); - return 0; - } - } - - duplicate.number = number; - error = get_current_file_name(&duplicate.file); - if (error) - return error; - - error = serial_numbers_add(&cert->serials, &duplicate); - if (error) - free(duplicate.file); - - return error; -} - -int -validation_store_subject(struct validation *state, struct rfc5280_name *subject) -{ - struct certificate *cert; - struct subject_name *cursor; - array_index i; - struct subject_name duplicate; - int error; - - /* - * There's something that's not clicking with me: - * - * "Each distinct subordinate CA and - * EE certified by the issuer MUST be identified using a subject name - * that is unique per issuer. In this context, 'distinct' is defined as - * an entity and a given public key." - * - * Does the last sentence have any significance to us? I don't even - * understand why the requirement exists. 5280 and 6487 don't even - * define "entity." I guess it's the same meaning from "End-Entity", - * and in this case it's supposed to function as a synonym for "subject - * name." - * - * "An issuer SHOULD use a different - * subject name if the subject's key pair has changed (i.e., when the CA - * issues a certificate as part of re-keying the subject.)" - * - * It's really weird that it seems to be rewording the same requirement - * except the first version is defined as MUST and the second one is - * defined as SHOULD. - * - * Ugh. Okay. Let's use some common sense. There are four possible - * situations: - * - * - Certificates do not share name nor public key. We should accept - * this. - * - Certificates share name, but not public key. We should reject this. - * - Certificates share public key, but not name. This is basically - * impossible, but fine nonetheless. Accept. - * (But maybe issue a warning. It sounds like the two children can - * impersonate each other.) - * - Certificates share name and public key. This likely means that we - * are looking at two different versions of the same certificate, but - * we can't tell for sure right now, and so we should accept it. - * - * This is all conjecture. We should probably mail the IETF. - * - * TODO (next iteration) The code below complains when certificates - * share names, and ignores public keys. I've decided to defer the - * fixing. - */ - - cert = SLIST_FIRST(&state->certs); - if (cert == NULL) - return 0; /* The TA lacks siblings, so subject is unique. */ - - /* See the large comment in validation_store_serial_number(). */ - ARRAYLIST_FOREACH(&cert->subjects, cursor, i) { - if (x509_name_equals(cursor->name, subject)) { - char const *serial = x509_name_serialNumber(subject); - pr_warn("Subject name '%s%s%s' is not unique. (Also found in '%s'.)", - x509_name_commonName(subject), - (serial != NULL) ? "/" : "", - (serial != NULL) ? serial : "", - cursor->file); - return 0; - } - } - - duplicate.name = subject; - x509_name_get(subject); - - error = get_current_file_name(&duplicate.file); - if (error) - goto revert_name; - - error = subjects_add(&cert->subjects, &duplicate); - if (error) - goto revert_file; - - return 0; - -revert_file: - free(duplicate.file); -revert_name: - x509_name_put(subject); - return error; -} - char * validation_get_ip_buffer1(struct validation *state) { diff --git a/src/state.h b/src/state.h index d7d39517..448ccf8a 100644 --- a/src/state.h +++ b/src/state.h @@ -2,9 +2,8 @@ #define SRC_STATE_H_ #include -#include "resource.h" +#include "cert_stack.h" #include "validation_handler.h" -#include "object/name.h" #include "object/tal.h" struct validation; @@ -15,7 +14,7 @@ void validation_destroy(struct validation *); struct tal *validation_tal(struct validation *); X509_STORE *validation_store(struct validation *); -STACK_OF(X509) *validation_certs(struct validation *); +struct cert_stack *validation_certstack(struct validation *state); enum pubkey_state { PKS_VALID, @@ -27,20 +26,10 @@ void validation_pubkey_valid(struct validation *); void validation_pubkey_invalid(struct validation *); enum pubkey_state validation_pubkey_state(struct validation *); -int validation_push_cert(struct validation *, struct rpki_uri const *, X509 *, - enum rpki_policy, bool); -int validation_pop_cert(struct validation *); -X509 *validation_peek_cert(struct validation *); -struct rpki_uri const *validation_peek_cert_uri(struct validation *); - -struct resources *validation_peek_resource(struct validation *); - -int validation_store_serial_number(struct validation *, BIGNUM *); -int validation_store_subject(struct validation *, struct rfc5280_name *); - char *validation_get_ip_buffer1(struct validation *); char *validation_get_ip_buffer2(struct validation *); -struct validation_handler const *validation_get_validation_handler(struct validation *); +struct validation_handler const * +validation_get_validation_handler(struct validation *); #endif /* SRC_STATE_H_ */ diff --git a/src/thread_var.c b/src/thread_var.c index b61b7cf1..05f1fdc4 100644 --- a/src/thread_var.c +++ b/src/thread_var.c @@ -159,9 +159,14 @@ fnstack_push(char const *file) files->filenames[files->len++] = file; } -/** See fnstack_push(). */ +/** + * See fnstack_push(). + * + * This function cannot claim a reference for @uri, so @uri will have to outlive + * the push/pop. + */ void -fnstack_push_uri(struct rpki_uri const *uri) +fnstack_push_uri(struct rpki_uri *uri) { fnstack_push(uri_get_printable(uri)); } @@ -198,7 +203,7 @@ addr2str(int af, void const *addr, char *(*buffer_cb)(struct validation *)) struct validation *state; state = state_retrieve(); - if (!state) + if (state == NULL) return NULL; return inet_ntop(af, addr, buffer_cb(state), INET6_ADDRSTRLEN); diff --git a/src/thread_var.h b/src/thread_var.h index 0f2f2d2a..15769964 100644 --- a/src/thread_var.h +++ b/src/thread_var.h @@ -12,7 +12,7 @@ void fnstack_init(void); void fnstack_cleanup(void); void fnstack_push(char const *); -void fnstack_push_uri(struct rpki_uri const *); +void fnstack_push_uri(struct rpki_uri *); char const *fnstack_peek(void); void fnstack_pop(void); diff --git a/src/uri.c b/src/uri.c index 351103ee..5f231b91 100644 --- a/src/uri.c +++ b/src/uri.c @@ -5,6 +5,57 @@ #include "log.h" #include "str.h" +/** + * All rpki_uris are guaranteed to be RSYNC URLs right now. + * + * Design notes: + * + * Because we need to generate @local from @global, @global's allowed character + * set must be a subset of @local. Because this is Unix, @local must never + * contain NULL (except as a terminating character). Therefore, even though IA5 + * allows NULL, @global won't. + * + * Because we will simply embed @global (minus "rsync://") into @local, @local's + * encoding must be IA5-compatible. In other words, UTF-16 and UTF-32 are out of + * the question. + * + * Aside from the reference counter, instances are meant to be immutable. + */ +struct rpki_uri { + /** + * "Global URI". + * The one that always starts with "rsync://". + * + * These things are IA5-encoded, which means you're not bound to get + * non-ASCII characters. + */ + char *global; + /** Length of @global. */ + size_t global_len; + + /** + * "Local URI". + * The file pointed by @global, but cached in the local filesystem. + * + * I can't find a standard that defines this, but lots of complaints on + * the Internet imply that Unix file paths are specifically meant to be + * C strings. + * + * So just to clarify: This is a string that permits all characters, + * printable or otherwise, except \0. (Because that's the terminating + * character.) + * + * Even though it might contain characters that are non-printable + * according to ASCII, we assume that we can just dump it into the + * output without trouble, because the input should have the same + * encoding as the output. + */ + char *local; + /* "local_len" is never needed right now. */ + + unsigned int references; +}; + /* * @character is an integer because we sometimes receive signed chars, and other * times we get unsigned chars. @@ -187,50 +238,72 @@ autocomplete_local(struct rpki_uri *uri) } int -uri_init(struct rpki_uri *uri, void const *guri, size_t guri_len) +uri_create(struct rpki_uri **result, void const *guri, size_t guri_len) { + struct rpki_uri *uri; int error; + uri = malloc(sizeof(struct rpki_uri)); + if (uri == NULL) + return pr_enomem(); + error = str2global(guri, guri_len, uri); - if (error) + if (error) { + free(uri); return error; + } error = autocomplete_local(uri); if (error) { free(uri->global); + free(uri); return error; } + uri->references = 1; + *result = uri; return 0; } int -uri_init_str(struct rpki_uri *uri, char const *guri, size_t guri_len) +uri_create_str(struct rpki_uri **uri, char const *guri, size_t guri_len) { - return uri_init(uri, guri, guri_len); + return uri_create(uri, guri, guri_len); } /* * Manifests URIs are a little special in that they are relative. */ int -uri_init_mft(struct rpki_uri *uri, char const *mft, IA5String_t *ia5) +uri_create_mft(struct rpki_uri **result, struct rpki_uri *mft, IA5String_t *ia5) { + struct rpki_uri *uri; int error; - error = ia5str2global(uri, mft, ia5); - if (error) + uri = malloc(sizeof(struct rpki_uri)); + if (uri == NULL) + return pr_enomem(); + + error = ia5str2global(uri, mft->global, ia5); + if (error) { + free(uri); return error; + } error = autocomplete_local(uri); - if (error) + if (error) { free(uri->global); + free(uri); + return error; + } - return error; + uri->references = 1; + *result = uri; + return 0; } int -uri_init_ad(struct rpki_uri *uri, ACCESS_DESCRIPTION *ad) +uri_create_ad(struct rpki_uri **uri, ACCESS_DESCRIPTION *ad) { ASN1_STRING *asn1_string; int type; @@ -272,37 +345,54 @@ uri_init_ad(struct rpki_uri *uri, ACCESS_DESCRIPTION *ad) * directory our g2l version of @asn1_string should contain. * But ask the testers to keep an eye on it anyway. */ - return uri_init(uri, ASN1_STRING_get0_data(asn1_string), + return uri_create(uri, ASN1_STRING_get0_data(asn1_string), ASN1_STRING_length(asn1_string)); } -int -uri_clone(struct rpki_uri const *old, struct rpki_uri *copy) +void +uri_refget(struct rpki_uri *uri) { - copy->global = strdup(old->global); - if (copy->global == NULL) - return pr_enomem(); - copy->global_len = old->global_len; + uri->references++; +} - copy->local = strdup(old->local); - if (copy->local == NULL) { - free(copy->global); - return pr_enomem(); +void +uri_refput(struct rpki_uri *uri) +{ + uri->references--; + if (uri->references == 0) { + free(uri->global); + free(uri->local); + free(uri); } +} - return 0; +char const * +uri_get_global(struct rpki_uri *uri) +{ + return uri->global; } -void -uri_cleanup(struct rpki_uri *uri) +char const * +uri_get_local(struct rpki_uri *uri) +{ + return uri->local; +} + +size_t +uri_get_global_len(struct rpki_uri *uri) +{ + return uri->global_len; +} + +bool +uri_equals(struct rpki_uri *u1, struct rpki_uri *u2) { - free(uri->global); - free(uri->local); + return strcmp(u1->global, u2->global) == 0; } /* @ext must include the period. */ bool -uri_has_extension(struct rpki_uri const *uri, char const *ext) +uri_has_extension(struct rpki_uri *uri, char const *ext) { size_t ext_len; int cmp; @@ -316,7 +406,7 @@ uri_has_extension(struct rpki_uri const *uri, char const *ext) } bool -uri_is_certificate(struct rpki_uri const *uri) +uri_is_certificate(struct rpki_uri *uri) { return uri_has_extension(uri, ".cer"); } @@ -329,7 +419,7 @@ get_filename(char const *file_path) } char const * -uri_get_printable(struct rpki_uri const *uri) +uri_get_printable(struct rpki_uri *uri) { enum filename_format format; diff --git a/src/uri.h b/src/uri.h index abd41d14..65f5056e 100644 --- a/src/uri.h +++ b/src/uri.h @@ -5,65 +5,27 @@ #include #include -/** - * These are expected to live on the stack, or as part of other objects. - * - * All rpki_uris are guaranteed to be RSYNC URLs right now. - * - * Design notes: - * - * Because we need to generate @local from @global, @global's allowed character - * set must be a subset of @local. Because this is Unix, @local must never - * contain NULL (except as a terminating character). Therefore, even though IA5 - * allows NULL, @global won't. - * - * Because we will simply embed @global (minus "rsync://") into @local, @local's - * encoding must be IA5-compatible. In other words, UTF-16 and UTF-32 are out of - * the question. - */ -struct rpki_uri { - /** - * "Global URI". - * The one that always starts with "rsync://". - * - * These things are IA5-encoded, which means you're not bound to get - * non-ASCII characters. - */ - char *global; - /** Length of @global. */ - size_t global_len; +struct rpki_uri; - /** - * "Local URI". - * The file pointed by @global, but cached in the local filesystem. - * - * I can't find a standard that defines this, but lots of complaints on - * the Internet imply that Unix file paths are specifically meant to be - * C strings. - * - * So just to clarify: This is a string that permits all characters, - * printable or otherwise, except \0. (Because that's the terminating - * character.) - * - * Even though it might contain characters that are non-printable - * according to ASCII, we assume that we can just dump it into the - * output without trouble, because the input should have the same - * encoding as the output. - */ - char *local; +int uri_create(struct rpki_uri **, void const *, size_t); +int uri_create_str(struct rpki_uri **, char const *, size_t); +int uri_create_mft(struct rpki_uri **, struct rpki_uri *, IA5String_t *); +int uri_create_ad(struct rpki_uri **, ACCESS_DESCRIPTION *); - /* "local_len" is not needed for now. */ -}; +void uri_refget(struct rpki_uri *); +void uri_refput(struct rpki_uri *); -int uri_init(struct rpki_uri *, void const *, size_t); -int uri_init_str(struct rpki_uri *, char const *, size_t); -int uri_init_mft(struct rpki_uri *, char const *, IA5String_t *); -int uri_init_ad(struct rpki_uri *, ACCESS_DESCRIPTION *); -int uri_clone(struct rpki_uri const *, struct rpki_uri *); -void uri_cleanup(struct rpki_uri *); +/* + * Note that, if you intend to print some URI, you're likely supposed to use + * uri_get_printable() instead. + */ +char const *uri_get_global(struct rpki_uri *); +char const *uri_get_local(struct rpki_uri *); +size_t uri_get_global_len(struct rpki_uri *); -bool uri_has_extension(struct rpki_uri const *, char const *); -bool uri_is_certificate(struct rpki_uri const *); -char const *uri_get_printable(struct rpki_uri const *); +bool uri_equals(struct rpki_uri *, struct rpki_uri *); +bool uri_has_extension(struct rpki_uri *, char const *); +bool uri_is_certificate(struct rpki_uri *); +char const *uri_get_printable(struct rpki_uri *); #endif /* SRC_URI_H_ */ diff --git a/src/validation_handler.c b/src/validation_handler.c index 0540d001..94861e85 100644 --- a/src/validation_handler.c +++ b/src/validation_handler.c @@ -4,13 +4,6 @@ #include "log.h" #include "thread_var.h" -int -vhandler_merge(struct validation_handler *handler) -{ - return (handler->merge != NULL) ? - handler->merge(handler->merge_arg, handler->arg) : 0; -} - int vhandler_reset(struct validation_handler *handler) { diff --git a/src/validation_handler.h b/src/validation_handler.h index 8c09d0ac..2bd44602 100644 --- a/src/validation_handler.h +++ b/src/validation_handler.h @@ -4,20 +4,41 @@ #include "address.h" #include "object/name.h" +/** + * Functions that handle validation results. + * + * At some point, I believe we will end up separating the validator code into a + * library, so it can be used by other applications aside from Fort's RTR + * server. + * + * This structure is designed with that in mind; it's the callback collection + * that the library's user application will fill up, so it can do whatever it + * wants with the validated ROAs. + * + * Because it's intended to be used by arbitrary applications, it needs to be + * generic. Please refrain from adding callbacks that are specifically meant for + * a particular use case. + * + * All of these functions can be NULL. + */ struct validation_handler { - /* All of these can be NULL. */ - - int (*merge)(void *, void *); - void *merge_arg; + /** + * Reinitializator; called every time Fort needs to invalidate a tree + * that was presumed to be correct thus far. + * (Implementor should invalidate all ROAs collected by handle_roa_v4() + * and handle_roa_v6().) + */ int (*reset)(void *); + /** Called every time Fort has successfully validated an IPv4 ROA. */ int (*handle_roa_v4)(uint32_t, struct ipv4_prefix const *, uint8_t, void *); + /** Called every time Fort has successfully validated an IPv6 ROA. */ int (*handle_roa_v6)(uint32_t, struct ipv6_prefix const *, uint8_t, void *); + /** Generic user-defined argument for the functions above. */ void *arg; }; -int vhandler_merge(struct validation_handler *); int vhandler_reset(struct validation_handler *); int vhandler_handle_roa_v4(uint32_t, struct ipv4_prefix const *, uint8_t); int vhandler_handle_roa_v6(uint32_t, struct ipv6_prefix const *, uint8_t);