Skip to content

Commit 387835c

Browse files
goldroomgoldroom
committed
fix: fixed issues in real-world backwards-compatible handshake behavior.
1 parent 54e0471 commit 387835c

File tree

1 file changed

+41
-20
lines changed

1 file changed

+41
-20
lines changed

‎toxcore/net_crypto.c‎

Lines changed: 41 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -2110,8 +2110,8 @@ static int handle_data_packet_core(Net_Crypto *c, int crypt_connection_id, const
21102110
if (conn->noise_handshake != nullptr) {
21112111
crypto_memzero(conn->noise_handshake, sizeof(Noise_Handshake));
21122112
//TODO: remove? leads to nullptr in handle_new_connection_handshake()?
2113-
mem_delete(c->mem, conn->noise_handshake);
2114-
conn->noise_handshake = nullptr;
2113+
// mem_delete(c->mem, conn->noise_handshake);
2114+
// conn->noise_handshake = nullptr;
21152115
}
21162116

21172117
/* also crypto_memzero() non-Noise values from crypto connection */
@@ -2211,7 +2211,7 @@ static int handle_packet_cookie_response(const Net_Crypto *c, int crypt_connecti
22112211
return -1;
22122212
}
22132213

2214-
LOGGER_DEBUG(c->log, "Packet: %d/length: %d/crypto_connection_id: %d/conn->status: %d",
2214+
LOGGER_DEBUG(c->log, "Packet: %d/length: %d/crypt_connection_id: %d/conn->status: %d",
22152215
packet[0], length, crypt_connection_id, conn->status);
22162216

22172217
if (conn->status != CRYPTO_CONN_COOKIE_REQUESTING) {
@@ -2294,7 +2294,8 @@ static int handle_packet_crypto_hs(Net_Crypto *c, int crypt_connection_id, const
22942294
uint8_t cookie[COOKIE_LENGTH];
22952295

22962296
/* necessary for compatiblity to non-Noise */
2297-
if (length == HANDSHAKE_PACKET_LENGTH && c->noise_compatibility_enabled) {
2297+
//TODO: check for conn->noise_handshake_enabled => to not switch again after handle_new_connection_handshake()
2298+
if (length == HANDSHAKE_PACKET_LENGTH && c->noise_compatibility_enabled && conn->noise_handshake_enabled) {
22982299
if (conn->noise_handshake != nullptr) {
22992300
/* non-Noise: noise_handshake not necessary anymore => memzero and free */
23002301
crypto_memzero(conn->noise_handshake, sizeof(Noise_Handshake));
@@ -2785,13 +2786,17 @@ static int handle_new_connection_handshake(Net_Crypto *c, const IP_Port *source,
27852786
// char log_spub[CRYPTO_PUBLIC_KEY_SIZE*2+1];
27862787
// bytes2string(log_spub, sizeof(log_spub), n_c.peersessionpublic_key, CRYPTO_PUBLIC_KEY_SIZE, c->log);
27872788
// LOGGER_DEBUG(c->log, "RESPONDER: session pub: %s", log_spub);
2789+
char log_id_public[CRYPTO_PUBLIC_KEY_SIZE*2+1];
2790+
bytes2string(log_id_public, sizeof(log_id_public), n_c.peer_id_public_key, CRYPTO_PUBLIC_KEY_SIZE, c->log);
2791+
LOGGER_DEBUG(c->log, ": peer_id_public_key: %s", log_id_public);
27882792
// char log_cookie[COOKIE_LENGTH*2+1];
27892793
// bytes2string(log_cookie, sizeof(log_cookie), n_c.cookie, COOKIE_LENGTH, c->log);
27902794
// LOGGER_DEBUG(c->log, "RESPONDER: cookie: %s", log_cookie);
27912795

27922796
const int crypt_connection_id = getcryptconnection_id(c, n_c.peer_id_public_key);
27932797

27942798
//TODO: This is only called if new_crypto_connection() was already called in the meantime! Now Noise RESPONDER!
2799+
//TODO: why is this called twice in row via tcp_oob_callback() ?!
27952800
/* happens NoiseIK handshake (e.g. auto_tox_many_test) */
27962801
if (crypt_connection_id != -1) {
27972802
LOGGER_DEBUG(c->log, "RESPONDER: CRYPTO CONN EXISTING -> crypt_connection_id: %d", crypt_connection_id);
@@ -2838,29 +2843,34 @@ static int handle_new_connection_handshake(Net_Crypto *c, const IP_Port *source,
28382843
else {
28392844
LOGGER_DEBUG(c->log, "non-Noise handshake");
28402845
conn->noise_handshake_enabled = false;
2846+
28412847
//TODO: need to do the same here as in handle_crypto_hs() to switch? otherwise still think this is a Noise connection? (toxic_01022025_4.log)
28422848
//TODO: not sure if that fixed something, connections taking way longer now in toxic
2843-
// if (conn->noise_handshake != nullptr) {
2844-
// /* non-Noise: noise_handshake not necessary anymore => memzero and free */
2845-
// crypto_memzero(conn->noise_handshake, sizeof(Noise_Handshake));
2846-
// mem_delete(c->mem, conn->noise_handshake);
2847-
// conn->noise_handshake = nullptr;
2848-
// }
2849-
// conn->noise_handshake_enabled = false;
2850-
// /* Ephemeral key pair needed in non-Noise handshake */
2851-
// crypto_new_keypair(c->rng, conn->ephemeral_public_key, conn->ephemeral_secret_key);
2852-
// /* Random nonce needed in non-Noise handshake */
2853-
// random_nonce(c->rng, conn->send_nonce);
2854-
// LOGGER_DEBUG(c->log, "Switch to non-Noise handshake");
2855-
// if (conn->status != CRYPTO_CONN_COOKIE_REQUESTING && conn->status != CRYPTO_CONN_HANDSHAKE_SENT) {
2856-
// mem_delete(c->mem, n_c.peer_cookie);
2857-
// return -1;
2858-
// }
2849+
if (conn->noise_handshake != nullptr) {
2850+
/* non-Noise: noise_handshake not necessary anymore => memzero and free */
2851+
crypto_memzero(conn->noise_handshake, sizeof(Noise_Handshake));
2852+
mem_delete(c->mem, conn->noise_handshake);
2853+
conn->noise_handshake = nullptr;
2854+
}
2855+
2856+
uint8_t zeros[CRYPTO_PUBLIC_KEY_SIZE];
2857+
memset(zeros, 0, CRYPTO_PUBLIC_KEY_SIZE);
2858+
/* Need to check here, values overwritten if handle_new_connection_handshake() is called twice (happened in tests) */
2859+
if (memcmp(conn->ephemeral_public_key, zeros, CRYPTO_PUBLIC_KEY_SIZE) == 0
2860+
&& memcmp(conn->ephemeral_secret_key, zeros, CRYPTO_SECRET_KEY_SIZE) == 0
2861+
&& memcmp(conn->send_nonce, zeros, CRYPTO_NONCE_SIZE) == 0) {
2862+
/* Ephemeral key pair needed in non-Noise handshake */
2863+
crypto_new_keypair(c->rng, conn->ephemeral_public_key, conn->ephemeral_secret_key);
2864+
/* Random nonce needed in non-Noise handshake */
2865+
random_nonce(c->rng, conn->send_nonce);
2866+
LOGGER_DEBUG(c->log, "Switch to non-Noise handshake");
2867+
}
28592868

28602869
memcpy(conn->recv_nonce, n_c.recv_nonce, CRYPTO_NONCE_SIZE);
28612870
memcpy(conn->peer_ephemeral_public_key, n_c.peer_ephemeral_public_key, CRYPTO_PUBLIC_KEY_SIZE);
28622871
encrypt_precompute(conn->peer_ephemeral_public_key, conn->ephemeral_secret_key, conn->shared_key);
28632872

2873+
28642874
crypto_connection_add_source(c, crypt_connection_id, source);
28652875

28662876
if (create_send_handshake(c, crypt_connection_id, n_c.peer_cookie, n_c.peer_dht_public_key) != 0) {
@@ -2925,6 +2935,10 @@ int accept_crypto_connection(Net_Crypto *c, const New_Connection *n_c)
29252935

29262936
conn->connection_number_tcp = connection_number_tcp;
29272937

2938+
char log_id_public[CRYPTO_PUBLIC_KEY_SIZE*2+1];
2939+
bytes2string(log_id_public, sizeof(log_id_public), n_c->peer_id_public_key, CRYPTO_PUBLIC_KEY_SIZE, c->log);
2940+
LOGGER_DEBUG(c->log, "peer_id_public_key: %s", log_id_public);
2941+
29282942
// NoiseIK: only happening for RESPONDER
29292943
if (n_c->noise_handshake != nullptr) {
29302944
if (!n_c->noise_handshake->initiator) {
@@ -3040,6 +3054,10 @@ int new_crypto_connection(Net_Crypto *c, const uint8_t *real_public_key, const u
30403054
conn->connection_number_tcp = connection_number_tcp;
30413055
/* Necessary for backwards compatibility to switch to non-Noise handshake */
30423056
memcpy(conn->peer_id_public_key, real_public_key, CRYPTO_PUBLIC_KEY_SIZE);
3057+
char log_id_public[CRYPTO_PUBLIC_KEY_SIZE*2+1];
3058+
bytes2string(log_id_public, sizeof(log_id_public), conn->peer_id_public_key, CRYPTO_PUBLIC_KEY_SIZE, c->log);
3059+
LOGGER_DEBUG(c->log, "peer_id_public_key: %s", log_id_public);
3060+
30433061

30443062
/* Base nonces are a counter in transport phase after NoiseIK handshake */
30453063
/* Only necessary after handshake is finished, but would need to set in multiple different places */
@@ -3055,6 +3073,9 @@ int new_crypto_connection(Net_Crypto *c, const uint8_t *real_public_key, const u
30553073

30563074
/* Necessary for backwards compatibility to switch to non-Noise handshake (only if enabled with option noise_compatibility_enabled) */
30573075
conn->noise_handshake_enabled = true;
3076+
/* Need to set for check in handle_new_connection_handshake() */
3077+
memset(conn->ephemeral_public_key, 0, CRYPTO_PUBLIC_KEY_SIZE);
3078+
memset(conn->ephemeral_secret_key, 0, CRYPTO_SECRET_KEY_SIZE);
30583079

30593080
/* TODO(goldroom): Noise: only necessary if Cookie response was successful, but moved here to avoid saving peer_id_public_key twice
30603081
(to remove it a some point from struct Crypto_Connection) */

0 commit comments

Comments
 (0)