Skip to content

Commit

Permalink
Use the crypto random functions instead of rand().
Browse files Browse the repository at this point in the history 
Presumably the uses of `rand()` were fine because they were not used in
security-sensitive places, but having to think about whether a crappy RNG
is acceptable in each situation requires effort that could better be
spent elsewhere.

Also, this means that once we have a custom deterministic RNG for
testing, that RNG is used everywhere, so all the code is deterministic.

It also allowed us to delete a system-specific function that wasn't used
anywhere except in a call to `srand()`.
  • Loading branch information
@iphydf
iphydf committed Aug 10, 2018
1 parent afab28f commit 463cbcb
Show file tree
Hide file tree
Showing 8 changed files with 30 additions and 56 deletions.
14 changes: 7 additions & 7 deletions toxcore/DHT.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1740,10 +1740,10 @@ static uint8_t do_ping_and_sendnode_requests(DHT *dht, uint64_t *lastgetnode, co
}

if ((num_nodes != 0) && (is_timeout(*lastgetnode, GET_NODE_INTERVAL) || *bootstrap_times < MAX_BOOTSTRAP_TIMES)) {
uint32_t rand_node = rand() % num_nodes;
uint32_t rand_node = random_u32() % num_nodes;

if ((num_nodes - 1) != rand_node) {
rand_node += rand() % (num_nodes - (rand_node + 1));
rand_node += random_u32() % (num_nodes - (rand_node + 1));
}

getnodes(dht, assoc_list[rand_node]->ip_port, client_list[rand_node]->public_key, public_key, nullptr);
Expand Down Expand Up @@ -2050,7 +2050,7 @@ static int routeone_tofriend(DHT *dht, const uint8_t *friend_id, const uint8_t *
return 0;
}

const int retval = sendpacket(dht->net, ip_list[rand() % n], packet, length);
const int retval = sendpacket(dht->net, ip_list[random_u32() % n], packet, length);

if ((unsigned int)retval == length) {
return 1;
Expand Down Expand Up @@ -2492,7 +2492,7 @@ static Node_format random_node(DHT *dht, Family sa_family)
uint8_t id[CRYPTO_PUBLIC_KEY_SIZE];

for (uint32_t i = 0; i < CRYPTO_PUBLIC_KEY_SIZE / 4; ++i) { /* populate the id with pseudorandom bytes.*/
const uint32_t t = rand();
const uint32_t t = random_u32();
memcpy(id + i * sizeof(t), &t, sizeof(t));
}

Expand All @@ -2504,7 +2504,7 @@ static Node_format random_node(DHT *dht, Family sa_family)
return nodes_list[0];
}

return nodes_list[rand() % num_nodes];
return nodes_list[random_u32() % num_nodes];
}
#endif

Expand All @@ -2530,7 +2530,7 @@ static uint16_t list_nodes(Client_data *list, size_t length, Node_format *nodes,
if (!is_timeout(list[i - 1].assoc6.timestamp, BAD_NODE_TIMEOUT)) {
if (assoc == nullptr) {
assoc = &list[i - 1].assoc6;
} else if (rand() % 2) {
} else if (random_u08() % 2) {
assoc = &list[i - 1].assoc6;
}
}
Expand Down Expand Up @@ -2560,7 +2560,7 @@ uint16_t randfriends_nodes(DHT *dht, Node_format *nodes, uint16_t max_num)
}

uint16_t count = 0;
const unsigned int r = rand();
const uint32_t r = random_u32();

for (size_t i = 0; i < DHT_FAKE_FRIEND_NUMBER; ++i) {
count += list_nodes(dht->friends_list[(i + r) % DHT_FAKE_FRIEND_NUMBER].client_list, MAX_FRIEND_CLIENTS, nodes + count,
Expand Down
13 changes: 7 additions & 6 deletions toxcore/TCP_connection.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -371,10 +371,10 @@ int send_packet_tcp_connection(TCP_Connections *tcp_c, int connections_number, c
*/
int get_random_tcp_onion_conn_number(TCP_Connections *tcp_c)
{
unsigned int i, r = rand();
const uint32_t r = random_u32();

for (i = 0; i < tcp_c->tcp_connections_length; ++i) {
unsigned int index = ((i + r) % tcp_c->tcp_connections_length);
for (uint32_t i = 0; i < tcp_c->tcp_connections_length; ++i) {
uint32_t index = ((i + r) % tcp_c->tcp_connections_length);

if (tcp_c->tcp_connections[index].onion && tcp_c->tcp_connections[index].status == TCP_CONN_CONNECTED) {
return index;
Expand Down Expand Up @@ -1283,11 +1283,12 @@ unsigned int tcp_connection_to_online_tcp_relays(TCP_Connections *tcp_c, int con
* return number of relays copied to tcp_relays on success.
* return 0 on failure.
*/
unsigned int tcp_copy_connected_relays(TCP_Connections *tcp_c, Node_format *tcp_relays, uint16_t max_num)
uint32_t tcp_copy_connected_relays(TCP_Connections *tcp_c, Node_format *tcp_relays, uint16_t max_num)
{
unsigned int i, copied = 0, r = rand();
const uint32_t r = random_u32();
uint32_t copied = 0;

for (i = 0; (i < tcp_c->tcp_connections_length) && (copied < max_num); ++i) {
for (uint32_t i = 0; (i < tcp_c->tcp_connections_length) && (copied < max_num); ++i) {
TCP_con *tcp_con = get_tcp_connection(tcp_c, (i + r) % tcp_c->tcp_connections_length);

if (!tcp_con) {
Expand Down
2 changes: 1 addition & 1 deletion toxcore/TCP_connection.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ int add_tcp_relay_global(TCP_Connections *tcp_c, IP_Port ip_port, const uint8_t
* return number of relays copied to tcp_relays on success.
* return 0 on failure.
*/
unsigned int tcp_copy_connected_relays(TCP_Connections *tcp_c, Node_format *tcp_relays, uint16_t max_num);
uint32_t tcp_copy_connected_relays(TCP_Connections *tcp_c, Node_format *tcp_relays, uint16_t max_num);

/* Returns a new TCP_Connections object associated with the secret_key.
*
Expand Down
4 changes: 2 additions & 2 deletions toxcore/group.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1490,12 +1490,12 @@ static void handle_friend_invite_packet(Messenger *m, uint32_t friendnumber, con

/* TODO(irungentoo): what if two people enter the group at the same time and
are given the same peer_number by different nodes? */
uint16_t peer_number = rand();
uint16_t peer_number = random_u16();

unsigned int tries = 0;

while (get_peer_index(g, peer_number) != -1) {
peer_number = rand();
peer_number = random_u16();
++tries;

if (tries > 32) {
Expand Down
23 changes: 0 additions & 23 deletions toxcore/mono_time.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,29 +90,6 @@ int is_timeout(uint64_t timestamp, uint64_t timeout)
}



/* return current UNIX time in microseconds (us). */
uint64_t current_time_actual(void)
{
uint64_t time;
#ifdef OS_WIN32
/* This probably works fine */
FILETIME ft;
GetSystemTimeAsFileTime(&ft);
time = ft.dwHighDateTime;
time <<= 32;
time |= ft.dwLowDateTime;
time -= 116444736000000000ULL;
return time / 10;
#else
struct timeval a;
gettimeofday(&a, nullptr);
time = 1000000ULL * a.tv_sec + a.tv_usec;
return time;
#endif
}


//!TOKSTYLE-
// No global mutable state in Tokstyle.
#ifdef OS_WIN32
Expand Down
3 changes: 0 additions & 3 deletions toxcore/mono_time.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,6 @@ void unix_time_update(void);
uint64_t unix_time(void);
int is_timeout(uint64_t timestamp, uint64_t timeout);

/* return current UNIX time in microseconds (us). */
uint64_t current_time_actual(void);

/* return current monotonic time in milliseconds (ms). */
uint64_t current_time_monotonic(void);

Expand Down
1 change: 0 additions & 1 deletion toxcore/network.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -698,7 +698,6 @@ int networking_at_startup(void)
}

#endif
srand((uint32_t)current_time_actual());
at_startup_ran = 1;
return 0;
}
Expand Down
26 changes: 13 additions & 13 deletions toxcore/onion_client.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ static uint16_t random_nodes_path_onion(const Onion_Client *onion_c, Node_format
return 0;
}

unsigned int num_nodes = (onion_c->path_nodes_index < MAX_PATH_NODES) ? onion_c->path_nodes_index : MAX_PATH_NODES;
const uint32_t num_nodes = (onion_c->path_nodes_index < MAX_PATH_NODES) ? onion_c->path_nodes_index : MAX_PATH_NODES;

// if (dht_non_lan_connected(onion_c->dht)) {
if (dht_isconnected(onion_c->dht)) {
Expand All @@ -276,7 +276,7 @@ static uint16_t random_nodes_path_onion(const Onion_Client *onion_c, Node_format
}

for (i = 0; i < max_num; ++i) {
nodes[i] = onion_c->path_nodes[rand() % num_nodes];
nodes[i] = onion_c->path_nodes[random_u32() % num_nodes];
}
} else {
int random_tcp = get_random_tcp_con_number(onion_c->c);
Expand All @@ -290,7 +290,7 @@ static uint16_t random_nodes_path_onion(const Onion_Client *onion_c, Node_format
nodes[0].ip_port.ip.ip.v4.uint32 = random_tcp;

for (i = 1; i < max_num; ++i) {
nodes[i] = onion_c->path_nodes[rand() % num_nodes];
nodes[i] = onion_c->path_nodes[random_u32() % num_nodes];
}
} else {
unsigned int num_nodes_bs = (onion_c->path_nodes_index_bs < MAX_PATH_NODES) ? onion_c->path_nodes_index_bs :
Expand All @@ -304,7 +304,7 @@ static uint16_t random_nodes_path_onion(const Onion_Client *onion_c, Node_format
nodes[0].ip_port.ip.ip.v4.uint32 = random_tcp;

for (i = 1; i < max_num; ++i) {
nodes[i] = onion_c->path_nodes_bs[rand() % num_nodes_bs];
nodes[i] = onion_c->path_nodes_bs[random_u32() % num_nodes_bs];
}
}
}
Expand Down Expand Up @@ -371,7 +371,7 @@ static bool onion_node_timed_out(const Onion_Node *node)
static int random_path(const Onion_Client *onion_c, Onion_Client_Paths *onion_paths, uint32_t pathnum, Onion_Path *path)
{
if (pathnum == UINT32_MAX) {
pathnum = rand() % NUMBER_ONION_PATHS;
pathnum = random_u32() % NUMBER_ONION_PATHS;
} else {
pathnum = pathnum % NUMBER_ONION_PATHS;
}
Expand All @@ -394,7 +394,7 @@ static int random_path(const Onion_Client *onion_c, Onion_Client_Paths *onion_pa
onion_paths->last_path_success[pathnum] = onion_paths->path_creation_time[pathnum];
onion_paths->last_path_used_times[pathnum] = ONION_PATH_MAX_NO_RESPONSE_USES / 2;

uint32_t path_num = rand();
uint32_t path_num = random_u32();
path_num /= NUMBER_ONION_PATHS;
path_num *= NUMBER_ONION_PATHS;
path_num += pathnum;
Expand Down Expand Up @@ -1597,7 +1597,7 @@ static void do_friend(Onion_Client *onion_c, uint16_t friendnum)
}

if (is_timeout(list_nodes[i].last_pinged, interval)
|| (ping_random && rand() % (MAX_ONION_CLIENTS - i) == 0)) {
|| (ping_random && random_u32() % (MAX_ONION_CLIENTS - i) == 0)) {
if (client_send_announce_request(onion_c, friendnum + 1, list_nodes[i].ip_port,
list_nodes[i].public_key, nullptr, ~0) == 0) {
list_nodes[i].last_pinged = unix_time();
Expand All @@ -1616,12 +1616,12 @@ static void do_friend(Onion_Client *onion_c, uint16_t friendnum)
n = (MAX_ONION_CLIENTS / 2);
}

if (count <= (uint32_t)rand() % MAX_ONION_CLIENTS) {
if (count <= random_u32() % MAX_ONION_CLIENTS) {
if (num_nodes != 0) {
unsigned int j;

for (j = 0; j < n; ++j) {
unsigned int num = rand() % num_nodes;
const uint32_t num = random_u32() % num_nodes;
client_send_announce_request(onion_c, friendnum + 1, onion_c->path_nodes[num].ip_port,
onion_c->path_nodes[num].public_key, nullptr, ~0);
}
Expand Down Expand Up @@ -1708,7 +1708,7 @@ static void do_announce(Onion_Client *onion_c)

if (is_timeout(list_nodes[i].last_pinged, interval)
|| (is_timeout(onion_c->last_announce, ONION_NODE_PING_INTERVAL)
&& rand() % (MAX_ONION_CLIENTS_ANNOUNCE - i) == 0)) {
&& random_u32() % (MAX_ONION_CLIENTS_ANNOUNCE - i) == 0)) {
uint32_t path_to_use = list_nodes[i].path_used;

if (list_nodes[i].unsuccessful_pings == ONION_NODE_MAX_PINGS - 1
Expand All @@ -1730,18 +1730,18 @@ static void do_announce(Onion_Client *onion_c)
unsigned int num_nodes;
Node_format *path_nodes;

if (rand() % 2 == 0 || onion_c->path_nodes_index == 0) {
if (random_u08() % 2 == 0 || onion_c->path_nodes_index == 0) {
num_nodes = (onion_c->path_nodes_index_bs < MAX_PATH_NODES) ? onion_c->path_nodes_index_bs : MAX_PATH_NODES;
path_nodes = onion_c->path_nodes_bs;
} else {
num_nodes = (onion_c->path_nodes_index < MAX_PATH_NODES) ? onion_c->path_nodes_index : MAX_PATH_NODES;
path_nodes = onion_c->path_nodes;
}

if (count <= (uint32_t)rand() % MAX_ONION_CLIENTS_ANNOUNCE) {
if (count <= random_u32() % MAX_ONION_CLIENTS_ANNOUNCE) {
if (num_nodes != 0) {
for (i = 0; i < (MAX_ONION_CLIENTS_ANNOUNCE / 2); ++i) {
unsigned int num = rand() % num_nodes;
const uint32_t num = random_u32() % num_nodes;
client_send_announce_request(onion_c, 0, path_nodes[num].ip_port, path_nodes[num].public_key, nullptr, ~0);
}
}
Expand Down

0 comments on commit 463cbcb

Please sign in to comment.