From 8c7d30475f308c086b90632d7198442fcac0bead Mon Sep 17 00:00:00 2001 From: sudden6 Date: Mon, 23 May 2022 00:38:40 +0200 Subject: [PATCH] refactor: re-implement shared key cache in separate file The exisiting implementation is not clearly documented and used by multiple modules. --- CMakeLists.txt | 2 + .../docker/tox-bootstrapd.sha256 | 2 +- toxcore/BUILD.bazel | 22 +++ toxcore/DHT.c | 105 +++--------- toxcore/DHT.h | 34 +--- toxcore/Makefile.inc | 2 + toxcore/announce.c | 27 ++- toxcore/net_crypto.c | 7 +- toxcore/onion.c | 55 +++++- toxcore/onion.h | 7 +- toxcore/onion_announce.c | 23 ++- toxcore/ping.c | 20 +-- toxcore/shared_key_cache.c | 161 ++++++++++++++++++ toxcore/shared_key_cache.h | 51 ++++++ 14 files changed, 364 insertions(+), 154 deletions(-) create mode 100644 toxcore/shared_key_cache.c create mode 100644 toxcore/shared_key_cache.h diff --git a/CMakeLists.txt b/CMakeLists.txt index bf3d433d..f6228e4e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -284,6 +284,8 @@ set(toxcore_SOURCES toxcore/ping_array.h toxcore/ping.c toxcore/ping.h + toxcore/shared_key_cache.c + toxcore/shared_key_cache.h toxcore/state.c toxcore/state.h toxcore/TCP_client.c diff --git a/other/bootstrap_daemon/docker/tox-bootstrapd.sha256 b/other/bootstrap_daemon/docker/tox-bootstrapd.sha256 index fa4a0816..52982fe9 100644 --- a/other/bootstrap_daemon/docker/tox-bootstrapd.sha256 +++ b/other/bootstrap_daemon/docker/tox-bootstrapd.sha256 @@ -1 +1 @@ -b2bee729be40b0a0d8a4c6f0e2a527854d9a9d37712b73b915d7470bc91f5e6a /usr/local/bin/tox-bootstrapd +769233afaac07be03c094411e6ec8f031bde41beae475c74e154e51e51e9168b /usr/local/bin/tox-bootstrapd diff --git a/toxcore/BUILD.bazel b/toxcore/BUILD.bazel index 489f8109..8ac0594a 100644 --- a/toxcore/BUILD.bazel +++ b/toxcore/BUILD.bazel @@ -180,6 +180,24 @@ cc_test( ], ) +cc_library( + name = "shared_key_cache", + srcs = ["shared_key_cache.c"], + hdrs = ["shared_key_cache.h"], + visibility = [ + "//c-toxcore/auto_tests:__pkg__", + "//c-toxcore/other:__pkg__", + "//c-toxcore/other/bootstrap_daemon:__pkg__", + "//c-toxcore/testing:__pkg__", + "//c-toxcore/toxav:__pkg__", + ], + deps = [ + ":ccompat", + ":crypto_core", + ":mono_time", + ], +) + cc_library( name = "network", srcs = ["network.c"], @@ -289,6 +307,7 @@ cc_library( ":mono_time", ":network", ":ping_array", + ":shared_key_cache", ":state", ":util", ], @@ -326,6 +345,7 @@ cc_library( ":ccompat", ":crypto_core", ":mono_time", + ":shared_key_cache", ":util", ], ) @@ -366,6 +386,7 @@ cc_library( ":LAN_discovery", ":ccompat", ":forwarding", + ":shared_key_cache", ":timed_auth", ":util", ], @@ -480,6 +501,7 @@ cc_library( ":ccompat", ":mono_time", ":onion", + ":shared_key_cache", ":timed_auth", ":util", ], diff --git a/toxcore/DHT.c b/toxcore/DHT.c index 56198ff1..ffb89e1e 100644 --- a/toxcore/DHT.c +++ b/toxcore/DHT.c @@ -18,6 +18,7 @@ #include "mono_time.h" #include "network.h" #include "ping.h" +#include "shared_key_cache.h" #include "state.h" #include "util.h" @@ -46,6 +47,10 @@ // TODO(sudden6): find out why we need multiple callbacks and if we really need 32 #define DHT_FRIEND_MAX_LOCKS 32 +/* Settings for the shared key cache */ +#define MAX_KEYS_PER_SLOT 4 +#define KEYS_TIMEOUT 600 + typedef struct DHT_Friend_Callback { dht_ip_cb *ip_callback; void *data; @@ -107,8 +112,8 @@ struct DHT { uint32_t loaded_num_nodes; unsigned int loaded_nodes_index; - Shared_Keys shared_keys_recv; - Shared_Keys shared_keys_sent; + Shared_Key_Cache *shared_keys_recv; + Shared_Key_Cache *shared_keys_sent; struct Ping *ping; Ping_Array *dht_ping_array; @@ -255,74 +260,22 @@ unsigned int bit_by_bit_cmp(const uint8_t *pk1, const uint8_t *pk2) return i * 8 + j; } -/** - * Shared key generations are costly, it is therefore smart to store commonly used - * ones so that they can be re-used later without being computed again. - * - * 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) -{ - uint32_t num = -1; - uint32_t curr = 0; - - for (uint32_t i = 0; i < MAX_KEYS_PER_SLOT; ++i) { - const int index = public_key[30] * MAX_KEYS_PER_SLOT + i; - Shared_Key *const key = &shared_keys->keys[index]; - - if (key->stored) { - if (pk_equal(public_key, key->public_key)) { - memcpy(shared_key, key->shared_key, CRYPTO_SHARED_KEY_SIZE); - ++key->times_requested; - key->time_last_requested = mono_time_get(mono_time); - return; - } - - if (num != 0) { - if (mono_time_is_timeout(mono_time, key->time_last_requested, KEYS_TIMEOUT)) { - num = 0; - curr = index; - } else if (num > key->times_requested) { - num = key->times_requested; - curr = index; - } - } - } else if (num != 0) { - num = 0; - curr = index; - } - } - - encrypt_precompute(public_key, secret_key, shared_key); - - if (num != UINT32_MAX) { - Shared_Key *const key = &shared_keys->keys[curr]; - key->stored = true; - key->times_requested = 1; - memcpy(key->public_key, public_key, CRYPTO_PUBLIC_KEY_SIZE); - memcpy(key->shared_key, shared_key, CRYPTO_SHARED_KEY_SIZE); - key->time_last_requested = mono_time_get(mono_time); - } -} - /** * Copy shared_key to encrypt/decrypt DHT packet from public_key into shared_key * for packets that we receive. */ -void dht_get_shared_key_recv(DHT *dht, uint8_t *shared_key, const uint8_t *public_key) +const uint8_t *dht_get_shared_key_recv(DHT *dht, const uint8_t *public_key) { - get_shared_key(dht->mono_time, &dht->shared_keys_recv, shared_key, dht->self_secret_key, public_key); + return shared_key_cache_lookup(dht->shared_keys_recv, public_key); } /** * Copy shared_key to encrypt/decrypt DHT packet from public_key into shared_key * for packets that we send. */ -void dht_get_shared_key_sent(DHT *dht, uint8_t *shared_key, const uint8_t *public_key) +const uint8_t *dht_get_shared_key_sent(DHT *dht, const uint8_t *public_key) { - get_shared_key(dht->mono_time, &dht->shared_keys_sent, shared_key, dht->self_secret_key, public_key); + return shared_key_cache_lookup(dht->shared_keys_sent, public_key); } #define CRYPTO_SIZE (1 + CRYPTO_PUBLIC_KEY_SIZE * 2 + CRYPTO_NONCE_SIZE) @@ -1063,8 +1016,7 @@ static bool send_announce_ping(DHT *dht, const uint8_t *public_key, const IP_Por public_key, CRYPTO_PUBLIC_KEY_SIZE); memcpy(plain + CRYPTO_PUBLIC_KEY_SIZE, &ping_id, sizeof(ping_id)); - uint8_t shared_key[CRYPTO_SHARED_KEY_SIZE]; - dht_get_shared_key_sent(dht, shared_key, public_key); + const uint8_t *shared_key = dht_get_shared_key_sent(dht, public_key); uint8_t request[1 + CRYPTO_PUBLIC_KEY_SIZE + CRYPTO_NONCE_SIZE + sizeof(plain) + CRYPTO_MAC_SIZE]; @@ -1092,8 +1044,7 @@ static int handle_data_search_response(void *object, const IP_Port *source, VLA(uint8_t, plain, plain_len); const uint8_t *public_key = packet + 1; - uint8_t shared_key[CRYPTO_SHARED_KEY_SIZE]; - dht_get_shared_key_recv(dht, shared_key, public_key); + const uint8_t *shared_key = dht_get_shared_key_recv(dht, public_key); if (decrypt_data_symmetric(shared_key, packet + 1 + CRYPTO_PUBLIC_KEY_SIZE, @@ -1522,15 +1473,12 @@ bool dht_getnodes(DHT *dht, const IP_Port *ip_port, const uint8_t *public_key, c memcpy(plain, client_id, CRYPTO_PUBLIC_KEY_SIZE); memcpy(plain + CRYPTO_PUBLIC_KEY_SIZE, &ping_id, sizeof(ping_id)); - uint8_t shared_key[CRYPTO_SHARED_KEY_SIZE]; - dht_get_shared_key_sent(dht, shared_key, public_key); + const uint8_t *shared_key = dht_get_shared_key_sent(dht, public_key); const int len = dht_create_packet(dht->rng, dht->self_public_key, shared_key, NET_PACKET_GET_NODES, plain, sizeof(plain), data, sizeof(data)); - crypto_memzero(shared_key, sizeof(shared_key)); - if (len != sizeof(data)) { LOGGER_ERROR(dht->log, "getnodes packet encryption failed"); return false; @@ -1605,9 +1553,7 @@ static int handle_getnodes(void *object, const IP_Port *source, const uint8_t *p } uint8_t plain[CRYPTO_NODE_SIZE]; - uint8_t shared_key[CRYPTO_SHARED_KEY_SIZE]; - - dht_get_shared_key_recv(dht, shared_key, packet + 1); + const uint8_t *shared_key = dht_get_shared_key_recv(dht, packet + 1); const int len = decrypt_data_symmetric( shared_key, packet + 1 + CRYPTO_PUBLIC_KEY_SIZE, @@ -1616,7 +1562,6 @@ static int handle_getnodes(void *object, const IP_Port *source, const uint8_t *p plain); if (len != CRYPTO_NODE_SIZE) { - crypto_memzero(shared_key, sizeof(shared_key)); return 1; } @@ -1624,8 +1569,6 @@ static int handle_getnodes(void *object, const IP_Port *source, const uint8_t *p ping_add(dht->ping, packet + 1, source); - crypto_memzero(shared_key, sizeof(shared_key)); - return 0; } @@ -1670,8 +1613,7 @@ static bool handle_sendnodes_core(void *object, const IP_Port *source, const uin } VLA(uint8_t, plain, 1 + data_size + sizeof(uint64_t)); - uint8_t shared_key[CRYPTO_SHARED_KEY_SIZE]; - dht_get_shared_key_sent(dht, shared_key, packet + 1); + const uint8_t *shared_key = dht_get_shared_key_sent(dht, packet + 1); const int len = decrypt_data_symmetric( shared_key, packet + 1 + CRYPTO_PUBLIC_KEY_SIZE, @@ -1679,8 +1621,6 @@ static bool handle_sendnodes_core(void *object, const IP_Port *source, const uin 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 false; } @@ -2796,6 +2736,15 @@ DHT *new_dht(const Logger *log, const Random *rng, const Network *ns, Mono_Time crypto_new_keypair(rng, dht->self_public_key, dht->self_secret_key); + dht->shared_keys_recv = shared_key_cache_new(mono_time, dht->self_secret_key, KEYS_TIMEOUT, MAX_KEYS_PER_SLOT); + dht->shared_keys_sent = shared_key_cache_new(mono_time, dht->self_secret_key, KEYS_TIMEOUT, MAX_KEYS_PER_SLOT); + + if (dht->shared_keys_recv == nullptr || dht->shared_keys_sent == nullptr) { + kill_dht(dht); + return nullptr; + } + + dht->dht_ping_array = ping_array_new(DHT_PING_ARRAY_SIZE, PING_TIMEOUT); if (dht->dht_ping_array == nullptr) { @@ -2858,12 +2807,12 @@ void kill_dht(DHT *dht) networking_registerhandler(dht->net, NET_PACKET_LAN_DISCOVERY, nullptr, nullptr); cryptopacket_registerhandler(dht, CRYPTO_PACKET_NAT_PING, nullptr, nullptr); + shared_key_cache_free(dht->shared_keys_recv); + shared_key_cache_free(dht->shared_keys_sent); ping_array_kill(dht->dht_ping_array); 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 ee6470ab..86ac4f9d 100644 --- a/toxcore/DHT.h +++ b/toxcore/DHT.h @@ -254,24 +254,6 @@ non_null(1, 4) nullable(3) int unpack_nodes(Node_format *nodes, uint16_t max_num_nodes, uint16_t *processed_data_len, const uint8_t *data, uint16_t length, bool tcp_enabled); - -/*----------------------------------------------------------------------------------*/ -/* struct to store some shared keys so we don't have to regenerate them for each request. */ -#define MAX_KEYS_PER_SLOT 4 -#define KEYS_TIMEOUT 600 - -typedef struct Shared_Key { - uint8_t public_key[CRYPTO_PUBLIC_KEY_SIZE]; - uint8_t shared_key[CRYPTO_SHARED_KEY_SIZE]; - uint32_t times_requested; - bool stored; - uint64_t time_last_requested; -} Shared_Key; - -typedef struct Shared_Keys { - Shared_Key keys[256 * MAX_KEYS_PER_SLOT]; -} Shared_Keys; - /*----------------------------------------------------------------------------------*/ typedef int cryptopacket_handler_cb(void *object, const IP_Port *ip_port, const uint8_t *source_pubkey, @@ -295,31 +277,19 @@ non_null() const uint8_t *dht_get_friend_public_key(const DHT *dht, uint32_t fri /*----------------------------------------------------------------------------------*/ -/** - * Shared key generations are costly, it is therefore smart to store commonly used - * ones so that they can be re-used later without being computed again. - * - * 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 - */ -non_null() -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); - /** * Copy shared_key to encrypt/decrypt DHT packet from public_key into shared_key * for packets that we receive. */ non_null() -void dht_get_shared_key_recv(DHT *dht, uint8_t *shared_key, const uint8_t *public_key); +const uint8_t *dht_get_shared_key_recv(DHT *dht, const uint8_t *public_key); /** * Copy shared_key to encrypt/decrypt DHT packet from public_key into shared_key * for packets that we send. */ non_null() -void dht_get_shared_key_sent(DHT *dht, uint8_t *shared_key, const uint8_t *public_key); +const uint8_t *dht_get_shared_key_sent(DHT *dht, const uint8_t *public_key); /** * Sends a getnodes request to `ip_port` with the public key `public_key` for nodes diff --git a/toxcore/Makefile.inc b/toxcore/Makefile.inc index c7a01dc3..e5f0ddf9 100644 --- a/toxcore/Makefile.inc +++ b/toxcore/Makefile.inc @@ -61,6 +61,8 @@ libtoxcore_la_SOURCES = ../third_party/cmp/cmp.c \ ../toxcore/Messenger.c \ ../toxcore/ping.h \ ../toxcore/ping.c \ + ../toxcore/shared_key_cache.h \ + ../toxcore/shared_key_cache.c \ ../toxcore/state.h \ ../toxcore/state.c \ ../toxcore/tox.h \ diff --git a/toxcore/announce.c b/toxcore/announce.c index 7914d196..647bd21e 100644 --- a/toxcore/announce.c +++ b/toxcore/announce.c @@ -14,9 +14,14 @@ #include "LAN_discovery.h" #include "ccompat.h" +#include "shared_key_cache.h" #include "timed_auth.h" #include "util.h" +// Settings for the shared key cache +#define MAX_KEYS_PER_SLOT 4 +#define KEYS_TIMEOUT 600 + uint8_t announce_response_of_request_type(uint8_t request_type) { switch (request_type) { @@ -53,7 +58,7 @@ struct Announcements { const uint8_t *public_key; const uint8_t *secret_key; - Shared_Keys shared_keys; + Shared_Key_Cache *shared_keys; uint8_t hmac_key[CRYPTO_HMAC_KEY_SIZE]; int32_t synch_offset; @@ -429,10 +434,13 @@ static int create_reply_plain_store_announce_request(Announcements *announce, } VLA(uint8_t, plain, plain_len); - uint8_t shared_key[CRYPTO_SHARED_KEY_SIZE]; - get_shared_key(announce->mono_time, &announce->shared_keys, shared_key, - announce->secret_key, data_public_key); + const uint8_t* shared_key = shared_key_cache_lookup(announce->shared_keys, data_public_key); + + if (shared_key == nullptr) { + /* Error looking up/deriving the shared key */ + return -1; + } if (decrypt_data_symmetric(shared_key, data + CRYPTO_PUBLIC_KEY_SIZE, @@ -549,9 +557,7 @@ static int create_reply(Announcements *announce, const IP_Port *source, } VLA(uint8_t, plain, plain_len); - uint8_t shared_key[CRYPTO_SHARED_KEY_SIZE]; - - dht_get_shared_key_recv(announce->dht, shared_key, data + 1); + const uint8_t *shared_key = dht_get_shared_key_recv(announce->dht, data + 1); if (decrypt_data_symmetric(shared_key, data + 1 + CRYPTO_PUBLIC_KEY_SIZE, @@ -652,6 +658,11 @@ Announcements *new_announcements(const Logger *log, const Random *rng, const Mon announce->public_key = dht_get_self_public_key(announce->dht); announce->secret_key = dht_get_self_secret_key(announce->dht); new_hmac_key(announce->rng, announce->hmac_key); + announce->shared_keys = shared_key_cache_new(mono_time, announce->secret_key, KEYS_TIMEOUT, MAX_KEYS_PER_SLOT); + if (announce->shared_keys == nullptr) { + free(announce); + return nullptr; + } announce->start_time = mono_time_get(announce->mono_time); @@ -677,7 +688,7 @@ void kill_announcements(Announcements *announce) networking_registerhandler(announce->net, NET_PACKET_STORE_ANNOUNCE_REQUEST, nullptr, nullptr); crypto_memzero(announce->hmac_key, CRYPTO_HMAC_KEY_SIZE); - crypto_memzero(&announce->shared_keys, sizeof(Shared_Keys)); + shared_key_cache_free(announce->shared_keys); for (uint32_t i = 0; i < ANNOUNCE_BUCKETS * ANNOUNCE_BUCKET_SIZE; ++i) { if (announce->entries[i].data != nullptr) { diff --git a/toxcore/net_crypto.c b/toxcore/net_crypto.c index f649df46..71f6e39e 100644 --- a/toxcore/net_crypto.c +++ b/toxcore/net_crypto.c @@ -223,8 +223,8 @@ static int create_cookie_request(const Net_Crypto *c, uint8_t *packet, const uin memcpy(plain, c->self_public_key, CRYPTO_PUBLIC_KEY_SIZE); memcpy(plain + CRYPTO_PUBLIC_KEY_SIZE, padding, CRYPTO_PUBLIC_KEY_SIZE); memcpy(plain + (CRYPTO_PUBLIC_KEY_SIZE * 2), &number, sizeof(uint64_t)); - - dht_get_shared_key_sent(c->dht, shared_key, dht_public_key); + const uint8_t *tmp_shared_key = dht_get_shared_key_sent(c->dht, dht_public_key); + memcpy(shared_key, tmp_shared_key, CRYPTO_SHARED_KEY_SIZE); uint8_t nonce[CRYPTO_NONCE_SIZE]; random_nonce(c->rng, nonce); packet[0] = NET_PACKET_COOKIE_REQUEST; @@ -341,7 +341,8 @@ static int handle_cookie_request(const Net_Crypto *c, uint8_t *request_plain, ui } memcpy(dht_public_key, packet + 1, CRYPTO_PUBLIC_KEY_SIZE); - dht_get_shared_key_sent(c->dht, shared_key, dht_public_key); + const uint8_t *tmp_shared_key = dht_get_shared_key_sent(c->dht, dht_public_key); + memcpy(shared_key, tmp_shared_key, CRYPTO_SHARED_KEY_SIZE); const int len = decrypt_data_symmetric(shared_key, packet + 1 + CRYPTO_PUBLIC_KEY_SIZE, packet + 1 + CRYPTO_PUBLIC_KEY_SIZE + CRYPTO_NONCE_SIZE, COOKIE_REQUEST_PLAIN_LENGTH + CRYPTO_MAC_SIZE, request_plain); diff --git a/toxcore/onion.c b/toxcore/onion.c index f0e4e1db..63ff6d85 100644 --- a/toxcore/onion.c +++ b/toxcore/onion.c @@ -27,6 +27,11 @@ #define KEY_REFRESH_INTERVAL (2 * 60 * 60) + +// Settings for the shared key cache +#define MAX_KEYS_PER_SLOT 4 +#define KEYS_TIMEOUT 600 + /** Change symmetric keys every 2 hours to make paths expire eventually. */ non_null() static void change_symmetric_key(Onion *onion) @@ -314,9 +319,14 @@ static int handle_send_initial(void *object, const IP_Port *source, const uint8_ change_symmetric_key(onion); uint8_t plain[ONION_MAX_PACKET_SIZE]; - uint8_t shared_key[CRYPTO_SHARED_KEY_SIZE]; - get_shared_key(onion->mono_time, &onion->shared_keys_1, shared_key, dht_get_self_secret_key(onion->dht), - packet + 1 + CRYPTO_NONCE_SIZE); + const uint8_t *public_key = packet + 1 + CRYPTO_NONCE_SIZE; + const uint8_t *shared_key = shared_key_cache_lookup(onion->shared_keys_1, public_key); + + if (shared_key == nullptr) { + /* Error looking up/deriving the shared key */ + return 1; + } + const int len = decrypt_data_symmetric(shared_key, packet + 1, packet + 1 + CRYPTO_NONCE_SIZE + CRYPTO_PUBLIC_KEY_SIZE, length - (1 + CRYPTO_NONCE_SIZE + CRYPTO_PUBLIC_KEY_SIZE), plain); @@ -385,9 +395,14 @@ static int handle_send_1(void *object, const IP_Port *source, const uint8_t *pac change_symmetric_key(onion); uint8_t plain[ONION_MAX_PACKET_SIZE]; - uint8_t shared_key[CRYPTO_SHARED_KEY_SIZE]; - get_shared_key(onion->mono_time, &onion->shared_keys_2, shared_key, dht_get_self_secret_key(onion->dht), - packet + 1 + CRYPTO_NONCE_SIZE); + const uint8_t *public_key = packet + 1 + CRYPTO_NONCE_SIZE; + const uint8_t *shared_key = shared_key_cache_lookup(onion->shared_keys_2, public_key); + + if (shared_key == nullptr) { + /* Error looking up/deriving the shared key */ + return 1; + } + int len = decrypt_data_symmetric(shared_key, packet + 1, packet + 1 + CRYPTO_NONCE_SIZE + CRYPTO_PUBLIC_KEY_SIZE, length - (1 + CRYPTO_NONCE_SIZE + CRYPTO_PUBLIC_KEY_SIZE + RETURN_1), plain); @@ -443,9 +458,14 @@ static int handle_send_2(void *object, const IP_Port *source, const uint8_t *pac change_symmetric_key(onion); uint8_t plain[ONION_MAX_PACKET_SIZE]; - uint8_t shared_key[CRYPTO_SHARED_KEY_SIZE]; - get_shared_key(onion->mono_time, &onion->shared_keys_3, shared_key, dht_get_self_secret_key(onion->dht), - packet + 1 + CRYPTO_NONCE_SIZE); + const uint8_t *public_key = packet + 1 + CRYPTO_NONCE_SIZE; + const uint8_t *shared_key = shared_key_cache_lookup(onion->shared_keys_3, public_key); + + if (shared_key == nullptr) { + /* Error looking up/deriving the shared key */ + return 1; + } + int len = decrypt_data_symmetric(shared_key, packet + 1, packet + 1 + CRYPTO_NONCE_SIZE + CRYPTO_PUBLIC_KEY_SIZE, length - (1 + CRYPTO_NONCE_SIZE + CRYPTO_PUBLIC_KEY_SIZE + RETURN_2), plain); @@ -668,6 +688,19 @@ Onion *new_onion(const Logger *log, const Mono_Time *mono_time, const Random *rn new_symmetric_key(rng, onion->secret_symmetric_key); onion->timestamp = mono_time_get(onion->mono_time); + const uint8_t *secret_key = dht_get_self_secret_key(dht); + onion->shared_keys_1 = shared_key_cache_new(mono_time, secret_key, KEYS_TIMEOUT, MAX_KEYS_PER_SLOT); + onion->shared_keys_2 = shared_key_cache_new(mono_time, secret_key, KEYS_TIMEOUT, MAX_KEYS_PER_SLOT); + onion->shared_keys_3 = shared_key_cache_new(mono_time, secret_key, KEYS_TIMEOUT, MAX_KEYS_PER_SLOT); + + if (onion->shared_keys_1 == nullptr || + onion->shared_keys_2 == nullptr || + onion->shared_keys_3 == nullptr) { + kill_onion(onion); + return nullptr; + } + + networking_registerhandler(onion->net, NET_PACKET_ONION_SEND_INITIAL, &handle_send_initial, onion); networking_registerhandler(onion->net, NET_PACKET_ONION_SEND_1, &handle_send_1, onion); networking_registerhandler(onion->net, NET_PACKET_ONION_SEND_2, &handle_send_2, onion); @@ -695,5 +728,9 @@ void kill_onion(Onion *onion) crypto_memzero(onion->secret_symmetric_key, sizeof(onion->secret_symmetric_key)); + shared_key_cache_free(onion->shared_keys_1); + shared_key_cache_free(onion->shared_keys_2); + shared_key_cache_free(onion->shared_keys_3); + free(onion); } diff --git a/toxcore/onion.h b/toxcore/onion.h index 3da2137c..5c8f920b 100644 --- a/toxcore/onion.h +++ b/toxcore/onion.h @@ -12,6 +12,7 @@ #include "DHT.h" #include "logger.h" #include "mono_time.h" +#include "shared_key_cache.h" typedef int onion_recv_1_cb(void *object, const IP_Port *dest, const uint8_t *data, uint16_t length); @@ -24,9 +25,9 @@ typedef struct Onion { uint8_t secret_symmetric_key[CRYPTO_SYMMETRIC_KEY_SIZE]; uint64_t timestamp; - Shared_Keys shared_keys_1; - Shared_Keys shared_keys_2; - Shared_Keys shared_keys_3; + Shared_Key_Cache *shared_keys_1; + Shared_Key_Cache *shared_keys_2; + Shared_Key_Cache *shared_keys_3; onion_recv_1_cb *recv_1_function; void *callback_object; diff --git a/toxcore/onion_announce.c b/toxcore/onion_announce.c index 3bf8eba3..fb04d21f 100644 --- a/toxcore/onion_announce.c +++ b/toxcore/onion_announce.c @@ -16,6 +16,7 @@ #include "LAN_discovery.h" #include "ccompat.h" #include "mono_time.h" +#include "shared_key_cache.h" #include "util.h" #define PING_ID_TIMEOUT ONION_ANNOUNCE_TIMEOUT @@ -31,6 +32,10 @@ #define ONION_MINIMAL_SIZE (ONION_PING_ID_SIZE + CRYPTO_PUBLIC_KEY_SIZE * 2 + ONION_ANNOUNCE_SENDBACK_DATA_LENGTH) +/* Settings for the shared key cache */ +#define MAX_KEYS_PER_SLOT 4 +#define KEYS_TIMEOUT 600 + static_assert(ONION_PING_ID_SIZE == CRYPTO_PUBLIC_KEY_SIZE, "announce response packets assume that ONION_PING_ID_SIZE is equal to CRYPTO_PUBLIC_KEY_SIZE"); @@ -51,7 +56,7 @@ struct Onion_Announce { Onion_Announce_Entry entries[ONION_ANNOUNCE_MAX_ENTRIES]; uint8_t hmac_key[CRYPTO_HMAC_KEY_SIZE]; - Shared_Keys shared_keys_recv; + Shared_Key_Cache *shared_keys_recv; uint16_t extra_data_max_size; pack_extra_data_cb *extra_data_callback; @@ -426,9 +431,12 @@ static int handle_announce_request_common( pack_extra_data_cb *pack_extra_data_callback) { const uint8_t *packet_public_key = packet + 1 + CRYPTO_NONCE_SIZE; - uint8_t shared_key[CRYPTO_SHARED_KEY_SIZE]; - get_shared_key(onion_a->mono_time, &onion_a->shared_keys_recv, shared_key, dht_get_self_secret_key(onion_a->dht), - packet_public_key); + const uint8_t *shared_key = shared_key_cache_lookup(onion_a->shared_keys_recv, packet_public_key); + + if (shared_key == nullptr) { + /* Error looking up/deriving the shared key */ + return 1; + } uint8_t *plain = (uint8_t *)malloc(plain_size); @@ -653,6 +661,12 @@ Onion_Announce *new_onion_announce(const Logger *log, const Random *rng, const M onion_a->extra_data_object = nullptr; new_hmac_key(rng, onion_a->hmac_key); + onion_a->shared_keys_recv = shared_key_cache_new(mono_time, dht_get_self_secret_key(dht), KEYS_TIMEOUT, MAX_KEYS_PER_SLOT); + if (onion_a->shared_keys_recv == nullptr) { + kill_onion_announce(onion_a); + return nullptr; + } + networking_registerhandler(onion_a->net, NET_PACKET_ANNOUNCE_REQUEST, &handle_announce_request, onion_a); networking_registerhandler(onion_a->net, NET_PACKET_ANNOUNCE_REQUEST_OLD, &handle_announce_request_old, onion_a); networking_registerhandler(onion_a->net, NET_PACKET_ONION_DATA_REQUEST, &handle_data_request, onion_a); @@ -671,6 +685,7 @@ void kill_onion_announce(Onion_Announce *onion_a) networking_registerhandler(onion_a->net, NET_PACKET_ONION_DATA_REQUEST, nullptr, nullptr); crypto_memzero(onion_a->hmac_key, CRYPTO_HMAC_KEY_SIZE); + shared_key_cache_free(onion_a->shared_keys_recv); free(onion_a); } diff --git a/toxcore/ping.c b/toxcore/ping.c index 91a780b1..f44777b8 100644 --- a/toxcore/ping.c +++ b/toxcore/ping.c @@ -53,10 +53,9 @@ void ping_send_request(Ping *ping, const IP_Port *ipp, const uint8_t *public_key return; } - uint8_t shared_key[CRYPTO_SHARED_KEY_SIZE]; // generate key to encrypt ping_id with recipient privkey - dht_get_shared_key_sent(ping->dht, shared_key, public_key); + const uint8_t *shared_key = dht_get_shared_key_sent(ping->dht, public_key); // Generate random ping_id. uint8_t data[PING_DATA_SIZE]; pk_copy(data, public_key); @@ -64,7 +63,6 @@ void ping_send_request(Ping *ping, const IP_Port *ipp, const uint8_t *public_key ping_id = ping_array_add(ping->ping_array, ping->mono_time, ping->rng, data, sizeof(data)); if (ping_id == 0) { - crypto_memzero(shared_key, sizeof(shared_key)); return; } @@ -82,8 +80,6 @@ void ping_send_request(Ping *ping, const 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; } @@ -139,11 +135,11 @@ static int handle_ping_request(void *object, const IP_Port *source, const uint8_ return 1; } - uint8_t shared_key[CRYPTO_SHARED_KEY_SIZE]; + const uint8_t *shared_key = dht_get_shared_key_recv(dht, packet + 1); + uint8_t ping_plain[PING_PLAIN_SIZE]; // Decrypt ping_id - dht_get_shared_key_recv(dht, shared_key, packet + 1); const int rc = decrypt_data_symmetric(shared_key, packet + 1 + CRYPTO_PUBLIC_KEY_SIZE, packet + 1 + CRYPTO_PUBLIC_KEY_SIZE + CRYPTO_NONCE_SIZE, @@ -151,12 +147,10 @@ static int handle_ping_request(void *object, const IP_Port *source, const uint8_ 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; } @@ -166,8 +160,6 @@ static int handle_ping_request(void *object, const IP_Port *source, const uint8_ 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; } @@ -188,10 +180,8 @@ static int handle_ping_response(void *object, const IP_Port *source, const uint8 return 1; } - uint8_t shared_key[CRYPTO_SHARED_KEY_SIZE]; - // generate key to encrypt ping_id with recipient privkey - dht_get_shared_key_sent(ping->dht, shared_key, packet + 1); + const uint8_t *shared_key = dht_get_shared_key_sent(ping->dht, packet + 1); uint8_t ping_plain[PING_PLAIN_SIZE]; // Decrypt ping_id @@ -201,8 +191,6 @@ static int handle_ping_response(void *object, const IP_Port *source, const uint8 PING_PLAIN_SIZE + CRYPTO_MAC_SIZE, ping_plain); - crypto_memzero(shared_key, sizeof(shared_key)); - if (rc != sizeof(ping_plain)) { return 1; } diff --git a/toxcore/shared_key_cache.c b/toxcore/shared_key_cache.c new file mode 100644 index 00000000..7846ae4f --- /dev/null +++ b/toxcore/shared_key_cache.c @@ -0,0 +1,161 @@ +/* SPDX-License-Identifier: GPL-3.0-or-later + * Copyright © 2022 The TokTok team. + */ + +#include "shared_key_cache.h" + +#include +#include +#include // calloc(...) +#include // memcpy(...) + +#include "ccompat.h" +#include "crypto_core.h" +#include "mono_time.h" + +typedef struct Shared_Key { + uint8_t public_key[CRYPTO_PUBLIC_KEY_SIZE]; + uint8_t shared_key[CRYPTO_SHARED_KEY_SIZE]; + uint64_t time_last_requested; +} Shared_Key; + +struct Shared_Key_Cache { + Shared_Key *keys; + const uint8_t* self_secret_key; + uint64_t timeout; /** After this time (in seconds), a key is erased on the next housekeeping cycle */ + const Mono_Time *time; + uint8_t keys_per_slot; +}; + +non_null() +static bool shared_key_is_empty(const Shared_Key *k) { + assert(k != nullptr); + /* + * Since time can never be 0, we use that to determine if a key slot is empty. + * Additionally this allows us to use crypto_memzero and leave the slot in a valid state. + */ + return k->time_last_requested == 0; +} + +non_null() +static void shared_key_set_empty(Shared_Key *k) { + crypto_memzero(k, sizeof (Shared_Key)); + assert(shared_key_is_empty(k)); +} + +Shared_Key_Cache *shared_key_cache_new(const Mono_Time *time, const uint8_t *self_secret_key, uint64_t timeout, uint8_t keys_per_slot) +{ + if (time == nullptr || self_secret_key == nullptr || timeout == 0 || keys_per_slot == 0) { + return nullptr; + } + + // Time must not be zero, since we use that as special value for empty slots + if (mono_time_get(time) == 0) { + // Fail loudly in debug environments + assert(false); + return nullptr; + } + + Shared_Key_Cache *res = (Shared_Key_Cache *)calloc(1, sizeof (Shared_Key_Cache)); + if (res == nullptr) { + return nullptr; + } + + res->self_secret_key = self_secret_key; + res->time = time; + res->keys_per_slot = keys_per_slot; + // We take one byte from the public key for each bucket and store keys_per_slot elements there + const size_t cache_size = 256 * keys_per_slot; + res->keys = (Shared_Key *)calloc(cache_size, sizeof (Shared_Key)); + + if (res->keys == nullptr) { + free(res); + return nullptr; + } + + crypto_memlock(res->keys, cache_size * sizeof (Shared_Key)); + + return res; +} + +void shared_key_cache_free(Shared_Key_Cache *cache) +{ + if (cache == nullptr) { + return; + } + + const size_t cache_size = 256 * cache->keys_per_slot; + // Don't leave key material in memory + crypto_memzero(cache->keys, cache_size * sizeof (Shared_Key)); + crypto_memunlock(cache->keys, cache_size * sizeof (Shared_Key)); + free(cache->keys); + free(cache); +} + +/* NOTE: On each lookup housekeeping is performed to evict keys that did timeout. */ +const uint8_t *shared_key_cache_lookup(Shared_Key_Cache *cache, const uint8_t public_key[CRYPTO_PUBLIC_KEY_SIZE]) +{ + // caching the time is not necessary, but calls to mono_time_get(...) are not free + const uint64_t cur_time = mono_time_get(cache->time); + // We can't use the first and last bytes because they are masked in curve25519. Selected 8 for good alignment. + const uint8_t bucket_idx = public_key[8]; + Shared_Key* bucket_start = &cache->keys[bucket_idx*cache->keys_per_slot]; + + const uint8_t* found = nullptr; + + // Perform lookup + for(size_t i = 0; i < cache->keys_per_slot; ++i) { + if (shared_key_is_empty(&bucket_start[i])) { + continue; + } + + if (pk_equal(public_key, bucket_start[i].public_key)) { + found = bucket_start[i].shared_key; + bucket_start[i].time_last_requested = cur_time; + break; + } + } + + // Perform housekeeping for this bucket + for (size_t i = 0; i < cache->keys_per_slot; ++i) { + if (shared_key_is_empty(&bucket_start[i])) { + continue; + } + + const bool timed_out = (bucket_start[i].time_last_requested + cache->timeout) < cur_time; + if (timed_out) { + shared_key_set_empty(&bucket_start[i]); + } + } + + if (found == nullptr) { + // Insert into cache + + uint64_t oldest_timestamp = UINT64_MAX; + size_t oldest_index = 0; + + /* + * Find least recently used entry, unused entries are prioritised, + * because their time_last_requested field is zeroed. + */ + for (size_t i = 0; i < cache->keys_per_slot; ++i) { + if (bucket_start[i].time_last_requested < oldest_timestamp) { + oldest_timestamp = bucket_start[i].time_last_requested; + oldest_index = i; + } + } + + // Compute the shared key for the cache + if (encrypt_precompute(public_key, cache->self_secret_key, bucket_start[oldest_index].shared_key) != 0) { + // Don't put anything in the cache on error + return nullptr; + } + + // update cache entry + memcpy(bucket_start[oldest_index].public_key, public_key, CRYPTO_PUBLIC_KEY_SIZE); + bucket_start[oldest_index].time_last_requested = cur_time; + found = bucket_start[oldest_index].shared_key; + } + + return found; +} diff --git a/toxcore/shared_key_cache.h b/toxcore/shared_key_cache.h new file mode 100644 index 00000000..97d8b7a4 --- /dev/null +++ b/toxcore/shared_key_cache.h @@ -0,0 +1,51 @@ +/* SPDX-License-Identifier: GPL-3.0-or-later + * Copyright © 2022 The TokTok team. + */ + +#ifndef C_TOXCORE_TOXCORE_SHARED_KEY_CACHE_H +#define C_TOXCORE_TOXCORE_SHARED_KEY_CACHE_H + +#include // uint*_t + +#include "crypto_core.h" +#include "mono_time.h" + +/** + * This implements a cache for shared keys, since key generation is expensive. + */ + +typedef struct Shared_Key_Cache Shared_Key_Cache; + +/** + * @brief Initializes a new shared key cache. + * @param time Time object for retrieving current time. + * @param self_secret_key Our own secret key of length CRYPTO_SECRET_KEY_SIZE, + * it must not change during the lifetime of the cache. + * @param timeout Number of milliseconds, after which a key should be evicted. + * @param keys_per_slot There are 256 slots, this controls how many keys are stored per slot and the size of the cache. + * @return nullptr on error. + */ +non_null() +Shared_Key_Cache *shared_key_cache_new(const Mono_Time *time, + const uint8_t *self_secret_key, + uint64_t timeout, uint8_t keys_per_slot); + +/** + * @brief Deletes the cache and frees all resources. + * @param cache Cache to delete or nullptr. + */ +nullable(1) +void shared_key_cache_free(Shared_Key_Cache *cache); + +/** + * @brief Looks up a key from the cache or computes it if it didn't exist yet. + * @param cache Cache to perform the lookup on. + * @param public_key Public key, used for the lookup and computation. + * + * @return The shared key of length CRYPTO_SHARED_KEY_SIZE, matching the public key and our secret key. + * @return nullptr on error. + */ +non_null() +const uint8_t* shared_key_cache_lookup(Shared_Key_Cache *cache, const uint8_t public_key[CRYPTO_PUBLIC_KEY_SIZE]); + +#endif // C_TOXCORE_TOXCORE_SHARED_KEY_CACHE_H