diff --git a/toxcore/Messenger.c b/toxcore/Messenger.c index ae2d1d3bb..2dccd4a8d 100644 --- a/toxcore/Messenger.c +++ b/toxcore/Messenger.c @@ -458,6 +458,8 @@ int m_get_friend_connectionstatus(const Messenger *m, int32_t friendnumber) bool direct_connected = 0; unsigned int num_online_relays = 0; int crypt_conn_id = friend_connection_crypt_connection_id(m->fr_c, m->friendlist[friendnumber].friendcon_id); + + // FIXME(sudden6): handle return value crypto_connection_status(m->net_crypto, crypt_conn_id, &direct_connected, &num_online_relays); if (direct_connected) { diff --git a/toxcore/net_crypto.c b/toxcore/net_crypto.c index 3ace3beae..da525a1f3 100644 --- a/toxcore/net_crypto.c +++ b/toxcore/net_crypto.c @@ -48,6 +48,18 @@ typedef struct Packets_Array { uint32_t buffer_end; /* packet numbers in array: {buffer_start, buffer_end) */ } Packets_Array; +typedef enum Crypto_Conn_State { + CRYPTO_CONN_FREE = 0, /* the connection slot is free; this value is 0 so it is valid after + * crypto_memzero(...) of the parent struct + */ + CRYPTO_CONN_NO_CONNECTION, /* the connection is allocated, but not yet used */ + CRYPTO_CONN_COOKIE_REQUESTING, /* we are sending cookie request packets */ + CRYPTO_CONN_HANDSHAKE_SENT, /* we are sending handshake packets */ + CRYPTO_CONN_NOT_CONFIRMED, /* we are sending handshake packets; + * we have received one from the other, but no data */ + CRYPTO_CONN_ESTABLISHED, /* the connection is established */ +} Crypto_Conn_State; + typedef struct Crypto_Connection { uint8_t public_key[CRYPTO_PUBLIC_KEY_SIZE]; /* The real public key of the peer. */ uint8_t recv_nonce[CRYPTO_NONCE_SIZE]; /* Nonce of received packets. */ @@ -56,14 +68,7 @@ typedef struct Crypto_Connection { uint8_t sessionsecret_key[CRYPTO_SECRET_KEY_SIZE]; /* Our private key for this session. */ uint8_t peersessionpublic_key[CRYPTO_PUBLIC_KEY_SIZE]; /* The public key of the peer. */ uint8_t shared_key[CRYPTO_SHARED_KEY_SIZE]; /* The precomputed shared key from encrypt_precompute. */ - /** - * 0 if no connection, - * 1 we are sending cookie request packets, - * 2 if we are sending handshake packets, - * 3 if connection is not confirmed yet (we have received a handshake but no data packets yet), - * 4 if the connection is established. - */ - Crypto_Conn_State status; + Crypto_Conn_State status; /* See Crypto_Conn_State documentation */ uint64_t cookie_request_number; /* number used in the cookie request packets for this connection */ uint8_t dht_public_key[CRYPTO_PUBLIC_KEY_SIZE]; /* The dht public key of the peer */ @@ -125,7 +130,8 @@ typedef struct Crypto_Connection { uint8_t maximum_speed_reached; - pthread_mutex_t mutex; + /* Must be a pointer, because the struct is moved in memory */ + pthread_mutex_t *mutex; dht_pk_cb *dht_pk_callback; void *dht_pk_callback_object; @@ -193,7 +199,9 @@ static uint8_t crypt_connection_id_not_valid(const Net_Crypto *c, int crypt_conn return 1; } - if (c->crypto_connections[crypt_connection_id].status == CRYPTO_CONN_NO_CONNECTION) { + const Crypto_Conn_State status = c->crypto_connections[crypt_connection_id].status; + + if (status == CRYPTO_CONN_NO_CONNECTION || status == CRYPTO_CONN_FREE) { return 1; } @@ -680,21 +688,23 @@ static int send_packet_to(Net_Crypto *c, int crypt_connection_id, const uint8_t int direct_send_attempt = 0; - pthread_mutex_lock(&conn->mutex); + pthread_mutex_lock(conn->mutex); IP_Port ip_port = return_ip_port_connection(c, crypt_connection_id); // TODO(irungentoo): on bad networks, direct connections might not last indefinitely. if (!net_family_is_unspec(ip_port.ip.family)) { bool direct_connected = 0; + + // FIXME(sudden6): handle return value crypto_connection_status(c, crypt_connection_id, &direct_connected, nullptr); if (direct_connected) { if ((uint32_t)sendpacket(dht_get_net(c->dht), ip_port, data, length) == length) { - pthread_mutex_unlock(&conn->mutex); + pthread_mutex_unlock(conn->mutex); return 0; } - pthread_mutex_unlock(&conn->mutex); + pthread_mutex_unlock(conn->mutex); return -1; } @@ -710,18 +720,18 @@ static int send_packet_to(Net_Crypto *c, int crypt_connection_id, const uint8_t } } - pthread_mutex_unlock(&conn->mutex); + pthread_mutex_unlock(conn->mutex); pthread_mutex_lock(&c->tcp_mutex); int ret = send_packet_tcp_connection(c->tcp_c, conn->connection_number_tcp, data, length); pthread_mutex_unlock(&c->tcp_mutex); - pthread_mutex_lock(&conn->mutex); + pthread_mutex_lock(conn->mutex); if (ret == 0) { conn->last_tcp_sent = current_time_monotonic(c->mono_time); } - pthread_mutex_unlock(&conn->mutex); + pthread_mutex_unlock(conn->mutex); if (ret == 0 || direct_send_attempt) { return 0; @@ -1073,19 +1083,19 @@ static int send_data_packet(Net_Crypto *c, int crypt_connection_id, const uint8_ return -1; } - pthread_mutex_lock(&conn->mutex); + pthread_mutex_lock(conn->mutex); VLA(uint8_t, packet, 1 + sizeof(uint16_t) + length + CRYPTO_MAC_SIZE); packet[0] = NET_PACKET_CRYPTO_DATA; memcpy(packet + 1, conn->sent_nonce + (CRYPTO_NONCE_SIZE - sizeof(uint16_t)), sizeof(uint16_t)); const int len = encrypt_data_symmetric(conn->shared_key, conn->sent_nonce, data, length, packet + 1 + sizeof(uint16_t)); if (len + 1 + sizeof(uint16_t) != SIZEOF_VLA(packet)) { - pthread_mutex_unlock(&conn->mutex); + pthread_mutex_unlock(conn->mutex); return -1; } increment_nonce(conn->sent_nonce); - pthread_mutex_unlock(&conn->mutex); + pthread_mutex_unlock(conn->mutex); return send_packet_to(c, crypt_connection_id, packet, SIZEOF_VLA(packet)); } @@ -1172,9 +1182,9 @@ static int64_t send_lossless_packet(Net_Crypto *c, int crypt_connection_id, cons dt.sent_time = 0; dt.length = length; memcpy(dt.data, data, length); - pthread_mutex_lock(&conn->mutex); + pthread_mutex_lock(conn->mutex); int64_t packet_num = add_data_end_of_buffer(c->log, &conn->send_array, &dt); - pthread_mutex_unlock(&conn->mutex); + pthread_mutex_unlock(conn->mutex); if (packet_num == -1) { return -1; @@ -1578,9 +1588,9 @@ static int handle_data_packet_core(Net_Crypto *c, int crypt_connection_id, const } while (1) { - pthread_mutex_lock(&conn->mutex); + pthread_mutex_lock(conn->mutex); int ret = read_data_beg_buffer(c->log, &conn->recv_array, &dt); - pthread_mutex_unlock(&conn->mutex); + pthread_mutex_unlock(conn->mutex); if (ret == -1) { break; @@ -1748,12 +1758,6 @@ static int realloc_cryptoconnection(Net_Crypto *c, uint32_t num) */ static int create_crypto_connection(Net_Crypto *c) { - for (uint32_t i = 0; i < c->crypto_connections_length; ++i) { - if (c->crypto_connections[i].status == CRYPTO_CONN_NO_CONNECTION) { - return i; - } - } - while (1) { /* TODO(irungentoo): is this really the best way to do this? */ pthread_mutex_lock(&c->connections_mutex); @@ -1766,21 +1770,42 @@ static int create_crypto_connection(Net_Crypto *c) int id = -1; - if (realloc_cryptoconnection(c, c->crypto_connections_length + 1) == 0) { - id = c->crypto_connections_length; - ++c->crypto_connections_length; - memset(&c->crypto_connections[id], 0, sizeof(Crypto_Connection)); + for (uint32_t i = 0; i < c->crypto_connections_length; ++i) { + if (c->crypto_connections[i].status == CRYPTO_CONN_FREE) { + id = i; + break; + } + } + + if (id == -1) { + if (realloc_cryptoconnection(c, c->crypto_connections_length + 1) == 0) { + id = c->crypto_connections_length; + ++c->crypto_connections_length; + memset(&c->crypto_connections[id], 0, sizeof(Crypto_Connection)); + } + } + + if (id != -1) { // Memsetting float/double to 0 is non-portable, so we explicitly set them to 0 c->crypto_connections[id].packet_recv_rate = 0; c->crypto_connections[id].packet_send_rate = 0; c->crypto_connections[id].last_packets_left_rem = 0; c->crypto_connections[id].packet_send_rate_requested = 0; c->crypto_connections[id].last_packets_left_requested_rem = 0; + c->crypto_connections[id].mutex = (pthread_mutex_t *) malloc(sizeof(pthread_mutex_t)); - if (pthread_mutex_init(&c->crypto_connections[id].mutex, nullptr) != 0) { + if (c->crypto_connections[id].mutex == nullptr) { pthread_mutex_unlock(&c->connections_mutex); return -1; } + + if (pthread_mutex_init(c->crypto_connections[id].mutex, nullptr) != 0) { + free(c->crypto_connections[id].mutex); + pthread_mutex_unlock(&c->connections_mutex); + return -1; + } + + c->crypto_connections[id].status = CRYPTO_CONN_NO_CONNECTION; } pthread_mutex_unlock(&c->connections_mutex); @@ -1794,21 +1819,29 @@ static int create_crypto_connection(Net_Crypto *c) */ static int wipe_crypto_connection(Net_Crypto *c, int crypt_connection_id) { - if (crypt_connection_id_not_valid(c, crypt_connection_id)) { + if ((uint32_t)crypt_connection_id >= c->crypto_connections_length) { + return -1; + } + + if (c->crypto_connections == nullptr) { + return -1; + } + + const Crypto_Conn_State status = c->crypto_connections[crypt_connection_id].status; + + if (status == CRYPTO_CONN_FREE) { return -1; } uint32_t i; - /* Keep mutex, only destroy it when connection is realloced out. */ - pthread_mutex_t mutex = c->crypto_connections[crypt_connection_id].mutex; + pthread_mutex_destroy(c->crypto_connections[crypt_connection_id].mutex); + free(c->crypto_connections[crypt_connection_id].mutex); crypto_memzero(&c->crypto_connections[crypt_connection_id], sizeof(Crypto_Connection)); - c->crypto_connections[crypt_connection_id].mutex = mutex; + /* check if we can resize the connections array */ for (i = c->crypto_connections_length; i != 0; --i) { - if (c->crypto_connections[i - 1].status == CRYPTO_CONN_NO_CONNECTION) { - pthread_mutex_destroy(&c->crypto_connections[i - 1].mutex); - } else { + if (c->crypto_connections[i - 1].status != CRYPTO_CONN_FREE) { break; } } @@ -1829,10 +1862,12 @@ static int wipe_crypto_connection(Net_Crypto *c, int crypt_connection_id) static int getcryptconnection_id(const Net_Crypto *c, const uint8_t *public_key) { for (uint32_t i = 0; i < c->crypto_connections_length; ++i) { - if (c->crypto_connections[i].status != CRYPTO_CONN_NO_CONNECTION) { - if (public_key_cmp(public_key, c->crypto_connections[i].public_key) == 0) { - return i; - } + if (crypt_connection_id_not_valid(c, i)) { + continue; + } + + if (public_key_cmp(public_key, c->crypto_connections[i].public_key) == 0) { + return i; } } @@ -1975,6 +2010,7 @@ int accept_crypto_connection(Net_Crypto *c, New_Connection *n_c) Crypto_Connection *conn = &c->crypto_connections[crypt_connection_id]; if (n_c->cookie_length != COOKIE_LENGTH) { + wipe_crypto_connection(c, crypt_connection_id); return -1; } @@ -1983,6 +2019,7 @@ int accept_crypto_connection(Net_Crypto *c, New_Connection *n_c) pthread_mutex_unlock(&c->tcp_mutex); if (connection_number_tcp == -1) { + wipe_crypto_connection(c, crypt_connection_id); return -1; } @@ -1999,7 +2036,7 @@ int accept_crypto_connection(Net_Crypto *c, New_Connection *n_c) pthread_mutex_lock(&c->tcp_mutex); kill_tcp_connection_to(c->tcp_c, conn->connection_number_tcp); pthread_mutex_unlock(&c->tcp_mutex); - conn->status = CRYPTO_CONN_NO_CONNECTION; + wipe_crypto_connection(c, crypt_connection_id); return -1; } @@ -2039,6 +2076,7 @@ int new_crypto_connection(Net_Crypto *c, const uint8_t *real_public_key, const u pthread_mutex_unlock(&c->tcp_mutex); if (connection_number_tcp == -1) { + wipe_crypto_connection(c, crypt_connection_id); return -1; } @@ -2062,7 +2100,7 @@ int new_crypto_connection(Net_Crypto *c, const uint8_t *real_public_key, const u pthread_mutex_lock(&c->tcp_mutex); kill_tcp_connection_to(c->tcp_c, conn->connection_number_tcp); pthread_mutex_unlock(&c->tcp_mutex); - conn->status = CRYPTO_CONN_NO_CONNECTION; + wipe_crypto_connection(c, crypt_connection_id); return -1; } @@ -2259,19 +2297,24 @@ static void do_tcp(Net_Crypto *c, void *userdata) continue; } - if (conn->status == CRYPTO_CONN_ESTABLISHED) { - bool direct_connected = 0; - crypto_connection_status(c, i, &direct_connected, nullptr); + if (conn->status != CRYPTO_CONN_ESTABLISHED) { + continue; + } - if (direct_connected) { - pthread_mutex_lock(&c->tcp_mutex); - set_tcp_connection_to_status(c->tcp_c, conn->connection_number_tcp, 0); - pthread_mutex_unlock(&c->tcp_mutex); - } else { - pthread_mutex_lock(&c->tcp_mutex); - set_tcp_connection_to_status(c->tcp_c, conn->connection_number_tcp, 1); - pthread_mutex_unlock(&c->tcp_mutex); - } + bool direct_connected = 0; + + if (!crypto_connection_status(c, i, &direct_connected, nullptr)) { + continue; + } + + if (direct_connected) { + pthread_mutex_lock(&c->tcp_mutex); + set_tcp_connection_to_status(c->tcp_c, conn->connection_number_tcp, 0); + pthread_mutex_unlock(&c->tcp_mutex); + } else { + pthread_mutex_lock(&c->tcp_mutex); + set_tcp_connection_to_status(c->tcp_c, conn->connection_number_tcp, 1); + pthread_mutex_unlock(&c->tcp_mutex); } } } @@ -2425,7 +2468,7 @@ static int udp_handle_packet(void *object, IP_Port source, const uint8_t *packet return -1; } - pthread_mutex_lock(&conn->mutex); + pthread_mutex_lock(conn->mutex); if (net_family_is_ipv4(source.ip.family)) { conn->direct_lastrecv_timev4 = mono_time_get(c->mono_time); @@ -2433,7 +2476,7 @@ static int udp_handle_packet(void *object, IP_Port source, const uint8_t *packet conn->direct_lastrecv_timev6 = mono_time_get(c->mono_time); } - pthread_mutex_unlock(&conn->mutex); + pthread_mutex_unlock(conn->mutex); return 0; } @@ -2542,6 +2585,7 @@ static void send_crypto_packets(Net_Crypto *c) (CONGESTION_QUEUE_ARRAY_SIZE * CONGESTION_LAST_SENT_ARRAY_SIZE); bool direct_connected = 0; + /* return value can be ignored since the `if` above ensures the connection is established */ crypto_connection_status(c, i, &direct_connected, nullptr); /* When switching from TCP to UDP, don't change the packet send rate for CONGESTION_EVENT_TIMEOUT ms. */ @@ -2824,10 +2868,10 @@ int send_lossy_cryptpacket(Net_Crypto *c, int crypt_connection_id, const uint8_t int ret = -1; if (conn) { - pthread_mutex_lock(&conn->mutex); + pthread_mutex_lock(conn->mutex); uint32_t buffer_start = conn->recv_array.buffer_start; uint32_t buffer_end = conn->send_array.buffer_end; - pthread_mutex_unlock(&conn->mutex); + pthread_mutex_unlock(conn->mutex); ret = send_data_packet_helper(c, crypt_connection_id, buffer_start, buffer_end, data, length); } @@ -2881,18 +2925,13 @@ int crypto_kill(Net_Crypto *c, int crypt_connection_id) return ret; } -/* return one of CRYPTO_CONN_* values indicating the state of the connection. - * - * sets direct_connected to 1 if connection connects directly to other, 0 if it isn't. - * sets online_tcp_relays to the number of connected tcp relays this connection has. - */ -Crypto_Conn_State crypto_connection_status(const Net_Crypto *c, int crypt_connection_id, bool *direct_connected, - unsigned int *online_tcp_relays) +bool crypto_connection_status(const Net_Crypto *c, int crypt_connection_id, bool *direct_connected, + unsigned int *online_tcp_relays) { Crypto_Connection *conn = get_crypto_connection(c, crypt_connection_id); if (conn == nullptr) { - return CRYPTO_CONN_NO_CONNECTION; + return false; } if (direct_connected) { @@ -2913,7 +2952,7 @@ Crypto_Conn_State crypto_connection_status(const Net_Crypto *c, int crypt_connec *online_tcp_relays = tcp_connection_to_online_tcp_relays(c->tcp_c, conn->connection_number_tcp); } - return conn->status; + return true; } void new_keys(Net_Crypto *c) @@ -3002,10 +3041,6 @@ static void kill_timedout(Net_Crypto *c, void *userdata) continue; } - if (conn->status == CRYPTO_CONN_NO_CONNECTION) { - continue; - } - if (conn->status == CRYPTO_CONN_COOKIE_REQUESTING || conn->status == CRYPTO_CONN_HANDSHAKE_SENT || conn->status == CRYPTO_CONN_NOT_CONFIRMED) { if (conn->temp_packet_num_sent < MAX_NUM_SENDPACKET_TRIES) { diff --git a/toxcore/net_crypto.h b/toxcore/net_crypto.h index 45e1a05ea..6917301b4 100644 --- a/toxcore/net_crypto.h +++ b/toxcore/net_crypto.h @@ -81,16 +81,6 @@ #define PACKET_ID_REJOIN_CONFERENCE 100 #define PACKET_ID_LOSSY_CONFERENCE 199 -/*** Crypto connections. ***/ - -typedef enum Crypto_Conn_State { - CRYPTO_CONN_NO_CONNECTION = 0, - CRYPTO_CONN_COOKIE_REQUESTING = 1, // send cookie request packets - CRYPTO_CONN_HANDSHAKE_SENT = 2, // send handshake packets - CRYPTO_CONN_NOT_CONFIRMED = 3, // send handshake packets, we have received one from the other - CRYPTO_CONN_ESTABLISHED = 4, -} Crypto_Conn_State; - /* Maximum size of receiving and sending packet buffers. */ #define CRYPTO_PACKET_BUFFER_SIZE 32768 // Must be a power of 2 @@ -319,13 +309,13 @@ unsigned int copy_connected_tcp_relays(Net_Crypto *c, Node_format *tcp_relays, u */ int crypto_kill(Net_Crypto *c, int crypt_connection_id); -/* return one of CRYPTO_CONN_* values indicating the state of the connection. +/* return true if connection is valid, false otherwise * * sets direct_connected to 1 if connection connects directly to other, 0 if it isn't. * sets online_tcp_relays to the number of connected tcp relays this connection has. */ -Crypto_Conn_State crypto_connection_status(const Net_Crypto *c, int crypt_connection_id, bool *direct_connected, - unsigned int *online_tcp_relays); +bool crypto_connection_status(const Net_Crypto *c, int crypt_connection_id, bool *direct_connected, + unsigned int *online_tcp_relays); /* Generate our public and private keys. * Only call this function the first time the program starts.