commit 43098a7ddc27c90a1786f002eaac90a2c23c2d72
parent 90f0c977f8625fe929003cedbf7ef4372aff00f8
Author: Nick Mathewson <nickm@torproject.org>
Date: Wed, 28 May 2025 10:02:38 -0400
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.
Diffstat:
8 files changed, 44 insertions(+), 117 deletions(-)
diff --git 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 <b>crypto</b> 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 <b>cell</b> arriving on
* <b>circ</b> in direction <b>cell_direction</b>.
*
@@ -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
@@ -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
@@ -178,22 +178,12 @@ cpath_free(crypt_path_t *victim)
/************ cpath sendme API ***************************/
/** Return the sendme_digest of this <b>cpath</b>. */
-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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
