diff --git a/other/bootstrap_daemon/docker/tox-bootstrapd.sha256 b/other/bootstrap_daemon/docker/tox-bootstrapd.sha256 index 4987e1fa4..a7371f21b 100644 --- a/other/bootstrap_daemon/docker/tox-bootstrapd.sha256 +++ b/other/bootstrap_daemon/docker/tox-bootstrapd.sha256 @@ -1 +1 @@ -ce191b497af89aafb2837d053043469c725ea7af0b1aa2bbedc39f8de2686d56 /usr/local/bin/tox-bootstrapd +ac88db3ee0c9ca621846897bb508c469eba97c263637966d74483e266b5564b0 /usr/local/bin/tox-bootstrapd diff --git a/toxcore/DHT.c b/toxcore/DHT.c index ad66e6953..d776105a7 100644 --- a/toxcore/DHT.c +++ b/toxcore/DHT.c @@ -313,33 +313,44 @@ void dht_get_shared_key_sent(DHT *dht, uint8_t *shared_key, const uint8_t *publi #define CRYPTO_SIZE (1 + CRYPTO_PUBLIC_KEY_SIZE * 2 + CRYPTO_NONCE_SIZE) -/** Create a request to peer. - * send_public_key and send_secret_key are the pub/secret keys of the sender. - * recv_public_key is public key of receiver. - * packet must be an array of MAX_CRYPTO_REQUEST_SIZE big. - * Data represents the data we send with the request with length being the length of the data. - * request_id is the id of the request (32 = friend request, 254 = ping request). +/** + * @brief Create a request to peer. * - * return -1 on failure. - * return the length of the created packet on success. + * Packs the data and sender public key and encrypts the packet. + * + * @param[in] send_public_key public key of the sender. + * @param[in] send_secret_key secret key of the sender. + * @param[out] packet an array of @ref MAX_CRYPTO_REQUEST_SIZE big. + * @param[in] recv_public_key public key of the receiver. + * @param[in] data represents the data we send with the request. + * @param[in] data_length the length of the data. + * @param[in] request_id the id of the request (32 = friend request, 254 = ping request). + * + * @attention Constraints: + * @code + * sizeof(packet) >= MAX_CRYPTO_REQUEST_SIZE + * @endcode + * + * @retval -1 on failure. + * @return the length of the created packet on success. */ int create_request(const uint8_t *send_public_key, const uint8_t *send_secret_key, uint8_t *packet, - const uint8_t *recv_public_key, const uint8_t *data, uint32_t length, uint8_t request_id) + const uint8_t *recv_public_key, const uint8_t *data, uint32_t data_length, uint8_t request_id) { if (send_public_key == nullptr || packet == nullptr || recv_public_key == nullptr || data == nullptr) { return -1; } - if (MAX_CRYPTO_REQUEST_SIZE < length + CRYPTO_SIZE + 1 + CRYPTO_MAC_SIZE) { + if (MAX_CRYPTO_REQUEST_SIZE < data_length + CRYPTO_SIZE + 1 + CRYPTO_MAC_SIZE) { return -1; } uint8_t *const nonce = packet + 1 + CRYPTO_PUBLIC_KEY_SIZE * 2; random_nonce(nonce); uint8_t temp[MAX_CRYPTO_REQUEST_SIZE]; - memcpy(temp + 1, data, length); + memcpy(temp + 1, data, data_length); temp[0] = request_id; - const int len = encrypt_data(recv_public_key, send_secret_key, nonce, temp, length + 1, + const int len = encrypt_data(recv_public_key, send_secret_key, nonce, temp, data_length + 1, packet + CRYPTO_SIZE); if (len == -1) { @@ -355,21 +366,37 @@ int create_request(const uint8_t *send_public_key, const uint8_t *send_secret_ke return len + CRYPTO_SIZE; } -/** Puts the senders public key in the request in public_key, the data from the request - * in data if a friend or ping request was sent to us and returns the length of the data. - * packet is the request packet and length is its length. +/** + * @brief Decrypts and unpacks a DHT request packet. * - * return -1 if not valid request. + * Puts the senders public key in the request in @p public_key, the data from + * the request in @p data. + * + * @param[in] self_public_key public key of the receiver (us). + * @param[in] self_secret_key secret key of the receiver (us). + * @param[out] public_key public key of the sender, copied from the input packet. + * @param[out] data decrypted request data, copied from the input packet, must + * have room for @ref MAX_CRYPTO_REQUEST_SIZE bytes. + * @param[in] packet is the request packet. + * @param[in] packet_length length of the packet. + * + * @attention Constraints: + * @code + * sizeof(data) >= MAX_CRYPTO_REQUEST_SIZE + * @endcode + * + * @retval -1 if not valid request. + * @return the length of the unpacked data. */ int handle_request(const uint8_t *self_public_key, const uint8_t *self_secret_key, uint8_t *public_key, uint8_t *data, - uint8_t *request_id, const uint8_t *packet, uint16_t length) + uint8_t *request_id, const uint8_t *packet, uint16_t packet_length) { if (self_public_key == nullptr || public_key == nullptr || data == nullptr || request_id == nullptr || packet == nullptr) { return -1; } - if (length <= CRYPTO_SIZE + CRYPTO_MAC_SIZE || length > MAX_CRYPTO_REQUEST_SIZE) { + if (packet_length <= CRYPTO_SIZE + CRYPTO_MAC_SIZE || packet_length > MAX_CRYPTO_REQUEST_SIZE) { return -1; } @@ -381,13 +408,14 @@ int handle_request(const uint8_t *self_public_key, const uint8_t *self_secret_ke const uint8_t *const nonce = packet + 1 + CRYPTO_PUBLIC_KEY_SIZE * 2; uint8_t temp[MAX_CRYPTO_REQUEST_SIZE]; int len1 = decrypt_data(public_key, self_secret_key, nonce, - packet + CRYPTO_SIZE, length - CRYPTO_SIZE, temp); + packet + CRYPTO_SIZE, packet_length - CRYPTO_SIZE, temp); if (len1 == -1 || len1 == 0) { crypto_memzero(temp, MAX_CRYPTO_REQUEST_SIZE); return -1; } + assert(len1 > 0); request_id[0] = temp[0]; --len1; memcpy(data, temp + 1, len1); diff --git a/toxcore/DHT.h b/toxcore/DHT.h index 0a51209ee..3662c1f57 100644 --- a/toxcore/DHT.h +++ b/toxcore/DHT.h @@ -3,8 +3,8 @@ * Copyright © 2013 Tox project. */ -/** - * An implementation of the DHT as seen in docs/updates/DHT.md +/** @file + * @brief An implementation of the DHT as seen in docs/updates/DHT.md */ #ifndef C_TOXCORE_TOXCORE_DHT_H #define C_TOXCORE_TOXCORE_DHT_H @@ -53,37 +53,65 @@ extern "C" { /** The number of "fake" friends to add (for optimization purposes and so our paths for the onion part are more random) */ #define DHT_FAKE_FRIEND_NUMBER 2 +/** Maximum packet size for a DHT request packet. */ #define MAX_CRYPTO_REQUEST_SIZE 1024 #define CRYPTO_PACKET_FRIEND_REQ 32 // Friend request crypto packet ID. #define CRYPTO_PACKET_DHTPK 156 #define CRYPTO_PACKET_NAT_PING 254 // NAT ping crypto packet ID. -/** Create a request to peer. - * send_public_key and send_secret_key are the pub/secret keys of the sender. - * recv_public_key is public key of receiver. - * packet must be an array of MAX_CRYPTO_REQUEST_SIZE big. - * Data represents the data we send with the request with length being the length of the data. - * request_id is the id of the request (32 = friend request, 254 = ping request). +/** + * @brief Create a request to peer. * - * return -1 on failure. - * return the length of the created packet on success. + * Packs the data and sender public key and encrypts the packet. + * + * @param[in] send_public_key public key of the sender. + * @param[in] send_secret_key secret key of the sender. + * @param[out] packet an array of @ref MAX_CRYPTO_REQUEST_SIZE big. + * @param[in] recv_public_key public key of the receiver. + * @param[in] data represents the data we send with the request. + * @param[in] data_length the length of the data. + * @param[in] request_id the id of the request (32 = friend request, 254 = ping request). + * + * @attention Constraints: + * @code + * sizeof(packet) >= MAX_CRYPTO_REQUEST_SIZE + * @endcode + * + * @retval -1 on failure. + * @return the length of the created packet on success. */ non_null() int create_request( const uint8_t *send_public_key, const uint8_t *send_secret_key, uint8_t *packet, - const uint8_t *recv_public_key, const uint8_t *data, uint32_t length, uint8_t request_id); + const uint8_t *recv_public_key, const uint8_t *data, uint32_t data_length, uint8_t request_id); -/** Puts the senders public key in the request in public_key, the data from the request - * in data if a friend or ping request was sent to us and returns the length of the data. - * packet is the request packet and length is its length. +/** + * @brief Decrypts and unpacks a DHT request packet. * - * return -1 if not valid request. + * Puts the senders public key in the request in @p public_key, the data from + * the request in @p data. + * + * @param[in] self_public_key public key of the receiver (us). + * @param[in] self_secret_key secret key of the receiver (us). + * @param[out] public_key public key of the sender, copied from the input packet. + * @param[out] data decrypted request data, copied from the input packet, must + * have room for @ref MAX_CRYPTO_REQUEST_SIZE bytes. + * @param[in] packet is the request packet. + * @param[in] packet_length length of the packet. + * + * @attention Constraints: + * @code + * sizeof(data) >= MAX_CRYPTO_REQUEST_SIZE + * @endcode + * + * @retval -1 if not valid request. + * @return the length of the unpacked data. */ non_null() int handle_request( const uint8_t *self_public_key, const uint8_t *self_secret_key, uint8_t *public_key, uint8_t *data, - uint8_t *request_id, const uint8_t *packet, uint16_t length); + uint8_t *request_id, const uint8_t *packet, uint16_t packet_length); typedef struct IPPTs { IP_Port ip_port; diff --git a/toxcore/DHT_test.cc b/toxcore/DHT_test.cc index 89a093d92..86e38712f 100644 --- a/toxcore/DHT_test.cc +++ b/toxcore/DHT_test.cc @@ -10,23 +10,34 @@ namespace { using PublicKey = std::array; +using SecretKey = std::array; + +struct KeyPair { + PublicKey pk; + SecretKey sk; + + KeyPair() { crypto_new_keypair(pk.data(), sk.data()); } +}; template std::array to_array(T const (&arr)[N]) { std::array stdarr; - memcpy(stdarr.data(), arr, N); + std::copy(arr, arr + N, stdarr.begin()); return stdarr; } +PublicKey random_pk() +{ + PublicKey pk; + random_bytes(pk.data(), pk.size()); + return pk; +} + TEST(IdClosest, IdenticalKeysAreSameDistance) { - PublicKey pk0; - random_bytes(pk0.data(), CRYPTO_PUBLIC_KEY_SIZE); - - PublicKey pk1; - random_bytes(pk1.data(), CRYPTO_PUBLIC_KEY_SIZE); - + PublicKey pk0 = random_pk(); + PublicKey pk1 = random_pk(); PublicKey pk2 = pk1; EXPECT_EQ(id_closest(pk0.data(), pk1.data(), pk2.data()), 0); @@ -35,14 +46,9 @@ TEST(IdClosest, IdenticalKeysAreSameDistance) TEST(IdClosest, DistanceIsCommutative) { for (uint32_t i = 0; i < 100; ++i) { - PublicKey pk0; - random_bytes(pk0.data(), CRYPTO_PUBLIC_KEY_SIZE); - - PublicKey pk1; - random_bytes(pk1.data(), CRYPTO_PUBLIC_KEY_SIZE); - - PublicKey pk2; - random_bytes(pk2.data(), CRYPTO_PUBLIC_KEY_SIZE); + PublicKey pk0 = random_pk(); + PublicKey pk1 = random_pk(); + PublicKey pk2 = random_pk(); ASSERT_NE(pk1, pk2); // RNG can't produce the same random key twice @@ -116,4 +122,58 @@ TEST(AddToList, OverridesKeysWithCloserKeys) EXPECT_EQ(to_array(nodes[3].public_key), keys[2]); } +TEST(Request, CreateAndParse) +{ + // Peers. + const KeyPair sender; + const KeyPair receiver; + const uint8_t sent_pkt_id = CRYPTO_PACKET_FRIEND_REQ; + + // Encoded packet. + std::array packet; + + // Received components. + PublicKey pk; + std::array incoming; + uint8_t recvd_pkt_id; + + // Request data: maximum payload is 918 bytes, so create a payload 1 byte larger than max. + std::vector outgoing(919); + random_bytes(outgoing.data(), outgoing.size()); + + EXPECT_LT(create_request(sender.pk.data(), sender.sk.data(), packet.data(), receiver.pk.data(), + outgoing.data(), outgoing.size(), sent_pkt_id), + 0); + + // Pop one element so the payload is 918 bytes. Packing should now succeed. + outgoing.pop_back(); + + const int max_sent_length = create_request(sender.pk.data(), sender.sk.data(), packet.data(), + receiver.pk.data(), outgoing.data(), outgoing.size(), sent_pkt_id); + ASSERT_GT(max_sent_length, 0); // success. + + // Check that handle_request rejects packets larger than the maximum created packet size. + EXPECT_LT(handle_request(receiver.pk.data(), receiver.sk.data(), pk.data(), incoming.data(), + &recvd_pkt_id, packet.data(), max_sent_length + 1), + 0); + + // Now try all possible packet sizes from max (918) to 0. + while (!outgoing.empty()) { + // Pack: + const int sent_length = create_request(sender.pk.data(), sender.sk.data(), packet.data(), + receiver.pk.data(), outgoing.data(), outgoing.size(), sent_pkt_id); + ASSERT_GT(sent_length, 0); + + // Unpack: + const int recvd_length = handle_request(receiver.pk.data(), receiver.sk.data(), pk.data(), + incoming.data(), &recvd_pkt_id, packet.data(), sent_length); + ASSERT_GE(recvd_length, 0); + + EXPECT_EQ( + std::vector(incoming.begin(), incoming.begin() + recvd_length), outgoing); + + outgoing.pop_back(); + } +} + } // namespace