Skip to content

Commit ccfa6c6

Browse files
irungentooirungentoo
committed
Merge branch 'kpp-code_review'
2 parents 61f8e65 + 23b0c9c commit ccfa6c6

19 files changed

+149
-126
lines changed

‎auto_tests/TCP_test.c‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ START_TEST(test_basic)
117117
increment_nonce(f_nonce_r);
118118
ck_assert_msg(packet_resp_plain[0] == 1, "wrong packet id %u", packet_resp_plain[0]);
119119
ck_assert_msg(packet_resp_plain[1] == 0, "connection not refused %u", packet_resp_plain[1]);
120-
ck_assert_msg(memcmp(packet_resp_plain + 2, f_public_key, crypto_box_PUBLICKEYBYTES) == 0, "key in packet wrong");
120+
ck_assert_msg(public_key_cmp(packet_resp_plain + 2, f_public_key) == 0, "key in packet wrong");
121121
kill_TCP_server(tcp_s);
122122
}
123123
END_TEST
@@ -235,12 +235,12 @@ START_TEST(test_some)
235235
ck_assert_msg(len == 1 + 1 + crypto_box_PUBLICKEYBYTES, "wrong len %u", len);
236236
ck_assert_msg(data[0] == 1, "wrong packet id %u", data[0]);
237237
ck_assert_msg(data[1] == 16, "connection not refused %u", data[1]);
238-
ck_assert_msg(memcmp(data + 2, con3->public_key, crypto_box_PUBLICKEYBYTES) == 0, "key in packet wrong");
238+
ck_assert_msg(public_key_cmp(data + 2, con3->public_key) == 0, "key in packet wrong");
239239
len = read_packet_sec_TCP(con3, data, 2 + 1 + 1 + crypto_box_PUBLICKEYBYTES + crypto_box_MACBYTES);
240240
ck_assert_msg(len == 1 + 1 + crypto_box_PUBLICKEYBYTES, "wrong len %u", len);
241241
ck_assert_msg(data[0] == 1, "wrong packet id %u", data[0]);
242242
ck_assert_msg(data[1] == 16, "connection not refused %u", data[1]);
243-
ck_assert_msg(memcmp(data + 2, con1->public_key, crypto_box_PUBLICKEYBYTES) == 0, "key in packet wrong");
243+
ck_assert_msg(public_key_cmp(data + 2, con1->public_key) == 0, "key in packet wrong");
244244

245245
uint8_t test_packet[512] = {16, 17, 16, 86, 99, 127, 255, 189, 78};
246246
write_packet_TCP_secure_connection(con3, test_packet, sizeof(test_packet));
@@ -363,7 +363,7 @@ static int oob_data_callback(void *object, const uint8_t *public_key, const uint
363363
if (length != 5)
364364
return 1;
365365

366-
if (memcmp(public_key, oob_pubkey, crypto_box_PUBLICKEYBYTES) != 0)
366+
if (public_key_cmp(public_key, oob_pubkey) != 0)
367367
return 1;
368368

369369
if (data[0] == 1 && data[1] == 2 && data[2] == 3 && data[3] == 4 && data[4] == 5) {
@@ -447,7 +447,7 @@ START_TEST(test_client)
447447
do_TCP_connection(conn2);
448448
ck_assert_msg(oob_data_callback_good == 1, "oob callback not called");
449449
ck_assert_msg(response_callback_good == 1, "response callback not called");
450-
ck_assert_msg(memcmp(response_callback_public_key, f2_public_key, crypto_box_PUBLICKEYBYTES) == 0, "wrong public key");
450+
ck_assert_msg(public_key_cmp(response_callback_public_key, f2_public_key) == 0, "wrong public key");
451451
ck_assert_msg(status_callback_good == 1, "status callback not called");
452452
ck_assert_msg(status_callback_status == 2, "wrong status");
453453
ck_assert_msg(status_callback_connection_id == response_callback_connection_id, "connection ids not equal");
@@ -538,17 +538,17 @@ START_TEST(test_tcp_connection)
538538
uint8_t self_secret_key[crypto_box_SECRETKEYBYTES];
539539
crypto_box_keypair(self_public_key, self_secret_key);
540540
TCP_Server *tcp_s = new_TCP_server(1, NUM_PORTS, ports, self_secret_key, NULL);
541-
ck_assert_msg(memcmp(tcp_s->public_key, self_public_key, crypto_box_PUBLICKEYBYTES) == 0, "Wrong public key");
541+
ck_assert_msg(public_key_cmp(tcp_s->public_key, self_public_key) == 0, "Wrong public key");
542542

543543
TCP_Proxy_Info proxy_info;
544544
proxy_info.proxy_type = TCP_PROXY_NONE;
545545
crypto_box_keypair(self_public_key, self_secret_key);
546546
TCP_Connections *tc_1 = new_tcp_connections(self_secret_key, &proxy_info);
547-
ck_assert_msg(memcmp(tc_1->self_public_key, self_public_key, crypto_box_PUBLICKEYBYTES) == 0, "Wrong public key");
547+
ck_assert_msg(public_key_cmp(tc_1->self_public_key, self_public_key) == 0, "Wrong public key");
548548

549549
crypto_box_keypair(self_public_key, self_secret_key);
550550
TCP_Connections *tc_2 = new_tcp_connections(self_secret_key, &proxy_info);
551-
ck_assert_msg(memcmp(tc_2->self_public_key, self_public_key, crypto_box_PUBLICKEYBYTES) == 0, "Wrong public key");
551+
ck_assert_msg(public_key_cmp(tc_2->self_public_key, self_public_key) == 0, "Wrong public key");
552552

553553
IP_Port ip_port_tcp_s;
554554

@@ -641,17 +641,17 @@ START_TEST(test_tcp_connection2)
641641
uint8_t self_secret_key[crypto_box_SECRETKEYBYTES];
642642
crypto_box_keypair(self_public_key, self_secret_key);
643643
TCP_Server *tcp_s = new_TCP_server(1, NUM_PORTS, ports, self_secret_key, NULL);
644-
ck_assert_msg(memcmp(tcp_s->public_key, self_public_key, crypto_box_PUBLICKEYBYTES) == 0, "Wrong public key");
644+
ck_assert_msg(public_key_cmp(tcp_s->public_key, self_public_key) == 0, "Wrong public key");
645645

646646
TCP_Proxy_Info proxy_info;
647647
proxy_info.proxy_type = TCP_PROXY_NONE;
648648
crypto_box_keypair(self_public_key, self_secret_key);
649649
TCP_Connections *tc_1 = new_tcp_connections(self_secret_key, &proxy_info);
650-
ck_assert_msg(memcmp(tc_1->self_public_key, self_public_key, crypto_box_PUBLICKEYBYTES) == 0, "Wrong public key");
650+
ck_assert_msg(public_key_cmp(tc_1->self_public_key, self_public_key) == 0, "Wrong public key");
651651

652652
crypto_box_keypair(self_public_key, self_secret_key);
653653
TCP_Connections *tc_2 = new_tcp_connections(self_secret_key, &proxy_info);
654-
ck_assert_msg(memcmp(tc_2->self_public_key, self_public_key, crypto_box_PUBLICKEYBYTES) == 0, "Wrong public key");
654+
ck_assert_msg(public_key_cmp(tc_2->self_public_key, self_public_key) == 0, "Wrong public key");
655655

656656
IP_Port ip_port_tcp_s;
657657

‎testing/DHT_test.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ void print_clientlist(DHT *dht)
108108
for (i = 0; i < LCLIENT_LIST; i++) {
109109
Client_data *client = &dht->close_clientlist[i];
110110

111-
if (memcmp(client->public_key, zeroes_cid, crypto_box_PUBLICKEYBYTES) == 0)
111+
if (public_key_cmp(client->public_key, zeroes_cid) == 0)
112112
continue;
113113

114114
printf("ClientID: ");
@@ -139,7 +139,7 @@ void print_friendlist(DHT *dht)
139139
for (i = 0; i < MAX_FRIEND_CLIENTS; i++) {
140140
Client_data *client = &dht->friends_list[k].client_list[i];
141141

142-
if (memcmp(client->public_key, zeroes_cid, crypto_box_PUBLICKEYBYTES) == 0)
142+
if (public_key_cmp(client->public_key, zeroes_cid) == 0)
143143
continue;
144144

145145
printf("ClientID: ");

‎toxcore/DHT.c‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ void get_shared_key(Shared_Keys *shared_keys, uint8_t *shared_key, const uint8_t
126126
int index = public_key[30] * MAX_KEYS_PER_SLOT + i;
127127

128128
if (shared_keys->keys[index].stored) {
129-
if (memcmp(public_key, shared_keys->keys[index].public_key, crypto_box_PUBLICKEYBYTES) == 0) {
129+
if (public_key_cmp(public_key, shared_keys->keys[index].public_key) == 0) {
130130
memcpy(shared_key, shared_keys->keys[index].shared_key, crypto_box_BEFORENMBYTES);
131131
++shared_keys->keys[index].times_requested;
132132
shared_keys->keys[index].time_last_requested = unix_time();
@@ -233,6 +233,7 @@ int pack_nodes(uint8_t *data, uint16_t length, const Node_format *nodes, uint16_
233233
int ipv6 = -1;
234234
uint8_t net_family;
235235

236+
// FIXME use functions to convert endianness
236237
if (nodes[i].ip_port.ip.family == AF_INET) {
237238
ipv6 = 0;
238239
net_family = TOX_AF_INET;
@@ -844,7 +845,7 @@ static _Bool is_pk_in_client_list(Client_data *list, unsigned int client_list_le
844845
for (i = 0; i < client_list_length; ++i) {
845846
if ((ip_port.ip.family == AF_INET && !is_timeout(list[i].assoc4.timestamp, BAD_NODE_TIMEOUT))
846847
|| (ip_port.ip.family == AF_INET6 && !is_timeout(list[i].assoc6.timestamp, BAD_NODE_TIMEOUT))) {
847-
if (memcmp(list[i].public_key, public_key, crypto_box_PUBLICKEYBYTES) == 0) {
848+
if (public_key_cmp(list[i].public_key, public_key) == 0) {
848849
return 1;
849850
}
850851
}
@@ -944,7 +945,7 @@ int addto_lists(DHT *dht, IP_Port ip_port, const uint8_t *public_key)
944945

945946
DHT_Friend *friend = &dht->friends_list[i];
946947

947-
if (memcmp(public_key, friend->public_key, crypto_box_PUBLICKEYBYTES) == 0) {
948+
if (public_key_cmp(public_key, friend->public_key) == 0) {
948949
friend_foundip = friend;
949950
}
950951

@@ -953,7 +954,7 @@ int addto_lists(DHT *dht, IP_Port ip_port, const uint8_t *public_key)
953954
} else {
954955
DHT_Friend *friend = &dht->friends_list[i];
955956

956-
if (memcmp(public_key, friend->public_key, crypto_box_PUBLICKEYBYTES) == 0) {
957+
if (public_key_cmp(public_key, friend->public_key) == 0) {
957958
friend_foundip = friend;
958959
}
959960

@@ -1212,7 +1213,7 @@ static uint8_t sent_getnode_to_node(DHT *dht, const uint8_t *public_key, IP_Port
12121213
Node_format test;
12131214
memcpy(&test, data, sizeof(Node_format));
12141215

1215-
if (!ipport_equal(&test.ip_port, &node_ip_port) || memcmp(test.public_key, public_key, crypto_box_PUBLICKEYBYTES) != 0)
1216+
if (!ipport_equal(&test.ip_port, &node_ip_port) || public_key_cmp(test.public_key, public_key) != 0)
12161217
return 0;
12171218

12181219
return 1;
@@ -2081,7 +2082,7 @@ static IPPTsPng *get_closelist_IPPTsPng(DHT *dht, const uint8_t *public_key, sa_
20812082
uint32_t i;
20822083

20832084
for (i = 0; i < LCLIENT_LIST; ++i) {
2084-
if (memcmp(dht->close_clientlist[i].public_key, public_key, crypto_box_PUBLICKEYBYTES) != 0)
2085+
if (public_key_cmp(dht->close_clientlist[i].public_key, public_key) != 0)
20852086
continue;
20862087

20872088
if (sa_family == AF_INET)
@@ -2178,7 +2179,7 @@ static int handle_hardening(void *object, IP_Port source, const uint8_t *source_
21782179
if (is_timeout(temp->hardening.send_nodes_timestamp, HARDENING_INTERVAL))
21792180
return 1;
21802181

2181-
if (memcmp(temp->hardening.send_nodes_pingedid, source_pubkey, crypto_box_PUBLICKEYBYTES) != 0)
2182+
if (public_key_cmp(temp->hardening.send_nodes_pingedid, source_pubkey) != 0)
21822183
return 1;
21832184

21842185
/* If Nodes look good and the request checks out */
@@ -2351,7 +2352,7 @@ static int cryptopacket_handle(void *object, IP_Port source, const uint8_t *pack
23512352
length > MAX_CRYPTO_REQUEST_SIZE + crypto_box_MACBYTES)
23522353
return 1;
23532354

2354-
if (memcmp(packet + 1, dht->self_public_key, crypto_box_PUBLICKEYBYTES) == 0) { // Check if request is for us.
2355+
if (public_key_cmp(packet + 1, dht->self_public_key) == 0) { // Check if request is for us.
23552356
uint8_t public_key[crypto_box_PUBLICKEYBYTES];
23562357
uint8_t data[MAX_CRYPTO_REQUEST_SIZE];
23572358
uint8_t number;

‎toxcore/Messenger.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2660,7 +2660,7 @@ static int messenger_load_state_callback(void *outer, const uint8_t *data, uint3
26602660
set_nospam(&(m->fr), *(uint32_t *)data);
26612661
load_secret_key(m->net_crypto, (&data[sizeof(uint32_t)]) + crypto_box_PUBLICKEYBYTES);
26622662

2663-
if (memcmp((&data[sizeof(uint32_t)]), m->net_crypto->self_public_key, crypto_box_PUBLICKEYBYTES) != 0) {
2663+
if (public_key_cmp((&data[sizeof(uint32_t)]), m->net_crypto->self_public_key) != 0) {
26642664
return -1;
26652665
}
26662666
} else

‎toxcore/TCP_client.c‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ static int socks5_read_handshake_response(TCP_Client_Connection *TCP_conn)
151151
if (ret == -1)
152152
return 0;
153153

154-
if (data[0] == 5 && data[1] == 0)
154+
if (data[0] == 5 && data[1] == 0) // FIXME magic numbers
155155
return 1;
156156

157157
return -1;
@@ -251,7 +251,7 @@ static int handle_handshake(TCP_Client_Connection *TCP_conn, const uint8_t *data
251251

252252
memcpy(TCP_conn->recv_nonce, plain + crypto_box_PUBLICKEYBYTES, crypto_box_NONCEBYTES);
253253
encrypt_precompute(plain, TCP_conn->temp_secret_key, TCP_conn->shared_key);
254-
memset(TCP_conn->temp_secret_key, 0, crypto_box_SECRETKEYBYTES);
254+
sodium_memzero(TCP_conn->temp_secret_key, crypto_box_SECRETKEYBYTES);
255255
return 0;
256256
}
257257

@@ -962,6 +962,6 @@ void kill_TCP_connection(TCP_Client_Connection *TCP_connection)
962962

963963
wipe_priority_list(TCP_connection);
964964
kill_sock(TCP_connection->sock);
965-
memset(TCP_connection, 0, sizeof(TCP_Client_Connection));
965+
sodium_memzero(TCP_connection, sizeof(TCP_Client_Connection));
966966
free(TCP_connection);
967967
}

‎toxcore/TCP_connection.c‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -384,7 +384,7 @@ static int find_tcp_connection_to(TCP_Connections *tcp_c, const uint8_t *public_
384384
TCP_Connection_to *con_to = get_connection(tcp_c, i);
385385

386386
if (con_to) {
387-
if (memcmp(con_to->public_key, public_key, crypto_box_PUBLICKEYBYTES) == 0) {
387+
if (public_key_cmp(con_to->public_key, public_key) == 0) {
388388
return i;
389389
}
390390
}
@@ -407,11 +407,11 @@ static int find_tcp_connection_relay(TCP_Connections *tcp_c, const uint8_t *rela
407407

408408
if (tcp_con) {
409409
if (tcp_con->status == TCP_CONN_SLEEPING) {
410-
if (memcmp(tcp_con->relay_pk, relay_pk, crypto_box_PUBLICKEYBYTES) == 0) {
410+
if (public_key_cmp(tcp_con->relay_pk, relay_pk) == 0) {
411411
return i;
412412
}
413413
} else {
414-
if (memcmp(tcp_con->connection->public_key, relay_pk, crypto_box_PUBLICKEYBYTES) == 0) {
414+
if (public_key_cmp(tcp_con->connection->public_key, relay_pk) == 0) {
415415
return i;
416416
}
417417
}

‎toxcore/TCP_server.c‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,7 @@ static int del_accepted(TCP_Server *TCP_server, int index)
169169
if (!bs_list_remove(&TCP_server->accepted_key_list, TCP_server->accepted_connection_array[index].public_key, index))
170170
return -1;
171171

172-
memset(&TCP_server->accepted_connection_array[index], 0, sizeof(TCP_Secure_Connection));
172+
sodium_memzero(&TCP_server->accepted_connection_array[index], sizeof(TCP_Secure_Connection));
173173
--TCP_server->num_accepted_connections;
174174

175175
if (TCP_server->num_accepted_connections == 0)
@@ -447,7 +447,7 @@ static int write_packet_TCP_secure_connection(TCP_Secure_Connection *con, const
447447
static void kill_TCP_connection(TCP_Secure_Connection *con)
448448
{
449449
kill_sock(con->sock);
450-
memset(con, 0, sizeof(TCP_Secure_Connection));
450+
sodium_memzero(con, sizeof(TCP_Secure_Connection));
451451
}
452452

453453
static int rm_connection_index(TCP_Server *TCP_server, TCP_Secure_Connection *con, uint8_t con_number);
@@ -583,7 +583,7 @@ static int handle_TCP_routing_req(TCP_Server *TCP_server, uint32_t con_id, const
583583
TCP_Secure_Connection *con = &TCP_server->accepted_connection_array[con_id];
584584

585585
/* If person tries to cennect to himself we deny the request*/
586-
if (memcmp(con->public_key, public_key, crypto_box_PUBLICKEYBYTES) == 0) {
586+
if (public_key_cmp(con->public_key, public_key) == 0) {
587587
if (send_routing_response(con, 0, public_key) == -1)
588588
return -1;
589589

@@ -592,7 +592,7 @@ static int handle_TCP_routing_req(TCP_Server *TCP_server, uint32_t con_id, const
592592

593593
for (i = 0; i < NUM_CLIENT_CONNECTIONS; ++i) {
594594
if (con->connections[i].status != 0) {
595-
if (memcmp(public_key, con->connections[i].public_key, crypto_box_PUBLICKEYBYTES) == 0) {
595+
if (public_key_cmp(public_key, con->connections[i].public_key) == 0) {
596596
if (send_routing_response(con, i + NUM_RESERVED_PORTS, public_key) == -1) {
597597
return -1;
598598
} else {
@@ -629,7 +629,7 @@ static int handle_TCP_routing_req(TCP_Server *TCP_server, uint32_t con_id, const
629629

630630
for (i = 0; i < NUM_CLIENT_CONNECTIONS; ++i) {
631631
if (other_conn->connections[i].status == 1
632-
&& memcmp(other_conn->connections[i].public_key, con->public_key, crypto_box_PUBLICKEYBYTES) == 0) {
632+
&& public_key_cmp(other_conn->connections[i].public_key, con->public_key) == 0) {
633633
other_id = i;
634634
break;
635635
}
@@ -868,7 +868,7 @@ static int confirm_TCP_connection(TCP_Server *TCP_server, TCP_Secure_Connection
868868
return -1;
869869
}
870870

871-
memset(con, 0, sizeof(TCP_Secure_Connection));
871+
sodium_memzero(con, sizeof(TCP_Secure_Connection));
872872

873873
if (handle_TCP_packet(TCP_server, index, data, length) == -1) {
874874
kill_accepted(TCP_server, index);
@@ -1056,7 +1056,7 @@ static int do_incoming(TCP_Server *TCP_server, uint32_t i)
10561056
kill_TCP_connection(conn_new);
10571057

10581058
memcpy(conn_new, conn_old, sizeof(TCP_Secure_Connection));
1059-
memset(conn_old, 0, sizeof(TCP_Secure_Connection));
1059+
sodium_memzero(conn_old, sizeof(TCP_Secure_Connection));
10601060
++TCP_server->unconfirmed_connection_queue_index;
10611061

10621062
return index_new;

0 commit comments

Comments
 (0)