Skip to content

Commit

Permalink
cleanup net_crypto
Browse files Browse the repository at this point in the history 
- add a state for allocated but not yet used crypto connections
- change crypto_connection_status signature and use the return value
  • Loading branch information
@sudden6
sudden6 committed Nov 24, 2019
1 parent 2fdc181 commit fc7072f
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 58 deletions.
6 changes: 5 additions & 1 deletion toxcore/Messenger.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -458,7 +458,11 @@ 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);
crypto_connection_status(m->net_crypto, crypt_conn_id, &direct_connected, &num_online_relays);
bool connected = crypto_connection_status(m->net_crypto, crypt_conn_id, &direct_connected, &num_online_relays);

if (!connected) {
return CONNECTION_NONE;
}

if (direct_connected) {
return CONNECTION_UDP;
Expand Down
113 changes: 69 additions & 44 deletions toxcore/net_crypto.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,17 @@ 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, explicitly 0 to stay 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, /* send 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. */
Expand All @@ -56,14 +67,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 */

Expand Down Expand Up @@ -194,7 +198,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;
}

Expand Down Expand Up @@ -687,7 +693,12 @@ static int send_packet_to(Net_Crypto *c, int crypt_connection_id, const uint8_t
// TODO(irungentoo): on bad networks, direct connections might not last indefinitely.
if (!net_family_is_unspec(ip_port.ip.family)) {
bool direct_connected = 0;
crypto_connection_status(c, crypt_connection_id, &direct_connected, nullptr);
bool connected = crypto_connection_status(c, crypt_connection_id, &direct_connected, nullptr);

if (!connected) {
pthread_mutex_unlock(conn->mutex);
return -1;
}

if (direct_connected) {
if ((uint32_t)sendpacket(dht_get_net(c->dht), ip_port, data, length) == length) {
Expand Down Expand Up @@ -1760,7 +1771,7 @@ 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) {
if (c->crypto_connections[i].status == CRYPTO_CONN_FREE) {
/* On destruction the connection struct is zeroed and the mutex memory freed,
* allocate here again */
c->crypto_connections[i].mutex = (pthread_mutex_t *) malloc(sizeof(pthread_mutex_t));
Expand All @@ -1776,6 +1787,7 @@ static int create_crypto_connection(Net_Crypto *c)
return -1;
}

c->crypto_connections[i].status = CRYPTO_CONN_NO_CONNECTION;
pthread_mutex_unlock(&c->connections_mutex);
return i;
}
Expand Down Expand Up @@ -1805,6 +1817,8 @@ static int create_crypto_connection(Net_Crypto *c)
pthread_mutex_unlock(&c->connections_mutex);
return -1;
}

c->crypto_connections[id].status = CRYPTO_CONN_NO_CONNECTION;
}

pthread_mutex_unlock(&c->connections_mutex);
Expand All @@ -1818,9 +1832,18 @@ static int create_crypto_connection(Net_Crypto *c)
*/
static int wipe_crypto_connection(Net_Crypto *c, int crypt_connection_id)
{
// prevent double free, by checking if status == CRYPTO_CONN_NO_CONNECTION
// only this function is allowed to set status to CRYPTO_CONN_NO_CONNECTION
if (crypt_connection_id_not_valid(c, crypt_connection_id)) {
// prevent double free, by checking if status == CRYPTO_CONN_FREE
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;
}

Expand All @@ -1832,7 +1855,7 @@ static int wipe_crypto_connection(Net_Crypto *c, int crypt_connection_id)

/* 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) {
if (c->crypto_connections[i - 1].status != CRYPTO_CONN_FREE) {
break;
}
}
Expand All @@ -1853,10 +1876,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;
}
}

Expand Down Expand Up @@ -2007,6 +2032,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;
}

Expand Down Expand Up @@ -2063,6 +2089,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;
}

Expand Down Expand Up @@ -2283,19 +2310,21 @@ 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);
bool direct_connected = 0;
bool connected = crypto_connection_status(c, i, &direct_connected, nullptr);

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);
}
if (!connected) {
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);
}
}
}
Expand Down Expand Up @@ -2566,6 +2595,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. */
Expand Down Expand Up @@ -2905,18 +2935,17 @@ 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 (conn->status != CRYPTO_CONN_ESTABLISHED) {
return false;
}

if (direct_connected) {
Expand All @@ -2937,7 +2966,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)
Expand Down Expand Up @@ -3026,10 +3055,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) {
Expand Down
16 changes: 3 additions & 13 deletions toxcore/net_crypto.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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 established, 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.
Expand Down

0 comments on commit fc7072f

Please sign in to comment.