From: Nick Mathewson Date: Wed, 28 May 2025 14:02:38 +0000 (-0400) Subject: Refactor and simplify save_sendme logic in tor1. X-Git-Url: http://git.ipfire.org/gitweb/gitweb.cgi?a=commitdiff_plain;h=43098a7ddc27c90a1786f002eaac90a2c23c2d72;p=thirdparty%2Ftor.git Refactor and simplify save_sendme logic in tor1. Every time that we want a sendme_digest, we have already computed it once, either to originate a cell or to recognize a cell. Rather than figuring out when to compute the digest a second time, we instead refactor our tor1 digest code to _always_ store such digests in the relay_crypto_t. This saves a bit of complexity, and shouldn't involve a performance hit; rather, it has potential to speed things up by saving a sha1 call. --- diff --git a/src/core/crypto/relay_crypto.c b/src/core/crypto/relay_crypto.c index 73410a74ae..2f30306605 100644 --- a/src/core/crypto/relay_crypto.c +++ b/src/core/crypto/relay_crypto.c @@ -38,31 +38,33 @@ #define V0_DIGEST_LEN 4 #define V0_RECOGNIZED_OFFSET 1 -/** Update digest{ from the payload of cell. Assign integrity part to - * cell. +/** Update digest from the payload of cell. Assign integrity part to + * cell. Record full 20-byte digest in `buf`. */ -void -tor1_set_digest_v0(crypto_digest_t *digest, cell_t *cell) +static void +tor1_set_digest_v0(crypto_digest_t *digest, cell_t *cell, uint8_t *buf) { - char integrity[V0_DIGEST_LEN]; - crypto_digest_add_bytes(digest, (char*)cell->payload, CELL_PAYLOAD_SIZE); - crypto_digest_get_digest(digest, integrity, V0_DIGEST_LEN); + crypto_digest_get_digest(digest, (char*)buf, DIGEST_LEN); // log_fn(LOG_DEBUG,"Putting digest of %u %u %u %u into relay cell.", // integrity[0], integrity[1], integrity[2], integrity[3]); - memcpy(cell->payload + V0_DIGEST_OFFSET, integrity, V0_DIGEST_LEN); + memcpy(cell->payload + V0_DIGEST_OFFSET, buf, V0_DIGEST_LEN); } /** Does the digest for this circuit indicate that this cell is for us? * * Update digest from the payload of cell (with the integrity part set - * to 0). If the integrity part is valid, return 1, else restore digest + * to 0). If the integrity part is valid, + * return 1 and save the full digest in the 20-byte buffer `buf`, + * else restore digest * and cell to their original state and return 0. */ static int -tor1_relay_digest_matches_v0(crypto_digest_t *digest, cell_t *cell) +tor1_relay_digest_matches_v0(crypto_digest_t *digest, cell_t *cell, + uint8_t *buf) { uint32_t received_integrity, calculated_integrity; + uint8_t calculated_digest[DIGEST_LEN]; crypto_digest_checkpoint_t backup_digest; CTASSERT(sizeof(uint32_t) == V0_DIGEST_LEN); @@ -77,8 +79,8 @@ tor1_relay_digest_matches_v0(crypto_digest_t *digest, cell_t *cell) // received_integrity[2], received_integrity[3]); crypto_digest_add_bytes(digest, (char*) cell->payload, CELL_PAYLOAD_SIZE); - crypto_digest_get_digest(digest, (char*) &calculated_integrity, - V0_DIGEST_LEN); + crypto_digest_get_digest(digest, (char*) calculated_digest, DIGEST_LEN); + calculated_integrity = get_uint32(calculated_digest); int rv = 1; @@ -91,6 +93,8 @@ tor1_relay_digest_matches_v0(crypto_digest_t *digest, cell_t *cell) memcpy(cell->payload + V0_DIGEST_OFFSET, &received_integrity, V0_DIGEST_LEN); rv = 0; + } else { + memcpy(buf, calculated_digest, DIGEST_LEN); } memwipe(&backup_digest, 0, sizeof(backup_digest)); @@ -117,26 +121,18 @@ tor1_crypt_one_payload(crypto_cipher_t *cipher, uint8_t *in) /* XXXX DOCDOC */ void tor1_crypt_client_originate(relay_crypto_t *tor1, - cell_t *cell, - bool record_sendme_digest) + cell_t *cell) { - tor1_set_digest_v0(tor1->f_digest, cell); - if (record_sendme_digest) { - tor1_save_sendme_digest(tor1, true); - } + tor1_set_digest_v0(tor1->f_digest, cell, tor1->sendme_digest); tor1_crypt_one_payload(tor1->f_crypto, cell->payload); } /* XXXX DOCDOC */ void tor1_crypt_relay_originate(relay_crypto_t *tor1, - cell_t *cell, - bool save_sendme_digest) + cell_t *cell) { - tor1_set_digest_v0(tor1->b_digest, cell); - if (save_sendme_digest) { - tor1_save_sendme_digest(tor1, false); - } + tor1_set_digest_v0(tor1->b_digest, cell, tor1->sendme_digest); tor1_crypt_one_payload(tor1->b_crypto, cell->payload); } @@ -160,7 +156,8 @@ tor1_crypt_relay_forward(relay_crypto_t *tor1, cell_t *cell) { tor1_crypt_one_payload(tor1->f_crypto, cell->payload); if (relay_cell_is_recognized_v0(cell)) { - if (tor1_relay_digest_matches_v0(tor1->f_digest, cell)) { + if (tor1_relay_digest_matches_v0(tor1->f_digest, cell, + tor1->sendme_digest)) { return true; } } @@ -174,7 +171,8 @@ tor1_crypt_client_backward(relay_crypto_t *tor1, cell_t *cell) tor1_crypt_one_payload(tor1->b_crypto, cell->payload); if (relay_cell_is_recognized_v0(cell)) { - if (tor1_relay_digest_matches_v0(tor1->b_digest, cell)) { + if (tor1_relay_digest_matches_v0(tor1->b_digest, cell, + tor1->sendme_digest)) { return true; } } @@ -183,38 +181,18 @@ tor1_crypt_client_backward(relay_crypto_t *tor1, cell_t *cell) /** Return the sendme_digest within the crypto object. * - * Before calling this function, you must call relay_crypto_save_sendme_digest. + * This is the digest from the most recent cell that we originated + * or recognized, _in either direction_. + * Calls to any encryption function on `crypto` may invalidate + * this digest. */ -uint8_t * +const uint8_t * relay_crypto_get_sendme_digest(relay_crypto_t *crypto) { tor_assert(crypto); return crypto->sendme_digest; } -/** Save the cell digest, indicated by is_foward_digest or not, as the - * SENDME cell digest inside this relay_crypto_t. - * - * This function must be called before relay_crypto_get_sendme_digest - * will work correctly. - */ -void -tor1_save_sendme_digest(relay_crypto_t *crypto, - bool is_foward_digest) -{ - struct crypto_digest_t *digest; - - tor_assert(crypto); - - digest = crypto->b_digest; - if (is_foward_digest) { - digest = crypto->f_digest; - } - - crypto_digest_get_digest(digest, (char *) crypto->sendme_digest, - sizeof(crypto->sendme_digest)); -} - /** Do the appropriate en/decryptions for cell arriving on * circ in direction cell_direction. * @@ -300,10 +278,7 @@ relay_encrypt_cell_outbound(cell_t *cell, { crypt_path_t *thishop = layer_hint; - bool save_sendme = - circuit_sent_cell_for_sendme(TO_CIRCUIT(circ), thishop); - - tor1_crypt_client_originate(&thishop->pvt_crypto, cell, save_sendme); + tor1_crypt_client_originate(&thishop->pvt_crypto, cell); thishop = thishop->prev; while (thishop != circ->cpath->prev) { @@ -323,9 +298,7 @@ void relay_encrypt_cell_inbound(cell_t *cell, or_circuit_t *or_circ) { - bool save_sendme = - circuit_sent_cell_for_sendme(TO_CIRCUIT(or_circ), NULL); - tor1_crypt_relay_originate(&or_circ->crypto, cell, save_sendme); + tor1_crypt_relay_originate(&or_circ->crypto, cell); } /** diff --git a/src/core/crypto/relay_crypto.h b/src/core/crypto/relay_crypto.h index 41c5747aea..59e00d1ae9 100644 --- a/src/core/crypto/relay_crypto.h +++ b/src/core/crypto/relay_crypto.h @@ -27,23 +27,17 @@ void relay_crypto_clear(relay_crypto_t *crypto); void relay_crypto_assert_ok(const relay_crypto_t *crypto); -uint8_t *relay_crypto_get_sendme_digest(relay_crypto_t *crypto); - -void tor1_save_sendme_digest(tor1_crypt_t *crypto, - bool is_foward_digest); +const uint8_t *relay_crypto_get_sendme_digest(relay_crypto_t *crypto); void tor1_crypt_client_originate(tor1_crypt_t *tor1, - cell_t *cell, - bool record_sendme_digest); + cell_t *cell); void tor1_crypt_relay_originate(tor1_crypt_t *tor1, - cell_t *cell, - bool record_sendme_digest); + cell_t *cell); void tor1_crypt_relay_backward(tor1_crypt_t *tor1, cell_t *cell); bool tor1_crypt_relay_forward(tor1_crypt_t *tor1, cell_t *cell); bool tor1_crypt_client_backward(tor1_crypt_t *tor1, cell_t *cell); void tor1_crypt_client_forward(tor1_crypt_t *tor1, cell_t *cell); void tor1_crypt_one_payload(crypto_cipher_t *cipher, uint8_t *in); -void tor1_set_digest_v0(crypto_digest_t *digest, cell_t *cell); #endif /* !defined(TOR_RELAY_CRYPTO_H) */ diff --git a/src/core/or/crypt_path.c b/src/core/or/crypt_path.c index d2527dfb4b..56be7b2311 100644 --- a/src/core/or/crypt_path.c +++ b/src/core/or/crypt_path.c @@ -178,22 +178,12 @@ cpath_free(crypt_path_t *victim) /************ cpath sendme API ***************************/ /** Return the sendme_digest of this cpath. */ -uint8_t * +const uint8_t * cpath_get_sendme_digest(crypt_path_t *cpath) { return relay_crypto_get_sendme_digest(&cpath->pvt_crypto); } -/** Save the cell digest, indicated by is_foward_digest or not, as the - * SENDME cell digest inside the cpath's relay_crypto_t. - */ -void -cpath_sendme_save_cell_digest(crypt_path_t *cpath, bool is_foward_digest) -{ - tor_assert(cpath); - tor1_save_sendme_digest(&cpath->pvt_crypto, is_foward_digest); -} - /************ other cpath functions ***************************/ /** Return the first non-open hop in cpath, or return NULL if all diff --git a/src/core/or/crypt_path.h b/src/core/or/crypt_path.h index 514f4df853..2cf36b02b1 100644 --- a/src/core/or/crypt_path.h +++ b/src/core/or/crypt_path.h @@ -21,14 +21,11 @@ cpath_free(crypt_path_t *victim); void cpath_extend_linked_list(crypt_path_t **head_ptr, crypt_path_t *new_hop); -void cpath_sendme_save_cell_digest(crypt_path_t *cpath, - bool is_foward_digest); - crypt_path_t *cpath_get_next_non_open_hop(crypt_path_t *cpath); void cpath_sendme_circuit_record_inbound_cell(crypt_path_t *cpath); -uint8_t *cpath_get_sendme_digest(crypt_path_t *cpath); +const uint8_t *cpath_get_sendme_digest(crypt_path_t *cpath); #if defined(TOR_UNIT_TESTS) unsigned int cpath_get_n_hops(crypt_path_t **head_ptr); diff --git a/src/core/or/relay.c b/src/core/or/relay.c index 6616b92d77..de1ecce009 100644 --- a/src/core/or/relay.c +++ b/src/core/or/relay.c @@ -265,10 +265,6 @@ circuit_receive_relay_cell(cell_t *cell, circuit_t *circ, edge_connection_t *conn = NULL; relay_cell_fmt_t format = circuit_get_relay_format(circ, layer_hint); - /* Recognized cell, the cell digest has been updated, we'll record it for - * the SENDME if need be. */ - sendme_save_received_cell_digest(circ, layer_hint); - relay_msg_t msg_buf; if (relay_msg_decode_cell_in_place(format, cell, &msg_buf) < 0) { log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL, diff --git a/src/core/or/relay_crypto_st.h b/src/core/or/relay_crypto_st.h index 32ff86235a..11d1e7e699 100644 --- a/src/core/or/relay_crypto_st.h +++ b/src/core/or/relay_crypto_st.h @@ -30,7 +30,11 @@ struct relay_crypto_t { /** Digest state for cells heading away from the OR at this step. */ struct crypto_digest_t *b_digest; - /** Digest used for the next SENDME cell if any. */ + /** Digest used for the next SENDME cell if any. + * + * This digest is updated every time a cell is _originated_ or _recognized_ + * in either direction. Any operation with this object may + * invalidate this digest. */ uint8_t sendme_digest[DIGEST_LEN]; }; #undef crypto_cipher_t diff --git a/src/core/or/sendme.c b/src/core/or/sendme.c index 22fd803865..6128845373 100644 --- a/src/core/or/sendme.c +++ b/src/core/or/sendme.c @@ -338,7 +338,8 @@ record_cell_digest_on_circ(circuit_t *circ, const uint8_t *sendme_digest) * low in the stack when decrypting or encrypting a cell. The window is only * updated once the cell is actually put in the outbuf. */ -STATIC bool +// XXXX Todo remove if truly not needed. +ATTR_UNUSED STATIC bool circuit_sendme_cell_is_next(int deliver_window, int sendme_inc) { /* Are we at the limit of the increment and if not, we don't expect next @@ -697,7 +698,7 @@ sendme_note_stream_data_packaged(edge_connection_t *conn, size_t len) void sendme_record_cell_digest_on_circ(circuit_t *circ, crypt_path_t *cpath) { - uint8_t *sendme_digest; + const uint8_t *sendme_digest; tor_assert(circ); @@ -719,29 +720,3 @@ sendme_record_cell_digest_on_circ(circuit_t *circ, crypt_path_t *cpath) record_cell_digest_on_circ(circ, sendme_digest); } - -/* Called once we decrypted a cell and recognized it. Save the cell digest - * as the next sendme digest in the cpath's relay_crypto_t, - * only if the next cell we'll send on the circuit - * is expected to be a SENDME. */ -void -sendme_save_received_cell_digest(circuit_t *circ, crypt_path_t *cpath) -{ - // XXXX: all sendme_save functions probably belong inside tor1_* - tor_assert(circ); - - /* Only record if the next cell is expected to be a SENDME. */ - if (!circuit_sendme_cell_is_next(cpath ? cpath->deliver_window : - circ->deliver_window, - sendme_get_inc_count(circ, cpath))) { - return; - } - - if (cpath) { - /* Record incoming digest. */ - cpath_sendme_save_cell_digest(cpath, false); - } else { - /* Record forward digest. */ - tor1_save_sendme_digest(&TO_OR_CIRCUIT(circ)->crypto, true); - } -} diff --git a/src/core/or/sendme.h b/src/core/or/sendme.h index c06e590a43..8bb293471b 100644 --- a/src/core/or/sendme.h +++ b/src/core/or/sendme.h @@ -37,8 +37,6 @@ int sendme_note_stream_data_packaged(edge_connection_t *conn, size_t len); /* Record cell digest on circuit. */ void sendme_record_cell_digest_on_circ(circuit_t *circ, crypt_path_t *cpath); -/* Record cell digest as the SENDME digest. */ -void sendme_save_received_cell_digest(circuit_t *circ, crypt_path_t *cpath); /* Private section starts. */ #ifdef SENDME_PRIVATE