From: Alberto Leiva Popper Date: Fri, 10 May 2019 23:26:49 +0000 (-0500) Subject: Review, mostly on error responses to the client X-Git-Tag: v0.0.2~37 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=e42015d7d3607dbcddb2c3472168e7f1ed72f2e4;p=thirdparty%2FFORT-validator.git Review, mostly on error responses to the client - Add several validations - Some error pipelines were missing error responses - Patch "Erroneous PDU" fields. (The server was writing only headers, not as much of the offending PDU as possible.) --- diff --git a/src/clients.c b/src/clients.c index c212d726..f5611f98 100644 --- a/src/clients.c +++ b/src/clients.c @@ -4,6 +4,7 @@ #include "common.h" #include "log.h" #include "data_structure/uthash.h" +#include "rtr/pdu.h" /* * TODO uthash panics on memory allocations... @@ -39,8 +40,7 @@ clients_db_init(void) } static int -create_client(int fd, struct sockaddr_storage *addr, uint8_t rtr_version, - struct hashable_client **result) +create_client(struct rtr_client *client, struct hashable_client **result) { struct hashable_client *node; @@ -48,22 +48,21 @@ create_client(int fd, struct sockaddr_storage *addr, uint8_t rtr_version, if (node == NULL) return pr_enomem(); - node->meat.fd = fd; - node->meat.family = addr->ss_family; - switch (addr->ss_family) { + node->meat.fd = client->fd; + node->meat.family = client->addr.ss_family; + switch (client->addr.ss_family) { case AF_INET: - node->meat.sin = SADDR_IN(addr)->sin_addr; - node->meat.sin_port = SADDR_IN(addr)->sin_port; + node->meat.sin = SADDR_IN(&client->addr)->sin_addr; + node->meat.sin_port = SADDR_IN(&client->addr)->sin_port; break; case AF_INET6: - node->meat.sin6 = SADDR_IN6(addr)->sin6_addr; - node->meat.sin_port = SADDR_IN6(addr)->sin6_port; + node->meat.sin6 = SADDR_IN6(&client->addr)->sin6_addr; + node->meat.sin_port = SADDR_IN6(&client->addr)->sin6_port; break; default: free(node); - return pr_crit("Bad protocol: %u", addr->ss_family); + return pr_crit("Bad protocol: %u", client->addr.ss_family); } - node->meat.rtr_version = rtr_version; *result = node; return 0; @@ -71,35 +70,24 @@ create_client(int fd, struct sockaddr_storage *addr, uint8_t rtr_version, /* * If the client whose file descriptor is @fd isn't already stored, store it. - * - * Code error -ERTR_VERSION_MISMATCH will be returned when a client exists but - * its RTR version isn't the same as in the DB. */ int -clients_add(int fd, struct sockaddr_storage *addr, uint8_t rtr_version) +clients_add(struct rtr_client *client) { struct hashable_client *new_client = NULL; struct hashable_client *old_client; int error; - error = create_client(fd, addr, rtr_version, &new_client); + error = create_client(client, &new_client); if (error) return error; rwlock_write_lock(&lock); - HASH_FIND_INT(table, &fd, old_client); + HASH_FIND_INT(table, &client->fd, old_client); if (old_client == NULL) { HASH_ADD_INT(table, meat.fd, new_client); new_client = NULL; - } else { - /* - * Isn't ready to handle distinct version on clients - * reconnection, but for now there's no problem since only - * RTRv0 is supported. - */ - if (old_client->meat.rtr_version != rtr_version) - error = -ERTR_VERSION_MISMATCH; } rwlock_unlock(&lock); @@ -107,7 +95,7 @@ clients_add(int fd, struct sockaddr_storage *addr, uint8_t rtr_version) if (new_client != NULL) free(new_client); - return error; + return 0; } void diff --git a/src/clients.h b/src/clients.h index 5055e144..b014e915 100644 --- a/src/clients.h +++ b/src/clients.h @@ -2,10 +2,9 @@ #define SRC_CLIENTS_H_ #include +#include "rtr/pdu.h" #include "rtr/db/vrp.h" -#define ERTR_VERSION_MISMATCH 8754983 - struct client { int fd; sa_family_t family; @@ -14,13 +13,12 @@ struct client { struct in6_addr sin6; }; in_port_t sin_port; - uint8_t rtr_version; serial_t serial_number; }; int clients_db_init(void); -int clients_add(int, struct sockaddr_storage *, uint8_t); +int clients_add(struct rtr_client *); void clients_update_serial(int, serial_t); void clients_forget(int); typedef int (*clients_foreach_cb)(struct client const *, void *); diff --git a/src/notify.c b/src/notify.c index 949ade2f..6e64adcb 100644 --- a/src/notify.c +++ b/src/notify.c @@ -10,13 +10,11 @@ static int send_notify(struct client const *client, void *arg) { - struct sender_common common; serial_t *serial = arg; int error; /* Send Serial Notify PDU */ - init_sender_common(&common, client->fd, client->rtr_version); - error = send_serial_notify_pdu(&common, *serial); + error = send_serial_notify_pdu(client->fd, *serial); /* Error? Log it... */ if (error) diff --git a/src/rtr/err_pdu.c b/src/rtr/err_pdu.c index 06db7b2d..c8cb7f4c 100644 --- a/src/rtr/err_pdu.c +++ b/src/rtr/err_pdu.c @@ -9,21 +9,76 @@ * fatal. However, some callers of this function are terminating the connection * regardless of that. */ -int -err_pdu_send(int fd, uint8_t version, uint16_t code, void *err_pdu_header, - char const *message) +static int +err_pdu_send(int fd, uint16_t code, struct rtr_request const *request, + char const *message_const) { + /* + * This function must always return error so callers can interrupt + * themselves easily. + */ + int error; + char *message; + + /* TODO (now) Prevent errors to errors */ + + message = (message_const != NULL) ? strdup(message_const) : NULL; + error = send_error_report_pdu(fd, code, request, message); + free(message); - error = send_error_report_pdu(fd, version, code, err_pdu_header, - message); if (err_pdu_is_fatal(code)) { pr_warn("Fatal error report PDU sent [code %u], closing socket.", code); close(fd); } - return error; + return error ? error : -EINVAL; +} + +int +err_pdu_send_corrupt_data(int fd, struct rtr_request const *request, + char const *message) +{ + return err_pdu_send(fd, ERR_PDU_CORRUPT_DATA, request, message); +} + +int +err_pdu_send_internal_error(int fd) +{ + return err_pdu_send(fd, ERR_PDU_INTERNAL_ERROR, NULL, NULL); +} + +int +err_pdu_send_no_data_available(int fd) +{ + return err_pdu_send(fd, ERR_PDU_NO_DATA_AVAILABLE, NULL, NULL); +} + +int +err_pdu_send_invalid_request(int fd, struct rtr_request const *request, + char const *message) +{ + return err_pdu_send(fd, ERR_PDU_INVALID_REQUEST, request, message); +} + +/* Caution: @header is supposed to be in serialized form. */ +int +err_pdu_send_invalid_request_truncated(int fd, unsigned char *header, + char const *message) +{ + struct rtr_request request = { + .bytes = header, + .bytes_len = RTRPDU_HEADER_LEN, + .pdu = NULL, + }; + return err_pdu_send_invalid_request(fd, &request, message); +} + +int +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 diff --git a/src/rtr/err_pdu.h b/src/rtr/err_pdu.h index 1d06c7a0..0ac9ffd3 100644 --- a/src/rtr/err_pdu.h +++ b/src/rtr/err_pdu.h @@ -4,6 +4,8 @@ #include #include +#include "rtr/pdu.h" + #define ERR_PDU_CORRUPT_DATA 0 #define ERR_PDU_INTERNAL_ERROR 1 #define ERR_PDU_NO_DATA_AVAILABLE 2 @@ -14,8 +16,18 @@ #define ERR_PDU_DUPLICATE_ANNOUNCE 7 #define ERR_PDU_UNEXPECTED_PROTO_VERSION 8 +/* + * Wrappers for err_pdu_send(). + * Mainly, this is for the sake of making it easier to see whether the error is + * supposed to contain a message and/or the original PDU or not. + */ +int err_pdu_send_corrupt_data(int, struct rtr_request const *, char const *); +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_pdu_type(int, struct rtr_request const *); -int err_pdu_send(int, uint8_t, uint16_t, void *, char const *); bool err_pdu_is_fatal(uint16_t); void err_pdu_log(uint16_t, char *); diff --git a/src/rtr/pdu.c b/src/rtr/pdu.c index da216c3b..9070af00 100644 --- a/src/rtr/pdu.c +++ b/src/rtr/pdu.c @@ -1,4 +1,4 @@ -#include "pdu.h" +#include "rtr/pdu.h" #include #include @@ -6,81 +6,123 @@ #include "common.h" #include "log.h" +#include "rtr/err_pdu.h" #include "rtr/pdu_handler.h" -static int pdu_header_from_stream(int, struct pdu_header *); -static int serial_notify_from_stream(struct pdu_header *, int, void *); -static int serial_query_from_stream(struct pdu_header *, int, void *); -static int reset_query_from_stream(struct pdu_header *, int, void *); -static int cache_response_from_stream(struct pdu_header *, int, void *); -static int ipv4_prefix_from_stream(struct pdu_header *, int, void *); -static int ipv6_prefix_from_stream(struct pdu_header *, int, void *); -static int end_of_data_from_stream(struct pdu_header *, int, void *); -static int cache_reset_from_stream(struct pdu_header *, int, void *); -static int router_key_from_stream(struct pdu_header *, int, void *); -static int error_report_from_stream(struct pdu_header *, int, void *); -static void error_report_destroy(void *); +static int +pdu_header_from_reader(struct pdu_reader *reader, struct pdu_header *header) +{ + return read_int8(reader, &header->protocol_version) + || read_int8(reader, &header->pdu_type) + || read_int16(reader, &header->m.session_id) + || read_int32(reader, &header->length); +} int -pdu_load(int fd, void **pdu, struct pdu_metadata const **metadata, - uint8_t *rtr_version) +pdu_load(int fd, struct rtr_request *request, + struct pdu_metadata const **metadata) { + unsigned char hdr_bytes[RTRPDU_HEADER_LEN]; + struct pdu_reader reader; struct pdu_header header; struct pdu_metadata const *meta; - int err; + int error; - err = pdu_header_from_stream(fd, &header); - if (err) - return err; + /* Read the header into its buffer. */ + /* TODO If the first read yields no bytes, the connection was terminated. */ + error = pdu_reader_init(&reader, fd, hdr_bytes, RTRPDU_HEADER_LEN); + if (error) + /* Communication interrupted; omit error response */ + return error; + error = pdu_header_from_reader(&reader, &header); + if (error) + /* No error response because the PDU might have been an error */ + return error; - meta = pdu_get_metadata(header.pdu_type); - if (!meta) - return -ENOENT; /* TODO try to skip it anyway? */ + /* + * RTRv1 expects us to respond RTRv1 messages with RTRv0 messages, + * and future protocols will probably do the same. + * So don't validate the protocol version. + */ + + if (header.length < RTRPDU_HEADER_LEN) + return err_pdu_send_invalid_request_truncated(fd, hdr_bytes, + "PDU is too small. (< 8 bytes)"); + + /* + * Error messages can be quite large. + * But they're probably not legitimate, so drop 'em. + * 512 is like a 5-paragraph error message, so it's probably enough. + */ + if (header.length > 512) { + pr_warn("Got an extremely large PDU (%u bytes). WTF?", + header.length); + return err_pdu_send_invalid_request_truncated(fd, hdr_bytes, + "PDU is too large. (> 512 bytes)"); + } - *pdu = malloc(meta->length); - if (*pdu == NULL) - return -ENOMEM; + /* Read the rest of the PDU into its buffer. */ + request->bytes_len = header.length; + request->bytes = malloc(header.length); + if (request->bytes == NULL) + return pr_enomem(); + + memcpy(request->bytes, hdr_bytes, RTRPDU_HEADER_LEN); + error = pdu_reader_init(&reader, fd, + request->bytes + RTRPDU_HEADER_LEN, + header.length - RTRPDU_HEADER_LEN); + if (error) + goto revert_bytes; + + /* Deserialize the PDU. */ + meta = pdu_get_metadata(header.pdu_type); + if (!meta) { + error = err_pdu_send_unsupported_pdu_type(fd, request); + goto revert_bytes; + } - err = meta->from_stream(&header, fd, *pdu); - if (err) { - free(*pdu); - return err; + request->pdu = malloc(meta->length); + if (request->pdu == NULL) { + error = pr_enomem(); + goto revert_bytes; } - *rtr_version = header.protocol_version; - if (metadata) - *metadata = meta; + error = meta->from_stream(&header, &reader, request->pdu); + if (error) + goto revert_pdu; + + /* Happy path. */ + *metadata = meta; return 0; -} -static int -pdu_header_from_stream(int fd, struct pdu_header *header) -{ - /* If the first read yields no bytes, the connection was terminated. */ - return read_int8(fd, &header->protocol_version) - || read_int8(fd, &header->pdu_type) - || read_int16(fd, &header->m.session_id) - || read_int32(fd, &header->length); +revert_pdu: + free(request->pdu); +revert_bytes: + free(request->bytes); + return error; } static int -serial_notify_from_stream(struct pdu_header *header, int fd, void *pdu_void) +serial_notify_from_stream(struct pdu_header *header, struct pdu_reader *reader, + void *pdu_void) { struct serial_notify_pdu *pdu = pdu_void; memcpy(&pdu->header, header, sizeof(*header)); - return read_int32(fd, &pdu->serial_number); + return read_int32(reader, &pdu->serial_number); } static int -serial_query_from_stream(struct pdu_header *header, int fd, void *pdu_void) +serial_query_from_stream(struct pdu_header *header, struct pdu_reader *reader, + void *pdu_void) { struct serial_query_pdu *pdu = pdu_void; memcpy(&pdu->header, header, sizeof(*header)); - return read_int32(fd, &pdu->serial_number); + return read_int32(reader, &pdu->serial_number); } static int -reset_query_from_stream(struct pdu_header *header, int fd, void *pdu_void) +reset_query_from_stream(struct pdu_header *header, struct pdu_reader *reader, + void *pdu_void) { struct reset_query_pdu *pdu = pdu_void; memcpy(&pdu->header, header, sizeof(*header)); @@ -88,7 +130,8 @@ reset_query_from_stream(struct pdu_header *header, int fd, void *pdu_void) } static int -cache_response_from_stream(struct pdu_header *header, int fd, void *pdu_void) +cache_response_from_stream(struct pdu_header *header, struct pdu_reader *reader, + void *pdu_void) { struct cache_response_pdu *pdu = pdu_void; memcpy(&pdu->header, header, sizeof(*header)); @@ -96,41 +139,45 @@ cache_response_from_stream(struct pdu_header *header, int fd, void *pdu_void) } static int -ipv4_prefix_from_stream(struct pdu_header *header, int fd, void *pdu_void) +ipv4_prefix_from_stream(struct pdu_header *header, struct pdu_reader *reader, + void *pdu_void) { struct ipv4_prefix_pdu *pdu = pdu_void; memcpy(&pdu->header, header, sizeof(*header)); - return read_int8(fd, &pdu->flags) - || read_int8(fd, &pdu->prefix_length) - || read_int8(fd, &pdu->max_length) - || read_int8(fd, &pdu->zero) - || read_in_addr(fd, &pdu->ipv4_prefix) - || read_int32(fd, &pdu->asn); + return read_int8(reader, &pdu->flags) + || read_int8(reader, &pdu->prefix_length) + || read_int8(reader, &pdu->max_length) + || read_int8(reader, &pdu->zero) + || read_in_addr(reader, &pdu->ipv4_prefix) + || read_int32(reader, &pdu->asn); } static int -ipv6_prefix_from_stream(struct pdu_header *header, int fd, void *pdu_void) +ipv6_prefix_from_stream(struct pdu_header *header, struct pdu_reader *reader, + void *pdu_void) { struct ipv6_prefix_pdu *pdu = pdu_void; memcpy(&pdu->header, header, sizeof(*header)); - return read_int8(fd, &pdu->flags) - || read_int8(fd, &pdu->prefix_length) - || read_int8(fd, &pdu->max_length) - || read_int8(fd, &pdu->zero) - || read_in6_addr(fd, &pdu->ipv6_prefix) - || read_int32(fd, &pdu->asn); + return read_int8(reader, &pdu->flags) + || read_int8(reader, &pdu->prefix_length) + || read_int8(reader, &pdu->max_length) + || read_int8(reader, &pdu->zero) + || read_in6_addr(reader, &pdu->ipv6_prefix) + || read_int32(reader, &pdu->asn); } static int -end_of_data_from_stream(struct pdu_header *header, int fd, void *pdu_void) +end_of_data_from_stream(struct pdu_header *header, struct pdu_reader *reader, + void *pdu_void) { struct end_of_data_pdu *pdu = pdu_void; memcpy(&pdu->header, header, sizeof(*header)); - return read_int32(fd, &pdu->serial_number); + return read_int32(reader, &pdu->serial_number); } static int -cache_reset_from_stream(struct pdu_header *header, int fd, void *pdu_void) +cache_reset_from_stream(struct pdu_header *header, struct pdu_reader *reader, + void *pdu_void) { struct cache_reset_pdu *pdu = pdu_void; memcpy(&pdu->header, header, sizeof(*header)); @@ -138,7 +185,8 @@ cache_reset_from_stream(struct pdu_header *header, int fd, void *pdu_void) } static int -router_key_from_stream(struct pdu_header *header, int fd, void *pdu_void) +router_key_from_stream(struct pdu_header *header, struct pdu_reader *reader, + void *pdu_void) { struct router_key_pdu *pdu = pdu_void; memcpy(&pdu->header, header, sizeof(*header)); @@ -146,48 +194,33 @@ router_key_from_stream(struct pdu_header *header, int fd, void *pdu_void) } static int -error_report_from_stream(struct pdu_header *header, int fd, void *pdu_void) +error_report_from_stream(struct pdu_header *header, struct pdu_reader *reader, + void *pdu_void) { struct error_report_pdu *pdu = pdu_void; - uint32_t sub_pdu_len; /* TODO use this for something */ - uint8_t rtr_version; int error; memcpy(&pdu->header, header, sizeof(*header)); - error = read_int32(fd, &sub_pdu_len); + error = read_int32(reader, &pdu->error_pdu_length); if (error) return error; - - error = pdu_load(fd, &pdu->erroneous_pdu, NULL, &rtr_version); + error = read_bytes(reader, pdu->erroneous_pdu, pdu->error_pdu_length); if (error) - return -EINVAL; - - error = read_string(fd, &pdu->error_message); - if (error) { - free(pdu->erroneous_pdu); return error; - } - - return 0; + error = read_int32(reader, &pdu->error_message_length); + if (error) + return error; + return read_string(reader, pdu->error_message_length, + &pdu->error_message); } static void error_report_destroy(void *pdu_void) { struct error_report_pdu *pdu = pdu_void; - struct pdu_header *sub_hdr; - struct pdu_metadata const *sub_meta; - - sub_hdr = pdu_get_header(pdu->erroneous_pdu); - sub_meta = pdu_get_metadata(sub_hdr->pdu_type); - if (sub_meta) - sub_meta->destructor(pdu->erroneous_pdu); - else - pr_warn("Unknown PDU type (%u).", sub_hdr->pdu_type); - free(pdu->error_message); - free(pdu_void); + free(pdu); } #define DEFINE_METADATA(name, dtor) \ diff --git a/src/rtr/pdu.h b/src/rtr/pdu.h index 103a21f0..dd809f22 100644 --- a/src/rtr/pdu.h +++ b/src/rtr/pdu.h @@ -9,6 +9,21 @@ #define RTR_V0 0 #define RTR_V1 1 +struct rtr_client { + int fd; + struct sockaddr_storage addr; +}; + +/** A request from an RTR client. */ +struct rtr_request { + /** Raw bytes. */ + unsigned char *bytes; + /** Length of @bytes. */ + size_t bytes_len; + /** Deserialized PDU. One of the *_pdu struct below. */ + void *pdu; +}; + enum pdu_type { PDU_TYPE_SERIAL_NOTIFY = 0, PDU_TYPE_SERIAL_QUERY = 1, @@ -22,6 +37,27 @@ enum pdu_type { PDU_TYPE_ERROR_REPORT = 10, }; +/* + * Note: It's probably best not to use sizeof for these lengths, because it + * risks including padding, and this is not the place for it. + * These numbers are constants from the RFC anyway. + */ + +/* Header length field is always 64 bits long */ +#define RTRPDU_HEADER_LEN 8 + +#define RTRPDU_SERIAL_NOTIFY_LEN 12 +#define RTRPDU_SERIAL_QUERY_LEN 12 +#define RTRPDU_RESET_QUERY_LEN 8 +#define RTRPDU_CACHE_RESPONSE_LEN 8 +#define RTRPDU_IPV4_PREFIX_LEN 20 +#define RTRPDU_IPV6_PREFIX_LEN 32 +#define RTRPDU_END_OF_DATA_LEN 12 +#define RTRPDU_CACHE_RESET_LEN 8 + +/* Ignores Error Report PDUs, which is fine. */ +#define RTRPDU_MAX_LEN RTRPDU_IPV6_PREFIX_LEN + struct pdu_header { uint8_t protocol_version; uint8_t pdu_type; @@ -95,19 +131,19 @@ struct router_key_pdu { struct error_report_pdu { struct pdu_header header; uint32_t error_pdu_length; - void *erroneous_pdu; + unsigned char erroneous_pdu[RTRPDU_MAX_LEN]; uint32_t error_message_length; rtr_char *error_message; }; struct pdu_metadata { size_t length; - int (*from_stream)(struct pdu_header *, int, void *); - int (*handle)(int, void *); + int (*from_stream)(struct pdu_header *, struct pdu_reader *, void *); + int (*handle)(int, struct rtr_request const *); void (*destructor)(void *); }; -int pdu_load(int, void **, struct pdu_metadata const **, uint8_t *); +int pdu_load(int, struct rtr_request *, struct pdu_metadata const **); struct pdu_metadata const *pdu_get_metadata(uint8_t); struct pdu_header *pdu_get_header(void *); diff --git a/src/rtr/pdu_handler.c b/src/rtr/pdu_handler.c index 462c6c81..f48af5dc 100644 --- a/src/rtr/pdu_handler.c +++ b/src/rtr/pdu_handler.c @@ -11,31 +11,28 @@ #include "rtr/db/vrps.h" static int -warn_unexpected_pdu(int fd, void *pdu, char const *pdu_name) +warn_unexpected_pdu(int fd, struct rtr_request const *request, + char const *pdu_name) { - struct pdu_header *pdu_header = pdu; - pr_warn("Unexpected %s PDU received", pdu_name); - err_pdu_send(fd, pdu_header->protocol_version, ERR_PDU_UNSUP_PDU_TYPE, - pdu_header, "PDU is unexpected or out of order."); + err_pdu_send_invalid_request(fd, request, + "PDU is unexpected or out of order."); return -EINVAL; } int -handle_serial_notify_pdu(int fd, void *pdu) +handle_serial_notify_pdu(int fd, struct rtr_request const *request) { - return warn_unexpected_pdu(fd, pdu, "Serial Notify"); + return warn_unexpected_pdu(fd, request, "Serial Notify"); } int -handle_serial_query_pdu(int fd, void *pdu) +handle_serial_query_pdu(int fd, struct rtr_request const *request) { - struct serial_query_pdu *received = pdu; - struct sender_common common; + struct serial_query_pdu *query = request->pdu; struct deltas_db deltas; serial_t final_serial; int error; - init_sender_common(&common, fd, received->header.protocol_version); /* * RFC 6810 and 8210: * "If [...] either the router or the cache finds that the value of the @@ -43,14 +40,9 @@ handle_serial_query_pdu(int fd, void *pdu) * the mismatch MUST immediately terminate the session with an Error * Report PDU with code 0 ("Corrupt Data")" */ - if (received->header.m.session_id != common.session_id) - return err_pdu_send(fd, common.version, ERR_PDU_CORRUPT_DATA, - &received->header, "Session ID doesn't match."); - - /* - * TODO (now) On certain errors, shouldn't we send error PDUs or - * something? - */ + if (query->header.m.session_id != get_current_session_id(RTR_V0)) + return err_pdu_send_corrupt_data(fd, request, + "Session ID doesn't match."); /* * For the record, there are two reasons why we want to work on a @@ -62,33 +54,30 @@ handle_serial_query_pdu(int fd, void *pdu) */ deltas_db_init(&deltas); - error = vrps_get_deltas_from(received->serial_number, &final_serial, + error = vrps_get_deltas_from(query->serial_number, &final_serial, &deltas); if (error == -EAGAIN) { - error = err_pdu_send(fd, common.version, - ERR_PDU_NO_DATA_AVAILABLE, NULL, NULL); + err_pdu_send_no_data_available(fd); + error = 0; goto end; } if (error == -ESRCH) { /* https://tools.ietf.org/html/rfc6810#section-6.3 */ - error = send_cache_reset_pdu(&common); + error = send_cache_reset_pdu(fd); goto end; } if (error) goto end; - /* - * https://tools.ietf.org/html/rfc6810#section-6.2 - * (Except the end of data PDU.) - */ + /* https://tools.ietf.org/html/rfc6810#section-6.2 */ - error = send_cache_response_pdu(&common); + error = send_cache_response_pdu(fd); if (error) goto end; - error = send_pdus_delta(&deltas, &common); + error = send_delta_pdus(fd, &deltas); if (error) goto end; /* TODO (now) maybe send something? */ - error = send_end_of_data_pdu(&common, final_serial); + error = send_end_of_data_pdu(fd, final_serial); end: deltas_db_cleanup(&deltas, deltagroup_cleanup); @@ -97,8 +86,7 @@ end: struct base_roa_args { bool started; - struct sender_common common; - serial_t last_serial; + int fd; }; static int @@ -108,26 +96,25 @@ send_base_roa(struct vrp const *vrp, void *arg) int error; if (!args->started) { - error = send_cache_response_pdu(&args->common); + error = send_cache_response_pdu(args->fd); if (error) return error; args->started = true; } /* TODO (now) maybe send something on error? */ - return send_prefix_pdu(&args->common, vrp, FLAG_ANNOUNCEMENT); + return send_prefix_pdu(args->fd, vrp, FLAG_ANNOUNCEMENT); } int -handle_reset_query_pdu(int fd, void *pdu) +handle_reset_query_pdu(int fd, struct rtr_request const *request) { - struct reset_query_pdu *received = pdu; struct base_roa_args args; serial_t current_serial; int error; args.started = false; - init_sender_common(&args.common, fd, received->header.protocol_version); + args.fd = fd; /* * It's probably best not to work on a copy, because the tree is large. @@ -137,55 +124,56 @@ handle_reset_query_pdu(int fd, void *pdu) */ error = vrps_foreach_base_roa(send_base_roa, &args, ¤t_serial); - if (error == -EAGAIN) - return err_pdu_send(fd, args.common.version, - ERR_PDU_NO_DATA_AVAILABLE, NULL, NULL); + if (error == -EAGAIN) { + err_pdu_send_no_data_available(fd); + return 0; + } if (error) return error; - return send_end_of_data_pdu(&args.common, current_serial); + return send_end_of_data_pdu(fd, current_serial); } int -handle_cache_response_pdu(int fd, void *pdu) +handle_cache_response_pdu(int fd, struct rtr_request const *request) { - return warn_unexpected_pdu(fd, pdu, "Cache Response"); + return warn_unexpected_pdu(fd, request, "Cache Response"); } int -handle_ipv4_prefix_pdu(int fd, void *pdu) +handle_ipv4_prefix_pdu(int fd, struct rtr_request const *request) { - return warn_unexpected_pdu(fd, pdu, "IPv4 Prefix"); + return warn_unexpected_pdu(fd, request, "IPv4 Prefix"); } int -handle_ipv6_prefix_pdu(int fd, void *pdu) +handle_ipv6_prefix_pdu(int fd, struct rtr_request const *request) { - return warn_unexpected_pdu(fd, pdu, "IPv6 Prefix"); + return warn_unexpected_pdu(fd, request, "IPv6 Prefix"); } int -handle_end_of_data_pdu(int fd, void *pdu) +handle_end_of_data_pdu(int fd, struct rtr_request const *request) { - return warn_unexpected_pdu(fd, pdu, "End of Data"); + return warn_unexpected_pdu(fd, request, "End of Data"); } int -handle_cache_reset_pdu(int fd, void *pdu) +handle_cache_reset_pdu(int fd, struct rtr_request const *request) { - return warn_unexpected_pdu(fd, pdu, "Cache Reset"); + return warn_unexpected_pdu(fd, request, "Cache Reset"); } int -handle_router_key_pdu(int fd, void *pdu) +handle_router_key_pdu(int fd, struct rtr_request const *request) { - return warn_unexpected_pdu(fd, pdu, "Router Key"); + return warn_unexpected_pdu(fd, request, "Router Key"); } int -handle_error_report_pdu(int fd, void *pdu) +handle_error_report_pdu(int fd, struct rtr_request const *request) { - struct error_report_pdu *received = pdu; + struct error_report_pdu *received = request->pdu; if (err_pdu_is_fatal(received->header.m.error_code)) { pr_warn("Fatal error report PDU received [code %u], closing socket.", diff --git a/src/rtr/pdu_handler.h b/src/rtr/pdu_handler.h index d53c7103..c45b8f55 100644 --- a/src/rtr/pdu_handler.h +++ b/src/rtr/pdu_handler.h @@ -1,18 +1,17 @@ #ifndef RTR_PDU_HANDLER_H_ #define RTR_PDU_HANDLER_H_ -#include "../common.h" +#include "rtr/pdu.h" - -int handle_serial_notify_pdu(int, void *); -int handle_serial_query_pdu(int, void *); -int handle_reset_query_pdu(int, void *); -int handle_cache_response_pdu(int, void *); -int handle_ipv4_prefix_pdu(int, void *); -int handle_ipv6_prefix_pdu(int, void *); -int handle_end_of_data_pdu(int, void *); -int handle_cache_reset_pdu(int, void *); -int handle_router_key_pdu(int, void *); -int handle_error_report_pdu(int, void *); +int handle_serial_notify_pdu(int, struct rtr_request const *); +int handle_serial_query_pdu(int, struct rtr_request const *); +int handle_reset_query_pdu(int, struct rtr_request const *); +int handle_cache_response_pdu(int, struct rtr_request const *); +int handle_ipv4_prefix_pdu(int, struct rtr_request const *); +int handle_ipv6_prefix_pdu(int, struct rtr_request const *); +int handle_end_of_data_pdu(int, struct rtr_request const *); +int handle_cache_reset_pdu(int, struct rtr_request const *); +int handle_router_key_pdu(int, struct rtr_request const *); +int handle_error_report_pdu(int, struct rtr_request const *); #endif /* RTR_PDU_HANDLER_H_ */ diff --git a/src/rtr/pdu_sender.c b/src/rtr/pdu_sender.c index 827a7bc0..4625b898 100644 --- a/src/rtr/pdu_sender.c +++ b/src/rtr/pdu_sender.c @@ -13,8 +13,6 @@ #include "rtr/pdu_serializer.h" #include "rtr/db/vrps.h" -/* Header length field is always 64 bits long */ -#define HEADER_LENGTH 8 /* IPvN PDUs length without header */ #define IPV4_PREFIX_LENGTH 12 #define IPV6_PREFIX_LENGTH 24 @@ -28,54 +26,17 @@ struct vrp_node { /** Sorted list to filter deltas */ SLIST_HEAD(vrp_slist, vrp_node); -void -init_sender_common(struct sender_common *common, int fd, uint8_t version) -{ - common->fd = fd; - common->version = version; - common->session_id = get_current_session_id(version); -} /* * Set all the header values, EXCEPT length field. */ static void -set_header_values(struct pdu_header *header, uint8_t version, uint8_t type, - uint16_t reserved) +set_header_values(struct pdu_header *header, uint8_t type, uint16_t reserved) { - header->protocol_version = version; + header->protocol_version = RTR_V0; header->pdu_type = type; header->m.reserved = reserved; } -static uint32_t -length_serial_notify_pdu(struct serial_notify_pdu *pdu) -{ - return HEADER_LENGTH + sizeof(pdu->serial_number); -} - -static uint32_t -length_ipvx_prefix_pdu(bool isv4) -{ - return HEADER_LENGTH + - (isv4 ? IPV4_PREFIX_LENGTH : IPV6_PREFIX_LENGTH); -} - -static uint32_t -length_end_of_data_pdu(struct end_of_data_pdu *pdu) -{ - uint32_t len; - - len = HEADER_LENGTH; - len += sizeof(pdu->serial_number); - if (pdu->header.protocol_version == RTR_V1) { - len += sizeof(pdu->refresh_interval); - len += sizeof(pdu->retry_interval); - len += sizeof(pdu->expire_interval); - } - - return len; -} - /* TODO Include Router Key PDU serials */ /** * static uint32_t @@ -86,14 +47,6 @@ length_end_of_data_pdu(struct end_of_data_pdu *pdu) * } */ -static uint32_t -length_error_report_pdu(struct error_report_pdu *pdu) -{ - return HEADER_LENGTH + - pdu->error_pdu_length + sizeof(pdu->error_pdu_length) + - pdu->error_message_length + sizeof(pdu->error_message_length); -} - /* * TODO Needs some testing, this is just a beta version */ @@ -157,66 +110,71 @@ send_response(int fd, unsigned char *data, size_t data_len) } int -send_serial_notify_pdu(struct sender_common *common, serial_t start_serial) +send_serial_notify_pdu(int fd, serial_t start_serial) { struct serial_notify_pdu pdu; - unsigned char data[BUFFER_SIZE]; + unsigned char data[RTRPDU_SERIAL_NOTIFY_LEN]; size_t len; - set_header_values(&pdu.header, common->version, PDU_TYPE_SERIAL_NOTIFY, - common->session_id); + set_header_values(&pdu.header, PDU_TYPE_SERIAL_NOTIFY, + get_current_session_id(RTR_V0)); pdu.serial_number = start_serial; - pdu.header.length = length_serial_notify_pdu(&pdu); + pdu.header.length = RTRPDU_SERIAL_NOTIFY_LEN; len = serialize_serial_notify_pdu(&pdu, data); + if (len != RTRPDU_SERIAL_NOTIFY_LEN) + return pr_crit("Serialized Serial Notify is %zu bytes.", len); - return send_response(common->fd, data, len); + return send_response(fd, data, len); } int -send_cache_reset_pdu(struct sender_common *common) +send_cache_reset_pdu(int fd) { struct cache_reset_pdu pdu; - unsigned char data[BUFFER_SIZE]; + unsigned char data[RTRPDU_CACHE_RESET_LEN]; size_t len; /* This PDU has only the header */ - set_header_values(&pdu.header, common->version, PDU_TYPE_CACHE_RESET, - 0); - pdu.header.length = HEADER_LENGTH; + set_header_values(&pdu.header, PDU_TYPE_CACHE_RESET, 0); + pdu.header.length = RTRPDU_CACHE_RESET_LEN; len = serialize_cache_reset_pdu(&pdu, data); - return send_response(common->fd, data, len); + if (len != RTRPDU_CACHE_RESET_LEN) + return pr_crit("Serialized Cache Reset is %zu bytes.", len); + + return send_response(fd, data, len); } int -send_cache_response_pdu(struct sender_common *common) +send_cache_response_pdu(int fd) { struct cache_response_pdu pdu; - unsigned char data[BUFFER_SIZE]; + unsigned char data[RTRPDU_CACHE_RESPONSE_LEN]; size_t len; /* This PDU has only the header */ - set_header_values(&pdu.header, common->version, PDU_TYPE_CACHE_RESPONSE, - common->session_id); - pdu.header.length = HEADER_LENGTH; + set_header_values(&pdu.header, PDU_TYPE_CACHE_RESPONSE, + get_current_session_id(RTR_V0)); + pdu.header.length = RTRPDU_CACHE_RESPONSE_LEN; len = serialize_cache_response_pdu(&pdu, data); + if (len != RTRPDU_CACHE_RESPONSE_LEN) + return pr_crit("Serialized Cache Response is %zu bytes.", len); - return send_response(common->fd, data, len); + return send_response(fd, data, len); } static int -send_ipv4_prefix_pdu(struct sender_common *common, struct vrp const *vrp, - uint8_t flags) +send_ipv4_prefix_pdu(int fd, struct vrp const *vrp, uint8_t flags) { struct ipv4_prefix_pdu pdu; - unsigned char data[BUFFER_SIZE]; + unsigned char data[RTRPDU_IPV4_PREFIX_LEN]; size_t len; - set_header_values(&pdu.header, common->version, PDU_TYPE_IPV4_PREFIX, - 0); + set_header_values(&pdu.header, PDU_TYPE_IPV4_PREFIX, 0); + pdu.header.length = RTRPDU_IPV4_PREFIX_LEN; pdu.flags = flags; pdu.prefix_length = vrp->prefix_length; @@ -224,23 +182,23 @@ send_ipv4_prefix_pdu(struct sender_common *common, struct vrp const *vrp, pdu.zero = 0; pdu.ipv4_prefix = vrp->prefix.v4; pdu.asn = vrp->asn; - pdu.header.length = length_ipvx_prefix_pdu(true); len = serialize_ipv4_prefix_pdu(&pdu, data); + if (len != RTRPDU_IPV4_PREFIX_LEN) + return pr_crit("Serialized IPv4 Prefix is %zu bytes.", len); - return send_response(common->fd, data, len); + return send_response(fd, data, len); } static int -send_ipv6_prefix_pdu(struct sender_common *common, struct vrp const *vrp, - uint8_t flags) +send_ipv6_prefix_pdu(int fd, struct vrp const *vrp, uint8_t flags) { struct ipv6_prefix_pdu pdu; - unsigned char data[BUFFER_SIZE]; + unsigned char data[RTRPDU_IPV6_PREFIX_LEN]; size_t len; - set_header_values(&pdu.header, common->version, PDU_TYPE_IPV6_PREFIX, - 0); + set_header_values(&pdu.header, PDU_TYPE_IPV6_PREFIX, 0); + pdu.header.length = RTRPDU_IPV6_PREFIX_LEN; pdu.flags = flags; pdu.prefix_length = vrp->prefix_length; @@ -248,22 +206,22 @@ send_ipv6_prefix_pdu(struct sender_common *common, struct vrp const *vrp, pdu.zero = 0; pdu.ipv6_prefix = vrp->prefix.v6; pdu.asn = vrp->asn; - pdu.header.length = length_ipvx_prefix_pdu(false); len = serialize_ipv6_prefix_pdu(&pdu, data); + if (len != RTRPDU_IPV6_PREFIX_LEN) + return pr_crit("Serialized IPv6 Prefix is %zu bytes.", len); - return send_response(common->fd, data, len); + return send_response(fd, data, len); } int -send_prefix_pdu(struct sender_common *common, struct vrp const *vrp, - uint8_t flags) +send_prefix_pdu(int fd, struct vrp const *vrp, uint8_t flags) { switch (vrp->addr_fam) { case AF_INET: - return send_ipv4_prefix_pdu(common, vrp, flags); + return send_ipv4_prefix_pdu(fd, vrp, flags); case AF_INET6: - return send_ipv6_prefix_pdu(common, vrp, flags); + return send_ipv6_prefix_pdu(fd, vrp, flags); } return -EINVAL; @@ -286,7 +244,8 @@ vrp_equals(struct vrp const *left, struct vrp const *right) static int vrp_simply_send(struct delta const *delta, void *arg) { - return send_prefix_pdu(arg, &delta->vrp, delta->flags); + int *fd = arg; + return send_prefix_pdu(*fd, &delta->vrp, delta->flags); } /** @@ -319,7 +278,7 @@ vrp_ovrd_remove(struct delta const *delta, void *arg) } int -send_pdus_delta(struct deltas_db *deltas, struct sender_common *common) +send_delta_pdus(int fd, struct deltas_db *deltas) { struct vrp_slist filtered_vrps; struct delta_group *group; @@ -333,7 +292,7 @@ send_pdus_delta(struct deltas_db *deltas, struct sender_common *common) if (deltas->len == 1) { group = &deltas->array[0]; return deltas_foreach(group->serial, group->deltas, - vrp_simply_send, common); + vrp_simply_send, &fd); } /* @@ -351,8 +310,7 @@ send_pdus_delta(struct deltas_db *deltas, struct sender_common *common) /* Now send the filtered deltas */ SLIST_FOREACH(ptr, &filtered_vrps, next) { - error = send_prefix_pdu(common, &ptr->delta.vrp, - ptr->delta.flags); + error = send_prefix_pdu(fd, &ptr->delta.vrp, ptr->delta.flags); if (error) break; } @@ -368,64 +326,79 @@ release_list: } int -send_end_of_data_pdu(struct sender_common *common, serial_t end_serial) +send_end_of_data_pdu(int fd, serial_t end_serial) { struct end_of_data_pdu pdu; - unsigned char data[BUFFER_SIZE]; + unsigned char data[RTRPDU_END_OF_DATA_LEN]; size_t len; int error; - set_header_values(&pdu.header, common->version, PDU_TYPE_END_OF_DATA, - common->session_id); + set_header_values(&pdu.header, PDU_TYPE_END_OF_DATA, + get_current_session_id(RTR_V0)); + pdu.header.length = RTRPDU_END_OF_DATA_LEN; + pdu.serial_number = end_serial; - if (common->version == RTR_V1) { + if (pdu.header.protocol_version == RTR_V1) { pdu.refresh_interval = config_get_refresh_interval(); pdu.retry_interval = config_get_retry_interval(); pdu.expire_interval = config_get_expire_interval(); } - pdu.header.length = length_end_of_data_pdu(&pdu); len = serialize_end_of_data_pdu(&pdu, data); + if (len != RTRPDU_END_OF_DATA_LEN) + return pr_crit("Serialized End of Data is %zu bytes.", len); - error = send_response(common->fd, data, len); + error = send_response(fd, data, len); if (error) return error; - clients_update_serial(common->fd, pdu.serial_number); - return error; + clients_update_serial(fd, pdu.serial_number); + return 0; } int -send_error_report_pdu(int fd, uint8_t version, uint16_t code, -struct pdu_header *err_pdu_header, char const *message) +send_error_report_pdu(int fd, uint16_t code, struct rtr_request const *request, + char *message) { struct error_report_pdu pdu; - unsigned char data[BUFFER_SIZE]; + unsigned char *data; size_t len; + int error; - set_header_values(&pdu.header, version, PDU_TYPE_ERROR_REPORT, code); - - pdu.error_pdu_length = 0; - pdu.erroneous_pdu = (void *)err_pdu_header; - if (err_pdu_header != NULL) - pdu.error_pdu_length = sizeof(err_pdu_header); - - pdu.error_message_length = 0; - pdu.error_message = NULL; - if (message != NULL) { - pdu.error_message = malloc(strlen(message) + 1); - if (pdu.error_message == NULL) - pr_warn("Error message couldn't be allocated, removed from PDU"); - else { - pdu.error_message_length = strlen(message) + 1; - strcpy(pdu.error_message, message); - } + set_header_values(&pdu.header, PDU_TYPE_ERROR_REPORT, code); + + if (request != NULL) { + pdu.error_pdu_length = (request->bytes_len > RTRPDU_MAX_LEN) + ? RTRPDU_MAX_LEN + : request->bytes_len; + memcpy(pdu.erroneous_pdu, request->bytes, pdu.error_pdu_length); + } else { + pdu.error_pdu_length = 0; } - /* Calculate lengths */ - pdu.header.length = length_error_report_pdu(&pdu); + pdu.error_message_length = (message != NULL) ? strlen(message) : 0; + pdu.error_message = message; + + pdu.header.length = RTRPDU_HEADER_LEN + + 4 /* Length of Encapsulated PDU field */ + + pdu.error_pdu_length + + 4 /* Length of Error Text field */ + + pdu.error_message_length; + + data = malloc(pdu.header.length); + if (data == NULL) + return pr_enomem(); len = serialize_error_report_pdu(&pdu, data); - free(pdu.error_message); - return send_response(fd, data, len); + if (len != pdu.header.length) { + error = pr_crit("Serialized Error Report PDU is %zu bytes, not the expected %u.", + len, pdu.header.length); + goto end; + } + + error = send_response(fd, data, len); + +end: + free(data); + return error; } diff --git a/src/rtr/pdu_sender.h b/src/rtr/pdu_sender.h index 4464a7a2..83464718 100644 --- a/src/rtr/pdu_sender.h +++ b/src/rtr/pdu_sender.h @@ -4,22 +4,15 @@ #include "pdu.h" #include "rtr/db/vrps.h" -struct sender_common { - int fd; - uint8_t version; - uint16_t session_id; -}; +void init_sender_common(int, int, uint8_t); -void init_sender_common(struct sender_common *, int, uint8_t); - -int send_serial_notify_pdu(struct sender_common *, serial_t); -int send_cache_reset_pdu(struct sender_common *); -int send_cache_response_pdu(struct sender_common *); -int send_prefix_pdu(struct sender_common *, struct vrp const *, uint8_t); -int send_pdus_delta(struct deltas_db *, struct sender_common *); -int send_end_of_data_pdu(struct sender_common *, serial_t); -int send_error_report_pdu(int, uint8_t, uint16_t, struct pdu_header *, - char const *); +int send_serial_notify_pdu(int, serial_t); +int send_cache_reset_pdu(int); +int send_cache_response_pdu(int); +int send_prefix_pdu(int, struct vrp const *, uint8_t); +int send_delta_pdus(int, struct deltas_db *); +int send_end_of_data_pdu(int, serial_t); +int send_error_report_pdu(int, uint16_t, struct rtr_request const *, char *); #endif /* SRC_RTR_PDU_SENDER_H_ */ diff --git a/src/rtr/pdu_serializer.c b/src/rtr/pdu_serializer.c index fbb74b20..bcf5bbe9 100644 --- a/src/rtr/pdu_serializer.c +++ b/src/rtr/pdu_serializer.c @@ -155,30 +155,22 @@ serialize_router_key_pdu(struct router_key_pdu *pdu, unsigned char *buf) size_t serialize_error_report_pdu(struct error_report_pdu *pdu, unsigned char *buf) { - struct pdu_header *err_pdu_header; - size_t head_size; unsigned char *ptr; - char *tmp_ptr; - int i; - - head_size = serialize_pdu_header(&pdu->header, pdu->header.m.error_code, - buf); - ptr = buf + head_size; + ptr = buf; + ptr += serialize_pdu_header(&pdu->header, pdu->header.m.error_code, buf); ptr = write_int32(ptr, pdu->error_pdu_length); if (pdu->error_pdu_length > 0) { - /* Set only the header of err PDU */ - err_pdu_header = (struct pdu_header *)pdu->erroneous_pdu; - head_size = serialize_pdu_header(err_pdu_header, - err_pdu_header->m.reserved, ptr); - ptr = ptr + head_size; + memcpy(ptr, pdu->erroneous_pdu, pdu->error_pdu_length); + ptr += pdu->error_pdu_length; } ptr = write_int32(ptr, pdu->error_message_length); - tmp_ptr = pdu->error_message; - for (i = 0; i < pdu->error_message_length; i++) - ptr = write_int8(ptr, tmp_ptr[i]); + if (pdu->error_message_length > 0) { + memcpy(ptr, pdu->error_message, pdu->error_message_length); + ptr += pdu->error_message_length; + } return ptr - buf; } diff --git a/src/rtr/primitive_reader.c b/src/rtr/primitive_reader.c index 64701a71..7f085c9b 100644 --- a/src/rtr/primitive_reader.c +++ b/src/rtr/primitive_reader.c @@ -4,17 +4,16 @@ #include #include #include +#include #include #include #include "log.h" -static int read_exact(int, unsigned char *, size_t); -static int read_and_waste(int, unsigned char *, size_t, uint32_t); static int get_octets(unsigned char); static void place_null_character(rtr_char *, size_t); -static int +int read_exact(int fd, unsigned char *buffer, size_t buffer_len) { ssize_t read_result; @@ -34,95 +33,81 @@ read_exact(int fd, unsigned char *buffer, size_t buffer_len) return 0; } +/** + * BTW: I think it's best not to use sizeof for @size, because it risks + * including padding. + */ int -read_int8(int fd, uint8_t *result) +pdu_reader_init(struct pdu_reader *reader, int fd, unsigned char *buffer, + size_t size) { - return read_exact(fd, result, sizeof(uint8_t)); + reader->buffer = buffer; + reader->size = size; + return read_exact(fd, reader->buffer, size); } -/** Big Endian. */ -int -read_int16(int fd, uint16_t *result) +static int +insufficient_bytes(void) { - unsigned char buffer[2]; - int err; + pr_crit("Attempted to read past the end of a PDU Reader."); + return -EPIPE; +} - err = read_exact(fd, buffer, sizeof(buffer)); - if (err) - return err; +int +read_int8(struct pdu_reader *reader, uint8_t *result) +{ + if (reader->size < 1) + return insufficient_bytes(); - *result = (((uint16_t)buffer[0]) << 8) | ((uint16_t)buffer[1]); + *result = reader->buffer[0]; + reader->buffer++; + reader->size--; return 0; } /** Big Endian. */ int -read_int32(int fd, uint32_t *result) +read_int16(struct pdu_reader *reader, uint16_t *result) { - unsigned char buffer[4]; - int err; + if (reader->size < 2) + return insufficient_bytes(); - err = read_exact(fd, buffer, sizeof(buffer)); - if (err) - return err; - - *result = (((uint32_t)buffer[0]) << 24) - | (((uint32_t)buffer[1]) << 16) - | (((uint32_t)buffer[2]) << 8) - | (((uint32_t)buffer[3]) ); + *result = (((uint16_t)reader->buffer[0]) << 8) + | (((uint16_t)reader->buffer[1]) ); + reader->buffer += 2; + reader->size -= 2; return 0; } +/** Big Endian. */ int -read_in_addr(int fd, struct in_addr *result) +read_int32(struct pdu_reader *reader, uint32_t *result) { - return read_int32(fd, &result->s_addr); + if (reader->size < 4) + return insufficient_bytes(); + + *result = (((uint32_t)reader->buffer[0]) << 24) + | (((uint32_t)reader->buffer[1]) << 16) + | (((uint32_t)reader->buffer[2]) << 8) + | (((uint32_t)reader->buffer[3]) ); + reader->buffer += 4; + reader->size -= 4; + return 0; } int -read_in6_addr(int fd, struct in6_addr *result) +read_in_addr(struct pdu_reader *reader, struct in_addr *result) { - return read_int32(fd, &result->s6_addr32[0]) - || read_int32(fd, &result->s6_addr32[1]) - || read_int32(fd, &result->s6_addr32[2]) - || read_int32(fd, &result->s6_addr32[3]); + return read_int32(reader, &result->s_addr); } -/* - * Consumes precisely @total_len bytes from @fd. - * The first @str_len bytes are stored in @str. - * - * It is required that @str_len <= @total_len. - */ -static int -read_and_waste(int fd, unsigned char *str, size_t str_len, uint32_t total_len) +int +read_in6_addr(struct pdu_reader *reader, struct in6_addr *result) { -#define TLEN 1024 /* "Trash length" */ - unsigned char *trash; - size_t offset; - int err; - - err = read_exact(fd, str, str_len); - if (err) - return err; - - if (str_len == total_len) - return 0; - - trash = malloc(TLEN); - if (trash == NULL) - return pr_enomem(); - - for (offset = str_len; offset < total_len; offset += TLEN) { - err = read_exact(fd, trash, - (total_len - offset >= TLEN) ? TLEN : (total_len - offset)); - if (err) - break; - } - - free(trash); - return err; -#undef TLEN + return read_int32(reader, &result->s6_addr32[0]) + || read_int32(reader, &result->s6_addr32[1]) + || read_int32(reader, &result->s6_addr32[2]) + || read_int32(reader, &result->s6_addr32[3]); } #define EINVALID_UTF8 -0xFFFF @@ -172,7 +157,7 @@ place_null_character(rtr_char *str, size_t len) null_chara_pos = str; cursor = str; - while (cursor < str + len - 1) { + while (cursor < str + len) { octets = get_octets(*UCHAR(cursor)); if (octets == EINVALID_UTF8) break; @@ -180,7 +165,7 @@ place_null_character(rtr_char *str, size_t len) for (octet = 1; octet < octets; octet++) { /* Memory ends in the middle of this code point? */ - if (cursor >= str + len - 1) + if (cursor >= str + len) goto end; /* All continuation octets must begin with 0b10. */ if ((*(UCHAR(cursor)) >> 6) != 2 /* 0b10 */) @@ -207,43 +192,41 @@ end: * (Including the NULL chara.) */ int -read_string(int fd, rtr_char **result) +read_string(struct pdu_reader *reader, uint32_t string_len, rtr_char **result) { /* Actual string length claimed by the PDU, in octets. */ - uint32_t full_length32; /* Excludes the null chara */ - uint64_t full_length64; /* Includes the null chara */ + rtr_char *string; + + if (reader->size < string_len) + return pr_err("Erroneous PDU's error message is larger than its slot in the PDU."); + /* - * Actual length that we allocate. Octets. - * This exists because there might be value in truncating the string; - * full_length is a fucking 32-bit integer for some reason. - * Note that, because this is UTF-8 we're dealing with, this might not - * necessarily end up being the actual octet length of the final - * string; since our truncation can land in the middle of a code point, - * the null character might need to be shifted left slightly. + * Ok. Since the PDU size is already sanitized, string_len is guaranteed + * to be relatively small now. */ - size_t alloc_length; /* Includes the null chara */ - rtr_char *str; - int err; - err = read_int32(fd, &full_length32); - if (err) - return err; + string = malloc(string_len + 1); /* Include NULL chara. */ + if (!string) + return -ENOMEM; - full_length64 = ((uint64_t) full_length32) + 1; + memcpy(string, reader->buffer, string_len); + reader->buffer += string_len; + reader->size -= string_len; - alloc_length = (full_length64 > 4096) ? 4096 : full_length64; - str = malloc(alloc_length); - if (!str) - return -ENOMEM; + place_null_character(string, string_len); - err = read_and_waste(fd, UCHAR(str), alloc_length - 1, full_length32); - if (err) { - free(str); - return err; - } + *result = string; + return 0; +} - place_null_character(str, alloc_length); +int +read_bytes(struct pdu_reader *reader, unsigned char *result, size_t num) +{ + if (reader->size < num) + return insufficient_bytes(); - *result = str; + memcpy(result, reader->buffer, num); + reader->buffer += num; + reader->size -= num; return 0; } diff --git a/src/rtr/primitive_reader.h b/src/rtr/primitive_reader.h index 104b1239..3876894d 100644 --- a/src/rtr/primitive_reader.h +++ b/src/rtr/primitive_reader.h @@ -3,15 +3,23 @@ #include -#include "../common.h" +#include "common.h" typedef char rtr_char; -int read_int8(int, uint8_t *); -int read_int16(int, uint16_t *); -int read_int32(int, uint32_t *); -int read_in_addr(int, struct in_addr *); -int read_in6_addr(int, struct in6_addr *); -int read_string(int, rtr_char **); +struct pdu_reader { + unsigned char *buffer; + size_t size; +}; + +int pdu_reader_init(struct pdu_reader *, int, unsigned char *, size_t size); + +int read_int8(struct pdu_reader *, uint8_t *); +int read_int16(struct pdu_reader *, uint16_t *); +int read_int32(struct pdu_reader *, uint32_t *); +int read_in_addr(struct pdu_reader *, struct in_addr *); +int read_in6_addr(struct pdu_reader *, struct in6_addr *); +int read_string(struct pdu_reader *, uint32_t, rtr_char **); +int read_bytes(struct pdu_reader *, unsigned char *, size_t); #endif /* RTR_PRIMITIVE_READER_H_ */ diff --git a/src/rtr/rtr.c b/src/rtr/rtr.c index 8bc26276..3a7630d6 100644 --- a/src/rtr/rtr.c +++ b/src/rtr/rtr.c @@ -19,24 +19,12 @@ #include "rtr/err_pdu.h" #include "rtr/pdu.h" -/* TODO (next iteration) Support both RTR v0 and v1 */ -#define RTR_VERSION_SUPPORTED RTR_V0 - /* TODO (urgent) this needs to be atomic */ volatile bool loop; -/* - * Arguments that the server socket thread will send to the client - * socket threads whenever it creates them. - */ -struct thread_param { - int client_fd; - struct sockaddr_storage client_addr; -}; - struct thread_node { pthread_t tid; - struct thread_param params; + struct rtr_client client; SLIST_ENTRY(thread_node) next; }; @@ -160,12 +148,17 @@ handle_accept_result(int client_fd, int err) return VERDICT_EXIT; } +static void +clean_request(struct rtr_request *request, const struct pdu_metadata *meta) +{ + free(request->bytes); + meta->destructor(request->pdu); +} + static void * -end_client(int client_fd, const struct pdu_metadata *meta, void *pdu) +end_client(struct rtr_client *client) { - if (meta != NULL && pdu != NULL) - meta->destructor(pdu); - clients_forget(client_fd); + clients_forget(client->fd); return NULL; } @@ -173,43 +166,32 @@ end_client(int client_fd, const struct pdu_metadata *meta, void *pdu) * The client socket threads' entry routine. */ static void * -client_thread_cb(void *param_void) +client_thread_cb(void *arg) { - struct thread_param *param = param_void; + struct rtr_client *client = arg; struct pdu_metadata const *meta; - void *pdu; - int err; - uint8_t rtr_version; + struct rtr_request request; + int error; while (loop) { /* For each PDU... */ - err = pdu_load(param->client_fd, &pdu, &meta, &rtr_version); - if (err) - return end_client(param->client_fd, NULL, NULL); - - /* Protocol Version Negotiation */ - if (rtr_version != RTR_VERSION_SUPPORTED) { - err_pdu_send(param->client_fd, RTR_VERSION_SUPPORTED, - ERR_PDU_UNSUP_PROTO_VERSION, pdu, NULL); - return end_client(param->client_fd, meta, pdu); + error = pdu_load(client->fd, &request, &meta); + if (error) + return end_client(client); + + error = clients_add(client); + if (error) { + clean_request(&request, meta); + err_pdu_send_internal_error(client->fd); + return end_client(client); } - /* RTR Version ready, now update client */ - err = clients_add(param->client_fd, ¶m->client_addr, - rtr_version); - if (err) { - if (err == -ERTR_VERSION_MISMATCH) { - err_pdu_send(param->client_fd, rtr_version, - (rtr_version == RTR_V0) - ? ERR_PDU_UNSUP_PROTO_VERSION - : ERR_PDU_UNEXPECTED_PROTO_VERSION, - pdu, NULL); - } - return end_client(param->client_fd, meta, pdu); + + error = meta->handle(client->fd, &request); + if (error) { + clean_request(&request, meta); + return end_client(client); } - err = meta->handle(param->client_fd, pdu); - meta->destructor(pdu); - if (err) - return end_client(param->client_fd, NULL, NULL); + clean_request(&request, meta); } return NULL; /* Unreachable. */ @@ -256,11 +238,11 @@ handle_client_connections(int server_fd) continue; } - new_thread->params.client_fd = client_fd; - new_thread->params.client_addr = client_addr; + new_thread->client.fd = client_fd; + new_thread->client.addr = client_addr; errno = pthread_create(&new_thread->tid, NULL, - client_thread_cb, &new_thread->params); + client_thread_cb, &new_thread->client); if (errno) { pr_errno(errno, "Could not spawn the client's thread"); free(new_thread); diff --git a/test/client_test.c b/test/client_test.c index f565201e..b162d585 100644 --- a/test/client_test.c +++ b/test/client_test.c @@ -35,13 +35,13 @@ START_TEST(basic_test) */ struct sockaddr_in addr; - struct sockaddr_storage *addr_ptr; + struct rtr_client client; + unsigned int i; unsigned int state; + memset(&addr, 0, sizeof(addr)); addr.sin_family = AF_INET; - addr.sin_addr.s_addr = htonl(0xc0000201u); - addr.sin_port = 1234; - addr_ptr = (struct sockaddr_storage *) &addr; + memcpy(&client.addr, &addr, sizeof(client.addr)); ck_assert_int_eq(0, clients_db_init()); @@ -49,15 +49,17 @@ START_TEST(basic_test) * The address is actually supposed to be unique, but this is rather * enforced by calling code, so whatever. */ - ck_assert_int_eq(0, clients_add(1, addr_ptr, 0)); - ck_assert_int_eq(0, clients_add(2, addr_ptr, 0)); - ck_assert_int_eq(0, clients_add(3, addr_ptr, 1)); - ck_assert_int_eq(0, clients_add(4, addr_ptr, 1)); - - ck_assert_int_eq(0, clients_add(1, addr_ptr, 0)); - ck_assert_int_eq(-ERTR_VERSION_MISMATCH, clients_add(2, addr_ptr, 1)); - ck_assert_int_eq(0, clients_add(3, addr_ptr, 1)); - ck_assert_int_eq(-ERTR_VERSION_MISMATCH, clients_add(4, addr_ptr, 0)); + + for (i = 0; i < 4; i++) { + client.fd = 1; + ck_assert_int_eq(0, clients_add(&client)); + client.fd = 2; + ck_assert_int_eq(0, clients_add(&client)); + client.fd = 3; + ck_assert_int_eq(0, clients_add(&client)); + client.fd = 4; + ck_assert_int_eq(0, clients_add(&client)); + } clients_forget(3); diff --git a/test/rtr/pdu_handler_test.c b/test/rtr/pdu_handler_test.c index b4629dcc..129f3daf 100644 --- a/test/rtr/pdu_handler_test.c +++ b/test/rtr/pdu_handler_test.c @@ -67,8 +67,10 @@ init_db_full(void) } static void -init_reset_query(struct reset_query_pdu *query) +init_reset_query(struct rtr_request *request, struct reset_query_pdu *query) { + request->pdu = query; + request->bytes_len = 0; query->header.protocol_version = RTR_V0; query->header.pdu_type = PDU_TYPE_RESET_QUERY; query->header.m.reserved = 0; @@ -76,8 +78,11 @@ init_reset_query(struct reset_query_pdu *query) } static void -init_serial_query(struct serial_query_pdu *query, uint32_t serial) +init_serial_query(struct rtr_request *request, struct serial_query_pdu *query, + uint32_t serial) { + request->pdu = query; + request->bytes_len = 0; query->header.protocol_version = RTR_V0; query->header.pdu_type = PDU_TYPE_SERIAL_QUERY; query->header.m.session_id = get_current_session_id(RTR_V0); @@ -93,16 +98,8 @@ clients_get_min_serial(void) return 0; } -void -init_sender_common(struct sender_common *common, int fd, uint8_t version) -{ - common->fd = fd; - common->version = version; - common->session_id = get_current_session_id(version); -} - int -send_cache_reset_pdu(struct sender_common *common) +send_cache_reset_pdu(int fd) { pr_info(" Server sent Cache Reset."); ck_assert_int_eq(pop_expected_pdu(), PDU_TYPE_CACHE_RESET); @@ -110,7 +107,7 @@ send_cache_reset_pdu(struct sender_common *common) } int -send_cache_response_pdu(struct sender_common *common) +send_cache_response_pdu(int fd) { pr_info(" Server sent Cache Response."); ck_assert_int_eq(pop_expected_pdu(), PDU_TYPE_CACHE_RESPONSE); @@ -118,8 +115,7 @@ send_cache_response_pdu(struct sender_common *common) } int -send_prefix_pdu(struct sender_common *common, struct vrp const *vrp, - uint8_t flags) +send_prefix_pdu(int fd, struct vrp const *vrp, uint8_t flags) { /* * We don't care about order. @@ -137,24 +133,25 @@ send_prefix_pdu(struct sender_common *common, struct vrp const *vrp, static int handle_delta(struct delta const *delta, void *arg) { - ck_assert_int_eq(0, send_prefix_pdu(arg, &delta->vrp, delta->flags)); + int *fd = arg; + ck_assert_int_eq(0, send_prefix_pdu(*fd, &delta->vrp, delta->flags)); return 0; } int -send_pdus_delta(struct deltas_db *deltas, struct sender_common *common) +send_delta_pdus(int fd, struct deltas_db *deltas) { struct delta_group *group; ARRAYLIST_FOREACH(deltas, group) ck_assert_int_eq(0, deltas_foreach(group->serial, group->deltas, - handle_delta, common)); + handle_delta, &fd)); return 0; } int -send_end_of_data_pdu(struct sender_common *common, serial_t end_serial) +send_end_of_data_pdu(int fd, serial_t end_serial) { pr_info(" Server sent End of Data."); ck_assert_int_eq(pop_expected_pdu(), PDU_TYPE_END_OF_DATA); @@ -162,8 +159,8 @@ send_end_of_data_pdu(struct sender_common *common, serial_t end_serial) } int -send_error_report_pdu(int fd, uint8_t version, uint16_t code, -struct pdu_header *err_pdu_header, char const *message) +send_error_report_pdu(int fd, uint16_t code, struct rtr_request const *request, + char *message) { pr_info(" Server sent Error Report %u: '%s'", code, message); ck_assert_int_eq(pop_expected_pdu(), PDU_TYPE_ERROR_REPORT); @@ -175,6 +172,7 @@ struct pdu_header *err_pdu_header, char const *message) /* https://tools.ietf.org/html/rfc6810#section-6.1 */ START_TEST(test_start_or_restart) { + struct rtr_request request; struct reset_query_pdu client_pdu; pr_info("-- Start or Restart --"); @@ -183,7 +181,7 @@ START_TEST(test_start_or_restart) init_db_full(); /* Init client request */ - init_reset_query(&client_pdu); + init_reset_query(&request, &client_pdu); /* Define expected server response */ expected_pdu_add(PDU_TYPE_CACHE_RESPONSE); @@ -192,7 +190,7 @@ START_TEST(test_start_or_restart) expected_pdu_add(PDU_TYPE_END_OF_DATA); /* Run and validate */ - ck_assert_int_eq(0, handle_reset_query_pdu(0, &client_pdu)); + ck_assert_int_eq(0, handle_reset_query_pdu(0, &request)); ck_assert_uint_eq(false, has_expected_pdus()); /* Clean up */ @@ -203,6 +201,7 @@ END_TEST /* https://tools.ietf.org/html/rfc6810#section-6.2 */ START_TEST(test_typical_exchange) { + struct rtr_request request; struct serial_query_pdu client_pdu; pr_info("-- Typical Exchange --"); @@ -211,7 +210,7 @@ START_TEST(test_typical_exchange) init_db_full(); /* From serial 0: Init client request */ - init_serial_query(&client_pdu, 0); + init_serial_query(&request, &client_pdu, 0); /* From serial 0: Define expected server response */ expected_pdu_add(PDU_TYPE_CACHE_RESPONSE); @@ -222,11 +221,11 @@ START_TEST(test_typical_exchange) expected_pdu_add(PDU_TYPE_END_OF_DATA); /* From serial 0: Run and validate */ - ck_assert_int_eq(0, handle_serial_query_pdu(0, &client_pdu)); + ck_assert_int_eq(0, handle_serial_query_pdu(0, &request)); ck_assert_uint_eq(false, has_expected_pdus()); /* From serial 1: Init client request */ - init_serial_query(&client_pdu, 1); + init_serial_query(&request, &client_pdu, 1); /* From serial 1: Define expected server response */ expected_pdu_add(PDU_TYPE_CACHE_RESPONSE); @@ -235,18 +234,18 @@ START_TEST(test_typical_exchange) expected_pdu_add(PDU_TYPE_END_OF_DATA); /* From serial 1: Run and validate */ - ck_assert_int_eq(0, handle_serial_query_pdu(0, &client_pdu)); + ck_assert_int_eq(0, handle_serial_query_pdu(0, &request)); ck_assert_uint_eq(false, has_expected_pdus()); /* From serial 2: Init client request */ - init_serial_query(&client_pdu, 2); + init_serial_query(&request, &client_pdu, 2); /* From serial 2: Define expected server response */ expected_pdu_add(PDU_TYPE_CACHE_RESPONSE); expected_pdu_add(PDU_TYPE_END_OF_DATA); /* From serial 2: Run and validate */ - ck_assert_int_eq(0, handle_serial_query_pdu(0, &client_pdu)); + ck_assert_int_eq(0, handle_serial_query_pdu(0, &request)); ck_assert_uint_eq(false, has_expected_pdus()); /* Clean up */ @@ -257,6 +256,7 @@ END_TEST /* https://tools.ietf.org/html/rfc6810#section-6.3 */ START_TEST(test_no_incremental_update_available) { + struct rtr_request request; struct serial_query_pdu serial_query; pr_info("-- No Incremental Update Available --"); @@ -265,13 +265,13 @@ START_TEST(test_no_incremental_update_available) init_db_full(); /* Init client request */ - init_serial_query(&serial_query, 10000); + init_serial_query(&request, &serial_query, 10000); /* Define expected server response */ expected_pdu_add(PDU_TYPE_CACHE_RESET); /* Run and validate */ - ck_assert_int_eq(0, handle_serial_query_pdu(0, &serial_query)); + ck_assert_int_eq(0, handle_serial_query_pdu(0, &request)); ck_assert_uint_eq(false, has_expected_pdus()); /* The Reset Query is already tested in start_or_restart. */ @@ -284,6 +284,7 @@ END_TEST /* https://tools.ietf.org/html/rfc6810#section-6.4 */ START_TEST(test_cache_has_no_data_available) { + struct rtr_request request; struct serial_query_pdu serial_query; struct reset_query_pdu reset_query; @@ -293,23 +294,23 @@ START_TEST(test_cache_has_no_data_available) ck_assert_int_eq(0, vrps_init()); /* Serial Query: Init client request */ - init_serial_query(&serial_query, 0); + init_serial_query(&request, &serial_query, 0); /* Serial Query: Define expected server response */ expected_pdu_add(PDU_TYPE_ERROR_REPORT); /* Serial Query: Run and validate */ - ck_assert_int_eq(0, handle_serial_query_pdu(0, &serial_query)); + ck_assert_int_eq(0, handle_serial_query_pdu(0, &request)); ck_assert_uint_eq(false, has_expected_pdus()); /* Reset Query: Init client request */ - init_reset_query(&reset_query); + init_reset_query(&request, &reset_query); /* Reset Query: Define expected server response */ expected_pdu_add(PDU_TYPE_ERROR_REPORT); /* Reset Query: Run and validate */ - ck_assert_int_eq(0, handle_reset_query_pdu(0, &reset_query)); + ck_assert_int_eq(0, handle_reset_query_pdu(0, &request)); ck_assert_uint_eq(false, has_expected_pdus()); /* Clean up */ diff --git a/test/rtr/pdu_test.c b/test/rtr/pdu_test.c index 1e4e11b8..9829561e 100644 --- a/test/rtr/pdu_test.c +++ b/test/rtr/pdu_test.c @@ -48,7 +48,7 @@ START_TEST(test_pdu_header_from_stream) fd = buffer2fd(input, sizeof(input)); ck_assert_int_ge(fd, 0); - err = pdu_header_from_stream(fd, &header); + err = pdu_header_from_reader(fd, &header); close(fd); ck_assert_int_eq(err, 0);