diff --git a/other/bootstrap_daemon/docker/tox-bootstrapd.sha256 b/other/bootstrap_daemon/docker/tox-bootstrapd.sha256 index ce279f62465..4439dee07af 100644 --- a/other/bootstrap_daemon/docker/tox-bootstrapd.sha256 +++ b/other/bootstrap_daemon/docker/tox-bootstrapd.sha256 @@ -1 +1 @@ -96618672392dc44a2f0a480f6415380e3fc588ace646345d31a297e80f7c271f /usr/local/bin/tox-bootstrapd +97015923b4bee5f557e2c8b60c766392c9610c661acf82e659044ffa536d3c9c /usr/local/bin/tox-bootstrapd diff --git a/toxcore/BUILD.bazel b/toxcore/BUILD.bazel index c852eb0580d..5daf4cc7340 100644 --- a/toxcore/BUILD.bazel +++ b/toxcore/BUILD.bazel @@ -285,6 +285,7 @@ cc_library( ], deps = [ ":LAN_discovery", + ":bin_pack", ":ccompat", ":crypto_core", ":logger", diff --git a/toxcore/DHT.c b/toxcore/DHT.c index 145b2e97192..ddd61b72323 100644 --- a/toxcore/DHT.c +++ b/toxcore/DHT.c @@ -9,10 +9,12 @@ #include "DHT.h" #include +#include #include #include #include "LAN_discovery.h" +#include "bin_pack.h" #include "ccompat.h" #include "logger.h" #include "mono_time.h" @@ -452,19 +454,32 @@ int packed_node_size(Family ip_family) } -/** @brief Pack an IP_Port structure into data of max size length. +/** @brief Packs an IP structure. * - * Packed_length is the offset of data currently packed. + * It's the caller's responsibility to make sure `is_ipv4` tells the truth. This + * function is an implementation detail of @ref bin_pack_ip_port. * - * @return size of packed IP_Port data on success. - * @retval -1 on failure. + * @param is_ipv4 whether this IP is an IP4 or IP6. + * + * @retval true on success. */ -int pack_ip_port(const Logger *logger, uint8_t *data, uint16_t length, const IP_Port *ip_port) +non_null() +static bool bin_pack_ip(Bin_Pack *bp, const IP *ip, bool is_ipv4) { - if (data == nullptr) { - return -1; + if (is_ipv4) { + return bin_pack_bin_b(bp, ip->ip.v4.uint8, SIZE_IP4); + } else { + return bin_pack_bin_b(bp, ip->ip.v6.uint8, SIZE_IP6); } +} +/** @brief Packs an IP_Port structure. + * + * @retval true on success. + */ +non_null() +static bool bin_pack_ip_port(Bin_Pack *bp, const IP_Port *ip_port) +{ bool is_ipv4; uint8_t family; @@ -482,36 +497,41 @@ int pack_ip_port(const Logger *logger, uint8_t *data, uint16_t length, const IP_ is_ipv4 = false; family = TOX_TCP_INET6; } else { - char ip_str[IP_NTOA_LEN]; - // TODO(iphydf): Find out why we're trying to pack invalid IPs, stop - // doing that, and turn this into an error. - LOGGER_TRACE(logger, "cannot pack invalid IP: %s", ip_ntoa(&ip_port->ip, ip_str, sizeof(ip_str))); - return -1; + return false; } - if (is_ipv4) { - const uint32_t size = 1 + SIZE_IP4 + sizeof(uint16_t); + return bin_pack_u08_b(bp, family) + && bin_pack_ip(bp, &ip_port->ip, is_ipv4) + && bin_pack_u16_b(bp, net_ntohs(ip_port->port)); +} - if (size > length) { - return -1; - } +non_null() +static bool bin_pack_ip_port_handler(Bin_Pack *bp, const void *obj) +{ + return bin_pack_ip_port(bp, (const IP_Port *)obj); +} - data[0] = family; - memcpy(data + 1, &ip_port->ip.ip.v4, SIZE_IP4); - memcpy(data + 1 + SIZE_IP4, &ip_port->port, sizeof(uint16_t)); - return size; - } else { - const uint32_t size = 1 + SIZE_IP6 + sizeof(uint16_t); +/** @brief Pack an IP_Port structure into data of max size length. + * + * Packed_length is the offset of data currently packed. + * + * @return size of packed IP_Port data on success. + * @retval -1 on failure. + */ +int pack_ip_port(uint8_t *data, uint16_t length, const IP_Port *ip_port) +{ + const uint32_t size = bin_pack_obj_size(bin_pack_ip_port_handler, ip_port); - if (size > length) { - return -1; - } + if (size > length) { + return -1; + } - data[0] = family; - memcpy(data + 1, &ip_port->ip.ip.v6, SIZE_IP6); - memcpy(data + 1 + SIZE_IP6, &ip_port->port, sizeof(uint16_t)); - return size; + if (!bin_pack_obj(bin_pack_ip_port_handler, ip_port, data, length)) { + return -1; } + + assert(size < INT_MAX); + return (int)size; } /** @brief Encrypt plain and write resulting DHT packet into packet with max size length. @@ -560,6 +580,7 @@ int dht_create_packet(const Random *rng, const uint8_t public_key[CRYPTO_PUBLIC_ * @return size of unpacked ip_port on success. * @retval -1 on failure. */ +non_null() int unpack_ip_port(IP_Port *ip_port, const uint8_t *data, uint16_t length, bool tcp_enabled) { if (data == nullptr) { @@ -620,38 +641,30 @@ int unpack_ip_port(IP_Port *ip_port, const uint8_t *data, uint16_t length, bool } } +/** @brief Pack a single node from a node array. + * + * @retval true on success. + */ +non_null() +static bool bin_pack_node_handler(Bin_Pack *bp, const void *arr, uint32_t index) +{ + const Node_format *nodes = (const Node_format *)arr; + return bin_pack_ip_port(bp, &nodes[index].ip_port) + && bin_pack_bin_b(bp, nodes[index].public_key, CRYPTO_PUBLIC_KEY_SIZE); +} + /** @brief Pack number of nodes into data of maxlength length. * * @return length of packed nodes on success. * @retval -1 on failure. */ -int pack_nodes(const Logger *logger, uint8_t *data, uint16_t length, const Node_format *nodes, uint16_t number) +int pack_nodes(uint8_t *data, uint16_t length, const Node_format *nodes, uint16_t number) { - uint32_t packed_length = 0; - - for (uint32_t i = 0; i < number && packed_length < length; ++i) { - const int ipp_size = pack_ip_port(logger, data + packed_length, length - packed_length, &nodes[i].ip_port); - - if (ipp_size == -1) { - return -1; - } - - packed_length += ipp_size; - - if (packed_length + CRYPTO_PUBLIC_KEY_SIZE > length) { - return -1; - } - - memcpy(data + packed_length, nodes[i].public_key, CRYPTO_PUBLIC_KEY_SIZE); - packed_length += CRYPTO_PUBLIC_KEY_SIZE; - -#ifndef NDEBUG - const uint32_t increment = ipp_size + CRYPTO_PUBLIC_KEY_SIZE; -#endif - assert(increment == PACKED_NODE_SIZE_IP4 || increment == PACKED_NODE_SIZE_IP6); + const uint32_t size = bin_pack_obj_array_size(bin_pack_node_handler, nodes, number); + if (!bin_pack_obj_array(bin_pack_node_handler, nodes, number, data, length)) { + return -1; } - - return packed_length; + return size; } /** @brief Unpack data of length into nodes of size max_num_nodes. @@ -1494,7 +1507,7 @@ bool dht_getnodes(DHT *dht, const IP_Port *ip_port, const uint8_t *public_key, c memcpy(receiver.public_key, public_key, CRYPTO_PUBLIC_KEY_SIZE); receiver.ip_port = *ip_port; - if (pack_nodes(dht->log, plain_message, sizeof(plain_message), &receiver, 1) == -1) { + if (pack_nodes(plain_message, sizeof(plain_message), &receiver, 1) == -1) { return false; } @@ -1555,7 +1568,7 @@ static int sendnodes_ipv6(const DHT *dht, const IP_Port *ip_port, const uint8_t int nodes_length = 0; if (num_nodes > 0) { - nodes_length = pack_nodes(dht->log, plain + 1, node_format_size * MAX_SENT_NODES, nodes_list, num_nodes); + nodes_length = pack_nodes(plain + 1, node_format_size * MAX_SENT_NODES, nodes_list, num_nodes); if (nodes_length <= 0) { return -1; @@ -2906,8 +2919,9 @@ void dht_save(const DHT *dht, uint8_t *data) } } - state_write_section_header(old_data, DHT_STATE_COOKIE_TYPE, pack_nodes(dht->log, data, sizeof(Node_format) * num, - clients, num), DHT_STATE_TYPE_NODES); + state_write_section_header( + old_data, DHT_STATE_COOKIE_TYPE, pack_nodes(data, sizeof(Node_format) * num, clients, num), + DHT_STATE_TYPE_NODES); free(clients); } diff --git a/toxcore/DHT.h b/toxcore/DHT.h index 0109ed1b2ec..ae4e24efc2b 100644 --- a/toxcore/DHT.h +++ b/toxcore/DHT.h @@ -199,7 +199,17 @@ int packed_node_size(Family ip_family); * @retval -1 on failure. */ non_null() -int pack_ip_port(const Logger *logger, uint8_t *data, uint16_t length, const IP_Port *ip_port); +int pack_ip_port(uint8_t *data, uint16_t length, const IP_Port *ip_port); + +/** @brief Unpack IP_Port structure from data of max size length into ip_port. + * + * len_processed is the offset of data currently unpacked. + * + * @return size of unpacked ip_port on success. + * @retval -1 on failure. + */ +non_null() +int unpack_ip_port(IP_Port *ip_port, const uint8_t *data, uint16_t length, bool tcp_enabled); /** @brief Encrypt plain and write resulting DHT packet into packet with max size length. * @@ -213,23 +223,13 @@ int dht_create_packet(const Random *rng, const uint8_t *plain, size_t plain_length, uint8_t *packet, size_t length); -/** @brief Unpack IP_Port structure from data of max size length into ip_port. - * - * len_processed is the offset of data currently unpacked. - * - * @return size of unpacked ip_port on success. - * @retval -1 on failure. - */ -non_null() -int unpack_ip_port(IP_Port *ip_port, const uint8_t *data, uint16_t length, bool tcp_enabled); - /** @brief Pack number of nodes into data of maxlength length. * * @return length of packed nodes on success. * @retval -1 on failure. */ non_null() -int pack_nodes(const Logger *logger, uint8_t *data, uint16_t length, const Node_format *nodes, uint16_t number); +int pack_nodes(uint8_t *data, uint16_t length, const Node_format *nodes, uint16_t number); /** @brief Unpack data of length into nodes of size max_num_nodes. * Put the length of the data processed in processed_data_len. diff --git a/toxcore/DHT_fuzz_test.cc b/toxcore/DHT_fuzz_test.cc index e9673ae0ee5..28bebc3b989 100644 --- a/toxcore/DHT_fuzz_test.cc +++ b/toxcore/DHT_fuzz_test.cc @@ -1,6 +1,8 @@ #include "DHT.h" +#include #include +#include #include #include "../testing/fuzzing/fuzz_support.h" @@ -31,11 +33,20 @@ void TestUnpackNodes(Fuzz_Data &input) if (packed_count > 0) { Logger *logger = logger_new(); std::vector packed(packed_count * PACKED_NODE_SIZE_IP6); - const int packed_size - = pack_nodes(logger, packed.data(), packed.size(), nodes, packed_count); + const int packed_size = pack_nodes(packed.data(), packed.size(), nodes, packed_count); LOGGER_ASSERT(logger, packed_size == processed_data_len, "packed size (%d) != unpacked size (%d)", packed_size, processed_data_len); logger_kill(logger); + + // Check that packed nodes can be unpacked again and result in the + // original unpacked nodes. + Node_format nodes2[node_count]; + uint16_t processed_data_len2; + const int packed_count2 = unpack_nodes( + nodes2, node_count, &processed_data_len2, packed.data(), packed.size(), tcp_enabled); + assert(processed_data_len2 == processed_data_len); + assert(packed_count2 == packed_count); + assert(memcmp(nodes, nodes2, sizeof(Node_format) * packed_count) == 0); } } diff --git a/toxcore/Messenger.c b/toxcore/Messenger.c index 28c8b0da2f8..f6f94d37ccc 100644 --- a/toxcore/Messenger.c +++ b/toxcore/Messenger.c @@ -2981,7 +2981,7 @@ static uint8_t *save_tcp_relays(const Messenger *m, uint8_t *data) uint32_t num = m->num_loaded_relays; num += copy_connected_tcp_relays(m->net_crypto, relays + num, NUM_SAVED_TCP_RELAYS - num); - const int l = pack_nodes(m->log, data, NUM_SAVED_TCP_RELAYS * packed_node_size(net_family_tcp_ipv6), relays, num); + const int l = pack_nodes(data, NUM_SAVED_TCP_RELAYS * packed_node_size(net_family_tcp_ipv6), relays, num); if (l > 0) { const uint32_t len = l; @@ -3025,7 +3025,7 @@ static uint8_t *save_path_nodes(const Messenger *m, uint8_t *data) data = state_write_section_header(data, STATE_COOKIE_TYPE, 0, STATE_TYPE_PATH_NODE); memset(nodes, 0, sizeof(nodes)); const unsigned int num = onion_backup_nodes(m->onion_c, nodes, NUM_SAVED_PATH_NODES); - const int l = pack_nodes(m->log, data, NUM_SAVED_PATH_NODES * packed_node_size(net_family_tcp_ipv6), nodes, num); + const int l = pack_nodes(data, NUM_SAVED_PATH_NODES * packed_node_size(net_family_tcp_ipv6), nodes, num); if (l > 0) { const uint32_t len = l; diff --git a/toxcore/TCP_client.c b/toxcore/TCP_client.c index 53866c854e3..729f4fdfe38 100644 --- a/toxcore/TCP_client.c +++ b/toxcore/TCP_client.c @@ -542,7 +542,7 @@ int send_forward_request_tcp(const Logger *logger, TCP_Client_Connection *con, c VLA(uint8_t, packet, 1 + MAX_PACKED_IPPORT_SIZE + length); packet[0] = TCP_PACKET_FORWARD_REQUEST; - const int ipport_length = pack_ip_port(logger, packet + 1, MAX_PACKED_IPPORT_SIZE, dest); + const int ipport_length = pack_ip_port(packet + 1, MAX_PACKED_IPPORT_SIZE, dest); if (ipport_length == -1) { return 0; diff --git a/toxcore/announce.c b/toxcore/announce.c index e7be00c7022..1f19b836b98 100644 --- a/toxcore/announce.c +++ b/toxcore/announce.c @@ -275,7 +275,7 @@ static int create_data_search_to_auth(const Logger *logger, const uint8_t *data_ memcpy(dest, data_public_key, CRYPTO_PUBLIC_KEY_SIZE); memcpy(dest + CRYPTO_PUBLIC_KEY_SIZE, requester_key, CRYPTO_PUBLIC_KEY_SIZE); - const int ipport_length = pack_ip_port(logger, dest + CRYPTO_PUBLIC_KEY_SIZE * 2, MAX_PACKED_IPPORT_SIZE, source); + const int ipport_length = pack_ip_port(dest + CRYPTO_PUBLIC_KEY_SIZE * 2, MAX_PACKED_IPPORT_SIZE, source); if (ipport_length == -1) { return -1; @@ -353,7 +353,7 @@ static int create_reply_plain_data_search_request(Announcements *announce, *p = num_nodes; ++p; - p += pack_nodes(announce->log, p, nodes_max_length, nodes_list, num_nodes); + p += pack_nodes(p, nodes_max_length, nodes_list, num_nodes); const uint32_t reply_len = p - reply; diff --git a/toxcore/bin_pack.c b/toxcore/bin_pack.c index ca8940ee448..c59222e405a 100644 --- a/toxcore/bin_pack.c +++ b/toxcore/bin_pack.c @@ -77,6 +77,28 @@ uint32_t bin_pack_obj_size(bin_pack_cb *callback, const void *obj) return bp.bytes_pos; } +uint32_t bin_pack_obj_array_size(bin_pack_array_cb *callback, const void *arr, uint32_t count) +{ + Bin_Pack bp; + bin_pack_init(&bp, nullptr, 0); + for (uint32_t i = 0; i < count; ++i) { + callback(&bp, arr, i); + } + return bp.bytes_pos; +} + +bool bin_pack_obj_array(bin_pack_array_cb *callback, const void *arr, uint32_t count, uint8_t *buf, uint32_t buf_size) +{ + Bin_Pack bp; + bin_pack_init(&bp, buf, buf_size); + for (uint32_t i = 0; i < count; ++i) { + if (!callback(&bp, arr, i)) { + return false; + } + } + return true; +} + Bin_Pack *bin_pack_new(uint8_t *buf, uint32_t buf_size) { Bin_Pack *bp = (Bin_Pack *)calloc(1, sizeof(Bin_Pack)); diff --git a/toxcore/bin_pack.h b/toxcore/bin_pack.h index 542f533b883..a17b60ae5ef 100644 --- a/toxcore/bin_pack.h +++ b/toxcore/bin_pack.h @@ -25,13 +25,23 @@ typedef struct Bin_Pack Bin_Pack; */ typedef bool bin_pack_cb(Bin_Pack *bp, const void *obj); +/** @brief Function used to pack an array of objects. + * + * This function would typically cast the `void *` to the actual object pointer type and then call + * more appropriately typed packing functions. + * + * @param arr is the object array as void pointer. + * @param index is the index in the object array that is currently being packed. + */ +typedef bool bin_pack_array_cb(Bin_Pack *bp, const void *arr, uint32_t index); + /** @brief Determine the serialised size of an object. * * @param callback The function called on the created packer and packed object. * @param obj The object to be packed, passed as `obj` to the callback. * - * @return The packed size of the passed object according to the callback. UINT32_MAX in case of - * errors such as buffer overflow. + * @return The packed size of the passed object according to the callback. + * @retval UINT32_MAX in case of errors such as buffer overflow. */ non_null(1) nullable(2) uint32_t bin_pack_obj_size(bin_pack_cb *callback, const void *obj); @@ -54,6 +64,36 @@ uint32_t bin_pack_obj_size(bin_pack_cb *callback, const void *obj); non_null(1, 3) nullable(2) bool bin_pack_obj(bin_pack_cb *callback, const void *obj, uint8_t *buf, uint32_t buf_size); +/** @brief Determine the serialised size of an object array. + * + * @param callback The function called on the created packer and packed object + * array. + * @param arr The object array to be packed, passed as `arr` to the callback. + * @param count The number of elements in the object array. + * + * @return The packed size of the passed object array according to the callback. + * @retval UINT32_MAX in case of errors such as buffer overflow. + */ +non_null() +uint32_t bin_pack_obj_array_size(bin_pack_array_cb *callback, const void *arr, uint32_t count); + +/** @brief Pack an object array into a buffer of a given size. + * + * Similar to `bin_pack_obj` but for arrays. Does not write the array length, so + * if you need that, write it manually using `bin_pack_array`. + * + * @param callback The function called on the created packer and packed object + * array. + * @param arr The object array to be packed, passed as `arr` to the callback. + * @param count The number of elements in the object array. + * @param buf A byte array large enough to hold the serialised representation of `arr`. + * @param buf_size The size of the byte array. Can be `UINT32_MAX` to disable bounds checking. + * + * @retval false if an error occurred (e.g. buffer overflow). + */ +non_null() +bool bin_pack_obj_array(bin_pack_array_cb *callback, const void *arr, uint32_t count, uint8_t *buf, uint32_t buf_size); + /** @brief Allocate a new packer object. * * This is the only function that allocates memory in this module. diff --git a/toxcore/forwarding.c b/toxcore/forwarding.c index 5e885abd211..5f15da715dd 100644 --- a/toxcore/forwarding.c +++ b/toxcore/forwarding.c @@ -173,7 +173,7 @@ static int handle_forward_request(void *object, const IP_Port *source, const uin uint8_t sendback_data[1 + MAX_PACKED_IPPORT_SIZE]; sendback_data[0] = SENDBACK_IPPORT; - const int ipport_length = pack_ip_port(forwarding->log, sendback_data + 1, MAX_PACKED_IPPORT_SIZE, source); + const int ipport_length = pack_ip_port(sendback_data + 1, MAX_PACKED_IPPORT_SIZE, source); if (ipport_length == -1) { return 1; @@ -284,7 +284,7 @@ static int handle_forwarding(void *object, const IP_Port *source, const uint8_t VLA(uint8_t, sendback_data, 1 + MAX_PACKED_IPPORT_SIZE + sendback_len); sendback_data[0] = SENDBACK_FORWARD; - const int ipport_length = pack_ip_port(forwarding->log, sendback_data + 1, MAX_PACKED_IPPORT_SIZE, source); + const int ipport_length = pack_ip_port(sendback_data + 1, MAX_PACKED_IPPORT_SIZE, source); if (ipport_length == -1) { return 1; diff --git a/toxcore/friend_connection.c b/toxcore/friend_connection.c index 66e4026340c..1f11af54d51 100644 --- a/toxcore/friend_connection.c +++ b/toxcore/friend_connection.c @@ -303,7 +303,7 @@ static unsigned int send_relays(Friend_Connections *fr_c, int friendcon_id) friend_add_tcp_relay(fr_c, friendcon_id, &nodes[i].ip_port, nodes[i].public_key); } - int length = pack_nodes(fr_c->logger, data + 1, sizeof(data) - 1, nodes, n); + int length = pack_nodes(data + 1, sizeof(data) - 1, nodes, n); if (length <= 0) { return 0; diff --git a/toxcore/onion_announce.c b/toxcore/onion_announce.c index ac983c05e46..edfb01d5d29 100644 --- a/toxcore/onion_announce.c +++ b/toxcore/onion_announce.c @@ -492,7 +492,7 @@ static int handle_announce_request_common( int nodes_length = 0; if (num_nodes != 0) { - nodes_length = pack_nodes(onion_a->log, response + nodes_offset, sizeof(nodes_list), nodes_list, + nodes_length = pack_nodes(response + nodes_offset, sizeof(nodes_list), nodes_list, (uint16_t)num_nodes); if (nodes_length <= 0) { diff --git a/toxcore/onion_client.c b/toxcore/onion_client.c index 9701f325cf6..073d9873b25 100644 --- a/toxcore/onion_client.c +++ b/toxcore/onion_client.c @@ -1243,7 +1243,7 @@ static int send_dhtpk_announce(Onion_Client *onion_c, uint16_t friend_num, uint8 int nodes_len = 0; if (num_nodes != 0) { - nodes_len = pack_nodes(onion_c->logger, data + DHTPK_DATA_MIN_LENGTH, DHTPK_DATA_MAX_LENGTH - DHTPK_DATA_MIN_LENGTH, nodes, num_nodes); + nodes_len = pack_nodes(data + DHTPK_DATA_MIN_LENGTH, DHTPK_DATA_MAX_LENGTH - DHTPK_DATA_MIN_LENGTH, nodes, num_nodes); if (nodes_len <= 0) { return -1;