Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: friend requests with very long messages are no longer dropped #2720

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 56 additions & 18 deletions auto_tests/friend_request_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -10,44 +10,57 @@
#include "../toxcore/tox.h"
#include "../toxcore/util.h"
#include "../testing/misc_tools.h"

#include "auto_test_support.h"
#include "check_compat.h"

#define FR_MESSAGE "Gentoo"
#define REQUEST_MESSAGE "Hello, I would like to be your friend. Please respond."

typedef struct Callback_Data {
Tox *tox1; // request sender
Tox *tox2; // request receiver
uint8_t message[TOX_MAX_FRIEND_REQUEST_LENGTH];
uint16_t length;
} Callback_Data;

static void accept_friend_request(const Tox_Event_Friend_Request *event,
void *userdata)
static void accept_friend_request(const Tox_Event_Friend_Request *event, void *userdata)
{
Tox *state_tox = (Tox *)userdata;
Callback_Data *cb_data = (Callback_Data *)userdata;

const uint8_t *public_key = tox_event_friend_request_get_public_key(event);
const uint8_t *data = tox_event_friend_request_get_message(event);
const size_t length = tox_event_friend_request_get_message_length(event);

ck_assert_msg(length == sizeof(FR_MESSAGE) && memcmp(FR_MESSAGE, data, sizeof(FR_MESSAGE)) == 0,
ck_assert_msg(length == cb_data->length && memcmp(cb_data->message, data, cb_data->length) == 0,
"unexpected friend request message");
tox_friend_add_norequest(state_tox, public_key, nullptr);

fprintf(stderr, "Tox2 accepts friend request.\n");

Tox_Err_Friend_Add err;
tox_friend_add_norequest(cb_data->tox2, public_key, &err);

ck_assert_msg(err == TOX_ERR_FRIEND_ADD_OK, "tox_friend_add_norequest failed: %d", err);
}

static void iterate2_wait(const Tox_Dispatch *dispatch, Tox *tox1, Tox *tox2)
static void iterate2_wait(const Tox_Dispatch *dispatch, Callback_Data *cb_data)
{
Tox_Err_Events_Iterate err;
Tox_Events *events;

events = tox_events_iterate(tox1, true, &err);
events = tox_events_iterate(cb_data->tox1, true, &err);
ck_assert(err == TOX_ERR_EVENTS_ITERATE_OK);
tox_dispatch_invoke(dispatch, events, tox1);
tox_dispatch_invoke(dispatch, events, cb_data);
tox_events_free(events);

events = tox_events_iterate(tox2, true, &err);
events = tox_events_iterate(cb_data->tox2, true, &err);
ck_assert(err == TOX_ERR_EVENTS_ITERATE_OK);
tox_dispatch_invoke(dispatch, events, tox2);
tox_dispatch_invoke(dispatch, events, cb_data);
tox_events_free(events);

c_sleep(ITERATION_INTERVAL);
}

static void test_friend_request(void)
static void test_friend_request(const uint8_t *message, uint16_t length)
{
printf("Initialising 2 toxes.\n");
uint32_t index[] = { 1, 2 };
Expand All @@ -60,7 +73,7 @@ static void test_friend_request(void)
tox_events_init(tox1);
tox_events_init(tox2);

printf("Bootstrapping tox2 off tox1.\n");
printf("Bootstrapping Tox2 off Tox1.\n");
uint8_t dht_key[TOX_PUBLIC_KEY_SIZE];
tox_self_get_dht_id(tox1, dht_key);
const uint16_t dht_port = tox_self_get_udp_port(tox1, nullptr);
Expand All @@ -70,25 +83,36 @@ static void test_friend_request(void)
Tox_Dispatch *dispatch = tox_dispatch_new(nullptr);
ck_assert(dispatch != nullptr);

Callback_Data cb_data = {nullptr};
cb_data.tox1 = tox1;
cb_data.tox2 = tox2;

ck_assert(length <= sizeof(cb_data.message));
memcpy(cb_data.message, message, length);
cb_data.length = length;

do {
iterate2_wait(dispatch, tox1, tox2);
iterate2_wait(dispatch, &cb_data);
} while (tox_self_get_connection_status(tox1) == TOX_CONNECTION_NONE ||
tox_self_get_connection_status(tox2) == TOX_CONNECTION_NONE);

printf("Toxes are online, took %lu seconds.\n", (unsigned long)(time(nullptr) - cur_time));
const time_t con_time = time(nullptr);

printf("Tox1 adds tox2 as friend, tox2 accepts.\n");
printf("Tox1 adds Tox2 as friend. Waiting for Tox2 to accept.\n");
tox_events_callback_friend_request(dispatch, accept_friend_request);

uint8_t address[TOX_ADDRESS_SIZE];
tox_self_get_address(tox2, address);

const uint32_t test = tox_friend_add(tox1, address, (const uint8_t *)FR_MESSAGE, sizeof(FR_MESSAGE), nullptr);
Tox_Err_Friend_Add err;
const uint32_t test = tox_friend_add(tox1, address, message, length, &err);

ck_assert_msg(err == TOX_ERR_FRIEND_ADD_OK, "tox_friend_add failed: %d", err);
ck_assert_msg(test == 0, "failed to add friend error code: %u", test);

do {
iterate2_wait(dispatch, tox1, tox2);
iterate2_wait(dispatch, &cb_data);
} while (tox_friend_get_connection_status(tox1, 0, nullptr) != TOX_CONNECTION_UDP ||
tox_friend_get_connection_status(tox2, 0, nullptr) != TOX_CONNECTION_UDP);

Expand All @@ -104,6 +128,20 @@ int main(void)
{
setvbuf(stdout, nullptr, _IONBF, 0);

test_friend_request();
fprintf(stderr, "Testing friend request with the smallest allowed message length.\n");
test_friend_request((const uint8_t *)"a", 1);

fprintf(stderr, "Testing friend request with an average sized message length.\n");
test_friend_request((const uint8_t *)REQUEST_MESSAGE, sizeof(REQUEST_MESSAGE) - 1);

uint8_t long_message[TOX_MAX_FRIEND_REQUEST_LENGTH];

for (uint16_t i = 0; i < sizeof(long_message); ++i) {
long_message[i] = 'a';
}

fprintf(stderr, "Testing friend request with the largest allowed message length.\n");
test_friend_request(long_message, sizeof(long_message));

return 0;
}
4 changes: 4 additions & 0 deletions toxcore/friend_connection.c
Original file line number Diff line number Diff line change
Expand Up @@ -875,6 +875,10 @@ void set_friend_request_callback(Friend_Connections *fr_c, fr_request_cb *fr_req
int send_friend_request_packet(Friend_Connections *fr_c, int friendcon_id, uint32_t nospam_num, const uint8_t *data,
uint16_t length)
{
// FIXME: This max packet size is too large to be handled by receiving clients
// when sent via the onion. We currently limit the length at a higher level, but
// this bounds check should be fixed to represent the max size of a packet that
// the onion client can handle.
if (1 + sizeof(nospam_num) + length > ONION_CLIENT_MAX_DATA_SIZE || length == 0) {
return -1;
}
Expand Down
3 changes: 2 additions & 1 deletion toxcore/friend_requests.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@
#include "attributes.h"
#include "friend_connection.h"

#define MAX_FRIEND_REQUEST_DATA_SIZE (ONION_CLIENT_MAX_DATA_SIZE - (1 + sizeof(uint32_t)))
// FIXME: This should be the maximum size that an onion client can handle.
#define MAX_FRIEND_REQUEST_DATA_SIZE (ONION_CLIENT_MAX_DATA_SIZE - 100)

typedef struct Friend_Requests Friend_Requests;

Expand Down
2 changes: 2 additions & 0 deletions toxcore/onion_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,8 @@ non_null()
unsigned int onion_getfriend_dht_pubkey(const Onion_Client *onion_c, int friend_num, uint8_t *dht_key);

#define ONION_DATA_IN_RESPONSE_MIN_SIZE (CRYPTO_PUBLIC_KEY_SIZE + CRYPTO_MAC_SIZE)

// FIXME: This is not the correct value; data this large will be dropped by the onion client.
#define ONION_CLIENT_MAX_DATA_SIZE (MAX_DATA_REQUEST_SIZE - ONION_DATA_IN_RESPONSE_MIN_SIZE)

/** @brief Send data of length length to friendnum.
Expand Down
2 changes: 1 addition & 1 deletion toxcore/tox.h
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ uint32_t tox_max_status_message_length(void);
*
* @deprecated The macro will be removed in 0.3.0. Use the function instead.
*/
#define TOX_MAX_FRIEND_REQUEST_LENGTH 1016
#define TOX_MAX_FRIEND_REQUEST_LENGTH 921

uint32_t tox_max_friend_request_length(void);

Expand Down
Loading