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

[tor/master] Use htonll() when INT_8 is used.
by nickm＠torproject.org 09 Aug '17

09 Aug '17

commit 1397ac11d69b9a48ef5f17485ac1fd5e3ada922e Author: George Kadianakis <desnacked(a)riseup.net> Date: Thu Aug 3 12:01:52 2017 +0300 Use htonll() when INT_8 is used. Also prepend period_length to any period_num, as specified by the spec. --- src/or/hs_common.c | 69 ++++++++++++++++++++++++++++++++++++++++---------- src/or/hs_descriptor.c | 2 +- 2 files changed, 56 insertions(+), 15 deletions(-) diff --git a/src/or/hs_common.c b/src/or/hs_common.c index 20b470ff6..f3604d67c 100644 --- a/src/or/hs_common.c +++ b/src/or/hs_common.c @@ -510,11 +510,26 @@ get_disaster_srv(uint64_t time_period_num, uint8_t *srv_out) tor_assert(srv_out); digest = crypto_digest256_new(DIGEST_SHA3_256); - /* Setup payload: H("shared-random-disaster" | INT_8(period_num)) */ + + /* Start setting up payload: + * H("shared-random-disaster" | INT_8(period_length) | INT_8(period_num)) */ crypto_digest_add_bytes(digest, HS_SRV_DISASTER_PREFIX, HS_SRV_DISASTER_PREFIX_LEN); - crypto_digest_add_bytes(digest, (const char *) &time_period_num, - sizeof(time_period_num)); + + /* Setup INT_8(period_length) | INT_8(period_num) */ + { + uint64_t time_period_length = get_time_period_length(); + char period_stuff[sizeof(uint64_t)*2]; + size_t offset = 0; + set_uint64(period_stuff, tor_htonll(time_period_num)); + offset += sizeof(uint64_t); + set_uint64(period_stuff+offset, tor_htonll(time_period_length)); + offset += sizeof(uint64_t); + tor_assert(offset == sizeof(period_stuff)); + + crypto_digest_add_bytes(digest, period_stuff, sizeof(period_stuff)); + } + crypto_digest_get_digest(digest, (char *) srv_out, DIGEST256_LEN); crypto_digest_free(digest); } @@ -532,7 +547,7 @@ get_disaster_srv(uint64_t time_period_num, uint8_t *srv_out) static void build_blinded_key_param(const ed25519_public_key_t *pubkey, const uint8_t *secret, size_t secret_len, - uint64_t period_num, uint64_t start_time_period, + uint64_t period_num, uint64_t period_length, uint8_t *param_out) { size_t offset = 0; @@ -543,12 +558,12 @@ build_blinded_key_param(const ed25519_public_key_t *pubkey, tor_assert(param_out); /* Create the nonce N. The construction is as follow: - * N = "key-blind" || INT_8(period_num) || INT_8(start_period_sec) */ + * N = "key-blind" || INT_8(period_num) || INT_8(period_length) */ memcpy(nonce, HS_KEYBLIND_NONCE_PREFIX, HS_KEYBLIND_NONCE_PREFIX_LEN); offset += HS_KEYBLIND_NONCE_PREFIX_LEN; - set_uint64(nonce + offset, period_num); + set_uint64(nonce + offset, tor_htonll(period_num)); offset += sizeof(uint64_t); - set_uint64(nonce + offset, start_time_period); + set_uint64(nonce + offset, tor_htonll(period_length)); offset += sizeof(uint64_t); tor_assert(offset == HS_KEYBLIND_NONCE_LEN); @@ -953,7 +968,7 @@ hs_service_requires_uptime_circ(const smartlist_t *ports) * value is used to select the responsible HSDir where their hsdir_index is * closest to this value. * SHA3-256("store-at-idx" | blinded_public_key | - * INT_8(replicanum) | INT_8(period_num) ) + * INT_8(replicanum) | INT_8(period_length) | INT_8(period_num) ) * * hs_index_out must be large enough to receive DIGEST256_LEN bytes. */ void @@ -970,9 +985,23 @@ hs_build_hs_index(uint64_t replica, const ed25519_public_key_t *blinded_pk, crypto_digest_add_bytes(digest, HS_INDEX_PREFIX, HS_INDEX_PREFIX_LEN); crypto_digest_add_bytes(digest, (const char *) blinded_pk->pubkey, ED25519_PUBKEY_LEN); - crypto_digest_add_bytes(digest, (const char *) &replica, sizeof(replica)); - crypto_digest_add_bytes(digest, (const char *) &period_num, - sizeof(period_num)); + + /* Now setup INT_8(replicanum) | INT_8(period_length) | INT_8(period_num) */ + { + uint64_t period_length = get_time_period_length(); + char buf[sizeof(uint64_t)*3]; + size_t offset = 0; + set_uint64(buf, tor_htonll(replica)); + offset += sizeof(uint64_t); + set_uint64(buf, tor_htonll(period_length)); + offset += sizeof(uint64_t); + set_uint64(buf, tor_htonll(period_num)); + offset += sizeof(uint64_t); + tor_assert(offset == sizeof(buf)); + + crypto_digest_add_bytes(digest, buf, sizeof(buf)); + } + crypto_digest_get_digest(digest, (char *) hs_index_out, DIGEST256_LEN); crypto_digest_free(digest); } @@ -980,7 +1009,7 @@ hs_build_hs_index(uint64_t replica, const ed25519_public_key_t *blinded_pk, /* Build hsdir_index which is used to find the responsible hsdirs. This is the * index value that is compare to the hs_index when selecting an HSDir. * SHA3-256("node-idx" | node_identity | - * shared_random_value | INT_8(period_num) ) + * shared_random_value | INT_8(period_length) | INT_8(period_num) ) * * hsdir_index_out must be large enough to receive DIGEST256_LEN bytes. */ void @@ -1000,8 +1029,20 @@ hs_build_hsdir_index(const ed25519_public_key_t *identity_pk, crypto_digest_add_bytes(digest, (const char *) identity_pk->pubkey, ED25519_PUBKEY_LEN); crypto_digest_add_bytes(digest, (const char *) srv_value, DIGEST256_LEN); - crypto_digest_add_bytes(digest, (const char *) &period_num, - sizeof(period_num)); + + { + uint64_t time_period_length = get_time_period_length(); + char period_stuff[sizeof(uint64_t)*2]; + size_t offset = 0; + set_uint64(period_stuff, tor_htonll(period_num)); + offset += sizeof(uint64_t); + set_uint64(period_stuff+offset, tor_htonll(time_period_length)); + offset += sizeof(uint64_t); + tor_assert(offset == sizeof(period_stuff)); + + crypto_digest_add_bytes(digest, period_stuff, sizeof(period_stuff)); + } + crypto_digest_get_digest(digest, (char *) hsdir_index_out, DIGEST256_LEN); crypto_digest_free(digest); } diff --git a/src/or/hs_descriptor.c b/src/or/hs_descriptor.c index 6f304d6d2..e18e25b26 100644 --- a/src/or/hs_descriptor.c +++ b/src/or/hs_descriptor.c @@ -236,7 +236,7 @@ build_secret_input(const hs_descriptor_t *desc, uint8_t *dst, size_t dstlen) memcpy(dst + offset, desc->subcredential, sizeof(desc->subcredential)); offset += sizeof(desc->subcredential); /* Copy revision counter value. */ - set_uint64(dst + offset, tor_ntohll(desc->plaintext_data.revision_counter)); + set_uint64(dst + offset, tor_htonll(desc->plaintext_data.revision_counter)); offset += sizeof(uint64_t); tor_assert(HS_DESC_ENCRYPTED_SECRET_INPUT_LEN == offset); }

1 0

[tor/master] Improve code based on Nick review:
by nickm＠torproject.org 09 Aug '17

09 Aug '17

commit c62da5cf95de0958d28d2dfd8cdc66cf11498ebe Author: George Kadianakis <desnacked(a)riseup.net> Date: Fri Aug 4 22:02:28 2017 +0300 Improve code based on Nick review: - Fix some more crazy ternary ops. - Fix the order of disaster SRV computation. - Whitespace fixes. - Remove a redundant warn. - Better docs. --- src/or/hs_circuit.c | 7 +++++-- src/or/hs_common.c | 4 ++-- src/or/hs_descriptor.c | 1 - src/or/hs_service.c | 4 ++-- src/or/networkstatus.c | 2 +- 5 files changed, 10 insertions(+), 8 deletions(-) diff --git a/src/or/hs_circuit.c b/src/or/hs_circuit.c index 527439599..faceb731c 100644 --- a/src/or/hs_circuit.c +++ b/src/or/hs_circuit.c @@ -699,8 +699,11 @@ hs_circ_retry_service_rendezvous_point(origin_circuit_t *circ) circ->hs_service_side_rend_circ_has_been_relaunched = 1; /* Legacy service don't have an hidden service ident. */ - (circ->hs_ident) ? retry_service_rendezvous_point(circ) : - rend_service_relaunch_rendezvous(circ); + if (circ->hs_ident) { + retry_service_rendezvous_point(circ); + } else { + rend_service_relaunch_rendezvous(circ); + } done: return; diff --git a/src/or/hs_common.c b/src/or/hs_common.c index a29b37749..f5c63cb6a 100644 --- a/src/or/hs_common.c +++ b/src/or/hs_common.c @@ -521,9 +521,9 @@ get_disaster_srv(uint64_t time_period_num, uint8_t *srv_out) uint64_t time_period_length = get_time_period_length(); char period_stuff[sizeof(uint64_t)*2]; size_t offset = 0; - set_uint64(period_stuff, tor_htonll(time_period_num)); + set_uint64(period_stuff, tor_htonll(time_period_length)); offset += sizeof(uint64_t); - set_uint64(period_stuff+offset, tor_htonll(time_period_length)); + set_uint64(period_stuff+offset, tor_htonll(time_period_num)); offset += sizeof(uint64_t); tor_assert(offset == sizeof(period_stuff)); diff --git a/src/or/hs_descriptor.c b/src/or/hs_descriptor.c index 430e2f6f9..0b10f205a 100644 --- a/src/or/hs_descriptor.c +++ b/src/or/hs_descriptor.c @@ -1007,7 +1007,6 @@ desc_encode_v3(const hs_descriptor_t *desc, tor_assert(desc->plaintext_data.version == 3); if (BUG(desc->subcredential == NULL)) { - log_warn(LD_GENERAL, "Asked to encode desc with no subcred. No!"); goto err; } diff --git a/src/or/hs_service.c b/src/or/hs_service.c index f1592efa8..bea760837 100644 --- a/src/or/hs_service.c +++ b/src/or/hs_service.c @@ -2881,14 +2881,14 @@ hs_service_circuit_has_opened(origin_circuit_t *circ) if (circ->hs_ident) { service_intro_circ_has_opened(circ); } else { - rend_service_intro_has_opened(circ); + rend_service_intro_has_opened(circ); } break; case CIRCUIT_PURPOSE_S_CONNECT_REND: if (circ->hs_ident) { service_rendezvous_circ_has_opened(circ); } else { - rend_service_rendezvous_has_opened(circ); + rend_service_rendezvous_has_opened(circ); } break; default: diff --git a/src/or/networkstatus.c b/src/or/networkstatus.c index 82ceb8a9e..69bff55cf 100644 --- a/src/or/networkstatus.c +++ b/src/or/networkstatus.c @@ -1400,7 +1400,7 @@ networkstatus_get_live_consensus,(time_t now)) return NULL; } -/** Given a consensus in ns, validate that it's currently live and +/** Given a consensus in ns, return true iff currently live and * unexpired. */ int networkstatus_is_live(const networkstatus_t *ns, time_t now)

1 0

[tor/master] prop224 tests: test_gen_establish_intro_cell() check cell contents.
by nickm＠torproject.org 09 Aug '17

09 Aug '17

commit a464d49aeb66ad2daa6b753c34ceadfe0b391490 Author: George Kadianakis <desnacked(a)riseup.net> Date: Fri Aug 4 22:53:53 2017 +0300 prop224 tests: test_gen_establish_intro_cell() check cell contents. --- src/test/test_hs_cell.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/test/test_hs_cell.c b/src/test/test_hs_cell.c index 1686fccee..1b3c788a6 100644 --- a/src/test/test_hs_cell.c +++ b/src/test/test_hs_cell.c @@ -30,7 +30,6 @@ test_gen_establish_intro_cell(void *arg) ssize_t ret; char circ_nonce[DIGEST_LEN] = {0}; uint8_t buf[RELAY_PAYLOAD_SIZE]; - trn_cell_establish_intro_t *cell_out = NULL; trn_cell_establish_intro_t *cell_in = NULL; crypto_rand(circ_nonce, sizeof(circ_nonce)); @@ -38,7 +37,6 @@ test_gen_establish_intro_cell(void *arg) /* Create outgoing ESTABLISH_INTRO cell and extract its payload so that we attempt to parse it. */ { - cell_out = trn_cell_establish_intro_new(); /* We only need the auth key pair here. */ hs_service_intro_point_t *ip = service_intro_point_new(NULL, 0); /* Auth key pair is generated in the constructor so we are all set for @@ -46,9 +44,18 @@ test_gen_establish_intro_cell(void *arg) ret = hs_cell_build_establish_intro(circ_nonce, ip, buf); service_intro_point_free(ip); tt_u64_op(ret, OP_GT, 0); + } - ret = trn_cell_establish_intro_encode(buf, sizeof(buf), cell_out); - tt_u64_op(ret, OP_GT, 0); + /* Check the contents of the cell */ + { + /* First byte is the auth key type: make sure its correct */ + tt_int_op(buf[0], OP_EQ, HS_INTRO_AUTH_KEY_TYPE_ED25519); + /* Next two bytes is auth key len */ + tt_int_op(ntohs(get_uint16(buf+1)), OP_EQ, ED25519_PUBKEY_LEN); + /* Skip to the number of extensions: no extensions */ + tt_int_op(buf[35], OP_EQ, 0); + /* Skip to the sig len. Make sure it's the size of an ed25519 sig */ + tt_int_op(ntohs(get_uint16(buf+35+1+32)), OP_EQ, ED25519_SIG_LEN); } /* Parse it as the receiver */ @@ -64,7 +71,6 @@ test_gen_establish_intro_cell(void *arg) done: trn_cell_establish_intro_free(cell_in); - trn_cell_establish_intro_free(cell_out); } /* Mocked ed25519_sign_prefixed() function that always fails :) */

1 0

[tor/master] Constify functions that can be constified.
by nickm＠torproject.org 09 Aug '17

09 Aug '17

commit 2c6f2e9be9db6d3889a0756be93d7203888eaa72 Author: George Kadianakis <desnacked(a)riseup.net> Date: Thu Aug 3 16:03:30 2017 +0300 Constify functions that can be constified. --- src/or/hs_circuit.c | 4 ++-- src/or/hs_service.c | 6 +++--- src/or/hs_service.h | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/or/hs_circuit.c b/src/or/hs_circuit.c index 2a753f659..75c946799 100644 --- a/src/or/hs_circuit.c +++ b/src/or/hs_circuit.c @@ -214,8 +214,8 @@ count_opened_desc_intro_point_circuits(const hs_service_t *service, DIGEST256MAP_FOREACH(desc->intro_points.map, key, const hs_service_intro_point_t *, ip) { - circuit_t *circ; - origin_circuit_t *ocirc = hs_circ_service_get_intro_circ(ip); + const circuit_t *circ; + const origin_circuit_t *ocirc = hs_circ_service_get_intro_circ(ip); if (ocirc == NULL) { continue; } diff --git a/src/or/hs_service.c b/src/or/hs_service.c index b46fd6329..592b1f255 100644 --- a/src/or/hs_service.c +++ b/src/or/hs_service.c @@ -2048,8 +2048,8 @@ update_revision_counters_in_state(void) * counter. Else set service_found_out to False. */ STATIC uint64_t check_state_line_for_service_rev_counter(const char *state_line, - ed25519_public_key_t *blinded_pubkey, - int *service_found_out) + const ed25519_public_key_t *blinded_pubkey, + int *service_found_out) { smartlist_t *items = NULL; int ok; @@ -2111,7 +2111,7 @@ check_state_line_for_service_rev_counter(const char *state_line, * service with blinded key blinded_pubkey. If no revision counter is * found, return 0. */ static uint64_t -get_rev_counter_for_service(ed25519_public_key_t *blinded_pubkey) +get_rev_counter_for_service(const ed25519_public_key_t *blinded_pubkey) { or_state_t *state = get_or_state(); config_line_t *line; diff --git a/src/or/hs_service.h b/src/or/hs_service.h index 1ab0c1a4f..93d2710cd 100644 --- a/src/or/hs_service.h +++ b/src/or/hs_service.h @@ -332,8 +332,8 @@ STATIC void service_descriptor_free(hs_service_descriptor_t *desc); STATIC uint64_t check_state_line_for_service_rev_counter(const char *state_line, - ed25519_public_key_t *blinded_pubkey, - int *service_found_out); + const ed25519_public_key_t *blinded_pubkey, + int *service_found_out); #endif /* TOR_UNIT_TESTS */

1 0

[tor/master] Improve setting hsdir index procedure.
by nickm＠torproject.org 09 Aug '17

09 Aug '17

commit d88984a137b9f06fd72f57636b6ec321044c8908 Author: George Kadianakis <desnacked(a)riseup.net> Date: Thu Aug 3 15:54:42 2017 +0300 Improve setting hsdir index procedure. - Fix memleak. --- src/or/nodelist.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/or/nodelist.c b/src/or/nodelist.c index c5a5979f3..abbe15ecc 100644 --- a/src/or/nodelist.c +++ b/src/or/nodelist.c @@ -199,11 +199,6 @@ node_set_hsdir_index(node_t *node, const networkstatus_t *ns) current_time_period_num = hs_get_time_period_num(now); next_time_period_num = hs_get_next_time_period_num(now); - /* If NOT in overlap mode, we only need to compute the current hsdir index - * for the ongoing time period and thus the current SRV. If it can't be - * found, the disaster one is returned. */ - current_hsdir_index_srv = hs_get_current_srv(current_time_period_num); - if (hs_overlap_mode_is_active(ns, now)) { /* We are in overlap mode, this means that our consensus has just cycled * from current SRV to previous SRV so for the _next_ upcoming time @@ -214,6 +209,11 @@ node_set_hsdir_index(node_t *node, const networkstatus_t *ns) /* The following can be confusing so again, in overlap mode, we use our * previous SRV for our _current_ hsdir index. */ current_hsdir_index_srv = hs_get_previous_srv(current_time_period_num); + } else { + /* If NOT in overlap mode, we only need to compute the current hsdir index + * for the ongoing time period and thus the current SRV. If it can't be + * found, the disaster one is returned. */ + current_hsdir_index_srv = hs_get_current_srv(current_time_period_num); } /* Build the current hsdir index. */ @@ -223,6 +223,8 @@ node_set_hsdir_index(node_t *node, const networkstatus_t *ns) /* Build the next hsdir index if we have a next SRV that we can use. */ hs_build_hsdir_index(node_identity_pk, next_hsdir_index_srv, next_time_period_num, node->hsdir_index->next); + } else { + memset(node->hsdir_index->next, 0, sizeof(node->hsdir_index->next)); } done:

1 0

[tor/master] Improve documentation all around the codebase.
by nickm＠torproject.org 09 Aug '17

09 Aug '17

commit 5ca9b830eacf46acde56469e5f19f977e5f02668 Author: George Kadianakis <desnacked(a)riseup.net> Date: Thu Aug 3 16:02:51 2017 +0300 Improve documentation all around the codebase. --- src/or/hs_cell.h | 17 +++++++++++------ src/or/hs_circuit.c | 18 +++++++++++------- src/or/hs_common.c | 9 +++++---- src/or/hs_ident.h | 4 +++- src/or/hs_service.c | 47 +++++++++++++++++++++++++++++++--------------- src/or/hs_service.h | 10 +++++----- src/or/or.h | 4 ++-- src/test/test_hs_common.c | 3 +++ src/test/test_hs_service.c | 12 ++++++++++++ 9 files changed, 84 insertions(+), 40 deletions(-) diff --git a/src/or/hs_cell.h b/src/or/hs_cell.h index b7c3d1127..f32f7a421 100644 --- a/src/or/hs_cell.h +++ b/src/or/hs_cell.h @@ -20,22 +20,27 @@ typedef enum { /* This data structure contains data that we need to parse an INTRODUCE2 cell * which is used by the INTRODUCE2 cell parsing function. On a successful * parsing, the onion_pk and rendezvous_cookie will be populated with the - * computed key material from the cell data. */ + * computed key material from the cell data. This structure is only used during + * INTRO2 parsing and discarded after that. */ typedef struct hs_cell_introduce2_data_t { - /*** Immutable Section. ***/ + /*** Immutable Section: Set on structure init. ***/ - /* Introduction point authentication public key. */ + /* Introduction point authentication public key. Pointer owned by the + introduction point object through which we received the INTRO2 cell. */ const ed25519_public_key_t *auth_pk; - /* Introduction point encryption keypair for the ntor handshake. */ + /* Introduction point encryption keypair for the ntor handshake. Pointer + owned by the introduction point object through which we received the + INTRO2 cell*/ const curve25519_keypair_t *enc_kp; - /* Subcredentials of the service. */ + /* Subcredentials of the service. Pointer owned by the descriptor that owns + the introduction point through which we received the INTRO2 cell. */ const uint8_t *subcredential; /* Payload of the received encoded cell. */ const uint8_t *payload; /* Size of the payload of the received encoded cell. */ size_t payload_len; - /*** Muttable Section. ***/ + /*** Mutable Section: Set upon parsing INTRODUCE2 cell. ***/ /* Onion public key computed using the INTRODUCE2 encrypted section. */ curve25519_public_key_t onion_pk; diff --git a/src/or/hs_circuit.c b/src/or/hs_circuit.c index 3d67f24cb..2a753f659 100644 --- a/src/or/hs_circuit.c +++ b/src/or/hs_circuit.c @@ -233,7 +233,7 @@ count_opened_desc_intro_point_circuits(const hs_service_t *service, return count; } -/* From a given service, rendezvous cookie and handshake infor, create a +/* From a given service, rendezvous cookie and handshake info, create a * rendezvous point circuit identifier. This can't fail. */ static hs_ident_circuit_t * create_rp_circuit_identifier(const hs_service_t *service, @@ -351,7 +351,7 @@ send_establish_intro(const hs_service_t *service, * if direct_conn, IPv6 is prefered if we have one available. * if firewall does not allow the chosen address, error. * - * Return NULL if we can fulfill the conditions. */ + * Return NULL if we can't fulfill the conditions. */ static extend_info_t * get_rp_extend_info(const smartlist_t *link_specifiers, const curve25519_public_key_t *onion_key, int direct_conn) @@ -778,6 +778,8 @@ hs_circ_service_intro_has_opened(hs_service_t *service, tor_assert(desc); tor_assert(circ); + /* Cound opened circuits that have sent ESTABLISH_INTRO cells or are already + * established introduction circuits */ num_intro_circ = count_opened_desc_intro_point_circuits(service, desc); num_needed_circ = service->config.num_intro_points; if (num_intro_circ > num_needed_circ) { @@ -879,8 +881,9 @@ hs_circ_service_rp_has_opened(const hs_service_t *service, memwipe(payload, 0, sizeof(payload)); } -/* Handle an INTRO_ESTABLISHED cell payload of length payload_len arriving on - * the given introduction circuit circ. The service is only used for logging +/* Circ has been expecting an INTRO_ESTABLISHED cell that just arrived. Handle + * the INTRO_ESTABLISHED cell payload of length payload_len arriving on the + * given introduction circuit circ. The service is only used for logging * purposes. Return 0 on success else a negative value. */ int hs_circ_handle_intro_established(const hs_service_t *service, @@ -919,9 +922,10 @@ hs_circ_handle_intro_established(const hs_service_t *service, return ret; } -/* Handle an INTRODUCE2 unparsed payload of payload_len for the given circuit - * and service. This cell is associated with the intro point object ip and the - * subcredential. Return 0 on success else a negative value. */ +/* We just received an INTRODUCE2 cell on the established introduction circuit + * circ. Handle the INTRODUCE2 payload of size payload_len for the given + * circuit and service. This cell is associated with the intro point object ip + * and the subcredential. Return 0 on success else a negative value. */ int hs_circ_handle_introduce2(const hs_service_t *service, const origin_circuit_t *circ, diff --git a/src/or/hs_common.c b/src/or/hs_common.c index d68c446fd..570c1132c 100644 --- a/src/or/hs_common.c +++ b/src/or/hs_common.c @@ -669,7 +669,8 @@ hs_parse_address_impl(const char *address, ed25519_public_key_t *key_out, } /* Using the given identity public key and a blinded public key, compute the - * subcredential and put it in subcred_out. This can't fail. */ + * subcredential and put it in subcred_out (must be of size DIGEST256_LEN). + * This can't fail. */ void hs_get_subcredential(const ed25519_public_key_t *identity_pk, const ed25519_public_key_t *blinded_pk, @@ -707,9 +708,9 @@ hs_get_subcredential(const ed25519_public_key_t *identity_pk, memwipe(credential, 0, sizeof(credential)); } -/* From the given list of hidden service ports, find the matching one from the - * given edge connection conn and set the connection address from the service - * port object. Return 0 on success or -1 if none. */ +/* From the given list of hidden service ports, find the ones that much the + * given edge connection conn, pick one at random and use it to set the + * connection address. Return 0 on success or -1 if none. */ int hs_set_conn_addr_port(const smartlist_t *ports, edge_connection_t *conn) { diff --git a/src/or/hs_ident.h b/src/or/hs_ident.h index a3ebd07da..e259fde54 100644 --- a/src/or/hs_ident.h +++ b/src/or/hs_ident.h @@ -53,7 +53,9 @@ typedef struct hs_ident_circuit_t { hs_ident_circuit_type_t circuit_type; /* (All circuit) Introduction point authentication key. It's also needed on - * the rendezvous circuit for the ntor handshake. */ + * the rendezvous circuit for the ntor handshake. It's used as the unique key + * of the introduction point so it should not be shared between multiple + * intro points. */ ed25519_public_key_t intro_auth_pk; /* (Only client rendezvous circuit) Introduction point encryption public diff --git a/src/or/hs_service.c b/src/or/hs_service.c index a6f548d31..b46fd6329 100644 --- a/src/or/hs_service.c +++ b/src/or/hs_service.c @@ -206,8 +206,9 @@ service_clear_config(hs_service_config_t *config) memset(config, 0, sizeof(*config)); } -/* Return the number of minimum INTRODUCE2 cell defined by a consensus - * parameter or the default value. */ +/* Return the lower bound of maximum INTRODUCE2 cells per circuit before we + * rotate intro point (defined by a consensus parameter or the default + * value). */ static int32_t get_intro_point_min_introduce2(void) { @@ -218,8 +219,9 @@ get_intro_point_min_introduce2(void) 0, INT32_MAX); } -/* Return the number of maximum INTRODUCE2 cell defined by a consensus - * parameter or the default value. */ +/* Return the upper bound of maximum INTRODUCE2 cells per circuit before we + * rotate intro point (defined by a consensus parameter or the default + * value). */ static int32_t get_intro_point_max_introduce2(void) { @@ -230,8 +232,8 @@ get_intro_point_max_introduce2(void) 0, INT32_MAX); } -/* Return the minimum lifetime of an introduction point defined by a consensus - * parameter or the default value. */ +/* Return the minimum lifetime in seconds of an introduction point defined by a + * consensus parameter or the default value. */ static int32_t get_intro_point_min_lifetime(void) { @@ -247,8 +249,8 @@ get_intro_point_min_lifetime(void) 0, INT32_MAX); } -/* Return the maximum lifetime of an introduction point defined by a consensus - * parameter or the default value. */ +/* Return the maximum lifetime in seconds of an introduction point defined by a + * consensus parameter or the default value. */ static int32_t get_intro_point_max_lifetime(void) { @@ -466,7 +468,16 @@ service_intro_point_find(const hs_service_t *service, tor_assert(service); tor_assert(auth_key); - /* Trying all descriptors. */ + /* Trying all descriptors to find the right intro point. + * + * Even if we use the same node as intro point in both descriptors, the node + * will have a different intro auth key for each descriptor since we generate + * a new one everytime we pick an intro point. + * + * After #22893 gets implemented, intro points will be moved to be + * per-service instead of per-descriptor so this function will need to + * change. + */ FOR_EACH_DESCRIPTOR_BEGIN(service, desc) { if ((ip = digest256map_get(desc->intro_points.map, auth_key->pubkey)) != NULL) { @@ -1647,6 +1658,10 @@ rotate_service_descriptors(hs_service_t *service) STATIC void rotate_all_descriptors(time_t now) { + /* XXX We rotate all our service descriptors at once. In the future it might + * be wise, to rotate service descriptors independently to hide that all + * those descriptors are on the same tor instance */ + FOR_EACH_SERVICE_BEGIN(service) { /* We are _not_ in the overlap period so skip rotation. */ if (!hs_overlap_mode_is_active(NULL, now)) { @@ -2379,7 +2394,7 @@ service_rendezvous_circ_has_opened(origin_circuit_t *circ) tor_assert(circ); tor_assert(circ->cpath); - /* Getting here means this is a v3 intro circuit. */ + /* Getting here means this is a v3 rendezvous circuit. */ tor_assert(circ->hs_ident); tor_assert(TO_CIRCUIT(circ)->purpose == CIRCUIT_PURPOSE_S_CONNECT_REND); @@ -2411,8 +2426,9 @@ service_rendezvous_circ_has_opened(origin_circuit_t *circ) return; } -/* Handle an INTRO_ESTABLISHED cell arriving on the given introduction - * circuit. Return 0 on success else a negative value. */ +/* We've been expecting an INTRO_ESTABLISHED cell on this circuit and it just + * arrived. Handle the INTRO_ESTABLISHED cell arriving on the given + * introduction circuit. Return 0 on success else a negative value. */ static int service_handle_intro_established(origin_circuit_t *circ, const uint8_t *payload, @@ -2446,7 +2462,8 @@ service_handle_intro_established(origin_circuit_t *circ, } /* Try to parse the payload into a cell making sure we do actually have a - * valid cell. On success, the ip object is updated. */ + * valid cell. On success, the ip object and circuit purpose is updated to + * reflect the fact that the introduction circuit is established. */ if (hs_circ_handle_intro_established(service, ip, circ, payload, payload_len) < 0) { goto err; @@ -2467,8 +2484,8 @@ service_handle_intro_established(origin_circuit_t *circ, return -1; } -/* Handle an INTRODUCE2 cell arriving on the given introduction circuit. - * Return 0 on success else a negative value. */ +/* We just received an INTRODUCE2 cell on the established introduction circuit + * circ. Handle the cell and return 0 on success else a negative value. */ static int service_handle_introduce2(origin_circuit_t *circ, const uint8_t *payload, size_t payload_len) diff --git a/src/or/hs_service.h b/src/or/hs_service.h index cf2e1fa6f..1ab0c1a4f 100644 --- a/src/or/hs_service.h +++ b/src/or/hs_service.h @@ -123,11 +123,11 @@ typedef struct hs_service_descriptor_t { * couldn't pick any nodes. */ unsigned int missing_intro_points : 1; - /* List of hidden service directories node_t object to which we couldn't - * upload this descriptor because we didn't have its router descriptor at - * the time. If this list is non-empty, only the relays in this list are - * re-tried to upload this descriptor when our directory information have - * been updated. */ + /* List of identity digests for hidden service directories to which we + * couldn't upload this descriptor because we didn't have its router + * descriptor at the time. If this list is non-empty, only the relays in this + * list are re-tried to upload this descriptor when our directory information + * have been updated. */ smartlist_t *hsdir_missing_info; } hs_service_descriptor_t; diff --git a/src/or/or.h b/src/or/or.h index 14dd4e0d6..1e498aecf 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -421,9 +421,9 @@ typedef enum { #define DIR_PURPOSE_FETCH_RENDDESC_V2 18 /** A connection to a directory server: download a microdescriptor. */ #define DIR_PURPOSE_FETCH_MICRODESC 19 -/** A connetion to a hidden service directory: upload a descriptor. */ +/** A connection to a hidden service directory: upload a v3 descriptor. */ #define DIR_PURPOSE_UPLOAD_HSDESC 20 -/** A connetion to a hidden service directory: fetch a descriptor. */ +/** A connection to a hidden service directory: fetch a v3 descriptor. */ #define DIR_PURPOSE_FETCH_HSDESC 21 #define DIR_PURPOSE_MAX_ 21 diff --git a/src/test/test_hs_common.c b/src/test/test_hs_common.c index a5c499112..60577a2a5 100644 --- a/src/test/test_hs_common.c +++ b/src/test/test_hs_common.c @@ -16,6 +16,7 @@ #include "hs_common.h" #include "config.h" +/** Test the validation of HS v3 addresses */ static void test_validate_address(void *arg) { @@ -69,6 +70,7 @@ test_validate_address(void *arg) ; } +/** Test building HS v3 onion addresses */ static void test_build_address(void *arg) { @@ -133,6 +135,7 @@ test_time_period(void *arg) ; } +/** Test that we can correctly find the start time of the next time period */ static void test_start_time_of_next_time_period(void *arg) { diff --git a/src/test/test_hs_service.c b/src/test/test_hs_service.c index 4ee2cdac8..10eeaa432 100644 --- a/src/test/test_hs_service.c +++ b/src/test/test_hs_service.c @@ -397,6 +397,7 @@ test_access_service(void *arg) hs_free_all(); } +/** Test that we can create intro point objects, index them and find them */ static void test_service_intro_point(void *arg) { @@ -609,6 +610,7 @@ test_helper_functions(void *arg) UNMOCK(node_get_by_id); } +/** Test that we do the right operations when an intro circuit opens */ static void test_intro_circuit_opened(void *arg) { @@ -666,6 +668,8 @@ test_intro_circuit_opened(void *arg) UNMOCK(relay_send_command_from_edge_); } +/** Test the operations we do on a circuit after we learn that we successfuly + * established an intro point on it */ static void test_intro_established(void *arg) { @@ -735,6 +739,8 @@ test_intro_established(void *arg) UNMOCK(circuit_mark_for_close_); } +/** Check the operations we do on a rendezvous circuit after we learn it's + * open */ static void test_rdv_circuit_opened(void *arg) { @@ -776,6 +782,7 @@ test_rdv_circuit_opened(void *arg) UNMOCK(relay_send_command_from_edge_); } +/** Test sending and receiving introduce2 cells */ static void test_introduce2(void *arg) { @@ -852,6 +859,8 @@ test_introduce2(void *arg) UNMOCK(circuit_mark_for_close_); } +/** Test basic hidden service housekeeping operations (maintaining intro + * points, etc) */ static void test_service_event(void *arg) { @@ -933,6 +942,7 @@ test_service_event(void *arg) UNMOCK(circuit_mark_for_close_); } +/** Test that we rotate descriptors correctly in overlap period */ static void test_rotate_descriptors(void *arg) { @@ -1013,6 +1023,8 @@ test_rotate_descriptors(void *arg) UNMOCK(networkstatus_get_live_consensus); } +/** Test building descriptors: picking intro points, setting up their link + * specifiers, etc. */ static void test_build_update_descriptors(void *arg) {

1 0

[tor/master] Validate intro point limits to avoid asserts.
by nickm＠torproject.org 09 Aug '17

09 Aug '17

commit 3bc52dae8932b42c809e2b233d5c194b74fa4f9b Author: George Kadianakis <desnacked(a)riseup.net> Date: Thu Aug 3 15:49:42 2017 +0300 Validate intro point limits to avoid asserts. --- src/or/hs_service.c | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/src/or/hs_service.c b/src/or/hs_service.c index 22739334d..430fb36a5 100644 --- a/src/or/hs_service.c +++ b/src/or/hs_service.c @@ -346,12 +346,25 @@ service_intro_point_new(const extend_info_t *ei, unsigned int is_legacy) * term keys. */ ed25519_keypair_generate(&ip->auth_key_kp, 0); - ip->introduce2_max = - crypto_rand_int_range(get_intro_point_min_introduce2(), - get_intro_point_max_introduce2()); - ip->time_to_expire = time(NULL) + - crypto_rand_int_range(get_intro_point_min_lifetime(), - get_intro_point_max_lifetime()); + { /* Set introduce2 max cells limit */ + int32_t min_introduce2_cells = get_intro_point_min_introduce2(); + int32_t max_introduce2_cells = get_intro_point_max_introduce2(); + if (BUG(max_introduce2_cells < min_introduce2_cells)) { + goto err; + } + ip->introduce2_max = crypto_rand_int_range(min_introduce2_cells, + max_introduce2_cells); + } + { /* Set intro point lifetime */ + int32_t intro_point_min_lifetime = get_intro_point_min_lifetime(); + int32_t intro_point_max_lifetime = get_intro_point_max_lifetime(); + if (BUG(intro_point_max_lifetime < intro_point_min_lifetime)) { + goto err; + } + ip->time_to_expire = time(NULL) + + crypto_rand_int_range(intro_point_min_lifetime,intro_point_max_lifetime); + } + ip->replay_cache = replaycache_new(0, 0); /* Initialize the base object. We don't need the certificate object. */

1 0

[tor/master] Fix small easy bugs all around
by nickm＠torproject.org 09 Aug '17

09 Aug '17

commit a561a10da726b426d326515ca7f75988b405bab7 Author: George Kadianakis <desnacked(a)riseup.net> Date: Thu Aug 3 16:04:25 2017 +0300 Fix small easy bugs all around - Fix log message format string. - Do extra circuit purpose check. - wipe memory in a clear function - Make sure we don't double add intro points in our list - Make sure we don't double close intro circuits. - s/tt_u64_op/tt_i64_op/ --- src/or/hs_cell.c | 4 ++-- src/or/hs_circuit.c | 4 ++++ src/or/hs_intropoint.c | 1 + src/or/hs_service.c | 12 ++++++++---- src/test/test_hs_cell.c | 2 +- 5 files changed, 16 insertions(+), 7 deletions(-) diff --git a/src/or/hs_cell.c b/src/or/hs_cell.c index 922ff7346..064e26334 100644 --- a/src/or/hs_cell.c +++ b/src/or/hs_cell.c @@ -169,9 +169,9 @@ parse_introduce2_encrypted(const uint8_t *decrypted_data, if (trn_cell_introduce_encrypted_getlen_onion_key(enc_cell) != CURVE25519_PUBKEY_LEN) { - log_info(LD_REND, "INTRODUCE2 onion key length is invalid. Got %ld but " + log_info(LD_REND, "INTRODUCE2 onion key length is invalid. Got %u but " "expected %d on circuit %u for service %s", - trn_cell_introduce_encrypted_getlen_onion_key(enc_cell), + (unsigned)trn_cell_introduce_encrypted_getlen_onion_key(enc_cell), CURVE25519_PUBKEY_LEN, TO_CIRCUIT(circ)->n_circ_id, safe_str_client(service->onion_address)); goto err; diff --git a/src/or/hs_circuit.c b/src/or/hs_circuit.c index 75c946799..527439599 100644 --- a/src/or/hs_circuit.c +++ b/src/or/hs_circuit.c @@ -898,6 +898,10 @@ hs_circ_handle_intro_established(const hs_service_t *service, tor_assert(circ); tor_assert(payload); + if (BUG(TO_CIRCUIT(circ)->purpose != CIRCUIT_PURPOSE_S_ESTABLISH_INTRO)) { + goto done; + } + /* Try to parse the payload into a cell making sure we do actually have a * valid cell. For a legacy node, it's an empty payload so as long as we * have the cell, we are good. */ diff --git a/src/or/hs_intropoint.c b/src/or/hs_intropoint.c index a0453841f..644611fea 100644 --- a/src/or/hs_intropoint.c +++ b/src/or/hs_intropoint.c @@ -607,5 +607,6 @@ hs_intropoint_clear(hs_intropoint_t *ip) SMARTLIST_FOREACH(ip->link_specifiers, hs_desc_link_specifier_t *, ls, hs_desc_link_specifier_free(ls)); smartlist_free(ip->link_specifiers); + memset(ip, 0, sizeof(hs_intropoint_t)); } diff --git a/src/or/hs_service.c b/src/or/hs_service.c index 592b1f255..f1592efa8 100644 --- a/src/or/hs_service.c +++ b/src/or/hs_service.c @@ -433,14 +433,18 @@ service_intro_point_new(const extend_info_t *ei, unsigned int is_legacy) STATIC void service_intro_point_add(digest256map_t *map, hs_service_intro_point_t *ip) { + hs_service_intro_point_t *old_ip_entry; + tor_assert(map); tor_assert(ip); - digest256map_set(map, ip->auth_key_kp.pubkey.pubkey, ip); + old_ip_entry = digest256map_set(map, ip->auth_key_kp.pubkey.pubkey, ip); + /* Make sure we didn't just try to double-add an intro point */ + tor_assert_nonfatal(!old_ip_entry); } -/* For a given service, remove the intro point from that service which will - * look in both descriptors. */ +/* For a given service, remove the intro point from that service's descriptors + * (check both current and next descriptor) */ STATIC void service_intro_point_remove(const hs_service_t *service, const hs_service_intro_point_t *ip) @@ -1623,7 +1627,7 @@ cleanup_intro_points(hs_service_t *service, time_t now) * descriptor created and uploaded. There is no difference to an * attacker between the timing of a new consensus and intro point * rotation (possibly?). */ - if (ocirc) { + if (ocirc && !TO_CIRCUIT(ocirc)->marked_for_close) { /* After this, no new cells will be handled on the circuit. */ circuit_mark_for_close(TO_CIRCUIT(ocirc), END_CIRC_REASON_FINISHED); } diff --git a/src/test/test_hs_cell.c b/src/test/test_hs_cell.c index 9c963bcb1..1686fccee 100644 --- a/src/test/test_hs_cell.c +++ b/src/test/test_hs_cell.c @@ -106,7 +106,7 @@ test_gen_establish_intro_cell_bad(void *arg) expect_log_msg_containing("Unable to make signature for " "ESTABLISH_INTRO cell."); teardown_capture_of_logs(); - tt_u64_op(cell_len, OP_EQ, -1); + tt_i64_op(cell_len, OP_EQ, -1); done: trn_cell_establish_intro_free(cell);

1 0

[tor/master] prop224: Don't use nodes as HSDirs if they don't have an HSDir index.
by nickm＠torproject.org 09 Aug '17

09 Aug '17

commit e42c55626abf7447a154f5271e3bc35743340a21 Author: George Kadianakis <desnacked(a)riseup.net> Date: Wed Aug 2 16:50:15 2017 +0300 prop224: Don't use nodes as HSDirs if they don't have an HSDir index. --- src/or/hs_common.c | 28 +++++++++++++++++++++++++++- src/or/nodelist.c | 6 +++--- 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/src/or/hs_common.c b/src/or/hs_common.c index f63adf51b..d68c446fd 100644 --- a/src/or/hs_common.c +++ b/src/or/hs_common.c @@ -1121,6 +1121,30 @@ hs_get_hsdir_spread_store(void) HS_DEFAULT_HSDIR_SPREAD_STORE, 1, 128); } +/** node is an HSDir so make sure that we have assigned an hsdir index. + * Return 0 if everything is as expected, else return -1. */ +static int +node_has_hsdir_index(const node_t *node) +{ + tor_assert(node_supports_v3_hsdir(node)); + + /* A node can't have an HSDir index without a descriptor since we need desc + * to get its ed25519 key */ + if (!node_has_descriptor(node)) { + return 0; + } + + /* At this point, since the node has a desc, this node must also have an + * hsdir index. If not, something went wrong, so BUG out. */ + if (BUG(node->hsdir_index == NULL) || + BUG(tor_mem_is_zero((const char*)node->hsdir_index->current, + DIGEST256_LEN))) { + return 0; + } + + return 1; +} + /* For a given blinded key and time period number, get the responsible HSDir * and put their routerstatus_t object in the responsible_dirs list. If * is_next_period is true, the next hsdir_index of the node_t is used. If @@ -1162,7 +1186,9 @@ hs_get_responsible_hsdirs(const ed25519_public_key_t *blinded_pk, node_t *n = node_get_mutable_by_id(rs->identity_digest); tor_assert(n); if (node_supports_v3_hsdir(n) && rs->is_hs_dir) { - if (BUG(n->hsdir_index == NULL)) { + if (!node_has_hsdir_index(n)) { + log_info(LD_GENERAL, "Node %s was found without hsdir index.", + node_describe(n)); continue; } smartlist_add(sorted_nodes, n); diff --git a/src/or/nodelist.c b/src/or/nodelist.c index 1666fffb7..c5a5979f3 100644 --- a/src/or/nodelist.c +++ b/src/or/nodelist.c @@ -189,9 +189,9 @@ node_set_hsdir_index(node_t *node, const networkstatus_t *ns) node_identity_pk = node_get_ed25519_id(node); if (node_identity_pk == NULL) { - log_warn(LD_BUG, "ed25519 identity public key not found when " - "trying to build the hsdir indexes for node %s", - node_describe(node)); + log_debug(LD_GENERAL, "ed25519 identity public key not found when " + "trying to build the hsdir indexes for node %s", + node_describe(node)); goto done; }

1 0

[tor/master] Correctly assign HSDir flags based on protocol list
by nickm＠torproject.org 09 Aug '17

09 Aug '17

commit 440eaa9b22573cdb0d38bf5c13200cc1077a453f Author: George Kadianakis <desnacked(a)riseup.net> Date: Thu Aug 3 16:08:17 2017 +0300 Correctly assign HSDir flags based on protocol list In Nick's words: "We want to always return false if the platform is a Tor version, and it is not as new as 0.3.0.8 -- but if the platform is not a Tor version, or if the version is as new as 0.3.0.8, then we want to obey the protocol list. That way, other implementations of our protocol won't have to claim any particular Tor version, and future versions of Tor will have the freedom to drop this protocol in the distant future." --- src/or/nodelist.c | 9 +++++++-- src/or/routerparse.c | 8 +++++--- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/or/nodelist.c b/src/or/nodelist.c index abbe15ecc..e900b5145 100644 --- a/src/or/nodelist.c +++ b/src/or/nodelist.c @@ -800,9 +800,14 @@ node_supports_v3_hsdir(const node_t *node) if (node->ri->protocol_list == NULL) { return 0; } + /* Bug #22447 forces us to filter on tor version: + * If platform is a Tor version, and older than 0.3.0.8, return False. + * Else, obey the protocol list. */ if (node->ri->platform) { - /* Bug #22447 forces us to filter on this version. */ - return tor_version_as_new_as(node->ri->platform, "0.3.0.8"); + if (!strcmpstart(node->ri->platform, "Tor ") && + !tor_version_as_new_as(node->ri->platform, "0.3.0.8")) { + return 0; + } } return protocol_list_supports_protocol(node->ri->protocol_list, PRT_HSDIR, PROTOVER_HSDIR_V3); diff --git a/src/or/routerparse.c b/src/or/routerparse.c index ec63aef4d..db42a44ee 100644 --- a/src/or/routerparse.c +++ b/src/or/routerparse.c @@ -2706,7 +2706,8 @@ routerstatus_parse_entry_from_string(memarea_t *area, rs->supports_ed25519_hs_intro = protocol_list_supports_protocol(tok->args[0], PRT_HSINTRO, 4); rs->supports_v3_hsdir = - protocol_list_supports_protocol(tok->args[0], PRT_HSDIR, 2); + protocol_list_supports_protocol(tok->args[0], PRT_HSDIR, + PROTOVER_HSDIR_V3); } if ((tok = find_opt_by_keyword(tokens, K_V))) { tor_assert(tok->n_args == 1); @@ -2720,8 +2721,9 @@ routerstatus_parse_entry_from_string(memarea_t *area, } if (!strcmpstart(tok->args[0], "Tor ") && found_protocol_list) { /* Bug #22447 forces us to filter on this version. */ - rs->supports_v3_hsdir = - tor_version_as_new_as(tok->args[0], "0.3.0.8"); + if (!tor_version_as_new_as(tok->args[0], "0.3.0.8")) { + rs->supports_v3_hsdir = 0; + } } if (vote_rs) { vote_rs->version = tor_strdup(tok->args[0]);

1 0