From 59b9eecc19877f38b2c9d8b4f7964c6e9875f4c0 Mon Sep 17 00:00:00 2001 From: David Goulet Date: Tue, 7 May 2019 09:16:39 -0400 Subject: [PATCH 1/7] sendme: Record cell digest on both client and exit It turns out that only the exit side is validating the authenticated SENDME v1 logic and never the client side. Which means that if a client ever uploaded data towards an exit, the authenticated SENDME logic wouldn't apply. For this to work, we have to record the cell digest client side as well which introduced a new function that supports both type of edges. This also removes a test that is not valid anymore which was that we didn't allow cell recording on an origin circuit (client). Part of #30428 Signed-off-by: David Goulet --- scripts/maint/practracker/exceptions.txt | 2 +- src/core/or/relay.c | 2 +- src/core/or/sendme.c | 49 +++++++++++++++++------- src/core/or/sendme.h | 3 +- src/test/test_sendme.c | 28 ++++---------- 5 files changed, 46 insertions(+), 38 deletions(-) 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. */ From 69e0d5bfc7d52f223d686bcd87f629f01b03561a Mon Sep 17 00:00:00 2001 From: David Goulet Date: Tue, 7 May 2019 09:19:41 -0400 Subject: [PATCH 2/7] sendme: Validate v1 SENDMEs on both client and exit side The validation of the SENDME cell is now done as the very first thing when receiving it for both client and exit. On failure to validate, the circuit is closed as detailed in the specification. Part of #30428 Signed-off-by: David Goulet --- src/core/or/sendme.c | 17 ++++++++--------- src/test/test_relaycell.c | 2 +- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/src/core/or/sendme.c b/src/core/or/sendme.c index 586d4d0ae0..baa57f4f25 100644 --- a/src/core/or/sendme.c +++ b/src/core/or/sendme.c @@ -204,10 +204,10 @@ sendme_is_valid(const circuit_t *circ, const uint8_t *cell_payload, /* Valid cell. */ sendme_cell_free(cell); - return 1; + return true; invalid: sendme_cell_free(cell); - return 0; + return false; } /* Build and encode a version 1 SENDME cell into payload, which must be at @@ -424,6 +424,12 @@ sendme_process_circuit_level(crypt_path_t *layer_hint, tor_assert(circ); tor_assert(cell_payload); + /* Validate the SENDME cell. Depending on the version, different validation + * can be done. An invalid SENDME requires us to close the circuit. */ + if (!sendme_is_valid(circ, cell_payload, cell_payload_len)) { + return -END_CIRC_REASON_TORPROTOCOL; + } + /* If we are the origin of the circuit, we are the Client so we use the * layer hint (the Exit hop) for the package window tracking. */ if (CIRCUIT_IS_ORIGIN(circ)) { @@ -448,13 +454,6 @@ sendme_process_circuit_level(crypt_path_t *layer_hint, * are rate limited. */ circuit_read_valid_data(TO_ORIGIN_CIRCUIT(circ), cell_payload_len); } else { - /* Validate the SENDME cell. Depending on the version, different - * validation can be done. An invalid SENDME requires us to close the - * circuit. It is only done if we are the Exit of the circuit. */ - if (!sendme_is_valid(circ, cell_payload, cell_payload_len)) { - return -END_CIRC_REASON_TORPROTOCOL; - } - /* We aren't the origin of this circuit so we are the Exit and thus we * track the package window with the circuit object. */ if ((circ->package_window + CIRCWINDOW_INCREMENT) > diff --git a/src/test/test_relaycell.c b/src/test/test_relaycell.c index 0623583511..d6372d3956 100644 --- a/src/test/test_relaycell.c +++ b/src/test/test_relaycell.c @@ -812,7 +812,7 @@ test_circbw_relay(void *arg) ASSERT_UNCOUNTED_BW(); /* Sendme on circuit with non-full window: counted */ - PACK_CELL(0, RELAY_COMMAND_SENDME, "Data1234"); + PACK_CELL(0, RELAY_COMMAND_SENDME, ""); circ->cpath->package_window = 900; connection_edge_process_relay_cell(&cell, TO_CIRCUIT(circ), edgeconn, circ->cpath); From 44265dd6716887b997bb03d2db1641efd7ae9c19 Mon Sep 17 00:00:00 2001 From: David Goulet Date: Tue, 7 May 2019 09:44:10 -0400 Subject: [PATCH 3/7] sendme: Never fallback to v0 if unknown version There was a missing cell version check against our max supported version. In other words, we do not fallback to v0 anymore in case we do know the SENDME version. We can either handle it or not, never fallback to the unauthenticated version in order to avoid gaming the authenticated logic. Add a unit tests making sure we properly test that and also test that we can always handle the default emit and accepted versions. Fixes #30428 Signed-off-by: David Goulet --- src/core/or/sendme.c | 52 +++++++++++++++++++++--------------------- src/core/or/sendme.h | 16 ++++++++++++- src/test/test_sendme.c | 33 +++++++++++++++++++++++++-- 3 files changed, 72 insertions(+), 29 deletions(-) diff --git a/src/core/or/sendme.c b/src/core/or/sendme.c index baa57f4f25..80d6b7d165 100644 --- a/src/core/or/sendme.c +++ b/src/core/or/sendme.c @@ -25,20 +25,6 @@ #include "lib/ctime/di_ops.h" #include "trunnel/sendme.h" -/* The maximum supported version. Above that value, the cell can't be - * recognized as a valid SENDME. */ -#define SENDME_MAX_SUPPORTED_VERSION 1 - -/* The cell version constants for when emitting a cell. */ -#define SENDME_EMIT_MIN_VERSION_DEFAULT 0 -#define SENDME_EMIT_MIN_VERSION_MIN 0 -#define SENDME_EMIT_MIN_VERSION_MAX UINT8_MAX - -/* The cell version constants for when accepting a cell. */ -#define SENDME_ACCEPT_MIN_VERSION_DEFAULT 0 -#define SENDME_ACCEPT_MIN_VERSION_MIN 0 -#define SENDME_ACCEPT_MIN_VERSION_MAX UINT8_MAX - /* Return the minimum version given by the consensus (if any) that should be * used when emitting a SENDME cell. */ STATIC int @@ -123,30 +109,41 @@ cell_v1_is_valid(const sendme_cell_t *cell, const circuit_t *circ) /* Return true iff the given cell version can be handled or if the minimum * accepted version from the consensus is known to us. */ STATIC bool -cell_version_is_valid(uint8_t cell_version) +cell_version_can_be_handled(uint8_t cell_version) { int accept_version = get_accept_min_version(); - /* Can we handle this version? */ + /* We will first check if the consensus minimum accepted version can be + * handled by us and if not, regardless of the cell version we got, we can't + * continue. */ if (accept_version > SENDME_MAX_SUPPORTED_VERSION) { log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL, "Unable to accept SENDME version %u (from consensus). " - "We only support <= %d. Probably your tor is too old?", - accept_version, cell_version); + "We only support <= %u. Probably your tor is too old?", + accept_version, SENDME_MAX_SUPPORTED_VERSION); goto invalid; } - /* We only accept a SENDME cell from what the consensus tells us. */ + /* Then, is this version below the accepted version from the consensus? If + * yes, we must not handle it. */ if (cell_version < accept_version) { - log_info(LD_PROTOCOL, "Unacceptable SENDME version %d. Only " + log_info(LD_PROTOCOL, "Unacceptable SENDME version %u. Only " "accepting %u (from consensus). Closing circuit.", cell_version, accept_version); goto invalid; } - return 1; + /* Is this cell version supported by us? */ + if (cell_version > SENDME_MAX_SUPPORTED_VERSION) { + log_info(LD_PROTOCOL, "SENDME cell version %u is not supported by us. " + "We only support <= %u", + cell_version, SENDME_MAX_SUPPORTED_VERSION); + goto invalid; + } + + return true; invalid: - return 0; + return false; } /* Return true iff the encoded SENDME cell in cell_payload of length @@ -183,7 +180,7 @@ sendme_is_valid(const circuit_t *circ, const uint8_t *cell_payload, } /* Validate that we can handle this cell version. */ - if (!cell_version_is_valid(cell_version)) { + if (!cell_version_can_be_handled(cell_version)) { goto invalid; } @@ -195,10 +192,13 @@ sendme_is_valid(const circuit_t *circ, const uint8_t *cell_payload, } break; case 0x00: - /* Fallthrough. Version 0, there is no work to be done on the payload so - * it is necessarily valid if we pass the version validation. */ + /* Version 0, there is no work to be done on the payload so it is + * necessarily valid if we pass the version validation. */ + break; default: - /* Unknown version means we can't handle it so fallback to version 0. */ + log_warn(LD_PROTOCOL, "Unknown SENDME cell version %d received.", + cell_version); + tor_assert_nonfatal_unreached(); break; } diff --git a/src/core/or/sendme.h b/src/core/or/sendme.h index 2fb73f76d9..a71891996b 100644 --- a/src/core/or/sendme.h +++ b/src/core/or/sendme.h @@ -45,6 +45,20 @@ bool sendme_circuit_cell_is_next(int window); /* Private section starts. */ #ifdef SENDME_PRIVATE +/* The maximum supported version. Above that value, the cell can't be + * recognized as a valid SENDME. */ +#define SENDME_MAX_SUPPORTED_VERSION 1 + +/* The cell version constants for when emitting a cell. */ +#define SENDME_EMIT_MIN_VERSION_DEFAULT 0 +#define SENDME_EMIT_MIN_VERSION_MIN 0 +#define SENDME_EMIT_MIN_VERSION_MAX UINT8_MAX + +/* The cell version constants for when accepting a cell. */ +#define SENDME_ACCEPT_MIN_VERSION_DEFAULT 0 +#define SENDME_ACCEPT_MIN_VERSION_MIN 0 +#define SENDME_ACCEPT_MIN_VERSION_MAX UINT8_MAX + /* * Unit tests declaractions. */ @@ -53,7 +67,7 @@ bool sendme_circuit_cell_is_next(int window); STATIC int get_emit_min_version(void); STATIC int get_accept_min_version(void); -STATIC bool cell_version_is_valid(uint8_t cell_version); +STATIC bool cell_version_can_be_handled(uint8_t cell_version); STATIC ssize_t build_cell_payload_v1(const uint8_t *cell_digest, uint8_t *payload); diff --git a/src/test/test_sendme.c b/src/test/test_sendme.c index 463a0ec086..a36c904c28 100644 --- a/src/test/test_sendme.c +++ b/src/test/test_sendme.c @@ -122,9 +122,9 @@ test_v1_consensus_params(void *arg) smartlist_add(current_md_consensus->net_params, (void *) "sendme_accept_min_version=1"); /* Minimum acceptable value is 1. */ - tt_int_op(cell_version_is_valid(1), OP_EQ, true); + tt_int_op(cell_version_can_be_handled(1), OP_EQ, true); /* Minimum acceptable value is 1 so a cell version of 0 is refused. */ - tt_int_op(cell_version_is_valid(0), OP_EQ, false); + tt_int_op(cell_version_can_be_handled(0), OP_EQ, false); done: free_mock_consensus(); @@ -239,6 +239,33 @@ test_cell_payload_pad(void *arg) ; } +static void +test_cell_version_validation(void *arg) +{ + (void) arg; + + /* We currently only support up to SENDME_MAX_SUPPORTED_VERSION so we are + * going to test the boundaries there. */ + + tt_assert(cell_version_can_be_handled(SENDME_MAX_SUPPORTED_VERSION)); + + /* Version below our supported should pass. */ + tt_assert(cell_version_can_be_handled(SENDME_MAX_SUPPORTED_VERSION - 1)); + + /* Extra version from our supported should fail. */ + tt_assert(!cell_version_can_be_handled(SENDME_MAX_SUPPORTED_VERSION + 1)); + + /* Simple check for version 0. */ + tt_assert(cell_version_can_be_handled(0)); + + /* We MUST handle the default cell version that we emit or accept. */ + tt_assert(cell_version_can_be_handled(SENDME_EMIT_MIN_VERSION_DEFAULT)); + tt_assert(cell_version_can_be_handled(SENDME_ACCEPT_MIN_VERSION_DEFAULT)); + + done: + ; +} + struct testcase_t sendme_tests[] = { { "v1_record_digest", test_v1_record_digest, TT_FORK, NULL, NULL }, @@ -248,6 +275,8 @@ struct testcase_t sendme_tests[] = { NULL, NULL }, { "cell_payload_pad", test_cell_payload_pad, TT_FORK, NULL, NULL }, + { "cell_version_validation", test_cell_version_validation, TT_FORK, + NULL, NULL }, END_OF_TESTCASES }; From 3835a3acf57426f692a787e7729de929b40dc62e Mon Sep 17 00:00:00 2001 From: David Goulet Date: Wed, 15 May 2019 10:16:05 -0400 Subject: [PATCH 4/7] sendme: Properly record SENDMEs on both edges Turns out that we were only recording the "b_digest" but to have bidirectionnal authenticated SENDMEs, we need to use the "f_digest" in the forward cell situation. Because of the cpath refactoring, this commit plays with the crypt_path_ and relay_crypto_t API a little bit in order to respect the abstractions. Previously, we would record the cell digest as the SENDME digest in the decrypt cell function but to avoid code duplication (both directions needs to record), we now do that right after iff the cell is recognized (at the edge). It is now done in circuit_receive_relay_cell() instead. We now also record the cell digest as the SENDME digest in both relay cell encryption functions since they are split depending on the direction. relay_encrypt_cell_outbound() and relay_encrypt_cell_inbound() need to consider recording the cell digest depending on their direction (f vs b digest). Fixes #30428 Signed-off-by: David Goulet --- scripts/maint/practracker/exceptions.txt | 2 +- src/core/crypto/relay_crypto.c | 31 +++++++----- src/core/crypto/relay_crypto.h | 5 +- src/core/or/crypt_path.c | 18 +++---- src/core/or/crypt_path.h | 3 ++ src/core/or/relay.c | 4 ++ src/core/or/sendme.c | 61 ++++++++++++++++++++---- src/core/or/sendme.h | 5 +- 8 files changed, 93 insertions(+), 36 deletions(-) diff --git a/scripts/maint/practracker/exceptions.txt b/scripts/maint/practracker/exceptions.txt index a83bb62bb7..2671f723ba 100644 --- a/scripts/maint/practracker/exceptions.txt +++ b/scripts/maint/practracker/exceptions.txt @@ -121,7 +121,7 @@ problem file-size /src/core/or/policies.c 3249 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:circuit_receive_relay_cell() 127 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 diff --git a/src/core/crypto/relay_crypto.c b/src/core/crypto/relay_crypto.c index 74cccd2223..8a285131a8 100644 --- a/src/core/crypto/relay_crypto.c +++ b/src/core/crypto/relay_crypto.c @@ -100,12 +100,22 @@ relay_crypto_get_sendme_digest(relay_crypto_t *crypto) return crypto->sendme_digest; } -/** Record the b_digest from crypto and put it in the sendme_digest. */ +/** Record the cell digest, indicated by is_foward_digest or not, as the + * SENDME cell digest. */ void -relay_crypto_record_sendme_digest(relay_crypto_t *crypto) +relay_crypto_record_sendme_digest(relay_crypto_t *crypto, + bool is_foward_digest) { + struct crypto_digest_t *digest; + tor_assert(crypto); - crypto_digest_get_digest(crypto->b_digest, (char *) crypto->sendme_digest, + + 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)); } @@ -161,11 +171,6 @@ relay_decrypt_cell(circuit_t *circ, cell_t *cell, if (relay_digest_matches(cpath_get_incoming_digest(thishop), cell)) { *recognized = 1; *layer_hint = thishop; - /* This cell is for us. Keep a record of this cell because we will - * use it in the next SENDME cell. */ - if (sendme_circuit_cell_is_next(thishop->deliver_window)) { - cpath_sendme_circuit_record_inbound_cell(thishop); - } return 0; } } @@ -213,6 +218,9 @@ relay_encrypt_cell_outbound(cell_t *cell, crypt_path_t *thishop; /* counter for repeated crypts */ cpath_set_cell_forward_digest(layer_hint, cell); + /* Record cell digest as the SENDME digest if need be. */ + sendme_record_sending_cell_digest(TO_CIRCUIT(circ), layer_hint); + thishop = layer_hint; /* moving from farthest to nearest hop */ do { @@ -237,11 +245,8 @@ relay_encrypt_cell_inbound(cell_t *cell, { relay_set_digest(or_circ->crypto.b_digest, cell); - /* We are about to send this cell outbound on the circuit. Keep a record of - * this cell if we are expecting that the next cell is a SENDME. */ - if (sendme_circuit_cell_is_next(TO_CIRCUIT(or_circ)->package_window)) { - sendme_circuit_record_outbound_cell(or_circ); - } + /* Record cell digest as the SENDME digest if need be. */ + sendme_record_sending_cell_digest(TO_CIRCUIT(or_circ), NULL); /* encrypt one layer */ relay_crypt_one_payload(or_circ->crypto.b_crypto, cell->payload); diff --git a/src/core/crypto/relay_crypto.h b/src/core/crypto/relay_crypto.h index 7f09219c7f..9478f8d359 100644 --- a/src/core/crypto/relay_crypto.h +++ b/src/core/crypto/relay_crypto.h @@ -28,7 +28,10 @@ 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 relay_crypto_record_sendme_digest(relay_crypto_t *crypto); + +void relay_crypto_record_sendme_digest(relay_crypto_t *crypto, + bool is_foward_digest); + void relay_crypt_one_payload(crypto_cipher_t *cipher, uint8_t *in); diff --git a/src/core/or/crypt_path.c b/src/core/or/crypt_path.c index a4b7190e21..6d5245510f 100644 --- a/src/core/or/crypt_path.c +++ b/src/core/or/crypt_path.c @@ -204,15 +204,6 @@ cpath_set_cell_forward_digest(crypt_path_t *cpath, cell_t *cell) /************ cpath sendme API ***************************/ -/** Keep the current inbound cell digest for the next SENDME digest. This part - * is only done by the client as the circuit came back from the Exit. */ -void -cpath_sendme_circuit_record_inbound_cell(crypt_path_t *cpath) -{ - tor_assert(cpath); - relay_crypto_record_sendme_digest(&cpath->pvt_crypto); -} - /** Return the sendme_digest of this cpath. */ uint8_t * cpath_get_sendme_digest(crypt_path_t *cpath) @@ -220,6 +211,15 @@ cpath_get_sendme_digest(crypt_path_t *cpath) return relay_crypto_get_sendme_digest(&cpath->pvt_crypto); } +/** Record the cell digest, indicated by is_foward_digest or not, as the + * SENDME cell digest. */ +void +cpath_sendme_record_cell_digest(crypt_path_t *cpath, bool is_foward_digest) +{ + tor_assert(cpath); + relay_crypto_record_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 30c14b3dce..9850610ef7 100644 --- a/src/core/or/crypt_path.h +++ b/src/core/or/crypt_path.h @@ -27,6 +27,9 @@ cpath_crypt_cell(const crypt_path_t *cpath, uint8_t *payload, bool is_decrypt); struct crypto_digest_t * cpath_get_incoming_digest(const crypt_path_t *cpath); +void cpath_sendme_record_cell_digest(crypt_path_t *cpath, + bool is_foward_digest); + void cpath_set_cell_forward_digest(crypt_path_t *cpath, cell_t *cell); diff --git a/src/core/or/relay.c b/src/core/or/relay.c index 91236e5fec..beff225af4 100644 --- a/src/core/or/relay.c +++ b/src/core/or/relay.c @@ -247,6 +247,10 @@ circuit_receive_relay_cell(cell_t *cell, circuit_t *circ, if (recognized) { edge_connection_t *conn = NULL; + /* Recognized cell, the cell digest has been updated, we'll record it for + * the SENDME if need be. */ + sendme_record_received_cell_digest(circ, layer_hint); + if (circ->purpose == CIRCUIT_PURPOSE_PATH_BIAS_TESTING) { if (pathbias_check_probe_response(circ, cell) == -1) { pathbias_count_valid_cells(circ, cell); diff --git a/src/core/or/sendme.c b/src/core/or/sendme.c index 80d6b7d165..eba7c37cdb 100644 --- a/src/core/or/sendme.c +++ b/src/core/or/sendme.c @@ -306,15 +306,6 @@ record_cell_digest_on_circ(circuit_t *circ, const uint8_t *sendme_digest) * Public API */ -/** Keep the current inbound cell digest for the next SENDME digest. This part - * is only done by the client as the circuit came back from the Exit. */ -void -sendme_circuit_record_outbound_cell(or_circuit_t *or_circ) -{ - tor_assert(or_circ); - relay_crypto_record_sendme_digest(&or_circ->crypto); -} - /** Return true iff the next cell for the given cell window is expected to be * a SENDME. * @@ -582,7 +573,8 @@ int sendme_note_stream_data_packaged(edge_connection_t *conn) { tor_assert(conn); - return --conn->package_window; + log_debug(LD_APP, "Stream package_window now %d.", --conn->package_window); + return conn->package_window; } /* Record the cell digest into the circuit sendme digest list depending on @@ -619,3 +611,52 @@ 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. Record the cell digest + * as the next sendme digest only if the next cell we'll send on the circuit + * is expected to be a SENDME. */ +void +sendme_record_received_cell_digest(circuit_t *circ, crypt_path_t *cpath) +{ + tor_assert(circ); + + /* Only record if the next cell is expected to be a SENDME. */ + if (!sendme_circuit_cell_is_next(cpath ? cpath->deliver_window : + circ->deliver_window)) { + return; + } + + if (cpath) { + /* Record incoming digest. */ + cpath_sendme_record_cell_digest(cpath, false); + } else { + /* Record foward digest. */ + relay_crypto_record_sendme_digest(&TO_OR_CIRCUIT(circ)->crypto, true); + } +} + +/* Called once we encrypted a cell. Record the cell digest as the next sendme + * digest only if the next cell we expect to receive is a SENDME so we can + * match the digests. */ +void +sendme_record_sending_cell_digest(circuit_t *circ, crypt_path_t *cpath) +{ + tor_assert(circ); + + /* Only record if the next cell is expected to be a SENDME. */ + if (!sendme_circuit_cell_is_next(cpath ? cpath->package_window: + circ->package_window)) { + goto end; + } + + if (cpath) { + /* Record the forward digest. */ + cpath_sendme_record_cell_digest(cpath, true); + } else { + /* Record the incoming digest. */ + relay_crypto_record_sendme_digest(&TO_OR_CIRCUIT(circ)->crypto, false); + } + + end: + return; +} diff --git a/src/core/or/sendme.h b/src/core/or/sendme.h index a71891996b..847fcdd672 100644 --- a/src/core/or/sendme.h +++ b/src/core/or/sendme.h @@ -34,10 +34,11 @@ int sendme_note_circuit_data_packaged(circuit_t *circ, crypt_path_t *layer_hint); int sendme_note_stream_data_packaged(edge_connection_t *conn); -/* Track cell digest. */ -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); +/* Record cell digest as the SENDME digest. */ +void sendme_record_received_cell_digest(circuit_t *circ, crypt_path_t *cpath); +void sendme_record_sending_cell_digest(circuit_t *circ, crypt_path_t *cpath); /* Circuit level information. */ bool sendme_circuit_cell_is_next(int window); From 482c4972b996fc7b1a3a8cc13f93c8ecc8748590 Mon Sep 17 00:00:00 2001 From: David Goulet Date: Tue, 21 May 2019 15:19:30 -0400 Subject: [PATCH 5/7] sendme: Clarify how sendme_circuit_cell_is_next() works Commit 4ef8470fa5480d3b was actually reverted before because in the end we needed to do this minus 1 check on the window. This commit clarifies that in the code, takes the useful comment changes from 4ef8470fa5480d3b and makes sendme_circuit_cell_is_next() private since it behaves in a very specific way that one external caller might expect. Part of #30428. Signed-off-by: David Goulet --- src/core/or/circuit_st.h | 9 ++++----- src/core/or/sendme.c | 30 ++++++++++++++++++++++-------- src/core/or/sendme.h | 3 --- 3 files changed, 26 insertions(+), 16 deletions(-) diff --git a/src/core/or/circuit_st.h b/src/core/or/circuit_st.h index a68547ecb1..499bf93d6b 100644 --- a/src/core/or/circuit_st.h +++ b/src/core/or/circuit_st.h @@ -115,12 +115,11 @@ struct circuit_t { * list can not contain more than 10 digests of DIGEST_LEN bytes (20). * * At position i in the list, the digest corresponds to the - * ((CIRCWINDOW_INCREMENT * i) - 1)-nth cell received since we expect the - * (CIRCWINDOW_INCREMENT * i)-nth cell to be the SENDME and thus containing - * the previous cell digest. + * (CIRCWINDOW_INCREMENT * i)-nth cell received since we expect a SENDME to + * be received containing that cell digest. * - * For example, position 2 (starting at 0) means that we've received 299 - * cells and the 299th cell digest is kept at index 2. + * For example, position 2 (starting at 0) means that we've received 300 + * cells so the 300th cell digest is kept at index 2. * * At maximum, this list contains 200 bytes plus the smartlist overhead. */ smartlist_t *sendme_last_digests; diff --git a/src/core/or/sendme.c b/src/core/or/sendme.c index eba7c37cdb..d914ba5e2e 100644 --- a/src/core/or/sendme.c +++ b/src/core/or/sendme.c @@ -312,15 +312,29 @@ record_cell_digest_on_circ(circuit_t *circ, const uint8_t *sendme_digest) * We are able to know that because the package or deliver window value minus * one cell (the possible SENDME cell) should be a multiple of the increment * window value. */ -bool -sendme_circuit_cell_is_next(int window) +static bool +circuit_sendme_cell_is_next(int window) { - /* Is this the last cell before a SENDME? The idea is that if the package or - * deliver window reaches a multiple of the increment, after this cell, we - * should expect a SENDME. */ + /* At the start of the window, no SENDME will be expected. */ + if (window == CIRCWINDOW_START) { + return false; + } + + /* Are we at the limit of the increment and if not, we don't expect next + * cell is a SENDME. + * + * We test against the window minus 1 because when we are looking if the + * next cell is a SENDME, the window (either package or deliver) hasn't been + * decremented just yet so when this is called, we are currently processing + * the "window - 1" cell. + * + * This function is used when recording a cell digest and this is done quite + * low in the stack when decrypting or encrypting a cell. The window is only + * updated once the cell is actually put in the outbuf. */ if (((window - 1) % CIRCWINDOW_INCREMENT) != 0) { return false; } + /* Next cell is expected to be a SENDME. */ return true; } @@ -596,7 +610,7 @@ sendme_record_cell_digest_on_circ(circuit_t *circ, crypt_path_t *cpath) /* 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(package_window)) { + if (!circuit_sendme_cell_is_next(package_window)) { return; } @@ -621,7 +635,7 @@ sendme_record_received_cell_digest(circuit_t *circ, crypt_path_t *cpath) tor_assert(circ); /* Only record if the next cell is expected to be a SENDME. */ - if (!sendme_circuit_cell_is_next(cpath ? cpath->deliver_window : + if (!circuit_sendme_cell_is_next(cpath ? cpath->deliver_window : circ->deliver_window)) { return; } @@ -644,7 +658,7 @@ sendme_record_sending_cell_digest(circuit_t *circ, crypt_path_t *cpath) tor_assert(circ); /* Only record if the next cell is expected to be a SENDME. */ - if (!sendme_circuit_cell_is_next(cpath ? cpath->package_window: + if (!circuit_sendme_cell_is_next(cpath ? cpath->package_window : circ->package_window)) { goto end; } diff --git a/src/core/or/sendme.h b/src/core/or/sendme.h index 847fcdd672..cdbdf55ac7 100644 --- a/src/core/or/sendme.h +++ b/src/core/or/sendme.h @@ -40,9 +40,6 @@ void sendme_record_cell_digest_on_circ(circuit_t *circ, crypt_path_t *cpath); void sendme_record_received_cell_digest(circuit_t *circ, crypt_path_t *cpath); void sendme_record_sending_cell_digest(circuit_t *circ, crypt_path_t *cpath); -/* Circuit level information. */ -bool sendme_circuit_cell_is_next(int window); - /* Private section starts. */ #ifdef SENDME_PRIVATE From 5479ffabf8ec1a3c63e9f237c6696c4ef0b74a55 Mon Sep 17 00:00:00 2001 From: David Goulet Date: Wed, 22 May 2019 10:37:27 -0400 Subject: [PATCH 6/7] sendme: Always pop last SENDME digest from circuit We must not accumulate digests on the circuit if the other end point is using another SENDME version that is not using those digests like v0. This commit makes it that we always pop the digest regardless of the version. Part of #30428 Signed-off-by: David Goulet --- src/core/or/sendme.c | 80 +++++++++++++++++++++++---------------- src/test/test_relaycell.c | 5 +++ src/test/test_sendme.c | 4 ++ 3 files changed, 57 insertions(+), 32 deletions(-) diff --git a/src/core/or/sendme.c b/src/core/or/sendme.c index d914ba5e2e..bfbb65e851 100644 --- a/src/core/or/sendme.c +++ b/src/core/or/sendme.c @@ -47,47 +47,46 @@ get_accept_min_version(void) SENDME_ACCEPT_MIN_VERSION_MAX); } +/* Pop the first cell digset on the given circuit from the SENDME last digests + * list. NULL is returned if the list is uninitialized or empty. + * + * The caller gets ownership of the returned digest thus is responsible for + * freeing the memory. */ +static uint8_t * +pop_first_cell_digest(const circuit_t *circ) +{ + uint8_t *circ_digest; + + tor_assert(circ); + + if (circ->sendme_last_digests == NULL || + smartlist_len(circ->sendme_last_digests) == 0) { + return NULL; + } + + circ_digest = smartlist_get(circ->sendme_last_digests, 0); + smartlist_del_keeporder(circ->sendme_last_digests, 0); + return circ_digest; +} + /* Return true iff the given cell digest matches the first digest in the * circuit sendme list. */ static bool -v1_digest_matches(const circuit_t *circ, const uint8_t *cell_digest) +v1_digest_matches(const uint8_t *circ_digest, const uint8_t *cell_digest) { - bool ret = false; - uint8_t *circ_digest = NULL; - - tor_assert(circ); + tor_assert(circ_digest); tor_assert(cell_digest); - /* We shouldn't have received a SENDME if we have no digests. Log at - * protocol warning because it can be tricked by sending many SENDMEs - * without prior data cell. */ - if (circ->sendme_last_digests == NULL || - smartlist_len(circ->sendme_last_digests) == 0) { - log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL, - "We received a SENDME but we have no cell digests to match. " - "Closing circuit."); - goto no_match; - } - - /* Pop the first element that was added (FIFO) and compare it. */ - circ_digest = smartlist_get(circ->sendme_last_digests, 0); - smartlist_del_keeporder(circ->sendme_last_digests, 0); - /* Compare the digest with the one in the SENDME. This cell is invalid * without a perfect match. */ if (tor_memneq(circ_digest, cell_digest, TRUNNEL_SENDME_V1_DIGEST_LEN)) { log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL, "SENDME v1 cell digest do not match."); - goto no_match; + return false; } - /* Digests matches! */ - ret = true; - no_match: - /* This digest was popped from the circuit list. Regardless of what happens, - * we have no more use for it. */ - tor_free(circ_digest); - return ret; + /* Digests matches! */ + return true; } /* Return true iff the given decoded SENDME version 1 cell is valid and @@ -97,13 +96,13 @@ v1_digest_matches(const circuit_t *circ, const uint8_t *cell_digest) * cell we saw which tells us that the other side has in fact seen that cell. * See proposal 289 for more details. */ static bool -cell_v1_is_valid(const sendme_cell_t *cell, const circuit_t *circ) +cell_v1_is_valid(const sendme_cell_t *cell, const uint8_t *circ_digest) { tor_assert(cell); - tor_assert(circ); + tor_assert(circ_digest); const uint8_t *cell_digest = sendme_cell_getconstarray_data_v1_digest(cell); - return v1_digest_matches(circ, cell_digest); + return v1_digest_matches(circ_digest, cell_digest); } /* Return true iff the given cell version can be handled or if the minimum @@ -160,6 +159,7 @@ sendme_is_valid(const circuit_t *circ, const uint8_t *cell_payload, size_t cell_payload_len) { uint8_t cell_version; + uint8_t *circ_digest = NULL; sendme_cell_t *cell = NULL; tor_assert(circ); @@ -184,10 +184,24 @@ sendme_is_valid(const circuit_t *circ, const uint8_t *cell_payload, goto invalid; } + /* Pop the first element that was added (FIFO). We do that regardless of the + * version so we don't accumulate on the circuit if v0 is used by the other + * end point. */ + circ_digest = pop_first_cell_digest(circ); + if (circ_digest == NULL) { + /* We shouldn't have received a SENDME if we have no digests. Log at + * protocol warning because it can be tricked by sending many SENDMEs + * without prior data cell. */ + log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL, + "We received a SENDME but we have no cell digests to match. " + "Closing circuit."); + goto invalid; + } + /* Validate depending on the version now. */ switch (cell_version) { case 0x01: - if (!cell_v1_is_valid(cell, circ)) { + if (!cell_v1_is_valid(cell, circ_digest)) { goto invalid; } break; @@ -204,9 +218,11 @@ sendme_is_valid(const circuit_t *circ, const uint8_t *cell_payload, /* Valid cell. */ sendme_cell_free(cell); + tor_free(circ_digest); return true; invalid: sendme_cell_free(cell); + tor_free(circ_digest); return false; } diff --git a/src/test/test_relaycell.c b/src/test/test_relaycell.c index d6372d3956..c65279fb25 100644 --- a/src/test/test_relaycell.c +++ b/src/test/test_relaycell.c @@ -17,6 +17,7 @@ #include "core/or/circuitbuild.h" #include "core/or/circuitlist.h" #include "core/or/connection_edge.h" +#include "core/or/sendme.h" #include "core/or/relay.h" #include "test/test.h" #include "test/log_test_helpers.h" @@ -813,6 +814,10 @@ test_circbw_relay(void *arg) /* Sendme on circuit with non-full window: counted */ PACK_CELL(0, RELAY_COMMAND_SENDME, ""); + /* Recording a cell, the window is updated after decryption so off by one in + * order to record and then we process it with the proper window. */ + circ->cpath->package_window = 901; + sendme_record_cell_digest_on_circ(TO_CIRCUIT(circ), circ->cpath); circ->cpath->package_window = 900; connection_edge_process_relay_cell(&cell, TO_CIRCUIT(circ), edgeconn, circ->cpath); diff --git a/src/test/test_sendme.c b/src/test/test_sendme.c index a36c904c28..fa5ae115ac 100644 --- a/src/test/test_sendme.c +++ b/src/test/test_sendme.c @@ -143,11 +143,13 @@ test_v1_build_cell(void *arg) or_circ = or_circuit_new(1, NULL); circ = TO_CIRCUIT(or_circ); + circ->sendme_last_digests = smartlist_new(); cell_digest = crypto_digest_new(); tt_assert(cell_digest); crypto_digest_add_bytes(cell_digest, "AAAAAAAAAAAAAAAAAAAA", 20); crypto_digest_get_digest(cell_digest, (char *) digest, sizeof(digest)); + smartlist_add(circ->sendme_last_digests, tor_memdup(digest, sizeof(digest))); /* SENDME v1 payload is 3 bytes + 20 bytes digest. See spec. */ ret = build_cell_payload_v1(digest, payload); @@ -157,6 +159,8 @@ test_v1_build_cell(void *arg) /* An empty payload means SENDME version 0 thus valid. */ tt_int_op(sendme_is_valid(circ, payload, 0), OP_EQ, true); + /* Current phoney digest should have been popped. */ + tt_int_op(smartlist_len(circ->sendme_last_digests), OP_EQ, 0); /* An unparseable cell means invalid. */ setup_full_capture_of_logs(LOG_INFO); From 0cad83bea4019b3c448756d718f5b65718e81f6e Mon Sep 17 00:00:00 2001 From: David Goulet Date: Wed, 22 May 2019 11:05:36 -0400 Subject: [PATCH 7/7] sendme: Add non fatal asserts for extra safety Two non fatal asserts are added in this commit. First one is to see if the SENDME digest list kept on the circuit for validation ever grows bigger than the maximum number of expected SENDME on a circuit (currently 10). The second one is to know if we ever send more than one SENDME at a time on a circuit. In theory, we shouldn't but if we ever do, the v1 implementation wouldn't work because we only keep one single cell digest (the previous cell to the SENDME) on the circuit/cpath. Thus, sending two SENDME consecutively will lead to a mismatch on the other side because the same cell digest would be use and thus the circuit would collapse. Finally, add an extra debug log in case we emit a v0 which also includes the consensus emit version in that case. Part of #30428 Signed-off-by: David Goulet --- src/core/or/sendme.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/core/or/sendme.c b/src/core/or/sendme.c index bfbb65e851..98ed639940 100644 --- a/src/core/or/sendme.c +++ b/src/core/or/sendme.c @@ -64,6 +64,13 @@ pop_first_cell_digest(const circuit_t *circ) return NULL; } + /* More cell digest than the SENDME window is never suppose to happen. The + * cell should have been rejected before reaching this point due to its + * package_window down to 0 leading to a circuit close. Scream loudly but + * still pop the element so we don't memory leak. */ + tor_assert_nonfatal(smartlist_len(circ->sendme_last_digests) <= + CIRCWINDOW_START_MAX / CIRCWINDOW_INCREMENT); + circ_digest = smartlist_get(circ->sendme_last_digests, 0); smartlist_del_keeporder(circ->sendme_last_digests, 0); return circ_digest; @@ -290,6 +297,8 @@ send_circuit_level_sendme(circuit_t *circ, crypt_path_t *layer_hint, default: /* Unknown version, fallback to version 0 meaning no payload. */ payload_len = 0; + log_debug(LD_PROTOCOL, "Emitting SENDME version 0 cell. " + "Consensus emit version is %d", emit_version); break; } @@ -408,6 +417,7 @@ sendme_connection_edge_consider_sending(edge_connection_t *conn) void sendme_circuit_consider_sending(circuit_t *circ, crypt_path_t *layer_hint) { + bool sent_one_sendme = false; const uint8_t *digest; while ((layer_hint ? layer_hint->deliver_window : circ->deliver_window) <= @@ -423,6 +433,12 @@ sendme_circuit_consider_sending(circuit_t *circ, crypt_path_t *layer_hint) if (send_circuit_level_sendme(circ, layer_hint, digest) < 0) { return; /* The circuit's closed, don't continue */ } + /* Current implementation is not suppose to send multiple SENDME at once + * because this means we would use the same relay crypto digest for each + * SENDME leading to a mismatch on the other side and the circuit to + * collapse. Scream loudly if it ever happens so we can address it. */ + tor_assert_nonfatal(!sent_one_sendme); + sent_one_sendme = true; } }