From 346b9f17ba45a843804da93205bd8fe7bcd90776 Mon Sep 17 00:00:00 2001 From: jfreegman Date: Fri, 10 Dec 2021 16:31:34 -0500 Subject: [PATCH] use crypto_memzero to wipe secret keys when no longer in use --- toxcore/DHT.c | 16 +++++++++++++--- toxcore/DHT.h | 6 +++--- toxcore/TCP_connection.c | 2 ++ toxcore/TCP_server.c | 8 ++++++++ toxcore/onion.c | 4 ++++ toxcore/ping.c | 9 +++++++++ 6 files changed, 39 insertions(+), 6 deletions(-) diff --git a/toxcore/DHT.c b/toxcore/DHT.c index d4a6c6dc69..bd8a057ccc 100644 --- a/toxcore/DHT.c +++ b/toxcore/DHT.c @@ -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) @@ -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; } @@ -1423,6 +1425,7 @@ 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; } @@ -1430,6 +1433,8 @@ static int handle_getnodes(void *object, IP_Port source, const uint8_t *packet, ping_add(dht->ping, packet + 1, source); + crypto_memzero(shared_key, sizeof(shared_key)); + return false; } @@ -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; } @@ -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); } diff --git a/toxcore/DHT.h b/toxcore/DHT.h index fcc67251f6..835ce727fa 100644 --- a/toxcore/DHT.h +++ b/toxcore/DHT.h @@ -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); diff --git a/toxcore/TCP_connection.c b/toxcore/TCP_connection.c index 6ca33c8ef9..cb4b1ae58b 100644 --- a/toxcore/TCP_connection.c +++ b/toxcore/TCP_connection.c @@ -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); diff --git a/toxcore/TCP_server.c b/toxcore/TCP_server.c index 79a9e560c3..2f9a69acd3 100644 --- a/toxcore/TCP_server.c +++ b/toxcore/TCP_server.c @@ -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; } @@ -601,15 +602,20 @@ 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; } if (TCP_SERVER_HANDSHAKE_SIZE != net_send(con->sock, response, TCP_SERVER_HANDSHAKE_SIZE)) { + crypto_memzero(shared_key, sizeof(shared_key)); return -1; } encrypt_precompute(plain, temp_secret_key, con->shared_key); con->status = TCP_STATUS_UNCONFIRMED; + + crypto_memzero(shared_key, sizeof(shared_key)); + return 1; } @@ -1475,6 +1481,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); } diff --git a/toxcore/onion.c b/toxcore/onion.c index 89c15b4c99..8ed0ab1963 100644 --- a/toxcore/onion.c +++ b/toxcore/onion.c @@ -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; @@ -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); } diff --git a/toxcore/ping.c b/toxcore/ping.c index 305cce47c4..1d91ba38f7 100644 --- a/toxcore/ping.c +++ b/toxcore/ping.c @@ -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; } @@ -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; } @@ -148,10 +151,12 @@ 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; } if (ping_plain[0] != NET_PACKET_PING_REQUEST) { + crypto_memzero(shared_key, sizeof(shared_key)); return 1; } @@ -161,6 +166,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; } @@ -192,6 +199,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; }