May 2019 - tor-commits - lists.torproject.org

[torspec/master] prop289: Rewrite the Deployment Plan section
by asn＠torproject.org 02 May '19

02 May '19

commit 81ec28177b01c91c6b1a6ac691ef33c7a9fe2754 Author: David Goulet <dgoulet(a)torproject.org> Date: Thu Jan 10 14:18:35 2019 -0500 prop289: Rewrite the Deployment Plan section Split it into subsection for clarity. Add a new subsection describing the addition of a new protover value. This patch removes a paragraph that states the obvious with "We'll need to do a bunch of testing". It is nice but also huge part of our development work flow so no need for it in the proposal. Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- proposals/289-authenticated-sendmes.txt | 129 +++++++++++++++++++++++++++----- 1 file changed, 111 insertions(+), 18 deletions(-) diff --git a/proposals/289-authenticated-sendmes.txt b/proposals/289-authenticated-sendmes.txt index 07258ef..df5226b 100644 --- a/proposals/289-authenticated-sendmes.txt +++ b/proposals/289-authenticated-sendmes.txt @@ -302,21 +302,61 @@ Status: Open 4. Deployment Plan - In phase one, both sides begin remembering their expected digests, - and they learn how to parse sendme payloads. When they receive a - sendme with payload version 1, they verify its digest and tear down - the circuit if it's wrong. But they continue to send and accept - payload version 0 sendmes. + This section describes how we will be able to deploy this new mechanism on + the network. - In phase two, we flip a switch in the consensus, and everybody starts - sending payload version 1 sendmes. Payload version 0 sendmes are still - accepted. The newly proposed consensus parameter to achieve this is: + Alas, this deployment plan leaves a pretty large window until relays are + protected from attack. It's not all bad news though, since we could flip + the switches earlier than intended if we encounter a network-wide attack. + + There are 5 phases to this plan and detailed in the subsections. + +4.1. Phase One - Remembering Digests + + Both sides begin remembering their expected digests, and they learn how to + parse sendme version 1 payloads. When they receive a version 1 SENDME, they + verify its digest and tear down the circuit if it's wrong. But they + continue to send and accept payload version 0 sendmes. + +4.2. Phase Two - Sending Version 1 + + We flip a switch in the consensus, and everybody starts sending payload + version 1 sendmes. Payload version 0 sendmes are still accepted. The newly + proposed consensus parameter to achieve this is: "sendme_emit_min_version" - Minimum SENDME version that can be sent. - In phase three, we flip a different switch in the consensus, and - everybody starts refusing payload version 0 sendmes. The newly proposed - consensus parameter to achieve this is: +4.3. Phase Three - Protover + + On phase four (section 4.4), the new consensus parameter that tells us + which minimum version to accept, once flipped to version 1, has the + consequence of making every tor not supporting that version to fail to + operate on the network. It goes as far as unable to download a consensus. + + It is essentially a "false-kill" switch because tor will still run but will + simply not work. It will retry over and over to download a consensus. In + order to help us transition before only accepting v1 on the network, a new + protover value is proposed (see section 9 of tor-spec.txt for protover + details). + + Tor clients and relays that don't support this protover version from the + consensus "required-client-protocols" or "required-relay-protocols" lines + will exit and thus not try to join the network. Here is the proposed value: + + "FlowCtrl" + + Describes the flow control protocol at the circuit and stream level. If + there is no FlowCtrl protocol version, tor supports the unauthenticated + flow control features from its supported Relay protocols. + + "1" -- supports authenticated circuit level SENDMEs as of proposal + 289 in Tor 0.4.1.1-alpha. + +4.4. Phase Four - Accepting Version 1 + + We flip a different switch in the consensus, and everybody starts refusing + payload version 0 sendmes. The newly proposed consensus parameter to + achieve this is: "sendme_accept_min_version" - Minimum SENDME version that is accepted. @@ -324,14 +364,66 @@ Status: Open otherwise we'd have a race where relays learn about the update before clients know to start the new behavior.) - We'll want to do a bunch of testing in chutney before flipping the - switches in the real network: I've long suspected we still have bugs - in our sendme timing, and this proposal might expose some of them. + Phase three will shutdown every clients that understand protover. However, + one important case remains which is when a client not supporting verison 1 + boots up for the first time (no consensus). + + It won't be able to download a consensus and thus know that it can't join + the network making it re-try over and over again. To avoid that, we propose + to still allow v0 SENDMEs on one-hop directory circuits meaning the new + parameter above affects everything except these specific circuits. + + It will allow us to extend the period of time between Phase Four and too + old clients to bootstrap and fail properly. The last phase will then turn + this behavior off and we will be done once and for all with version 0. + +4.5. Phase Five - Turning Off v0 + + The last consensus switch to flip is the one refusing SENDME v0 on one-hop + directory circuit as described in previous phase. + + The newly proposed consensus parameter to achieve this is: + + "sendme_onehop_dir_min_version" - Minimum SENDME version that is + accepted on one-hop directory circuits. + + After this phase, any tor still not supporting v0 will retry over and over + again to bootstrap. We can't avoid that but at least we can give enough + time to minimize the amount of old clients unable to boostrap with the + proposed timeline below. + +4.6. Timeline + + The proposed timeline for the deployment phases: + + Phase 1 and 2: + + Once this proposal is merged into tor (expected: 0.4.1.1-alpha). + + Those two phases will start roughly at the same time. The reason we + can do that is because SENDME payloads are ignored for version 0 thus + sending v1 right now will not affect Tor current behavior. + + Phase 3: + + This phase will effectively exit() all tor not supporting + "FlowCtrl=1". The earliest date we can do that is when all versions + not supporting v1 are EOL. + + According to our release schedule[4], this can happen when our latest + LTS (0.3.5) goes EOL that is on Feb 1st, 2022. + + Phase 4: + + We recommend to pass at least one version after Phase 3 so we can + take the time to see the effect that it had on the network. + Considering 6 months release time frame we expect to do this phase + around July 2022. + + Phase 5: - Alas, this deployment plan leaves a pretty large window until relays - are protected from attack. It's not all bad news though, since we - could flip the switches earlier than intended if we encounter a - network-wide attack. + Depends on how Phase 4 goes. It could be we'll do that very quickly + or not depending on our users and also Tor situation/health in 2022. 5. Security Discussion @@ -420,6 +512,7 @@ Status: Open [1] https://www.freehaven.net/anonbib/#sniper14 [2] https://www.freehaven.net/anonbib/#torta05 [3] https://www.freehaven.net/anonbib/#congestion-longpaths + [4] https://trac.torproject.org/projects/tor/wiki/org/teams/NetworkTeam/CoreTor… 8. Acknowledgements

1 0

[tor/master] sendme: Better handle the random padding
by asn＠torproject.org 02 May '19

02 May '19

commit d084f9115d7d46ad5e029b9c75cea716fa7d65a5 Author: David Goulet <dgoulet(a)torproject.org> Date: Wed Apr 24 15:39:10 2019 -0400 sendme: Better handle the random padding We add random padding to every cell if there is room. This commit not only fixes how we compute that random padding length/offset but also improves its safety with helper functions and a unit test. Part of #26288 Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/core/or/relay.c | 74 ++++++++++++++++++++++++++++++++++++++------------ src/core/or/relay.h | 1 + src/test/test_sendme.c | 46 +++++++++++++++++++++++++++++++ 3 files changed, 103 insertions(+), 18 deletions(-) diff --git a/src/core/or/relay.c b/src/core/or/relay.c index 504f391d9..d273facd5 100644 --- a/src/core/or/relay.c +++ b/src/core/or/relay.c @@ -529,6 +529,60 @@ relay_command_to_string(uint8_t command) } } +/** Return the offset where the padding should start. The data_len is + * the relay payload length expected to be put in the cell. It can not be + * bigger than RELAY_PAYLOAD_SIZE else this function assert(). + * + * Value will always be smaller than CELL_PAYLOAD_SIZE because this offset is + * for the entire cell length not just the data payload length. Zero is + * returned if there is no room for padding. + * + * This function always skips the first 4 bytes after the payload because + * having some unused zero bytes has saved us a lot of times in the past. */ + +STATIC size_t +get_pad_cell_offset(size_t data_len) +{ + /* This is never suppose to happen but in case it does, stop right away + * because if tor is tricked somehow into not adding random bytes to the + * payload with this function returning 0 for a bad data_len, the entire + * authenticated SENDME design can be bypassed leading to bad denial of + * service attacks. */ + tor_assert(data_len <= RELAY_PAYLOAD_SIZE); + + /* If the offset is larger than the cell payload size, we return an offset + * of zero indicating that no padding needs to be added. */ + size_t offset = RELAY_HEADER_SIZE + data_len + 4; + if (offset >= CELL_PAYLOAD_SIZE) { + return 0; + } + return offset; +} + +/* Add random bytes to the unused portion of the payload, to foil attacks + * where the other side can predict all of the bytes in the payload and thus + * compute the authenticated SENDME cells without seeing the traffic. See + * proposal 289. */ +static void +pad_cell_payload(uint8_t *cell_payload, size_t data_len) +{ + size_t pad_offset, pad_len; + + tor_assert(cell_payload); + + pad_offset = get_pad_cell_offset(data_len); + if (pad_offset == 0) { + /* We can't add padding so we are done. */ + return; + } + + /* Remember here that the cell_payload is the length of the header and + * payload size so we offset it using the full lenght of the cell. */ + pad_len = CELL_PAYLOAD_SIZE - pad_offset; + crypto_fast_rng_getbytes(get_thread_fast_rng(), + cell_payload + pad_offset, pad_len); +} + /** Make a relay cell out of relay_command and payload, and send * it onto the open circuit circ. stream_id is the ID on * circ for the stream that's sending the relay cell, or 0 if it's a @@ -547,8 +601,6 @@ relay_send_command_from_edge_,(streamid_t stream_id, circuit_t *circ, cell_t cell; relay_header_t rh; cell_direction_t cell_direction; - int random_bytes_len; - size_t random_bytes_offset = 0; /* XXXX NM Split this function into a separate versions per circuit type? */ tor_assert(circ); @@ -574,22 +626,8 @@ relay_send_command_from_edge_,(streamid_t stream_id, circuit_t *circ, if (payload_len) memcpy(cell.payload+RELAY_HEADER_SIZE, payload, payload_len); - /* Add random bytes to the unused portion of the payload, to foil attacks - * where the other side can predict all of the bytes in the payload and thus - * compute authenticated sendme cells without seeing the traffic. See - * proposal 289. - * - * We'll skip the first 4 bytes of unused data because having some unused - * zero bytes has saved us a lot of times in the past. */ - random_bytes_len = RELAY_PAYLOAD_SIZE - - (RELAY_HEADER_SIZE + payload_len + 4); - if (random_bytes_len < 0) { - random_bytes_len = 0; - } - random_bytes_offset = RELAY_PAYLOAD_SIZE - random_bytes_len; - crypto_fast_rng_getbytes(get_thread_fast_rng(), - cell.payload + random_bytes_offset, - random_bytes_len); + /* Add random padding to the cell if we can. */ + pad_cell_payload(cell.payload, payload_len); log_debug(LD_OR,"delivering %d cell %s.", relay_command, cell_direction == CELL_DIRECTION_OUT ? "forward" : "backward"); diff --git a/src/core/or/relay.h b/src/core/or/relay.h index ea1b358ff..2248cdf38 100644 --- a/src/core/or/relay.h +++ b/src/core/or/relay.h @@ -120,6 +120,7 @@ STATIC int cell_queues_check_size(void); STATIC int connection_edge_process_relay_cell(cell_t *cell, circuit_t *circ, edge_connection_t *conn, crypt_path_t *layer_hint); +STATIC size_t get_pad_cell_offset(size_t payload_len); #endif /* defined(RELAY_PRIVATE) */ diff --git a/src/test/test_sendme.c b/src/test/test_sendme.c index d6410a748..50943e5f4 100644 --- a/src/test/test_sendme.c +++ b/src/test/test_sendme.c @@ -6,11 +6,13 @@ #define CIRCUITLIST_PRIVATE #define NETWORKSTATUS_PRIVATE #define SENDME_PRIVATE +#define RELAY_PRIVATE #include "core/or/circuit_st.h" #include "core/or/or_circuit_st.h" #include "core/or/origin_circuit_st.h" #include "core/or/circuitlist.h" +#include "core/or/relay.h" #include "core/or/sendme.h" #include "feature/nodelist/networkstatus.h" @@ -209,6 +211,48 @@ test_v1_build_cell(void *arg) circuit_free_(circ); } +static void +test_cell_payload_pad(void *arg) +{ + size_t pad_offset, payload_len, expected_offset; + + (void) arg; + + /* Offset should be 0, not enough room for padding. */ + payload_len = RELAY_PAYLOAD_SIZE; + pad_offset = get_pad_cell_offset(payload_len); + tt_int_op(pad_offset, OP_EQ, 0); + tt_int_op(CELL_PAYLOAD_SIZE - pad_offset, OP_LE, CELL_PAYLOAD_SIZE); + + /* Still no room because we keep 4 extra bytes. */ + pad_offset = get_pad_cell_offset(payload_len - 4); + tt_int_op(pad_offset, OP_EQ, 0); + tt_int_op(CELL_PAYLOAD_SIZE - pad_offset, OP_LE, CELL_PAYLOAD_SIZE); + + /* We should have 1 byte of padding. Meaning, the offset should be the + * CELL_PAYLOAD_SIZE minus 1 byte. */ + expected_offset = CELL_PAYLOAD_SIZE - 1; + pad_offset = get_pad_cell_offset(payload_len - 5); + tt_int_op(pad_offset, OP_EQ, expected_offset); + tt_int_op(CELL_PAYLOAD_SIZE - pad_offset, OP_LE, CELL_PAYLOAD_SIZE); + + /* Now some arbitrary small payload length. The cell size is header + 10 + + * extra 4 bytes we keep so the offset should be there. */ + expected_offset = RELAY_HEADER_SIZE + 10 + 4; + pad_offset = get_pad_cell_offset(10); + tt_int_op(pad_offset, OP_EQ, expected_offset); + tt_int_op(CELL_PAYLOAD_SIZE - pad_offset, OP_LE, CELL_PAYLOAD_SIZE); + + /* Data length of 0. */ + expected_offset = RELAY_HEADER_SIZE + 4; + pad_offset = get_pad_cell_offset(0); + tt_int_op(pad_offset, OP_EQ, expected_offset); + tt_int_op(CELL_PAYLOAD_SIZE - pad_offset, OP_LE, CELL_PAYLOAD_SIZE); + + done: + ; +} + struct testcase_t sendme_tests[] = { { "v1_note_digest", test_v1_note_digest, TT_FORK, NULL, NULL }, @@ -216,6 +260,8 @@ struct testcase_t sendme_tests[] = { NULL, NULL }, { "v1_build_cell", test_v1_build_cell, TT_FORK, NULL, NULL }, + { "cell_payload_pad", test_cell_payload_pad, TT_FORK, + NULL, NULL }, END_OF_TESTCASES };

1 0

[tor/master] sendme: Keep cell digest only if a SENDME is next
by asn＠torproject.org 02 May '19

02 May '19

commit c7385b5b14b30774c1768798c4495465da4d995d Author: David Goulet <dgoulet(a)torproject.org> Date: Wed Apr 24 13:38:47 2019 -0400 sendme: Keep cell digest only if a SENDME is next This way, we reduce the load by only hashing when we absolutely must. Part of #26288 Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/core/crypto/relay_crypto.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/src/core/crypto/relay_crypto.c b/src/core/crypto/relay_crypto.c index 94e906065..eddc4298e 100644 --- a/src/core/crypto/relay_crypto.c +++ b/src/core/crypto/relay_crypto.c @@ -12,6 +12,7 @@ #include "core/crypto/hs_ntor.h" // for HS_NTOR_KEY_EXPANSION_KDF_OUT_LEN #include "core/or/relay.h" #include "core/crypto/relay_crypto.h" +#include "core/or/sendme.h" #include "core/or/cell_st.h" #include "core/or/or_circuit_st.h" @@ -142,10 +143,11 @@ relay_decrypt_cell(circuit_t *circ, cell_t *cell, if (relay_digest_matches(thishop->crypto.b_digest, cell)) { *recognized = 1; *layer_hint = thishop; - /* Keep current digest of this cell for the possible SENDME. */ - crypto_digest_get_digest(thishop->crypto.b_digest, - (char *) thishop->crypto.sendme_digest, - sizeof(thishop->crypto.sendme_digest)); + /* This cell is for us. Keep a record of this cell because we will + * use it in the next SENDME cell. */ + if (sendme_circuit_is_next_cell(thishop->deliver_window)) { + sendme_circuit_note_inbound_cell(thishop); + } return 0; } } @@ -216,10 +218,13 @@ relay_encrypt_cell_inbound(cell_t *cell, or_circuit_t *or_circ) { relay_set_digest(or_circ->crypto.b_digest, cell); - /* Keep a record of this cell, we might use it for validating the SENDME. */ - crypto_digest_get_digest(or_circ->crypto.b_digest, - (char *) or_circ->crypto.sendme_digest, - sizeof(or_circ->crypto.sendme_digest)); + + /* 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_is_next_cell(TO_CIRCUIT(or_circ)->package_window)) { + sendme_circuit_note_outbound_cell(or_circ); + } + /* encrypt one layer */ relay_crypt_one_payload(or_circ->crypto.b_crypto, cell->payload); }

1 0

[tor/master] sendme: Move note_cell_digest() to relay_crypto module
by asn＠torproject.org 02 May '19

02 May '19

commit 67c22541830cf1dfc1c02843a1d4dd81c2df16ff Author: David Goulet <dgoulet(a)torproject.org> Date: Mon Apr 29 11:29:05 2019 -0400 sendme: Move note_cell_digest() to relay_crypto module Because this function is poking within the relay_crypto_t object, move the function to the module so we can keep it opaque as much as possible. Part of #26288 Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/core/crypto/relay_crypto.c | 9 +++++++++ src/core/crypto/relay_crypto.h | 2 ++ src/core/or/sendme.c | 14 +++----------- 3 files changed, 14 insertions(+), 11 deletions(-) diff --git a/src/core/crypto/relay_crypto.c b/src/core/crypto/relay_crypto.c index eddc4298e..3a6dad4b1 100644 --- a/src/core/crypto/relay_crypto.c +++ b/src/core/crypto/relay_crypto.c @@ -91,6 +91,15 @@ relay_crypt_one_payload(crypto_cipher_t *cipher, uint8_t *in) crypto_cipher_crypt_inplace(cipher, (char*) in, CELL_PAYLOAD_SIZE); } +/** Record the b_digest from crypto and put it in the sendme_digest. */ +void +relay_crypto_record_sendme_digest(relay_crypto_t *crypto) +{ + tor_assert(crypto); + crypto_digest_get_digest(crypto->b_digest, (char *) crypto->sendme_digest, + sizeof(crypto->sendme_digest)); +} + /** Do the appropriate en/decryptions for cell arriving on * circ in direction cell_direction. * diff --git a/src/core/crypto/relay_crypto.h b/src/core/crypto/relay_crypto.h index 45a21d14a..1009f1841 100644 --- a/src/core/crypto/relay_crypto.h +++ b/src/core/crypto/relay_crypto.h @@ -27,5 +27,7 @@ void relay_crypto_clear(relay_crypto_t *crypto); void relay_crypto_assert_ok(const relay_crypto_t *crypto); +void relay_crypto_record_sendme_digest(relay_crypto_t *crypto); + #endif /* !defined(TOR_RELAY_CRYPTO_H) */ diff --git a/src/core/or/sendme.c b/src/core/or/sendme.c index b384a1916..b65f30ba9 100644 --- a/src/core/or/sendme.c +++ b/src/core/or/sendme.c @@ -12,6 +12,7 @@ #include "core/or/or.h" #include "app/config/config.h" +#include "core/crypto/relay_crypto.h" #include "core/mainloop/connection.h" #include "core/or/cell_st.h" #include "core/or/circuitlist.h" @@ -286,15 +287,6 @@ send_circuit_level_sendme(circuit_t *circ, crypt_path_t *layer_hint, return 0; } -/* Put the crypto.b_digest in the sendme_digest. */ -static void -note_cell_digest(const relay_crypto_t *crypto) -{ - tor_assert(crypto); - crypto_digest_get_digest(crypto->b_digest, (char *) crypto->sendme_digest, - sizeof(crypto->sendme_digest)); -} - /* * Public API */ @@ -305,7 +297,7 @@ void sendme_circuit_note_outbound_cell(or_circuit_t *or_circ) { tor_assert(or_circ); - note_cell_digest(&or_circ->crypto); + relay_crypto_record_sendme_digest(&or_circ->crypto); } /** Keep the current inbound cell digest for the next SENDME digest. This part @@ -314,7 +306,7 @@ void sendme_circuit_note_inbound_cell(crypt_path_t *cpath) { tor_assert(cpath); - note_cell_digest(&cpath->crypto); + relay_crypto_record_sendme_digest(&cpath->crypto); } /** Return true iff the next cell for the given cell window is expected to be

1 0

[tor/master] sendme: Better function names
by asn＠torproject.org 02 May '19

02 May '19

commit 0d8b9b56c5332b8f0205f460d0b23bb7f5620eff Author: David Goulet <dgoulet(a)torproject.org> Date: Mon Apr 29 11:38:11 2019 -0400 sendme: Better function names From nickm's review, improve the names of some functions. Part of #26288 Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/core/crypto/relay_crypto.c | 8 ++++---- src/core/or/relay.c | 2 +- src/core/or/sendme.c | 10 +++++----- src/core/or/sendme.h | 8 ++++---- src/test/test_sendme.c | 18 +++++++++--------- 5 files changed, 23 insertions(+), 23 deletions(-) diff --git a/src/core/crypto/relay_crypto.c b/src/core/crypto/relay_crypto.c index 3a6dad4b1..5be45d6c7 100644 --- a/src/core/crypto/relay_crypto.c +++ b/src/core/crypto/relay_crypto.c @@ -154,8 +154,8 @@ relay_decrypt_cell(circuit_t *circ, cell_t *cell, *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_is_next_cell(thishop->deliver_window)) { - sendme_circuit_note_inbound_cell(thishop); + if (sendme_circuit_cell_is_next(thishop->deliver_window)) { + sendme_circuit_record_inbound_cell(thishop); } return 0; } @@ -230,8 +230,8 @@ relay_encrypt_cell_inbound(cell_t *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_is_next_cell(TO_CIRCUIT(or_circ)->package_window)) { - sendme_circuit_note_outbound_cell(or_circ); + if (sendme_circuit_cell_is_next(TO_CIRCUIT(or_circ)->package_window)) { + sendme_circuit_record_outbound_cell(or_circ); } /* encrypt one layer */ diff --git a/src/core/or/relay.c b/src/core/or/relay.c index d273facd5..1b2aafb86 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_note_cell_digest(circ); + sendme_record_cell_digest(circ); } return 0; diff --git a/src/core/or/sendme.c b/src/core/or/sendme.c index b65f30ba9..ff58c1489 100644 --- a/src/core/or/sendme.c +++ b/src/core/or/sendme.c @@ -294,7 +294,7 @@ send_circuit_level_sendme(circuit_t *circ, crypt_path_t *layer_hint, /** 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_note_outbound_cell(or_circuit_t *or_circ) +sendme_circuit_record_outbound_cell(or_circuit_t *or_circ) { tor_assert(or_circ); relay_crypto_record_sendme_digest(&or_circ->crypto); @@ -303,7 +303,7 @@ sendme_circuit_note_outbound_cell(or_circuit_t *or_circ) /** 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_note_inbound_cell(crypt_path_t *cpath) +sendme_circuit_record_inbound_cell(crypt_path_t *cpath) { tor_assert(cpath); relay_crypto_record_sendme_digest(&cpath->crypto); @@ -316,7 +316,7 @@ sendme_circuit_note_inbound_cell(crypt_path_t *cpath) * one cell (the possible SENDME cell) should be a multiple of the increment * window value. */ bool -sendme_circuit_is_next_cell(int window) +sendme_circuit_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 @@ -578,7 +578,7 @@ sendme_note_stream_data_packaged(edge_connection_t *conn) /* 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. */ void -sendme_note_cell_digest(circuit_t *circ) +sendme_record_cell_digest(circuit_t *circ) { const uint8_t *digest; @@ -592,7 +592,7 @@ sendme_note_cell_digest(circuit_t *circ) /* 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_is_next_cell(circ->package_window)) { + if (!sendme_circuit_cell_is_next(circ->package_window)) { return; } diff --git a/src/core/or/sendme.h b/src/core/or/sendme.h index 5889b41e9..78273eb9a 100644 --- a/src/core/or/sendme.h +++ b/src/core/or/sendme.h @@ -35,12 +35,12 @@ int sendme_note_circuit_data_packaged(circuit_t *circ, int sendme_note_stream_data_packaged(edge_connection_t *conn); /* Track cell digest. */ -void sendme_note_cell_digest(circuit_t *circ); -void sendme_circuit_note_inbound_cell(crypt_path_t *cpath); -void sendme_circuit_note_outbound_cell(or_circuit_t *or_circ); +void sendme_record_cell_digest(circuit_t *circ); +void sendme_circuit_record_inbound_cell(crypt_path_t *cpath); +void sendme_circuit_record_outbound_cell(or_circuit_t *or_circ); /* Circuit level information. */ -bool sendme_circuit_is_next_cell(int window); +bool sendme_circuit_cell_is_next(int window); /* Private section starts. */ #ifdef SENDME_PRIVATE diff --git a/src/test/test_sendme.c b/src/test/test_sendme.c index 50943e5f4..d40fbaf86 100644 --- a/src/test/test_sendme.c +++ b/src/test/test_sendme.c @@ -43,7 +43,7 @@ free_mock_consensus(void) } static void -test_v1_note_digest(void *arg) +test_v1_record_digest(void *arg) { or_circuit_t *or_circ = NULL; origin_circuit_t *orig_circ = NULL; @@ -61,7 +61,7 @@ test_v1_note_digest(void *arg) circ->purpose = CIRCUIT_PURPOSE_S_REND_JOINED; /* We should never note SENDME digest on origin circuit. */ - sendme_note_cell_digest(circ); + sendme_record_cell_digest(circ); tt_assert(!circ->sendme_last_digests); /* We do not need the origin circuit for now. */ orig_circ = NULL; @@ -73,23 +73,23 @@ test_v1_note_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_note_cell_digest(circ); + sendme_record_cell_digest(circ); tt_assert(!circ->sendme_last_digests); /* This should work now. Package window at CIRCWINDOW_INCREMENT + 1. */ circ->package_window++; - sendme_note_cell_digest(circ); + sendme_record_cell_digest(circ); 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_note_cell_digest(circ); + sendme_record_cell_digest(circ); 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_note_cell_digest(circ); + sendme_record_cell_digest(circ); tt_int_op(smartlist_len(circ->sendme_last_digests), OP_EQ, 2); done: @@ -188,7 +188,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_note_cell_digest(circ); + sendme_record_cell_digest(circ); 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 +200,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_note_cell_digest(circ); + sendme_record_cell_digest(circ); 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. */ @@ -254,7 +254,7 @@ test_cell_payload_pad(void *arg) } struct testcase_t sendme_tests[] = { - { "v1_note_digest", test_v1_note_digest, TT_FORK, + { "v1_record_digest", test_v1_record_digest, TT_FORK, NULL, NULL }, { "v1_consensus_params", test_v1_consensus_params, TT_FORK, NULL, NULL },

1 0

[tor/master] sendme: Add helper to note the cell digest
by asn＠torproject.org 02 May '19

02 May '19

commit 805c81efed9bc2c474d3f10675846ee445a908d5 Author: David Goulet <dgoulet(a)torproject.org> Date: Wed Apr 24 11:57:20 2019 -0400 sendme: Add helper to note the cell digest Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/core/or/sendme.c | 27 +++++++++++++++++++++++++++ src/core/or/sendme.h | 2 ++ 2 files changed, 29 insertions(+) diff --git a/src/core/or/sendme.c b/src/core/or/sendme.c index c66e947bc..b384a1916 100644 --- a/src/core/or/sendme.c +++ b/src/core/or/sendme.c @@ -286,10 +286,37 @@ send_circuit_level_sendme(circuit_t *circ, crypt_path_t *layer_hint, return 0; } +/* Put the crypto.b_digest in the sendme_digest. */ +static void +note_cell_digest(const relay_crypto_t *crypto) +{ + tor_assert(crypto); + crypto_digest_get_digest(crypto->b_digest, (char *) crypto->sendme_digest, + sizeof(crypto->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_note_outbound_cell(or_circuit_t *or_circ) +{ + tor_assert(or_circ); + note_cell_digest(&or_circ->crypto); +} + +/** 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_note_inbound_cell(crypt_path_t *cpath) +{ + tor_assert(cpath); + note_cell_digest(&cpath->crypto); +} + /** Return true iff the next cell for the given cell window is expected to be * a SENDME. * diff --git a/src/core/or/sendme.h b/src/core/or/sendme.h index 0965b5b22..5889b41e9 100644 --- a/src/core/or/sendme.h +++ b/src/core/or/sendme.h @@ -36,6 +36,8 @@ int sendme_note_stream_data_packaged(edge_connection_t *conn); /* Track cell digest. */ void sendme_note_cell_digest(circuit_t *circ); +void sendme_circuit_note_inbound_cell(crypt_path_t *cpath); +void sendme_circuit_note_outbound_cell(or_circuit_t *or_circ); /* Circuit level information. */ bool sendme_circuit_is_next_cell(int window);

1 0

[tor/master] sendme: Do not poke at crypto.sendme_digest directly
by asn＠torproject.org 02 May '19

02 May '19

commit 0f2ff267c5b509d697882989341d91b9fb4c249d Author: David Goulet <dgoulet(a)torproject.org> Date: Mon Apr 29 12:11:57 2019 -0400 sendme: Do not poke at crypto.sendme_digest directly As per review from nickm, keep as much as we can the relay_crypto_t object opaque. Part of #26288 Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/core/crypto/relay_crypto.c | 8 ++++++++ src/core/crypto/relay_crypto.h | 1 + src/core/or/sendme.c | 6 +++--- 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/core/crypto/relay_crypto.c b/src/core/crypto/relay_crypto.c index 5be45d6c7..893116316 100644 --- a/src/core/crypto/relay_crypto.c +++ b/src/core/crypto/relay_crypto.c @@ -91,6 +91,14 @@ relay_crypt_one_payload(crypto_cipher_t *cipher, uint8_t *in) crypto_cipher_crypt_inplace(cipher, (char*) in, CELL_PAYLOAD_SIZE); } +/** Return the sendme_digest within the crypto object. */ +uint8_t * +relay_crypto_get_sendme_digest(relay_crypto_t *crypto) +{ + tor_assert(crypto); + return crypto->sendme_digest; +} + /** Record the b_digest from crypto and put it in the sendme_digest. */ void relay_crypto_record_sendme_digest(relay_crypto_t *crypto) diff --git a/src/core/crypto/relay_crypto.h b/src/core/crypto/relay_crypto.h index 1009f1841..bcc153183 100644 --- a/src/core/crypto/relay_crypto.h +++ b/src/core/crypto/relay_crypto.h @@ -27,6 +27,7 @@ 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); #endif /* !defined(TOR_RELAY_CRYPTO_H) */ diff --git a/src/core/or/sendme.c b/src/core/or/sendme.c index ff58c1489..6f451d38e 100644 --- a/src/core/or/sendme.c +++ b/src/core/or/sendme.c @@ -388,10 +388,10 @@ sendme_circuit_consider_sending(circuit_t *circ, crypt_path_t *layer_hint) log_debug(LD_CIRC,"Queuing circuit sendme."); if (layer_hint) { layer_hint->deliver_window += CIRCWINDOW_INCREMENT; - digest = layer_hint->crypto.sendme_digest; + digest = relay_crypto_get_sendme_digest(&layer_hint->crypto); } else { circ->deliver_window += CIRCWINDOW_INCREMENT; - digest = TO_OR_CIRCUIT(circ)->crypto.sendme_digest; + digest = relay_crypto_get_sendme_digest(&TO_OR_CIRCUIT(circ)->crypto); } if (send_circuit_level_sendme(circ, layer_hint, digest) < 0) { return; /* The circuit's closed, don't continue */ @@ -597,7 +597,7 @@ sendme_record_cell_digest(circuit_t *circ) } /* Add the digest to the last seen list in the circuit. */ - digest = TO_OR_CIRCUIT(circ)->crypto.sendme_digest; + digest = relay_crypto_get_sendme_digest(&TO_OR_CIRCUIT(circ)->crypto); if (circ->sendme_last_digests == NULL) { circ->sendme_last_digests = smartlist_new(); }

1 0

[tor/master] Merge branch 'tor-github/pr/986'
by asn＠torproject.org 02 May '19

02 May '19

commit b2c2cb928722cd3ecd0781d51ce5c9a7c9029b55 Merge: 4d461e20f 562bcbcfc Author: George Kadianakis <desnacked(a)riseup.net> Date: Thu May 2 18:12:52 2019 +0300 Merge branch 'tor-github/pr/986' changes/ticket26288 | 6 + scripts/maint/practracker/exceptions.txt | 10 +- src/core/crypto/relay_crypto.c | 30 ++ src/core/crypto/relay_crypto.h | 3 + src/core/include.am | 2 + src/core/or/circuit_st.h | 20 + src/core/or/circuitlist.c | 6 + src/core/or/connection_edge.c | 24 +- src/core/or/connection_edge.h | 1 + src/core/or/relay.c | 344 +++++++++--------- src/core/or/relay.h | 1 + src/core/or/relay_crypto_st.h | 2 + src/core/or/sendme.c | 604 +++++++++++++++++++++++++++++++ src/core/or/sendme.h | 68 ++++ src/test/include.am | 1 + src/test/test.c | 1 + src/test/test.h | 1 + src/test/test_sendme.c | 267 ++++++++++++++ src/trunnel/include.am | 3 + src/trunnel/sendme.c | 347 ++++++++++++++++++ src/trunnel/sendme.h | 101 ++++++ src/trunnel/sendme.trunnel | 19 + 22 files changed, 1676 insertions(+), 185 deletions(-) diff --cc scripts/maint/practracker/exceptions.txt index c41b0a0b1,f8bb2bd37..9b4c03a72 --- a/scripts/maint/practracker/exceptions.txt +++ b/scripts/maint/practracker/exceptions.txt @@@ -119,10 -119,10 +119,10 @@@ problem function-size /src/core/or/conn problem function-size /src/core/or/connection_or.c:connection_or_compute_authenticate_cell_body() 235 problem file-size /src/core/or/policies.c 3171 problem function-size /src/core/or/policies.c:policy_summarize() 107 -problem function-size /src/core/or/protover.c:protover_all_supported() 116 +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_() 101 + problem function-size /src/core/or/relay.c:relay_send_command_from_edge_() 112 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

1 0

[tor/master] practracker: Update exceptions for #26288
by asn＠torproject.org 02 May '19

02 May '19

commit 535ba0d7c58c681af8251d0133b33dc5f787fb2f Author: David Goulet <dgoulet(a)torproject.org> Date: Mon Apr 29 12:27:53 2019 -0400 practracker: Update exceptions for #26288 Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- scripts/maint/practracker/exceptions.txt | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/scripts/maint/practracker/exceptions.txt b/scripts/maint/practracker/exceptions.txt index 37f11bb44..f8bb2bd37 100644 --- a/scripts/maint/practracker/exceptions.txt +++ b/scripts/maint/practracker/exceptions.txt @@ -85,7 +85,7 @@ problem function-size /src/core/or/circuitbuild.c:circuit_extend() 147 problem function-size /src/core/or/circuitbuild.c:choose_good_exit_server_general() 206 problem include-count /src/core/or/circuitlist.c 54 problem function-size /src/core/or/circuitlist.c:HT_PROTOTYPE() 128 -problem function-size /src/core/or/circuitlist.c:circuit_free_() 137 +problem function-size /src/core/or/circuitlist.c:circuit_free_() 143 problem function-size /src/core/or/circuitlist.c:circuit_find_to_cannibalize() 102 problem function-size /src/core/or/circuitlist.c:circuit_about_to_free() 120 problem function-size /src/core/or/circuitlist.c:circuits_handle_oom() 117 @@ -102,8 +102,8 @@ problem function-size /src/core/or/circuituse.c:circuit_get_open_circ_or_launch( problem function-size /src/core/or/circuituse.c:connection_ap_handshake_attach_circuit() 244 problem function-size /src/core/or/command.c:command_process_create_cell() 156 problem function-size /src/core/or/command.c:command_process_relay_cell() 132 -problem file-size /src/core/or/connection_edge.c 4575 -problem include-count /src/core/or/connection_edge.c 64 +problem file-size /src/core/or/connection_edge.c 4595 +problem include-count /src/core/or/connection_edge.c 65 problem function-size /src/core/or/connection_edge.c:connection_ap_expire_beginning() 117 problem function-size /src/core/or/connection_edge.c:connection_ap_handshake_rewrite() 192 problem function-size /src/core/or/connection_edge.c:connection_ap_handle_onion() 188 @@ -122,11 +122,11 @@ problem function-size /src/core/or/policies.c:policy_summarize() 107 problem function-size /src/core/or/protover.c:protover_all_supported() 116 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_() 101 +problem function-size /src/core/or/relay.c:relay_send_command_from_edge_() 112 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 -problem function-size /src/core/or/relay.c:connection_edge_package_raw_inbuf() 130 +problem function-size /src/core/or/relay.c:connection_edge_package_raw_inbuf() 132 problem function-size /src/core/or/relay.c:circuit_resume_edge_reading_helper() 148 problem function-size /src/core/or/scheduler_kist.c:kist_scheduler_run() 171 problem function-size /src/core/or/scheduler_vanilla.c:vanilla_scheduler_run() 109

1 0

[tor/master] sendme: Add changes file for prop289
by asn＠torproject.org 02 May '19

02 May '19

commit 562bcbcfc216df9f25cd72f1e227b9313c872172 Author: David Goulet <dgoulet(a)torproject.org> Date: Thu May 2 11:10:41 2019 -0400 sendme: Add changes file for prop289 Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- changes/ticket26288 | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/changes/ticket26288 b/changes/ticket26288 new file mode 100644 index 000000000..59bb856dd --- /dev/null +++ b/changes/ticket26288 @@ -0,0 +1,6 @@ + o Major features (flow control): + - Implement authenticated SENDMEs detailed in proposal 289. A SENDME cell + now includes the digest of the last cell received so once the end point + receives the SENDME, it can confirm the other side's knowledge of the + previous cells that were sent. This behavior is controlled by two new + consensus parameters, see proposal for more details. Fixes ticket 26288.

1 0