From fdd6352506ccf3d085238d970bef962b592da35d Mon Sep 17 00:00:00 2001 From: David Goulet Date: Wed, 11 Dec 2019 09:50:43 -0500 Subject: [PATCH 1/6] hs-v3: Report introduction failure SOCKS ExtendedErrors Signed-off-by: David Goulet --- src/feature/hs/hs_client.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/feature/hs/hs_client.c b/src/feature/hs/hs_client.c index cc1b01d2ef..4b9c9cb186 100644 --- a/src/feature/hs/hs_client.c +++ b/src/feature/hs/hs_client.c @@ -961,6 +961,20 @@ client_get_random_intro(const ed25519_public_key_t *service_pk) return ei; } +/** Called when introduction has failed meaning there is no more usable + * introduction points to be used (either NACKed or failed) for the given + * entry connection. + * + * This function only reports back the SOCKS5_HS_INTRO_FAILED (0xF2) code. The + * caller has to make sure to close the entry connections. */ +static void +socks_report_introduction_failed(entry_connection_t *conn) +{ + tor_assert(conn); + tor_assert(conn->socks_request); + conn->socks_request->socks_extended_error_code = SOCKS5_HS_INTRO_FAILED; +} + /** For this introduction circuit, we'll look at if we have any usable * introduction point left for this service. If so, we'll use the circuit to * re-extend to a new intro point. Else, we'll close the circuit and its @@ -1313,6 +1327,10 @@ client_desc_has_arrived(const smartlist_t *entry_conns) if (!hs_client_any_intro_points_usable(identity_pk, desc)) { log_info(LD_REND, "Hidden service descriptor is unusable. " "Closing streams."); + /* Report the extended socks error code that we were unable to introduce + * to the service. */ + socks_report_introduction_failed(entry_conn); + connection_mark_unattached_ap(entry_conn, END_STREAM_REASON_RESOLVEFAILED); /* We are unable to use the descriptor so remove the directory request From cf39276f7810c58a1a53bdccf3c3d15001808625 Mon Sep 17 00:00:00 2001 From: David Goulet Date: Wed, 11 Dec 2019 10:15:02 -0500 Subject: [PATCH 2/6] hs-v3: Report rendezvous circuit failure SOCKS ExtendedErrors Signed-off-by: David Goulet --- src/feature/hs/hs_circuit.c | 18 +++++++++++++ src/feature/hs/hs_client.c | 52 +++++++++++++++++++++++++++++++++++++ src/feature/hs/hs_client.h | 1 + 3 files changed, 71 insertions(+) diff --git a/src/feature/hs/hs_circuit.c b/src/feature/hs/hs_circuit.c index dc13c7045e..53f574c14a 100644 --- a/src/feature/hs/hs_circuit.c +++ b/src/feature/hs/hs_circuit.c @@ -621,6 +621,20 @@ setup_introduce1_data(const hs_desc_intro_point_t *ip, return ret; } +/** Helper: cleanup function for client circuit. This is for every HS version. + * It is called from hs_circ_cleanup_on_close() entry point. */ +static void +cleanup_on_close_client_circ(circuit_t *circ) +{ + tor_assert(circ); + + if (circuit_is_hs_v3(circ)) { + hs_client_circuit_cleanup_on_close(circ); + } + /* It is possible the circuit has an HS purpose but no identifier (rend_data + * or hs_ident). Thus possible that this passess through. */ +} + /** Helper: cleanup function for client circuit. This is for every HS version. * It is called from hs_circ_cleanup_on_free() entry point. */ static void @@ -1293,6 +1307,10 @@ hs_circ_cleanup_on_close(circuit_t *circ) { tor_assert(circ); + if (circuit_purpose_is_hs_client(circ->purpose)) { + cleanup_on_close_client_circ(circ); + } + /* On close, we simply remove it from the circuit map. It can not be used * anymore. We keep this code path fast and lean. */ diff --git a/src/feature/hs/hs_client.c b/src/feature/hs/hs_client.c index 4b9c9cb186..ceaba08ff9 100644 --- a/src/feature/hs/hs_client.c +++ b/src/feature/hs/hs_client.c @@ -961,6 +961,27 @@ client_get_random_intro(const ed25519_public_key_t *service_pk) return ei; } +/** Called when a rendezvous circuit has timed out. Every streams attached to + * the circuit will get set with the SOCKS5_HS_REND_FAILED (0xF3) extended + * error code so if the connection to the rendezvous point ends up not + * working, this code could be sent back as a reason. */ +static void +socks_report_rend_circuit_timed_out(const origin_circuit_t *rend_circ) +{ + tor_assert(rend_circ); + + /* For each entry connections attached to this rendezvous circuit, report + * the error. */ + for (edge_connection_t *edge = rend_circ->p_streams; edge; + edge = edge->next_stream) { + entry_connection_t *entry = EDGE_TO_ENTRY_CONN(edge); + if (entry->socks_request) { + entry->socks_request->socks_extended_error_code = + SOCKS5_HS_REND_FAILED; + } + } +} + /** Called when introduction has failed meaning there is no more usable * introduction points to be used (either NACKed or failed) for the given * entry connection. @@ -1779,6 +1800,37 @@ get_hs_client_auths_map(void) /* Public API */ /* ========== */ +/** Called when a circuit was just cleaned up. This is done right before the + * circuit is marked for close. */ +void +hs_client_circuit_cleanup_on_close(const circuit_t *circ) +{ + bool has_timed_out; + + tor_assert(circ); + tor_assert(CIRCUIT_IS_ORIGIN(circ)); + + has_timed_out = + (circ->marked_for_close_orig_reason == END_CIRC_REASON_TIMEOUT); + + switch (circ->purpose) { + case CIRCUIT_PURPOSE_C_ESTABLISH_REND: + case CIRCUIT_PURPOSE_C_REND_READY: + case CIRCUIT_PURPOSE_C_REND_READY_INTRO_ACKED: + case CIRCUIT_PURPOSE_C_REND_JOINED: + /* Report extended SOCKS error code when a rendezvous circuit timeouts. + * This MUST be done on_close() because it is possible the entry + * connection would get closed before the circuit is freed and thus + * failing to report the error code. */ + if (has_timed_out) { + socks_report_rend_circuit_timed_out(CONST_TO_ORIGIN_CIRCUIT(circ)); + } + break; + default: + break; + } +} + /** Called when a circuit was just cleaned up. This is done right before the * circuit is freed. */ void diff --git a/src/feature/hs/hs_client.h b/src/feature/hs/hs_client.h index 3660bfa96c..685b10f955 100644 --- a/src/feature/hs/hs_client.h +++ b/src/feature/hs/hs_client.h @@ -110,6 +110,7 @@ int hs_client_send_introduce1(origin_circuit_t *intro_circ, origin_circuit_t *rend_circ); void hs_client_circuit_has_opened(origin_circuit_t *circ); +void hs_client_circuit_cleanup_on_close(const circuit_t *circ); void hs_client_circuit_cleanup_on_free(const circuit_t *circ); int hs_client_receive_rendezvous_acked(origin_circuit_t *circ, From 6ab11bbf30e66ab4d63079657b689e09e127c939 Mon Sep 17 00:00:00 2001 From: David Goulet Date: Wed, 11 Dec 2019 10:30:31 -0500 Subject: [PATCH 3/6] hs-v3: Report SOCKS ExtendedErrors when all intro timed out Signed-off-by: David Goulet --- src/feature/hs/hs_client.c | 50 +++++++++++++++++++++++++++++++++---- src/lib/net/socks5_status.h | 1 + 2 files changed, 46 insertions(+), 5 deletions(-) diff --git a/src/feature/hs/hs_client.c b/src/feature/hs/hs_client.c index ceaba08ff9..40647ac256 100644 --- a/src/feature/hs/hs_client.c +++ b/src/feature/hs/hs_client.c @@ -961,6 +961,37 @@ client_get_random_intro(const ed25519_public_key_t *service_pk) return ei; } +/** Return true iff all intro points for the given service have timed out. */ +static bool +intro_points_all_timed_out(const ed25519_public_key_t *service_pk) +{ + bool ret = false; + + tor_assert(service_pk); + + const hs_descriptor_t *desc = hs_cache_lookup_as_client(service_pk); + + SMARTLIST_FOREACH_BEGIN(desc->encrypted_data.intro_points, + const hs_desc_intro_point_t *, ip) { + const hs_cache_intro_state_t *state = + hs_cache_client_intro_state_find(service_pk, + &ip->auth_key_cert->signed_key); + if (!state || !state->timed_out) { + /* No state or if this intro point has not timed out, we are done since + * clearly not all of them have timed out. */ + goto end; + } + } SMARTLIST_FOREACH_END(ip); + + /* Exiting the loop here means that all intro points we've looked at have + * timed out. Note that we can _not_ have a descriptor without intro points + * in the client cache. */ + ret = true; + + end: + return ret; +} + /** Called when a rendezvous circuit has timed out. Every streams attached to * the circuit will get set with the SOCKS5_HS_REND_FAILED (0xF3) extended * error code so if the connection to the rendezvous point ends up not @@ -986,14 +1017,23 @@ socks_report_rend_circuit_timed_out(const origin_circuit_t *rend_circ) * introduction points to be used (either NACKed or failed) for the given * entry connection. * - * This function only reports back the SOCKS5_HS_INTRO_FAILED (0xF2) code. The - * caller has to make sure to close the entry connections. */ + * This function only reports back the SOCKS5_HS_INTRO_FAILED (0xF2) code or + * SOCKS5_HS_INTRO_TIMEDOUT (0xF7) if all intros have timed out. The caller + * has to make sure to close the entry connections. */ static void -socks_report_introduction_failed(entry_connection_t *conn) +socks_report_introduction_failed(entry_connection_t *conn, + const ed25519_public_key_t *identity_pk) { + socks5_reply_status_t code = SOCKS5_HS_INTRO_FAILED; + tor_assert(conn); tor_assert(conn->socks_request); - conn->socks_request->socks_extended_error_code = SOCKS5_HS_INTRO_FAILED; + tor_assert(identity_pk); + + if (intro_points_all_timed_out(identity_pk)) { + code = SOCKS5_HS_INTRO_TIMEDOUT; + } + conn->socks_request->socks_extended_error_code = code; } /** For this introduction circuit, we'll look at if we have any usable @@ -1350,7 +1390,7 @@ client_desc_has_arrived(const smartlist_t *entry_conns) "Closing streams."); /* Report the extended socks error code that we were unable to introduce * to the service. */ - socks_report_introduction_failed(entry_conn); + socks_report_introduction_failed(entry_conn, identity_pk); connection_mark_unattached_ap(entry_conn, END_STREAM_REASON_RESOLVEFAILED); diff --git a/src/lib/net/socks5_status.h b/src/lib/net/socks5_status.h index 2b663e00c4..90c4305d29 100644 --- a/src/lib/net/socks5_status.h +++ b/src/lib/net/socks5_status.h @@ -37,6 +37,7 @@ typedef enum { SOCKS5_HS_MISSING_CLIENT_AUTH = 0xF4, SOCKS5_HS_BAD_CLIENT_AUTH = 0xF5, SOCKS5_HS_BAD_ADDRESS = 0xF6, + SOCKS5_HS_INTRO_TIMEDOUT = 0xF7, } socks5_reply_status_t; #endif /* !defined(TOR_SOCKS5_STATUS_H) */ From dd25805152342385c3fee1c1f2bee6cad452c5cf Mon Sep 17 00:00:00 2001 From: David Goulet Date: Wed, 11 Dec 2019 10:34:35 -0500 Subject: [PATCH 4/6] man: Update tor.1 ExtendedErrors section with latest Signed-off-by: David Goulet --- doc/tor.1.txt | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/doc/tor.1.txt b/doc/tor.1.txt index f2acf15f12..3cd9a2403a 100644 --- a/doc/tor.1.txt +++ b/doc/tor.1.txt @@ -1563,15 +1563,13 @@ The following options are useful only for clients (that is, if X'F2' Onion Service Introduction Failed - Client failed to introduce to the service meaning the descriptor - was found but the service is not connected anymore to the - introduction point. The service has likely changed its descriptor - or is not running. (v3 only) + All introduction attempts failed either due to a combination of + NACK by the intro point or time out. (v3 only) X'F3' Onion Service Rendezvous Failed - Client failed to rendezvous with the service which means that the - client is unable to finalize the connection. (v3 only) + Every rendezvous circuit has timed out and thus the client is + unable to rendezvous with the service. (v3 only) X'F4' Onion Service Missing Client Authorization @@ -1592,6 +1590,11 @@ The following options are useful only for clients (that is, if error is returned: address checksum doesn't match, ed25519 public key is invalid or the encoding is invalid. (v3 only) + X'F7' Onion Service Introduction Timed Out + + Similar to X'F2' code but in this case, all introduction attemps + have failed due to a time out. (v3 only) + // Anchor only for formatting, not visible in the man page. [[SocksPortFlagsMisc]]:: Flags are processed left to right. If flags conflict, the last flag on the From 9b72a561f59d5df7842909f5310209055f7c52eb Mon Sep 17 00:00:00 2001 From: David Goulet Date: Wed, 11 Dec 2019 11:08:54 -0500 Subject: [PATCH 5/6] test: Unit test for missing ExtendedErrors Signed-off-by: David Goulet --- src/test/test_hs_client.c | 72 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/src/test/test_hs_client.c b/src/test/test_hs_client.c index 3d6422a83c..0cd9e71fda 100644 --- a/src/test/test_hs_client.c +++ b/src/test/test_hs_client.c @@ -1189,7 +1189,11 @@ static void test_socks_hs_errors(void *arg) { int ret; + char digest[DIGEST_LEN]; char *desc_encoded = NULL; + circuit_t *circ = NULL; + origin_circuit_t *ocirc = NULL; + tor_addr_t addr; ed25519_keypair_t service_kp; ed25519_keypair_t signing_kp; entry_connection_t *socks_conn = NULL; @@ -1236,6 +1240,73 @@ test_socks_hs_errors(void *arg) desc = hs_helper_build_hs_desc_with_ip(&service_kp); tt_assert(desc); + /* Before testing the client authentication error code, encode the + * descriptor with no client auth. */ + ret = hs_desc_encode_descriptor(desc, &service_kp, NULL, &desc_encoded); + tt_int_op(ret, OP_EQ, 0); + tt_assert(desc_encoded); + + /* + * Test the introduction failure codes (X'F2' and X'F7') + */ + + /* First, we have to put all the IPs in the failure cache. */ + SMARTLIST_FOREACH_BEGIN(desc->encrypted_data.intro_points, + hs_desc_intro_point_t *, ip) { + hs_cache_client_intro_state_note(&service_kp.pubkey, + &ip->auth_key_cert->signed_key, + INTRO_POINT_FAILURE_GENERIC); + } SMARTLIST_FOREACH_END(ip); + + hs_client_dir_fetch_done(dir_conn, "Reason", desc_encoded, 200); + tt_int_op(socks_conn->socks_request->socks_extended_error_code, OP_EQ, + SOCKS5_HS_INTRO_FAILED); + + /* Purge client cache of the descriptor so we can go again. */ + hs_cache_purge_as_client(); + + /* Second, set all failures to be time outs. */ + SMARTLIST_FOREACH_BEGIN(desc->encrypted_data.intro_points, + hs_desc_intro_point_t *, ip) { + hs_cache_client_intro_state_note(&service_kp.pubkey, + &ip->auth_key_cert->signed_key, + INTRO_POINT_FAILURE_TIMEOUT); + } SMARTLIST_FOREACH_END(ip); + + hs_client_dir_fetch_done(dir_conn, "Reason", desc_encoded, 200); + tt_int_op(socks_conn->socks_request->socks_extended_error_code, OP_EQ, + SOCKS5_HS_INTRO_TIMEDOUT); + + /* Purge client cache of the descriptor so we can go again. */ + hs_cache_purge_as_client(); + + /* + * Test the rendezvous failure codes (X'F3') + */ + + circ = dummy_origin_circuit_new(0); + tt_assert(circ); + circ->purpose = CIRCUIT_PURPOSE_C_REND_READY; + ocirc = TO_ORIGIN_CIRCUIT(circ); + ocirc->hs_ident = hs_ident_circuit_new(&service_kp.pubkey); + ocirc->build_state = tor_malloc_zero(sizeof(cpath_build_state_t)); + /* Code path will log this exit so build it. */ + ocirc->build_state->chosen_exit = extend_info_new("TestNickname", digest, + NULL, NULL, NULL, &addr, + 4242); + /* Attach socks connection to this rendezvous circuit. */ + ocirc->p_streams = ENTRY_TO_EDGE_CONN(socks_conn); + /* Trigger the rendezvous failure. Timeout the circuit and free. */ + circuit_mark_for_close(circ, END_CIRC_REASON_TIMEOUT); + + tt_int_op(socks_conn->socks_request->socks_extended_error_code, OP_EQ, + SOCKS5_HS_REND_FAILED); + + /* + * Test client authorization codes. + */ + + tor_free(desc_encoded); crypto_rand((char *) descriptor_cookie, sizeof(descriptor_cookie)); ret = hs_desc_encode_descriptor(desc, &service_kp, descriptor_cookie, &desc_encoded); @@ -1277,6 +1348,7 @@ test_socks_hs_errors(void *arg) connection_free_minimal(TO_CONN(dir_conn)); hs_descriptor_free(desc); tor_free(desc_encoded); + circuit_free(circ); hs_free_all(); From 92ae8cea30d5c4b3cd5283abf571ffdb5ed06bcb Mon Sep 17 00:00:00 2001 From: David Goulet Date: Wed, 8 Apr 2020 08:17:41 -0400 Subject: [PATCH 6/6] changes: Add changes file for #32542 Signed-off-by: David Goulet --- changes/ticket32542 | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 changes/ticket32542 diff --git a/changes/ticket32542 b/changes/ticket32542 new file mode 100644 index 0000000000..c52335b059 --- /dev/null +++ b/changes/ticket32542 @@ -0,0 +1,3 @@ + o Minor feature (onion service client, SOCKS5): + - Add 3 new SocksPort ExtendedErrors (F2, F3, F7) that reports back new type + of onion service connection failures. Closes ticket 32542.