diff --git a/scripts/maint/practracker/exceptions.txt b/scripts/maint/practracker/exceptions.txt index cba3ddc1b3..a83bb62bb7 100644 --- a/scripts/maint/practracker/exceptions.txt +++ b/scripts/maint/practracker/exceptions.txt @@ -122,7 +122,7 @@ problem function-size /src/core/or/policies.c:policy_summarize() 107 problem function-size /src/core/or/protover.c:protover_all_supported() 117 problem file-size /src/core/or/relay.c 3173 problem function-size /src/core/or/relay.c:circuit_receive_relay_cell() 123 -problem function-size /src/core/or/relay.c:relay_send_command_from_edge_() 112 +problem function-size /src/core/or/relay.c:relay_send_command_from_edge_() 116 problem function-size /src/core/or/relay.c:connection_ap_process_end_not_open() 194 problem function-size /src/core/or/relay.c:connection_edge_process_relay_cell_not_open() 139 problem function-size /src/core/or/relay.c:connection_edge_process_relay_cell() 520 diff --git a/src/core/or/relay.c b/src/core/or/relay.c index 8b3a1be18e..91236e5fec 100644 --- a/src/core/or/relay.c +++ b/src/core/or/relay.c @@ -701,7 +701,7 @@ relay_send_command_from_edge_,(streamid_t stream_id, circuit_t *circ, * we need to. This call needs to be after the circuit_package_relay_cell() * because the cell digest is set within that function. */ if (relay_command == RELAY_COMMAND_DATA) { - sendme_record_cell_digest(circ); + sendme_record_cell_digest_on_circ(circ, cpath_layer); } return 0; diff --git a/src/core/or/sendme.c b/src/core/or/sendme.c index e7c65d99e2..586d4d0ae0 100644 --- a/src/core/or/sendme.c +++ b/src/core/or/sendme.c @@ -287,6 +287,21 @@ send_circuit_level_sendme(circuit_t *circ, crypt_path_t *layer_hint, return 0; } +/* Record the cell digest only if the next cell is expected to be a SENDME. */ +static void +record_cell_digest_on_circ(circuit_t *circ, const uint8_t *sendme_digest) +{ + tor_assert(circ); + tor_assert(sendme_digest); + + /* Add the digest to the last seen list in the circuit. */ + if (circ->sendme_last_digests == NULL) { + circ->sendme_last_digests = smartlist_new(); + } + smartlist_add(circ->sendme_last_digests, + tor_memdup(sendme_digest, DIGEST_LEN)); +} + /* * Public API */ @@ -571,31 +586,37 @@ sendme_note_stream_data_packaged(edge_connection_t *conn) return --conn->package_window; } -/* Note the cell digest in the circuit sendme last digests FIFO if applicable. - * It is safe to pass a circuit that isn't meant to track those digests. */ +/* Record the cell digest into the circuit sendme digest list depending on + * which edge we are. The digest is recorded only if we expect the next cell + * that we will receive is a SENDME so we can match the digest. */ void -sendme_record_cell_digest(circuit_t *circ) +sendme_record_cell_digest_on_circ(circuit_t *circ, crypt_path_t *cpath) { - const uint8_t *digest; + int package_window; + uint8_t *sendme_digest; tor_assert(circ); - /* We only keep the cell digest if we are the Exit on that circuit and if - * this cell is the last one before the client should send a SENDME. */ - if (CIRCUIT_IS_ORIGIN(circ)) { - return; + package_window = circ->package_window; + if (cpath) { + package_window = cpath->package_window; } + /* Is this the last cell before a SENDME? The idea is that if the * package_window reaches a multiple of the increment, after this cell, we * should expect a SENDME. */ - if (!sendme_circuit_cell_is_next(circ->package_window)) { + if (!sendme_circuit_cell_is_next(package_window)) { return; } - /* Add the digest to the last seen list in the circuit. */ - digest = relay_crypto_get_sendme_digest(&TO_OR_CIRCUIT(circ)->crypto); - if (circ->sendme_last_digests == NULL) { - circ->sendme_last_digests = smartlist_new(); + /* Getting the digest is expensive so we only do it once we are certain to + * record it on the circuit. */ + if (cpath) { + sendme_digest = cpath_get_sendme_digest(cpath); + } else { + sendme_digest = + relay_crypto_get_sendme_digest(&TO_OR_CIRCUIT(circ)->crypto); } - smartlist_add(circ->sendme_last_digests, tor_memdup(digest, DIGEST_LEN)); + + record_cell_digest_on_circ(circ, sendme_digest); } diff --git a/src/core/or/sendme.h b/src/core/or/sendme.h index ac18bbdd31..2fb73f76d9 100644 --- a/src/core/or/sendme.h +++ b/src/core/or/sendme.h @@ -35,8 +35,9 @@ int sendme_note_circuit_data_packaged(circuit_t *circ, int sendme_note_stream_data_packaged(edge_connection_t *conn); /* Track cell digest. */ -void sendme_record_cell_digest(circuit_t *circ); void sendme_circuit_record_outbound_cell(or_circuit_t *or_circ); +/* Record cell digest on circuit. */ +void sendme_record_cell_digest_on_circ(circuit_t *circ, crypt_path_t *cpath); /* Circuit level information. */ bool sendme_circuit_cell_is_next(int window); diff --git a/src/test/test_sendme.c b/src/test/test_sendme.c index d40fbaf862..463a0ec086 100644 --- a/src/test/test_sendme.c +++ b/src/test/test_sendme.c @@ -46,26 +46,12 @@ static void test_v1_record_digest(void *arg) { or_circuit_t *or_circ = NULL; - origin_circuit_t *orig_circ = NULL; circuit_t *circ = NULL; (void) arg; - /* Create our dummy circuits. */ - orig_circ = origin_circuit_new(); - tt_assert(orig_circ); + /* Create our dummy circuit. */ or_circ = or_circuit_new(1, NULL); - - /* Start by pointing to the origin circuit. */ - circ = TO_CIRCUIT(orig_circ); - circ->purpose = CIRCUIT_PURPOSE_S_REND_JOINED; - - /* We should never note SENDME digest on origin circuit. */ - sendme_record_cell_digest(circ); - tt_assert(!circ->sendme_last_digests); - /* We do not need the origin circuit for now. */ - orig_circ = NULL; - circuit_free_(circ); /* Points it to the OR circuit now. */ circ = TO_CIRCUIT(or_circ); @@ -73,23 +59,23 @@ test_v1_record_digest(void *arg) * in order to catched the CIRCWINDOW_INCREMENT-nth cell. Try something that * shouldn't be noted. */ circ->package_window = CIRCWINDOW_INCREMENT; - sendme_record_cell_digest(circ); + sendme_record_cell_digest_on_circ(circ, NULL); tt_assert(!circ->sendme_last_digests); /* This should work now. Package window at CIRCWINDOW_INCREMENT + 1. */ circ->package_window++; - sendme_record_cell_digest(circ); + sendme_record_cell_digest_on_circ(circ, NULL); tt_assert(circ->sendme_last_digests); tt_int_op(smartlist_len(circ->sendme_last_digests), OP_EQ, 1); /* Next cell in the package window shouldn't do anything. */ circ->package_window++; - sendme_record_cell_digest(circ); + sendme_record_cell_digest_on_circ(circ, NULL); tt_int_op(smartlist_len(circ->sendme_last_digests), OP_EQ, 1); /* The next CIRCWINDOW_INCREMENT should add one more digest. */ circ->package_window = (CIRCWINDOW_INCREMENT * 2) + 1; - sendme_record_cell_digest(circ); + sendme_record_cell_digest_on_circ(circ, NULL); tt_int_op(smartlist_len(circ->sendme_last_digests), OP_EQ, 2); done: @@ -188,7 +174,7 @@ test_v1_build_cell(void *arg) /* Note the wrong digest in the circuit, cell should fail validation. */ circ->package_window = CIRCWINDOW_INCREMENT + 1; - sendme_record_cell_digest(circ); + sendme_record_cell_digest_on_circ(circ, NULL); tt_int_op(smartlist_len(circ->sendme_last_digests), OP_EQ, 1); setup_full_capture_of_logs(LOG_INFO); tt_int_op(sendme_is_valid(circ, payload, sizeof(payload)), OP_EQ, false); @@ -200,7 +186,7 @@ test_v1_build_cell(void *arg) /* Record the cell digest into the circuit, cell should validate. */ memcpy(or_circ->crypto.sendme_digest, digest, sizeof(digest)); circ->package_window = CIRCWINDOW_INCREMENT + 1; - sendme_record_cell_digest(circ); + sendme_record_cell_digest_on_circ(circ, NULL); tt_int_op(smartlist_len(circ->sendme_last_digests), OP_EQ, 1); tt_int_op(sendme_is_valid(circ, payload, sizeof(payload)), OP_EQ, true); /* After a validation, the last digests is always popped out. */