Skip to content

Commit

Permalink
use crypto_memzero to wipe secret keys when no longer in use
Browse files Browse the repository at this point in the history
  • Loading branch information
JFreegman committed Dec 10, 2021
1 parent b3c757e commit 76f9fee
Show file tree
Hide file tree
Showing 6 changed files with 37 additions and 6 deletions.
16 changes: 13 additions & 3 deletions toxcore/DHT.c
Original file line number Diff line number Diff line change
Expand Up @@ -229,10 +229,10 @@ static unsigned int bit_by_bit_cmp(const uint8_t *pk1, const uint8_t *pk2)
}

/* Shared key generations are costly, it is therefore smart to store commonly used
* ones so that they can re used later without being computed again.
* ones so that they can be re-used later without being computed again.
*
* If shared key is already in shared_keys, copy it to shared_key.
* else generate it into shared_key and copy it to shared_keys
* If a shared key is already in shared_keys, copy it to shared_key.
* Otherwise generate it into shared_key and copy it to shared_keys
*/
void get_shared_key(const Mono_Time *mono_time, Shared_Keys *shared_keys, uint8_t *shared_key,
const uint8_t *secret_key, const uint8_t *public_key)
Expand Down Expand Up @@ -1342,6 +1342,8 @@ static int getnodes(DHT *dht, IP_Port ip_port, const uint8_t *public_key, const
const int len = dht_create_packet(dht->self_public_key, shared_key, NET_PACKET_GET_NODES,
plain, sizeof(plain), data);

crypto_memzero(shared_key, sizeof(shared_key));

if (len != sizeof(data)) {
return -1;
}
Expand Down Expand Up @@ -1423,13 +1425,16 @@ static int handle_getnodes(void *object, IP_Port source, const uint8_t *packet,
plain);

if (len != CRYPTO_NODE_SIZE) {
crypto_memzero(shared_key, sizeof(shared_key));
return true;
}

sendnodes_ipv6(dht, source, packet + 1, plain, plain + CRYPTO_PUBLIC_KEY_SIZE, sizeof(uint64_t), shared_key);

ping_add(dht->ping, packet + 1, source);

crypto_memzero(shared_key, sizeof(shared_key));

return false;
}

Expand Down Expand Up @@ -1492,6 +1497,8 @@ static int handle_sendnodes_core(void *object, IP_Port source, const uint8_t *pa
1 + data_size + sizeof(uint64_t) + CRYPTO_MAC_SIZE,
plain);

crypto_memzero(shared_key, sizeof(shared_key));

if ((unsigned int)len != SIZEOF_VLA(plain)) {
return 1;
}
Expand Down Expand Up @@ -2787,6 +2794,9 @@ void kill_dht(DHT *dht)
ping_kill(dht->ping);
free(dht->friends_list);
free(dht->loaded_nodes_list);
crypto_memzero(&dht->shared_keys_recv, sizeof(dht->shared_keys_recv));
crypto_memzero(&dht->shared_keys_sent, sizeof(dht->shared_keys_sent));
crypto_memzero(dht->self_secret_key, sizeof(dht->self_secret_key));
free(dht);
}

Expand Down
6 changes: 3 additions & 3 deletions toxcore/DHT.h
Original file line number Diff line number Diff line change
Expand Up @@ -225,10 +225,10 @@ const uint8_t *dht_get_friend_public_key(const DHT *dht, uint32_t friend_num);
/*----------------------------------------------------------------------------------*/

/* Shared key generations are costly, it is therefore smart to store commonly used
* ones so that they can re used later without being computed again.
* ones so that they can be re-used later without being computed again.
*
* If shared key is already in shared_keys, copy it to shared_key.
* else generate it into shared_key and copy it to shared_keys
* If a shared key is already in shared_keys, copy it to shared_key.
* Otherwise generate it into shared_key and copy it to shared_keys
*/
void get_shared_key(const Mono_Time *mono_time, Shared_Keys *shared_keys, uint8_t *shared_key,
const uint8_t *secret_key, const uint8_t *public_key);
Expand Down
2 changes: 2 additions & 0 deletions toxcore/TCP_connection.c
Original file line number Diff line number Diff line change
Expand Up @@ -1495,6 +1495,8 @@ void kill_tcp_connections(TCP_Connections *tcp_c)
kill_TCP_connection(tcp_c->tcp_connections[i].connection);
}

crypto_memzero(tcp_c->self_secret_key, sizeof(tcp_c->self_secret_key));

free(tcp_c->tcp_connections);
free(tcp_c->connections);
free(tcp_c);
Expand Down
7 changes: 7 additions & 0 deletions toxcore/TCP_server.c
Original file line number Diff line number Diff line change
Expand Up @@ -583,6 +583,7 @@ static int handle_TCP_handshake(TCP_Secure_Connection *con, const uint8_t *data,
data + CRYPTO_PUBLIC_KEY_SIZE + CRYPTO_NONCE_SIZE, TCP_HANDSHAKE_PLAIN_SIZE + CRYPTO_MAC_SIZE, plain);

if (len != TCP_HANDSHAKE_PLAIN_SIZE) {
crypto_memzero(shared_key, sizeof(shared_key));
return -1;
}

Expand All @@ -601,6 +602,7 @@ static int handle_TCP_handshake(TCP_Secure_Connection *con, const uint8_t *data,
response + CRYPTO_NONCE_SIZE);

if (len != TCP_HANDSHAKE_PLAIN_SIZE + CRYPTO_MAC_SIZE) {
crypto_memzero(shared_key, sizeof(shared_key));
return -1;
}

Expand All @@ -610,6 +612,9 @@ static int handle_TCP_handshake(TCP_Secure_Connection *con, const uint8_t *data,

encrypt_precompute(plain, temp_secret_key, con->shared_key);
con->status = TCP_STATUS_UNCONFIRMED;

crypto_memzero(shared_key, sizeof(shared_key));

return 1;
}

Expand Down Expand Up @@ -1475,6 +1480,8 @@ void kill_TCP_server(TCP_Server *tcp_server)

free_accepted_connection_array(tcp_server);

crypto_memzero(tcp_server->secret_key, sizeof(tcp_server->secret_key));

free(tcp_server->socks_listening);
free(tcp_server);
}
4 changes: 4 additions & 0 deletions toxcore/onion.c
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,8 @@ int create_onion_path(const DHT *dht, Onion_Path *new_path, const Node_format *n
encrypt_precompute(nodes[2].public_key, random_secret_key, new_path->shared_key3);
memcpy(new_path->public_key3, random_public_key, CRYPTO_PUBLIC_KEY_SIZE);

crypto_memzero(random_secret_key, sizeof(random_secret_key));

new_path->ip_port1 = nodes[0].ip_port;
new_path->ip_port2 = nodes[1].ip_port;
new_path->ip_port3 = nodes[2].ip_port;
Expand Down Expand Up @@ -694,5 +696,7 @@ void kill_onion(Onion *onion)
networking_registerhandler(onion->net, NET_PACKET_ONION_RECV_2, nullptr, nullptr);
networking_registerhandler(onion->net, NET_PACKET_ONION_RECV_1, nullptr, nullptr);

crypto_memzero(onion->secret_symmetric_key, sizeof(onion->secret_symmetric_key));

free(onion);
}
8 changes: 8 additions & 0 deletions toxcore/ping.c
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ int32_t ping_send_request(Ping *ping, IP_Port ipp, const uint8_t *public_key)
ping_id = ping_array_add(ping->ping_array, ping->mono_time, data, sizeof(data));

if (ping_id == 0) {
crypto_memzero(shared_key, sizeof(shared_key));
return 1;
}

Expand All @@ -83,6 +84,8 @@ int32_t ping_send_request(Ping *ping, IP_Port ipp, const uint8_t *public_key)
ping_plain, sizeof(ping_plain),
pk + 1 + CRYPTO_PUBLIC_KEY_SIZE + CRYPTO_NONCE_SIZE);

crypto_memzero(shared_key, sizeof(shared_key));

if (rc != PING_PLAIN_SIZE + CRYPTO_MAC_SIZE) {
return 1;
}
Expand Down Expand Up @@ -148,6 +151,7 @@ static int handle_ping_request(void *object, IP_Port source, const uint8_t *pack
ping_plain);

if (rc != sizeof(ping_plain)) {
crypto_memzero(shared_key, sizeof(shared_key));
return 1;
}

Expand All @@ -161,6 +165,8 @@ static int handle_ping_request(void *object, IP_Port source, const uint8_t *pack
ping_send_response(ping, source, packet + 1, ping_id, shared_key);
ping_add(ping, packet + 1, source);

crypto_memzero(shared_key, sizeof(shared_key));

return 0;
}

Expand Down Expand Up @@ -192,6 +198,8 @@ static int handle_ping_response(void *object, IP_Port source, const uint8_t *pac
PING_PLAIN_SIZE + CRYPTO_MAC_SIZE,
ping_plain);

crypto_memzero(shared_key, sizeof(shared_key));

if (rc != sizeof(ping_plain)) {
return 1;
}
Expand Down

0 comments on commit 76f9fee

Please sign in to comment.