August 2017 - tor-commits - lists.torproject.org

[tor/master] prop224: Update OOM counters when storing a client descriptor cache object
by nickm＠torproject.org 24 Aug '17

24 Aug '17

commit e046e6fb4816e118f1bb32bd709b5dce5b729994 Author: David Goulet <dgoulet(a)torproject.org> Date: Tue Aug 22 11:30:11 2017 -0400 prop224: Update OOM counters when storing a client descriptor cache object Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/or/hs_cache.c | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/src/or/hs_cache.c b/src/or/hs_cache.c index e0d89a70e..6962c5ce4 100644 --- a/src/or/hs_cache.c +++ b/src/or/hs_cache.c @@ -329,6 +329,14 @@ static digest256map_t *hs_cache_v3_client; * objects all related to a specific service. */ static digest256map_t *hs_cache_client_intro_state; +/* Return the size of a client cache entry in bytes. */ +static size_t +cache_get_client_entry_size(const hs_cache_client_descriptor_t *entry) +{ + return sizeof(*entry) + + strlen(entry->encoded_desc) + hs_desc_obj_size(entry->desc); +} + /* Remove a given descriptor from our cache. */ static void remove_v3_desc_as_client(const hs_cache_client_descriptor_t *desc) @@ -345,6 +353,8 @@ store_v3_desc_as_client(hs_cache_client_descriptor_t *desc) { tor_assert(desc); digest256map_set(hs_cache_v3_client, desc->key.pubkey, desc); + /* Update cache size with this entry for the OOM handler. */ + rend_cache_increment_allocation(cache_get_client_entry_size(desc)); } /* Query our cache and return the entry or NULL if not found. */ @@ -355,14 +365,6 @@ lookup_v3_desc_as_client(const uint8_t *key) return digest256map_get(hs_cache_v3_client, key); } -/* Return the size of a client cache entry in bytes. */ -static size_t -cache_get_client_entry_size(const hs_cache_client_descriptor_t *entry) -{ - return sizeof(*entry) + - strlen(entry->encoded_desc) + hs_desc_obj_size(entry->desc); -} - /* Parse the encoded descriptor in desc_str using * service_identity_pk to decrypt it first. * @@ -616,9 +618,6 @@ cache_store_as_client(hs_cache_client_descriptor_t *client_desc) /* Store descriptor in cache */ store_v3_desc_as_client(client_desc); - /* Update cache size with this entry for the OOM handler. */ - rend_cache_increment_allocation(cache_get_client_entry_size(client_desc)); - done: return 0; }

1 0

[tor/master] prop224: Check decoded descriptor matches the expected blinded key
by nickm＠torproject.org 24 Aug '17

24 Aug '17

commit 47672ec1c7202feb0422317b39f9bcefb231fe5b Author: David Goulet <dgoulet(a)torproject.org> Date: Tue Aug 22 09:02:23 2017 -0400 prop224: Check decoded descriptor matches the expected blinded key When a client decodes a descriptor, make sure it matches the expected blinded key which is derived from the hidden service identity key. Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/or/hs_client.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/or/hs_client.c b/src/or/hs_client.c index 77348f23e..71186b261 100644 --- a/src/or/hs_client.c +++ b/src/or/hs_client.c @@ -800,6 +800,7 @@ hs_client_decode_descriptor(const char *desc_str, { int ret; uint8_t subcredential[DIGEST256_LEN]; + ed25519_public_key_t blinded_pubkey; tor_assert(desc_str); tor_assert(service_identity_pk); @@ -807,7 +808,6 @@ hs_client_decode_descriptor(const char *desc_str, /* Create subcredential for this HS so that we can decrypt */ { - ed25519_public_key_t blinded_pubkey; uint64_t current_time_period = hs_get_time_period_num(approx_time()); hs_build_blinded_pubkey(service_identity_pk, NULL, 0, current_time_period, &blinded_pubkey); @@ -822,6 +822,16 @@ hs_client_decode_descriptor(const char *desc_str, goto err; } + /* Make sure the descriptor signing key cross certifies with the computed + * blinded key. Without this validation, anyone knowing the subcredential + * and onion address can forge a descriptor. */ + if (tor_cert_checksig((*desc)->plaintext_data.signing_key_cert, + &blinded_pubkey, approx_time()) < 0) { + log_warn(LD_GENERAL, "Descriptor signing key certificate signature " + "doesn't validate with computed blinded key."); + goto err; + } + return 0; err: return -1;

1 0

[tor/master] hs: Stub link_specifier_t in hs_descriptor.h
by nickm＠torproject.org 24 Aug '17

24 Aug '17

commit 61009d40d127e0435e9fd98a8daf81c50e6687f0 Author: David Goulet <dgoulet(a)torproject.org> Date: Tue Aug 22 14:10:30 2017 -0400 hs: Stub link_specifier_t in hs_descriptor.h Avoid including a huge trunnel interface everytime we include hs_descriptor.h Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/or/hs_descriptor.c | 5 ++--- src/or/hs_descriptor.h | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/or/hs_descriptor.c b/src/or/hs_descriptor.c index 71f16fee1..b9233c6d7 100644 --- a/src/or/hs_descriptor.c +++ b/src/or/hs_descriptor.c @@ -55,11 +55,10 @@ /* For unit tests.*/ #define HS_DESCRIPTOR_PRIVATE -#include "hs_descriptor.h" - #include "or.h" -#include "circuitbuild.h" #include "ed25519_cert.h" /* Trunnel interface. */ +#include "hs_descriptor.h" +#include "circuitbuild.h" #include "parsecommon.h" #include "rendcache.h" #include "hs_cache.h" diff --git a/src/or/hs_descriptor.h b/src/or/hs_descriptor.h index b4cda7b84..eeeb3e38a 100644 --- a/src/or/hs_descriptor.h +++ b/src/or/hs_descriptor.h @@ -19,7 +19,7 @@ #include "torcert.h" /* Trunnel */ -#include "ed25519_cert.h" +struct link_specifier_t; /* The earliest descriptor format version we support. */ #define HS_DESC_SUPPORTED_FORMAT_VERSION_MIN 3

1 0

[tor/master] prop224: Refactor how we use connection_ap_handle_onion
by nickm＠torproject.org 24 Aug '17

24 Aug '17

commit 5f94c4a0f17403060d5030fab3e81bde46693119 Author: David Goulet <dgoulet(a)torproject.org> Date: Wed Aug 23 10:48:48 2017 -0400 prop224: Refactor how we use connection_ap_handle_onion Simply directly return its returned value. Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/or/connection_edge.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c index 6409c5be8..a98b32450 100644 --- a/src/or/connection_edge.c +++ b/src/or/connection_edge.c @@ -2017,9 +2017,7 @@ connection_ap_handshake_rewrite_and_attach(entry_connection_t *conn, tor_assert(addresstype == ONION_V2_HOSTNAME || addresstype == ONION_V3_HOSTNAME); tor_assert(!automap); - if (connection_ap_handle_onion(conn, socks, circ, addresstype) < 0) { - return -1; - } + return connection_ap_handle_onion(conn, socks, circ, addresstype); } return 0; /* unreached but keeps the compiler happy */

1 0

[tor/master] prop224: Register RP circuit when it opens
by nickm＠torproject.org 24 Aug '17

24 Aug '17

commit dca105d58853c7ba6f8ef594cca34230dbb87394 Author: David Goulet <dgoulet(a)torproject.org> Date: Thu Aug 24 08:49:24 2017 -0400 prop224: Register RP circuit when it opens Only register the RP circuit when it opens and not when we send the INTRODUCE1 cell else, when re-extending to a new IP, we would register the same RP circuit with the same cookie twice leading to the circuit being closed. Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/or/hs_circuit.c | 4 ---- src/or/hs_circuitmap.c | 13 ++++++++++--- src/or/hs_client.c | 6 ++++++ src/test/test_circuitlist.c | 2 +- 4 files changed, 17 insertions(+), 8 deletions(-) diff --git a/src/or/hs_circuit.c b/src/or/hs_circuit.c index 7d41a33c8..2bfeac747 100644 --- a/src/or/hs_circuit.c +++ b/src/or/hs_circuit.c @@ -1070,10 +1070,6 @@ hs_circ_send_introduce1(origin_circuit_t *intro_circ, goto done; } - /* Register rend circuit in circuitmap */ - hs_circuitmap_register_rend_circ_client_side(rend_circ, - rend_circ->hs_ident->rendezvous_cookie); - /* Success. */ ret = 0; goto done; diff --git a/src/or/hs_circuitmap.c b/src/or/hs_circuitmap.c index f235ecc82..09704d796 100644 --- a/src/or/hs_circuitmap.c +++ b/src/or/hs_circuitmap.c @@ -408,7 +408,8 @@ hs_circuitmap_get_rend_circ_service_side(const uint8_t *cookie) /* Public function: Return client-side rendezvous circuit with rendezvous * cookie. It will first lookup for the CIRCUIT_PURPOSE_C_REND_READY - * purpose and then try for CIRCUIT_PURPOSE_C_REND_READY_INTRO_ACKED. + * purpose and then try for CIRCUIT_PURPOSE_C_REND_READY_INTRO_ACKED and then + * finally tries for CIRCUIT_PURPOSE_C_ESTABLISH_REND. * * Return NULL if no such circuit is found in the circuitmap. */ origin_circuit_t * @@ -426,6 +427,13 @@ hs_circuitmap_get_rend_circ_client_side(const uint8_t *cookie) circ = hs_circuitmap_get_origin_circuit(HS_TOKEN_REND_CLIENT_SIDE, REND_TOKEN_LEN, cookie, CIRCUIT_PURPOSE_C_REND_READY_INTRO_ACKED); + if (circ) { + return circ; + } + + circ = hs_circuitmap_get_origin_circuit(HS_TOKEN_REND_CLIENT_SIDE, + REND_TOKEN_LEN, cookie, + CIRCUIT_PURPOSE_C_ESTABLISH_REND); return circ; } @@ -472,8 +480,7 @@ hs_circuitmap_register_rend_circ_client_side(origin_circuit_t *or_circ, { circuit_t *circ = TO_CIRCUIT(or_circ); { /* Basic circ purpose sanity checking */ - tor_assert_nonfatal(circ->purpose == CIRCUIT_PURPOSE_C_REND_READY || - circ->purpose == CIRCUIT_PURPOSE_C_REND_READY_INTRO_ACKED); + tor_assert_nonfatal(circ->purpose == CIRCUIT_PURPOSE_C_ESTABLISH_REND); } hs_circuitmap_register_circuit(circ, HS_TOKEN_REND_CLIENT_SIDE, diff --git a/src/or/hs_client.c b/src/or/hs_client.c index 57bd87079..3e25126a6 100644 --- a/src/or/hs_client.c +++ b/src/or/hs_client.c @@ -434,6 +434,12 @@ client_rendezvous_circ_has_opened(origin_circuit_t *circ) /* Ignore returned value, nothing we can really do. On failure, the circuit * will be marked for close. */ hs_circ_send_establish_rendezvous(circ); + + /* Register rend circuit in circuitmap if it's still alive. */ + if (!TO_CIRCUIT(circ)->marked_for_close) { + hs_circuitmap_register_rend_circ_client_side(circ, + circ->hs_ident->rendezvous_cookie); + } } /* This is an helper function that convert a descriptor intro point object ip diff --git a/src/test/test_circuitlist.c b/src/test/test_circuitlist.c index 6b8dae31f..f622704ec 100644 --- a/src/test/test_circuitlist.c +++ b/src/test/test_circuitlist.c @@ -267,7 +267,7 @@ test_rend_token_maps(void *arg) tt_ptr_op(NULL, OP_EQ, hs_circuitmap_get_intro_circ_v2_relay_side(tok3)); /* Now let's do a check for the client-side rend circuitmap */ - c5->base_.purpose = CIRCUIT_PURPOSE_C_REND_READY; + c5->base_.purpose = CIRCUIT_PURPOSE_C_ESTABLISH_REND; hs_circuitmap_register_rend_circ_client_side(c5, tok1); tt_ptr_op(c5, OP_EQ, hs_circuitmap_get_rend_circ_client_side(tok1));

1 0

[tor/master] hs: Update intro circuit timestamp when re-extending
by nickm＠torproject.org 24 Aug '17

24 Aug '17

commit a1125625449617ef427d8816f6e0174806e2d947 Author: David Goulet <dgoulet(a)torproject.org> Date: Thu Aug 24 09:12:57 2017 -0400 hs: Update intro circuit timestamp when re-extending A client can re-extend up to 3 intro points on the same circuit. This happens when we get NACKed by the intro point for which we choose a new intro and re-extend the circuit to it. That process can be arbitrarly long so reset the dirty timestamp of the circuit everytime we choose to re-extend so we get a bit more time to actually do our introduction. This is a client circuit so it is short live once opened thus giving us a bit more time to complete the introduction is ok. Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/or/hs_client.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/or/hs_client.c b/src/or/hs_client.c index 3e25126a6..c0e24ac85 100644 --- a/src/or/hs_client.c +++ b/src/or/hs_client.c @@ -1195,6 +1195,13 @@ hs_client_reextend_intro_circuit(origin_circuit_t *circ) (unsigned int) TO_CIRCUIT(circ)->n_circ_id, safe_str_client(extend_info_describe(ei))); ret = circuit_extend_to_new_exit(circ, ei); + if (ret == 0) { + /* We were able to extend so update the timestamp so we avoid expiring + * this circuit too early. The intro circuit is short live so the + * linkability issue is minimized, we just need the circuit to hold a + * bit longer so we can introduce. */ + TO_CIRCUIT(circ)->timestamp_dirty = time(NULL); + } } else { log_info(LD_REND, "Closing intro circ %u (out of RELAY_EARLY cells).", (unsigned int) TO_CIRCUIT(circ)->n_circ_id);

1 0

[tor/master] circuit: Consider v3 hidden service circuit in circuit_expire_building()
by nickm＠torproject.org 24 Aug '17

24 Aug '17

commit e546ac6b32d5478f50644ca8f1eac0a66513fe8d Author: David Goulet <dgoulet(a)torproject.org> Date: Thu Aug 24 09:31:30 2017 -0400 circuit: Consider v3 hidden service circuit in circuit_expire_building() For a ready v3 rendezvous circuit, consider it so we don't expire. Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/or/circuituse.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/or/circuituse.c b/src/or/circuituse.c index 2472ac545..570b05e57 100644 --- a/src/or/circuituse.c +++ b/src/or/circuituse.c @@ -651,6 +651,7 @@ circuit_expire_building(void) * because that's set when they switch purposes */ if (TO_ORIGIN_CIRCUIT(victim)->rend_data || + TO_ORIGIN_CIRCUIT(victim)->hs_ident || victim->timestamp_dirty > cutoff.tv_sec) continue; break;

1 0

[tor/master] prop224: Add a function to clear INTRO data
by nickm＠torproject.org 24 Aug '17

24 Aug '17

commit 7c543412fc4c77a58fb928c6eb322e48fae97137 Author: David Goulet <dgoulet(a)torproject.org> Date: Tue Aug 22 14:37:33 2017 -0400 prop224: Add a function to clear INTRO data New function named hs_cell_introduce1_data_clear() is introduced to clear off an hs_cell_introduce1_data_t object. Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/or/hs_cell.c | 15 +++++++++++++++ src/or/hs_cell.h | 3 +++ src/or/hs_circuit.c | 6 +----- 3 files changed, 19 insertions(+), 5 deletions(-) diff --git a/src/or/hs_cell.c b/src/or/hs_cell.c index 482fa04e9..5244cfa3d 100644 --- a/src/or/hs_cell.c +++ b/src/or/hs_cell.c @@ -931,3 +931,18 @@ hs_cell_parse_rendezvous2(const uint8_t *payload, size_t payload_len, return ret; } +/* Clear the given INTRODUCE1 data structure data. */ +void +hs_cell_introduce1_data_clear(hs_cell_introduce1_data_t *data) +{ + if (data == NULL) { + return; + } + /* Object in this list have been moved to the cell object when building it + * so they've been freed earlier. We do that in order to avoid duplicating + * them leading to more memory and CPU time being used for nothing. */ + smartlist_free(data->link_specifiers); + /* The data object has no ownership of any members. */ + memwipe(data, 0, sizeof(hs_cell_introduce1_data_t)); +} + diff --git a/src/or/hs_cell.h b/src/or/hs_cell.h index 14dd31c9d..5136fce93 100644 --- a/src/or/hs_cell.h +++ b/src/or/hs_cell.h @@ -115,5 +115,8 @@ int hs_cell_parse_rendezvous2(const uint8_t *payload, size_t payload_len, uint8_t *handshake_info, size_t handshake_info_len); +/* Util API. */ +void hs_cell_introduce1_data_clear(hs_cell_introduce1_data_t *data); + #endif /* TOR_HS_CELL_H */ diff --git a/src/or/hs_circuit.c b/src/or/hs_circuit.c index 819d25432..7d41a33c8 100644 --- a/src/or/hs_circuit.c +++ b/src/or/hs_circuit.c @@ -1079,11 +1079,7 @@ hs_circ_send_introduce1(origin_circuit_t *intro_circ, goto done; done: - /* Object in this list have been moved to the cell object when building it - * so they've been freed earlier. We do that in order to avoid duplicating - * them leading to more memory and CPU time being used for nothing. */ - smartlist_free(intro1_data.link_specifiers); - memwipe(&intro1_data, 0, sizeof(intro1_data)); + hs_cell_introduce1_data_clear(&intro1_data); memwipe(payload, 0, sizeof(payload)); return ret; }

1 0

[tor/master] prop224: Add a function to find an intro point by legacy ID
by nickm＠torproject.org 24 Aug '17

24 Aug '17

commit a16627db1f099e8eaa2ac899cead604fe611f117 Author: David Goulet <dgoulet(a)torproject.org> Date: Tue Aug 22 14:51:27 2017 -0400 prop224: Add a function to find an intro point by legacy ID The client needs to find the right intro point object from the circuit identity digest it is opened to. This new function does that. Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/or/hs_client.c | 64 ++++++++++++++++++++++++++++++++++++++---------------- 1 file changed, 45 insertions(+), 19 deletions(-) diff --git a/src/or/hs_client.c b/src/or/hs_client.c index 90124fb8e..cfbda2327 100644 --- a/src/or/hs_client.c +++ b/src/or/hs_client.c @@ -214,6 +214,42 @@ find_desc_intro_point_by_ident(const hs_ident_circuit_t *ident, return intro_point; } +/* Find a descriptor intro point object from the descriptor object desc that + * matches the given legacy identity digest in legacy_id. Return NULL if not + * found. */ +static hs_desc_intro_point_t * +find_desc_intro_point_by_legacy_id(const char *legacy_id, + const hs_descriptor_t *desc) +{ + hs_desc_intro_point_t *ret_ip = NULL; + + tor_assert(legacy_id); + tor_assert(desc); + + /* We will go over every intro point and try to find which one is linked to + * that circuit. Those lists are small so it's not that expensive. */ + SMARTLIST_FOREACH_BEGIN(desc->encrypted_data.intro_points, + hs_desc_intro_point_t *, ip) { + SMARTLIST_FOREACH_BEGIN(ip->link_specifiers, + const hs_desc_link_specifier_t *, lspec) { + /* Not all tor node have an ed25519 identity key so we still rely on the + * legacy identity digest. */ + if (lspec->type != LS_LEGACY_ID) { + continue; + } + if (fast_memneq(legacy_id, lspec->u.legacy_id, DIGEST_LEN)) { + break; + } + /* Found it. */ + ret_ip = ip; + goto end; + } SMARTLIST_FOREACH_END(lspec); + } SMARTLIST_FOREACH_END(ip); + + end: + return ret_ip; +} + /* Send an INTRODUCE1 cell along the intro circuit and populate the rend * circuit identifier with the needed key material for the e2e encryption. * Return 0 on success, -1 if there is a transient error such that an action @@ -319,6 +355,7 @@ static void setup_intro_circ_auth_key(origin_circuit_t *circ) { const hs_descriptor_t *desc; + const hs_desc_intro_point_t *ip; tor_assert(circ); @@ -330,25 +367,14 @@ setup_intro_circ_auth_key(origin_circuit_t *circ) /* We will go over every intro point and try to find which one is linked to * that circuit. Those lists are small so it's not that expensive. */ - SMARTLIST_FOREACH_BEGIN(desc->encrypted_data.intro_points, - const hs_desc_intro_point_t *, ip) { - SMARTLIST_FOREACH_BEGIN(ip->link_specifiers, - const hs_desc_link_specifier_t *, lspec) { - /* Not all tor node have an ed25519 identity key so we still rely on the - * legacy identity digest. */ - if (lspec->type != LS_LEGACY_ID) { - continue; - } - if (fast_memneq(circ->build_state->chosen_exit->identity_digest, - lspec->u.legacy_id, DIGEST_LEN)) { - break; - } - /* We got it, copy its authentication key to the identifier. */ - ed25519_pubkey_copy(&circ->hs_ident->intro_auth_pk, - &ip->auth_key_cert->signed_key); - goto end; - } SMARTLIST_FOREACH_END(lspec); - } SMARTLIST_FOREACH_END(ip); + ip = find_desc_intro_point_by_legacy_id( + circ->build_state->chosen_exit->identity_digest, desc); + if (ip) { + /* We got it, copy its authentication key to the identifier. */ + ed25519_pubkey_copy(&circ->hs_ident->intro_auth_pk, + &ip->auth_key_cert->signed_key); + goto end; + } /* Reaching this point means we didn't find any intro point for this circuit * which is not suppose to happen. */

1 0

[tor/master] circuit: Avoid needless log info in circuit_get_best()
by nickm＠torproject.org 24 Aug '17

24 Aug '17

commit 1077e9894cd60d9b991e38d965e57be00fd83277 Author: David Goulet <dgoulet(a)torproject.org> Date: Thu Aug 24 09:00:18 2017 -0400 circuit: Avoid needless log info in circuit_get_best() When looking for an introduction circuit in circuit_get_best(), we log an info message if we are about to launch a new intro circuit in parallel. However, the condition was considering marked for close circuit leading to the function triggering the log info even though there is actually no valid intro circuit. Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/or/circuituse.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/or/circuituse.c b/src/or/circuituse.c index 93bccf0eb..2472ac545 100644 --- a/src/or/circuituse.c +++ b/src/or/circuituse.c @@ -337,7 +337,8 @@ circuit_get_best(const entry_connection_t *conn, /* Log an info message if we're going to launch a new intro circ in * parallel */ if (purpose == CIRCUIT_PURPOSE_C_INTRODUCE_ACK_WAIT && - !must_be_open && origin_circ->hs_circ_has_timed_out) { + !must_be_open && origin_circ->hs_circ_has_timed_out && + !circ->marked_for_close) { intro_going_on_but_too_old = 1; continue; }

1 0