From: Alberto Leiva Popper Date: Wed, 15 May 2019 20:57:09 +0000 (-0500) Subject: Another review X-Git-Tag: v0.0.2~33 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=f24a66b35786ef71a5aa1d5d6940ce656877d1bc;p=thirdparty%2FFORT-validator.git Another review - Remove the traverse_down() and traverse_up() callbacks, obsolete since the ROA database refactor. - Fix warnings reported by ultra-pedantic gcc. - Add abbreviations file. - Patch unit tests (broken in the last commit). - Some TODOs patched or discarded. --- diff --git a/src/abbreviations.txt b/src/abbreviations.txt new file mode 100644 index 00000000..92263bc1 --- /dev/null +++ b/src/abbreviations.txt @@ -0,0 +1,29 @@ +This file lists the abbreviations used through the code. +(Standard C, dependency and RFC-defined abbreviations are generally excluded.) +If you find an abbreviation that is not listed here, feel free to report it as +a bug. + +addr: address +addr4: IPv4 address +addr6: IPv6 address +be: Big Endian +db: database +eof: end of file +err: error +fd: File Descriptor (see `man 2 accept`) +hdr: header +id: identifier +len: length +max: maximum +min: minimum +msg: message +pdu: Protocol Data Unit (RFC 6810) +pr: print +ptr: pointer +str: string +tmp: temporal +uint: unsigned int +vrp: Validated ROA Payload (RFC 6811) +vrps: Validated ROA Payloads (VRP database) + +Single-character variables are always generic counters. diff --git a/src/asn1/signed_data.c b/src/asn1/signed_data.c index 0ad8b314..42e51224 100644 --- a/src/asn1/signed_data.c +++ b/src/asn1/signed_data.c @@ -31,7 +31,6 @@ signed_object_args_init(struct signed_object_args *args, args->uri = uri; args->crls = crls; memset(&args->refs, 0, sizeof(args->refs)); - args->subject_name = NULL; return 0; } @@ -40,8 +39,6 @@ signed_object_args_cleanup(struct signed_object_args *args) { resources_destroy(args->res); refs_cleanup(&args->refs); - if (args->subject_name != NULL) - x509_name_put(args->subject_name); } static int @@ -96,7 +93,7 @@ handle_sdata_certificate(ANY_t *cert_encoded, struct signed_object_args *args, error = certificate_validate_chain(cert, args->crls); if (error) goto end2; - error = certificate_validate_rfc6487(cert, &args->subject_name, false); + error = certificate_validate_rfc6487(cert, false); if (error) goto end2; error = certificate_validate_extensions_ee(cert, sid, &args->refs, diff --git a/src/asn1/signed_data.h b/src/asn1/signed_data.h index 67ce0ea6..75ca7089 100644 --- a/src/asn1/signed_data.h +++ b/src/asn1/signed_data.h @@ -23,8 +23,6 @@ struct signed_object_args { * recorded for future validation. */ struct certificate_refs refs; - /** Certificate's subject name field */ - struct rfc5280_name *subject_name; }; int signed_object_args_init(struct signed_object_args *, diff --git a/src/clients.c b/src/clients.c index d3d4288c..661b0bfe 100644 --- a/src/clients.c +++ b/src/clients.c @@ -6,10 +6,6 @@ #include "data_structure/uthash_nonfatal.h" #include "rtr/pdu.h" - -#define SADDR_IN(addr) ((struct sockaddr_in *) addr) -#define SADDR_IN6(addr) ((struct sockaddr_in6 *) addr) - struct hashable_client { struct client meat; UT_hash_handle hh; diff --git a/src/console_handler.c b/src/console_handler.c index 2c87072c..3ab71049 100644 --- a/src/console_handler.c +++ b/src/console_handler.c @@ -30,8 +30,6 @@ validate_into_console(void) handler.merge = NULL; handler.merge_arg = NULL; handler.reset = NULL; - handler.traverse_down = NULL; - handler.traverse_up = NULL; handler.handle_roa_v4 = print_v4_roa; handler.handle_roa_v6 = print_v6_roa; handler.arg = NULL; diff --git a/src/data_structure/array_list.h b/src/data_structure/array_list.h index e87c6be5..7ca927cc 100644 --- a/src/data_structure/array_list.h +++ b/src/data_structure/array_list.h @@ -73,11 +73,10 @@ DEFINE_ARRAY_LIST_STRUCT(name, elem_type); \ DEFINE_ARRAY_LIST_FUNCTIONS(name, elem_type, ) -/* c = cursor */ -#define ARRAYLIST_FOREACH(list, c) for ( \ - (c) = (list)->array; \ - ((c) != NULL) && (((c) - (typeof(c)) ((list)->array)) < (list)->len); \ - (c)++ \ +#define ARRAYLIST_FOREACH(list, node, index) for ( \ + (i) = 0, (node) = (list)->array; \ + (i) < (list)->len; \ + (i)++, (node)++ \ ) #endif /* SRC_DATA_STRUCTURE_ARRAY_LIST_H_ */ diff --git a/src/object/certificate.c b/src/object/certificate.c index 7416244f..f81824c3 100644 --- a/src/object/certificate.c +++ b/src/object/certificate.c @@ -92,7 +92,7 @@ validate_issuer(X509 *cert, bool is_ta) } static int -validate_subject(X509 *cert, struct rfc5280_name **subject_name) +validate_subject(X509 *cert) { struct validation *state; struct rfc5280_name *name; @@ -107,13 +107,9 @@ validate_subject(X509 *cert, struct rfc5280_name **subject_name) return error; error = validation_store_subject(state, name); - if (error) { - x509_name_put(name); - return error; - } - *subject_name = name; /* Transfer ownership */ - return 0; + x509_name_put(name); + return error; } static int @@ -267,8 +263,7 @@ validate_public_key(X509 *cert, bool is_root) } int -certificate_validate_rfc6487(X509 *cert, struct rfc5280_name **subject_name, - bool is_root) +certificate_validate_rfc6487(X509 *cert, bool is_root) { int error; @@ -310,7 +305,7 @@ certificate_validate_rfc6487(X509 *cert, struct rfc5280_name **subject_name, * "An issuer SHOULD use a different subject name if the subject's * key pair has changed" (it's a SHOULD, so [for now] avoid validation) */ - error = validate_subject(cert, subject_name); + error = validate_subject(cert); if (error) return error; @@ -1444,7 +1439,6 @@ certificate_traverse(struct rpp *rpp_parent, struct rpki_uri const *cert_uri, struct validation *state; X509 *cert; - struct rfc5280_name *subject_name; struct rpki_uri mft; struct certificate_refs refs; enum rpki_policy policy; @@ -1469,14 +1463,14 @@ certificate_traverse(struct rpp *rpp_parent, struct rpki_uri const *cert_uri, error = certificate_validate_chain(cert, crls); if (error) goto revert_cert; - error = certificate_validate_rfc6487(cert, &subject_name, IS_TA); + error = certificate_validate_rfc6487(cert, IS_TA); if (error) goto revert_cert; error = IS_TA ? certificate_validate_extensions_ta(cert, &mft, &policy) : certificate_validate_extensions_ca(cert, &mft, &refs, &policy); if (error) - goto revert_subject_name; + goto revert_cert; error = refs_validate_ca(&refs, rpp_parent); if (error) @@ -1492,25 +1486,14 @@ certificate_traverse(struct rpp *rpp_parent, struct rpki_uri const *cert_uri, goto revert_cert_push; /* -- Validate & traverse the RPP (@pp) described by the manifest -- */ - error = vhandler_traverse_down(subject_name); - if (error) - goto revert_rpp; - error = rpp_traverse(pp); - if (error) - goto revert_rpp; - - error = vhandler_traverse_up(); -revert_rpp: rpp_destroy(pp); revert_cert_push: validation_pop_cert(state); /* Error code is useless. */ revert_uri_and_refs: uri_cleanup(&mft); refs_cleanup(&refs); -revert_subject_name: - x509_name_put(subject_name); revert_cert: X509_free(cert); revert_fnstack_and_debug: diff --git a/src/object/certificate.h b/src/object/certificate.h index 023d538b..2c9e4e25 100644 --- a/src/object/certificate.h +++ b/src/object/certificate.h @@ -9,7 +9,6 @@ #include "resource.h" #include "rpp.h" #include "uri.h" -#include "object/name.h" int certificate_load(struct rpki_uri const *, X509 **); @@ -22,7 +21,7 @@ int certificate_validate_chain(X509 *, STACK_OF(X509_CRL) *); * Validates RFC 6487 compliance. * (Except extensions.) */ -int certificate_validate_rfc6487(X509 *, struct rfc5280_name **, bool); +int certificate_validate_rfc6487(X509 *, bool); int certificate_validate_signature(X509 *, ANY_t *coded, SignatureValue_t *); diff --git a/src/object/roa.c b/src/object/roa.c index 98362ad7..8e04fad7 100644 --- a/src/object/roa.c +++ b/src/object/roa.c @@ -209,15 +209,7 @@ roa_traverse(struct rpki_uri const *uri, struct rpp *pp, if (error) goto revert_roa; - error = vhandler_traverse_down(sobj_args.subject_name); - if (error) - goto revert_roa; - error = __handle_roa(roa, sobj_args.res); - if (error) - goto revert_roa; - - error = vhandler_traverse_up(); revert_roa: ASN_STRUCT_FREE(asn_DEF_RouteOriginAttestation, roa); diff --git a/src/rpp.c b/src/rpp.c index afed7e2d..bb74f77b 100644 --- a/src/rpp.c +++ b/src/rpp.c @@ -123,6 +123,7 @@ rpp_traverse(struct rpp *pp) */ STACK_OF(X509_CRL) *crls; struct rpki_uri *uri; + array_index i; int error; crls = sk_X509_CRL_new_null(); @@ -133,14 +134,14 @@ rpp_traverse(struct rpp *pp) goto end; /* Use CRL stack to validate certificates, and also traverse them. */ - ARRAYLIST_FOREACH(&pp->certs, uri) + ARRAYLIST_FOREACH(&pp->certs, uri, i) certificate_traverse(pp, uri, crls); /* Use valid address ranges to print ROAs that match them. */ - ARRAYLIST_FOREACH(&pp->roas, uri) + ARRAYLIST_FOREACH(&pp->roas, uri, i) roa_traverse(uri, pp, crls); - ARRAYLIST_FOREACH(&pp->ghostbusters, uri) + ARRAYLIST_FOREACH(&pp->ghostbusters, uri, i) ghostbusters_traverse(uri, pp, crls); end: diff --git a/src/rtr/db/delta.c b/src/rtr/db/delta.c index b8ffc998..3bcc1c5d 100644 --- a/src/rtr/db/delta.c +++ b/src/rtr/db/delta.c @@ -124,13 +124,14 @@ __foreach_v4(struct deltas_v4 *array, delta_foreach_cb cb, void *arg, { struct delta delta; struct delta_v4 *d; + array_index i; int error; delta.serial = serial; delta.vrp.addr_fam = AF_INET; delta.flags = flags; - ARRAYLIST_FOREACH(array, d) { + ARRAYLIST_FOREACH(array, d, i) { delta.vrp.asn = d->as; delta.vrp.prefix.v4 = d->prefix.addr; delta.vrp.prefix_length = d->prefix.len; @@ -149,13 +150,14 @@ __foreach_v6(struct deltas_v6 *array, delta_foreach_cb cb, void *arg, { struct delta delta; struct delta_v6 *d; + array_index i; int error; delta.serial = serial; delta.vrp.addr_fam = AF_INET6; delta.flags = flags; - ARRAYLIST_FOREACH(array, d) { + ARRAYLIST_FOREACH(array, d, i) { delta.vrp.asn = d->as; delta.vrp.prefix.v6 = d->prefix.addr; delta.vrp.prefix_length = d->prefix.len; diff --git a/src/rtr/db/roa_table.c b/src/rtr/db/roa_table.c index eba04e96..bf4c012d 100644 --- a/src/rtr/db/roa_table.c +++ b/src/rtr/db/roa_table.c @@ -3,11 +3,6 @@ #include "data_structure/uthash_nonfatal.h" struct hashable_roa { - /* - * TODO (whatever) flags is not useful here. - * Maybe separate struct vrp into two structures: One that doesn't - * contain flags, and one that contains the other. - */ struct vrp data; UT_hash_handle hh; }; diff --git a/src/rtr/db/vrp.h b/src/rtr/db/vrp.h index c002d928..79659b04 100644 --- a/src/rtr/db/vrp.h +++ b/src/rtr/db/vrp.h @@ -11,11 +11,6 @@ typedef uint32_t serial_t; struct vrp { uint32_t asn; - /* - * TODO (whatever) convert to ipv*_prefix? (from address.h) - * Most of the time, @prefix and @prefix_length are copied from or into - * ipv*_prefixes. - */ union { struct in_addr v4; struct in6_addr v6; @@ -23,7 +18,6 @@ struct vrp { uint8_t prefix_length; uint8_t max_prefix_length; uint8_t addr_fam; - /* uint8_t flags; */ /* TODO (now) commit remove */ }; struct delta { diff --git a/src/rtr/db/vrps.c b/src/rtr/db/vrps.c index f84fec9a..7a290b62 100644 --- a/src/rtr/db/vrps.c +++ b/src/rtr/db/vrps.c @@ -138,8 +138,6 @@ __perform_standalone_validation(struct roa_table **result) validation_handler.merge = __merge; validation_handler.merge_arg = global_roas; validation_handler.reset = __reset; - validation_handler.traverse_down = NULL; - validation_handler.traverse_up = NULL; validation_handler.handle_roa_v4 = __handle_roa_v4; validation_handler.handle_roa_v6 = __handle_roa_v6; validation_handler.arg = roas; @@ -184,12 +182,13 @@ static void vrps_purge(void) { struct delta_group *group; + array_index i; uint32_t min_serial; min_serial = clients_get_min_serial(); /** Assume is ordered by serial, so get the new initial pointer */ - ARRAYLIST_FOREACH(&state.deltas, group) + ARRAYLIST_FOREACH(&state.deltas, group, i) if (group->serial >= min_serial) break; @@ -340,6 +339,7 @@ int vrps_get_deltas_from(serial_t from, serial_t *to, struct deltas_db *result) { struct delta_group *group; + array_index i; bool from_found; int error; @@ -354,7 +354,7 @@ vrps_get_deltas_from(serial_t from, serial_t *to, struct deltas_db *result) return -EAGAIN; } - ARRAYLIST_FOREACH(&state.deltas, group) { + ARRAYLIST_FOREACH(&state.deltas, group, i) { if (!from_found) { if (group->serial == from) { from_found = true; diff --git a/src/rtr/err_pdu.c b/src/rtr/err_pdu.c index 3e521c65..2ade29eb 100644 --- a/src/rtr/err_pdu.c +++ b/src/rtr/err_pdu.c @@ -17,11 +17,6 @@ typedef enum rtr_error_code { ERR_PDU_UNEXPECTED_PROTO_VERSION = 8, } rtr_error_code_t; -/* - * TODO (urgent) According to the function below, NO_DATA_AVAILABLE is not - * fatal. However, some callers of this function are terminating the connection - * regardless of that. - */ static int err_pdu_send(int fd, rtr_error_code_t code, struct rtr_request const *request, char const *message_const) @@ -63,7 +58,12 @@ err_pdu_send_internal_error(int fd) int err_pdu_send_no_data_available(int fd) { - return err_pdu_send(fd, ERR_PDU_NO_DATA_AVAILABLE, NULL, NULL); + err_pdu_send(fd, ERR_PDU_NO_DATA_AVAILABLE, NULL, NULL); + /* + * The connection should not be terminated because of this error. + * So don't panic; client should retry later. + */ + return 0; } int @@ -80,16 +80,22 @@ err_pdu_send_invalid_request_truncated(int fd, unsigned char *header, { struct rtr_request request = { .bytes = header, - .bytes_len = RTRPDU_HEADER_LEN, + .bytes_len = RTRPDU_HDR_LEN, .pdu = NULL, }; return err_pdu_send_invalid_request(fd, &request, message); } int -err_pdu_send_unsupported_proto_version(int fd) +err_pdu_send_unsupported_proto_version(int fd, unsigned char *header, + char const *message) { - return err_pdu_send(fd, ERR_PDU_UNSUP_PROTO_VERSION, NULL, NULL); + struct rtr_request request = { + .bytes = header, + .bytes_len = RTRPDU_HDR_LEN, + .pdu = NULL, + }; + return err_pdu_send(fd, ERR_PDU_UNSUP_PROTO_VERSION, &request, message); } int @@ -98,19 +104,6 @@ err_pdu_send_unsupported_pdu_type(int fd, struct rtr_request const *request) return err_pdu_send(fd, ERR_PDU_UNSUP_PDU_TYPE, request, NULL); } -bool -err_pdu_is_fatal(uint16_t code) -{ - /* - * Only NO_DATA_AVAILABLE error isn't fatal - * - * Addendum: Note that this is only non-fatal if we're the ones sending - * it. If the clients is the one telling us this, then it probably - * counts as "erroneous Error Report PDU", which is totally fatal. - */ - return code != ERR_PDU_NO_DATA_AVAILABLE; -} - char const * err_pdu_to_string(uint16_t code) { diff --git a/src/rtr/err_pdu.h b/src/rtr/err_pdu.h index be6c00b6..1606b96d 100644 --- a/src/rtr/err_pdu.h +++ b/src/rtr/err_pdu.h @@ -16,10 +16,9 @@ int err_pdu_send_internal_error(int); int err_pdu_send_no_data_available(int); int err_pdu_send_invalid_request(int, struct rtr_request const *, char const *); int err_pdu_send_invalid_request_truncated(int, unsigned char *, char const *); -int err_pdu_send_unsupported_proto_version(int); +int err_pdu_send_unsupported_proto_version(int, unsigned char *, char const *); int err_pdu_send_unsupported_pdu_type(int, struct rtr_request const *); -bool err_pdu_is_fatal(uint16_t); char const *err_pdu_to_string(uint16_t); #endif /* SRC_RTR_ERR_PDU_H_ */ diff --git a/src/rtr/pdu.c b/src/rtr/pdu.c index 536dba7e..ca63af50 100644 --- a/src/rtr/pdu.c +++ b/src/rtr/pdu.c @@ -18,25 +18,22 @@ pdu_header_from_reader(struct pdu_reader *reader, struct pdu_header *header) || read_int32(reader, &header->length); } -#define ERROR(report_cb) \ +/* Do not use this macro before @header has been initialized, obviously. */ +#define RESPOND_ERROR(report_cb) \ ((header.pdu_type != PDU_TYPE_ERROR_REPORT) ? (report_cb) : -EINVAL); int pdu_load(int fd, struct rtr_request *request, struct pdu_metadata const **metadata) { - unsigned char hdr_bytes[RTRPDU_HEADER_LEN]; + unsigned char hdr_bytes[RTRPDU_HDR_LEN]; struct pdu_reader reader; struct pdu_header header; struct pdu_metadata const *meta; int error; /* Read the header into its buffer. */ - /* - * TODO (urgent) If the first read yields no bytes, the connection was - * terminated. We're not ending gracefully in those cases. - */ - error = pdu_reader_init(&reader, fd, hdr_bytes, RTRPDU_HEADER_LEN); + error = pdu_reader_init(&reader, fd, hdr_bytes, RTRPDU_HDR_LEN, true); if (error) /* Communication interrupted; omit error response */ return error; @@ -45,15 +42,21 @@ pdu_load(int fd, struct rtr_request *request, /* No error response because the PDU might have been an error */ return error; + /* + * DO NOT USE THE err_pdu_* functions directly. Wrap them with + * RESPOND_ERROR() INSTEAD. + */ + /* * For now, only RTRv0 is supported */ if (header.protocol_version != RTR_V0) - return err_pdu_send_unsupported_proto_version(fd); + return RESPOND_ERROR(err_pdu_send_unsupported_proto_version(fd, + hdr_bytes, "This server only supports RTRv0.")); - if (header.length < RTRPDU_HEADER_LEN) - return ERROR(err_pdu_send_invalid_request_truncated(fd, - hdr_bytes, "PDU is too small. (< 8 bytes)")); + if (header.length < RTRPDU_HDR_LEN) + return RESPOND_ERROR(err_pdu_send_invalid_request_truncated(fd, + hdr_bytes, "Invalid header length. (< 8 bytes)")); /* * Error messages can be quite large. @@ -62,12 +65,9 @@ pdu_load(int fd, struct rtr_request *request, * Most error messages are bound to be two phrases tops. * (Warning: I'm assuming english tho.) */ - if (header.length > 512) { - pr_warn("Got an extremely large PDU (%u bytes). WTF?", - header.length); - return ERROR(err_pdu_send_invalid_request_truncated(fd, + if (header.length > 512) + return RESPOND_ERROR(err_pdu_send_invalid_request_truncated(fd, hdr_bytes, "PDU is too large. (> 512 bytes)")); - } /* Read the rest of the PDU into its buffer. */ request->bytes_len = header.length; @@ -76,10 +76,11 @@ pdu_load(int fd, struct rtr_request *request, /* No error report PDU on allocation failures. */ return pr_enomem(); - memcpy(request->bytes, hdr_bytes, RTRPDU_HEADER_LEN); + memcpy(request->bytes, hdr_bytes, RTRPDU_HDR_LEN); error = pdu_reader_init(&reader, fd, - request->bytes + RTRPDU_HEADER_LEN, - header.length - RTRPDU_HEADER_LEN); + request->bytes + RTRPDU_HDR_LEN, + header.length - RTRPDU_HDR_LEN, + false); if (error) /* Communication interrupted; no error PDU. */ goto revert_bytes; @@ -87,7 +88,8 @@ pdu_load(int fd, struct rtr_request *request, /* Deserialize the PDU. */ meta = pdu_get_metadata(header.pdu_type); if (!meta) { - error = ERROR(err_pdu_send_unsupported_pdu_type(fd, request)); + error = RESPOND_ERROR(err_pdu_send_unsupported_pdu_type(fd, + request)); goto revert_bytes; } @@ -99,7 +101,7 @@ pdu_load(int fd, struct rtr_request *request, error = meta->from_stream(&header, &reader, request->pdu); if (error) { - ERROR(err_pdu_send_internal_error(fd)); + RESPOND_ERROR(err_pdu_send_internal_error(fd)); goto revert_pdu; } diff --git a/src/rtr/pdu.h b/src/rtr/pdu.h index 283fb973..8a905b35 100644 --- a/src/rtr/pdu.h +++ b/src/rtr/pdu.h @@ -11,10 +11,6 @@ struct rtr_client { int fd; - /* - * TODO (whatever) this is currently not being used for anything. - * (But consider the end_client() to-do.) - */ struct sockaddr_storage addr; }; @@ -48,7 +44,7 @@ enum pdu_type { */ /* Header length field is always 64 bits long */ -#define RTRPDU_HEADER_LEN 8 +#define RTRPDU_HDR_LEN 8 #define RTRPDU_SERIAL_NOTIFY_LEN 12 #define RTRPDU_SERIAL_QUERY_LEN 12 diff --git a/src/rtr/pdu_handler.c b/src/rtr/pdu_handler.c index 798e7bd1..c7b272f0 100644 --- a/src/rtr/pdu_handler.c +++ b/src/rtr/pdu_handler.c @@ -55,8 +55,7 @@ handle_serial_query_pdu(int fd, struct rtr_request const *request) case 0: break; case -EAGAIN: /* Database still under construction */ - err_pdu_send_no_data_available(fd); - error = 0; /* Don't panic; client should retry later */ + error = err_pdu_send_no_data_available(fd); goto end; case -ESRCH: /* Invalid serial */ /* https://tools.ietf.org/html/rfc6810#section-6.3 */ @@ -139,8 +138,7 @@ handle_reset_query_pdu(int fd, struct rtr_request const *request) case 0: break; case -EAGAIN: - err_pdu_send_no_data_available(fd); - return 0; + return err_pdu_send_no_data_available(fd); case EAGAIN: err_pdu_send_internal_error(fd); return error; diff --git a/src/rtr/pdu_sender.c b/src/rtr/pdu_sender.c index 5a1d97ac..d05b7d29 100644 --- a/src/rtr/pdu_sender.c +++ b/src/rtr/pdu_sender.c @@ -13,11 +13,6 @@ #include "rtr/pdu_serializer.h" #include "rtr/db/vrps.h" -/* IPvN PDUs length without header */ -#define IPV4_PREFIX_LENGTH 12 -#define IPV6_PREFIX_LENGTH 24 - - struct vrp_node { struct delta delta; SLIST_ENTRY(vrp_node) next; @@ -232,6 +227,7 @@ send_delta_pdus(int fd, struct deltas_db *deltas) { struct vrp_slist filtered_vrps; struct delta_group *group; + array_index i; struct vrp_node *ptr; int error = 0; @@ -251,7 +247,7 @@ send_delta_pdus(int fd, struct deltas_db *deltas) * are immutable.) */ SLIST_INIT(&filtered_vrps); - ARRAYLIST_FOREACH(deltas, group) { + ARRAYLIST_FOREACH(deltas, group, i) { error = deltas_foreach(group->serial, group->deltas, vrp_ovrd_remove, &filtered_vrps); if (error) @@ -329,7 +325,7 @@ send_error_report_pdu(int fd, uint16_t code, struct rtr_request const *request, pdu.error_message_length = (message != NULL) ? strlen(message) : 0; pdu.error_message = message; - pdu.header.length = RTRPDU_HEADER_LEN + pdu.header.length = RTRPDU_HDR_LEN + 4 /* Length of Encapsulated PDU field */ + pdu.error_pdu_length + 4 /* Length of Error Text field */ diff --git a/src/rtr/primitive_reader.c b/src/rtr/primitive_reader.c index 7f085c9b..cc5c0968 100644 --- a/src/rtr/primitive_reader.c +++ b/src/rtr/primitive_reader.c @@ -13,8 +13,19 @@ static int get_octets(unsigned char); static void place_null_character(rtr_char *, size_t); -int -read_exact(int fd, unsigned char *buffer, size_t buffer_len) +/** + * Reads exactly @buffer_len bytes from @buffer, erroring if this goal cannot be + * met. + * + * If @allow_end is true, will allow immediate EOF in place of the buffer bytes. + * (Though all this really means is that the corresponding warning message will + * not be printed, which is perfectly fine as far as the only current caller is + * concerned.) + * + * Returns 0 if exactly @buffer_len bytes could be read. + */ +static int +read_exact(int fd, unsigned char *buffer, size_t buffer_len, bool allow_eof) { ssize_t read_result; size_t offset; @@ -25,9 +36,12 @@ read_exact(int fd, unsigned char *buffer, size_t buffer_len) return -pr_errno(errno, "Client socket read interrupted"); if (read_result == 0) { - pr_warn("Stream ended mid-PDU."); + if (!allow_eof) + pr_warn("Stream ended mid-PDU."); return -EPIPE; } + + allow_eof = false; } return 0; @@ -39,11 +53,11 @@ read_exact(int fd, unsigned char *buffer, size_t buffer_len) */ int pdu_reader_init(struct pdu_reader *reader, int fd, unsigned char *buffer, - size_t size) + size_t size, bool allow_eof) { reader->buffer = buffer; reader->size = size; - return read_exact(fd, reader->buffer, size); + return read_exact(fd, reader->buffer, size, allow_eof); } static int diff --git a/src/rtr/primitive_reader.h b/src/rtr/primitive_reader.h index 3876894d..014b8ceb 100644 --- a/src/rtr/primitive_reader.h +++ b/src/rtr/primitive_reader.h @@ -1,6 +1,7 @@ #ifndef RTR_PRIMITIVE_READER_H_ #define RTR_PRIMITIVE_READER_H_ +#include #include #include "common.h" @@ -12,7 +13,8 @@ struct pdu_reader { size_t size; }; -int pdu_reader_init(struct pdu_reader *, int, unsigned char *, size_t size); +int pdu_reader_init(struct pdu_reader *, int, unsigned char *, size_t size, + bool); int read_int8(struct pdu_reader *, uint8_t *); int read_int16(struct pdu_reader *, uint16_t *); diff --git a/src/rtr/rtr.c b/src/rtr/rtr.c index cf12812d..e78342a7 100644 --- a/src/rtr/rtr.c +++ b/src/rtr/rtr.c @@ -154,15 +154,39 @@ clean_request(struct rtr_request *request, const struct pdu_metadata *meta) meta->destructor(request->pdu); } +static void +print_close_failure(int error, struct sockaddr_storage *sockaddr) +{ + char buffer[INET6_ADDRSTRLEN]; + void *addr = NULL; + char const *addr_str; + + switch (sockaddr->ss_family) { + case AF_INET: + addr = &((struct sockaddr_in *) sockaddr)->sin_addr; + break; + case AF_INET6: + addr = &((struct sockaddr_in6 *) sockaddr)->sin6_addr; + break; + default: + addr_str = "(protocol unknown)"; + goto done; + } + + addr_str = inet_ntop(sockaddr->ss_family, addr, buffer, + INET6_ADDRSTRLEN); + if (addr_str == NULL) + addr_str = "(unprintable address)"; + +done: + pr_errno(error, "close() failed on socket of client %s", addr_str); +} + static void * end_client(struct rtr_client *client) { - /* - * TODO It'd probably be a good idea to print the client's address in - * this message. - */ if (close(client->fd) != 0) - pr_errno(errno, "close() failed on client socket"); + print_close_failure(errno, &client->addr); clients_forget(client->fd); return NULL; diff --git a/src/slurm/slurm_db.c b/src/slurm/slurm_db.c index 837224ef..6b49b878 100644 --- a/src/slurm/slurm_db.c +++ b/src/slurm/slurm_db.c @@ -142,8 +142,9 @@ bgpsec_equal(struct slurm_bgpsec *left, struct slurm_bgpsec *right, name##_locate(type *obj) \ { \ type *cursor; \ + array_index i; \ \ - ARRAYLIST_FOREACH(db_list, cursor) \ + ARRAYLIST_FOREACH(db_list, cursor, i) \ if (equal_cb(cursor, obj, filter)) \ return cursor; \ \ @@ -189,9 +190,10 @@ int slurm_db_foreach_assertion_prefix(assertion_pfx_foreach_cb cb, void *arg) { struct slurm_prefix *cursor; + array_index i; int error; - ARRAYLIST_FOREACH(&array_lists_db.assertion_pfx_al, cursor) { + ARRAYLIST_FOREACH(&array_lists_db.assertion_pfx_al, cursor, i) { error = cb(cursor, arg); if (error) return error; diff --git a/src/state.c b/src/state.c index 82825925..675280c7 100644 --- a/src/state.c +++ b/src/state.c @@ -373,6 +373,7 @@ 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; @@ -404,7 +405,7 @@ validation_store_serial_number(struct validation *state, BIGNUM *number) * 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) { + 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'.)", @@ -432,6 +433,7 @@ 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; @@ -483,7 +485,7 @@ validation_store_subject(struct validation *state, struct rfc5280_name *subject) return 0; /* The TA lacks siblings, so subject is unique. */ /* See the large comment in validation_store_serial_number(). */ - ARRAYLIST_FOREACH(&cert->subjects, cursor) { + 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'.)", diff --git a/src/thread_var.h b/src/thread_var.h index a56b8811..0f2f2d2a 100644 --- a/src/thread_var.h +++ b/src/thread_var.h @@ -16,9 +16,10 @@ void fnstack_push_uri(struct rpki_uri const *); char const *fnstack_peek(void); void fnstack_pop(void); -char const *v4addr2str(struct in_addr const *addr); -char const *v4addr2str2(struct in_addr const *addr); -char const *v6addr2str(struct in6_addr const *addr); -char const *v6addr2str2(struct in6_addr const *addr); +/* Please remember that these functions can only be used during validations. */ +char const *v4addr2str(struct in_addr const *); +char const *v4addr2str2(struct in_addr const *); +char const *v6addr2str(struct in6_addr const *); +char const *v6addr2str2(struct in6_addr const *); #endif /* SRC_THREAD_VAR_H_ */ diff --git a/src/validation_handler.c b/src/validation_handler.c index 65633308..0540d001 100644 --- a/src/validation_handler.c +++ b/src/validation_handler.c @@ -34,36 +34,6 @@ get_current_threads_handler(struct validation_handler const **result) return 0; } -int -vhandler_traverse_down(struct rfc5280_name *subject_name) -{ - struct validation_handler const *handler; - int error; - - error = get_current_threads_handler(&handler); - if (error) - return error; - - return (handler->traverse_down != NULL) - ? handler->traverse_down(subject_name, handler->arg) - : 0; -} - -int -vhandler_traverse_up(void) -{ - struct validation_handler const *handler; - int error; - - error = get_current_threads_handler(&handler); - if (error) - return error; - - return (handler->traverse_up != NULL) - ? handler->traverse_up(handler->arg) - : 0; -} - int vhandler_handle_roa_v4(uint32_t as, struct ipv4_prefix const *prefix, uint8_t max_length) diff --git a/src/validation_handler.h b/src/validation_handler.h index 5b21be54..8c09d0ac 100644 --- a/src/validation_handler.h +++ b/src/validation_handler.h @@ -10,8 +10,6 @@ struct validation_handler { int (*merge)(void *, void *); void *merge_arg; int (*reset)(void *); - int (*traverse_down)(struct rfc5280_name *, void *); - int (*traverse_up)(void *); int (*handle_roa_v4)(uint32_t, struct ipv4_prefix const *, uint8_t, void *); int (*handle_roa_v6)(uint32_t, struct ipv6_prefix const *, uint8_t, @@ -21,8 +19,6 @@ struct validation_handler { int vhandler_merge(struct validation_handler *); int vhandler_reset(struct validation_handler *); -int vhandler_traverse_down(struct rfc5280_name *); -int vhandler_traverse_up(void); 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); diff --git a/test/impersonator.c b/test/impersonator.c index 3d0f0ed4..ca69311b 100644 --- a/test/impersonator.c +++ b/test/impersonator.c @@ -1,6 +1,7 @@ #include #include "config.h" +#include "incidence/incidence.h" /** * Some core functions, as linked from unit testing code. @@ -75,3 +76,11 @@ config_get_slurm_location(void) { return NULL; } + +enum incidence_action +incidence_get_action(enum incidence_id id) +{ + return (id == INID_SIGNATURE_ALGORITHM_HAS_PARAMS) + ? INAC_WARN + : INAC_ERROR; +} diff --git a/test/rtr/db/vrps_test.c b/test/rtr/db/vrps_test.c index c6dfccb0..80319835 100644 --- a/test/rtr/db/vrps_test.c +++ b/test/rtr/db/vrps_test.c @@ -175,7 +175,7 @@ check_deltas(serial_t from, serial_t to, bool const *expected_deltas) ck_assert_uint_eq(to, actual_serial); memset(actual_deltas, 0, sizeof(actual_deltas)); - ARRAYLIST_FOREACH(&deltas, group) + ARRAYLIST_FOREACH(&deltas, group, i) ck_assert_int_eq(0, deltas_foreach(group->serial, group->deltas, delta_check, actual_deltas)); for (i = 0; i < ARRAY_LEN(actual_deltas); i++) diff --git a/test/rtr/pdu_handler_test.c b/test/rtr/pdu_handler_test.c index 129f3daf..0b40b1f6 100644 --- a/test/rtr/pdu_handler_test.c +++ b/test/rtr/pdu_handler_test.c @@ -142,8 +142,9 @@ int send_delta_pdus(int fd, struct deltas_db *deltas) { struct delta_group *group; + array_index i; - ARRAYLIST_FOREACH(deltas, group) + ARRAYLIST_FOREACH(deltas, group, i) ck_assert_int_eq(0, deltas_foreach(group->serial, group->deltas, handle_delta, &fd));