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

[tor/master] hs-v3: Refactor descriptor dir fetch done code
by asn＠torproject.org 18 Nov '19

18 Nov '19

commit fbc18c8989bae0e2acb5d903f2bd5b0648bad828 Author: David Goulet <dgoulet(a)torproject.org> Date: Tue May 28 14:43:28 2019 -0400 hs-v3: Refactor descriptor dir fetch done code This commit extract most of the code that dirclient.c had to handle the end of a descriptor directory requests (fetch). It is moved into hs_client.c in order to have one single point of entry and the rest is fully handled by the HS subsystem. As part of #30382, depending on how the descriptor ended up stored (decoded or not), different SOCKS error code can be returned. Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/feature/dirclient/dirclient.c | 80 +--------- src/feature/hs/hs_client.c | 304 ++++++++++++++++++++++++++------------ src/feature/hs/hs_client.h | 6 +- src/test/test_hs_client.c | 8 +- 4 files changed, 221 insertions(+), 177 deletions(-) diff --git a/src/feature/dirclient/dirclient.c b/src/feature/dirclient/dirclient.c index abece62dd..6155a09a5 100644 --- a/src/feature/dirclient/dirclient.c +++ b/src/feature/dirclient/dirclient.c @@ -2722,91 +2722,13 @@ handle_response_fetch_hsdesc_v3(dir_connection_t *conn, const char *reason = args->reason; const char *body = args->body; const size_t body_len = args->body_len; - hs_desc_decode_status_t decode_status; tor_assert(conn->hs_ident); log_info(LD_REND,"Received v3 hsdesc (body size %d, status %d (%s))", (int)body_len, status_code, escaped(reason)); - switch (status_code) { - case 200: - /* We got something: Try storing it in the cache. */ - decode_status = hs_cache_store_as_client(body, - &conn->hs_ident->identity_pk); - switch (decode_status) { - case HS_DESC_DECODE_OK: - case HS_DESC_DECODE_NEED_CLIENT_AUTH: - case HS_DESC_DECODE_BAD_CLIENT_AUTH: - log_info(LD_REND, "Stored hidden service descriptor successfully."); - TO_CONN(conn)->purpose = DIR_PURPOSE_HAS_FETCHED_HSDESC; - if (decode_status == HS_DESC_DECODE_OK) { - hs_client_desc_has_arrived(conn->hs_ident); - } else { - /* This handles both client auth decode status. */ - hs_client_desc_missing_bad_client_auth(conn->hs_ident, decode_status); - log_info(LD_REND, "Stored hidden service descriptor requires " - "%s client authorization.", - decode_status == HS_DESC_DECODE_NEED_CLIENT_AUTH ? "missing" - : "new"); - } - /* Fire control port RECEIVED event. */ - hs_control_desc_event_received(conn->hs_ident, conn->identity_digest); - hs_control_desc_event_content(conn->hs_ident, conn->identity_digest, - body); - break; - case HS_DESC_DECODE_ENCRYPTED_ERROR: - case HS_DESC_DECODE_SUPERENC_ERROR: - case HS_DESC_DECODE_PLAINTEXT_ERROR: - case HS_DESC_DECODE_GENERIC_ERROR: - default: - log_info(LD_REND, "Failed to store hidden service descriptor. " - "Descriptor decoding status: %d", decode_status); - /* Fire control port FAILED event. */ - hs_control_desc_event_failed(conn->hs_ident, conn->identity_digest, - "BAD_DESC"); - hs_control_desc_event_content(conn->hs_ident, conn->identity_digest, - NULL); - break; - } - break; - case 404: - /* Not there. We'll retry when connection_about_to_close_connection() - * tries to clean this conn up. */ - log_info(LD_REND, "Fetching hidden service v3 descriptor not found: " - "Retrying at another directory."); - /* Fire control port FAILED event. */ - hs_control_desc_event_failed(conn->hs_ident, conn->identity_digest, - "NOT_FOUND"); - hs_control_desc_event_content(conn->hs_ident, conn->identity_digest, - NULL); - hs_client_desc_not_found(conn->hs_ident); - break; - case 400: - log_warn(LD_REND, "Fetching v3 hidden service descriptor failed: " - "http status 400 (%s). Dirserver didn't like our " - "query? Retrying at another directory.", - escaped(reason)); - /* Fire control port FAILED event. */ - hs_control_desc_event_failed(conn->hs_ident, conn->identity_digest, - "QUERY_REJECTED"); - hs_control_desc_event_content(conn->hs_ident, conn->identity_digest, - NULL); - break; - default: - log_warn(LD_REND, "Fetching v3 hidden service descriptor failed: " - "http status %d (%s) response unexpected from HSDir server " - "'%s:%d'. Retrying at another directory.", - status_code, escaped(reason), TO_CONN(conn)->address, - TO_CONN(conn)->port); - /* Fire control port FAILED event. */ - hs_control_desc_event_failed(conn->hs_ident, conn->identity_digest, - "UNEXPECTED"); - hs_control_desc_event_content(conn->hs_ident, conn->identity_digest, - NULL); - break; - } - + hs_client_dir_fetch_done(conn, reason, body, status_code); return 0; } diff --git a/src/feature/hs/hs_client.c b/src/feature/hs/hs_client.c index 0a10492e0..2716dca03 100644 --- a/src/feature/hs/hs_client.c +++ b/src/feature/hs/hs_client.c @@ -1258,6 +1258,189 @@ find_client_auth(const ed25519_public_key_t *service_identity_pk) return digest256map_get(client_auths, service_identity_pk->pubkey); } +/** This is called when a descriptor has arrived following a fetch request and + * has been stored in the client cache. The given entry connections, matching + * the service identity key, will get attached to the service circuit. */ +static void +client_desc_has_arrived(const smartlist_t *entry_conns) +{ + time_t now = time(NULL); + + tor_assert(entry_conns); + + SMARTLIST_FOREACH_BEGIN(entry_conns, entry_connection_t *, entry_conn) { + const hs_descriptor_t *desc; + edge_connection_t *edge_conn = ENTRY_TO_EDGE_CONN(entry_conn); + const ed25519_public_key_t *identity_pk = + &edge_conn->hs_ident->identity_pk; + + /* We were just called because we stored the descriptor for this service + * so not finding a descriptor means we have a bigger problem. */ + desc = hs_cache_lookup_as_client(identity_pk); + if (BUG(desc == NULL)) { + goto end; + } + + if (!hs_client_any_intro_points_usable(identity_pk, desc)) { + log_info(LD_REND, "Hidden service descriptor is unusable. " + "Closing streams."); + connection_mark_unattached_ap(entry_conn, + END_STREAM_REASON_RESOLVEFAILED); + /* We are unable to use the descriptor so remove the directory request + * from the cache so the next connection can try again. */ + note_connection_attempt_succeeded(edge_conn->hs_ident); + continue; + } + + log_info(LD_REND, "Descriptor has arrived. Launching circuits."); + + /* Mark connection as waiting for a circuit since we do have a usable + * descriptor now. */ + mark_conn_as_waiting_for_circuit(&edge_conn->base_, now); + } SMARTLIST_FOREACH_END(entry_conn); + + end: + return; +} + +/** This is called when a descriptor fetch was successful but the descriptor + * couldn't be decrypted due to missing or bad client authorization. */ +static void +client_desc_missing_bad_client_auth(const smartlist_t *entry_conns, + hs_desc_decode_status_t status) +{ + tor_assert(entry_conns); + + SMARTLIST_FOREACH_BEGIN(entry_conns, entry_connection_t *, entry_conn) { + socks5_reply_status_t code; + if (status == HS_DESC_DECODE_BAD_CLIENT_AUTH) { + code = SOCKS5_HS_BAD_CLIENT_AUTH; + } else if (status == HS_DESC_DECODE_NEED_CLIENT_AUTH) { + code = SOCKS5_HS_MISSING_CLIENT_AUTH; + } else { + /* We should not be called with another type of status. Recover by + * sending a generic error. */ + tor_assert_nonfatal_unreached(); + code = HS_DESC_DECODE_GENERIC_ERROR; + } + entry_conn->socks_request->socks_extended_error_code = code; + connection_mark_unattached_ap(entry_conn, END_STREAM_REASON_MISC); + } SMARTLIST_FOREACH_END(entry_conn); +} + +/** Called when we get a 200 directory fetch status code. */ +static void +client_dir_fetch_200(dir_connection_t *dir_conn, + const smartlist_t *entry_conns, const char *body) +{ + hs_desc_decode_status_t decode_status; + + tor_assert(dir_conn); + tor_assert(entry_conns); + tor_assert(body); + + /* We got something: Try storing it in the cache. */ + decode_status = hs_cache_store_as_client(body, + &dir_conn->hs_ident->identity_pk); + switch (decode_status) { + case HS_DESC_DECODE_OK: + case HS_DESC_DECODE_NEED_CLIENT_AUTH: + case HS_DESC_DECODE_BAD_CLIENT_AUTH: + log_info(LD_REND, "Stored hidden service descriptor successfully."); + TO_CONN(dir_conn)->purpose = DIR_PURPOSE_HAS_FETCHED_HSDESC; + if (decode_status == HS_DESC_DECODE_OK) { + client_desc_has_arrived(entry_conns); + } else { + /* This handles both client auth decode status. */ + client_desc_missing_bad_client_auth(entry_conns, decode_status); + log_info(LD_REND, "Stored hidden service descriptor requires " + "%s client authorization.", + decode_status == HS_DESC_DECODE_NEED_CLIENT_AUTH ? "missing" + : "new"); + } + /* Fire control port RECEIVED event. */ + hs_control_desc_event_received(dir_conn->hs_ident, + dir_conn->identity_digest); + hs_control_desc_event_content(dir_conn->hs_ident, + dir_conn->identity_digest, body); + break; + case HS_DESC_DECODE_ENCRYPTED_ERROR: + case HS_DESC_DECODE_SUPERENC_ERROR: + case HS_DESC_DECODE_PLAINTEXT_ERROR: + case HS_DESC_DECODE_GENERIC_ERROR: + default: + log_info(LD_REND, "Failed to store hidden service descriptor. " + "Descriptor decoding status: %d", decode_status); + /* Fire control port FAILED event. */ + hs_control_desc_event_failed(dir_conn->hs_ident, + dir_conn->identity_digest, "BAD_DESC"); + hs_control_desc_event_content(dir_conn->hs_ident, + dir_conn->identity_digest, NULL); + break; + } +} + +/** Called when we get a 404 directory fetch status code. */ +static void +client_dir_fetch_404(dir_connection_t *dir_conn, + const smartlist_t *entry_conns) +{ + tor_assert(entry_conns); + + /* Not there. We'll retry when connection_about_to_close_connection() tries + * to clean this conn up. */ + log_info(LD_REND, "Fetching hidden service v3 descriptor not found: " + "Retrying at another directory."); + /* Fire control port FAILED event. */ + hs_control_desc_event_failed(dir_conn->hs_ident, dir_conn->identity_digest, + "NOT_FOUND"); + hs_control_desc_event_content(dir_conn->hs_ident, dir_conn->identity_digest, + NULL); + + /* Flag every entry connections that the descriptor was not found. */ + SMARTLIST_FOREACH_BEGIN(entry_conns, entry_connection_t *, entry_conn) { + entry_conn->socks_request->socks_extended_error_code = + SOCKS5_HS_NOT_FOUND; + } SMARTLIST_FOREACH_END(entry_conn); +} + +/** Called when we get a 400 directory fetch status code. */ +static void +client_dir_fetch_400(dir_connection_t *dir_conn, const char *reason) +{ + tor_assert(dir_conn); + + log_warn(LD_REND, "Fetching v3 hidden service descriptor failed: " + "http status 400 (%s). Dirserver didn't like our " + "query? Retrying at another directory.", + escaped(reason)); + + /* Fire control port FAILED event. */ + hs_control_desc_event_failed(dir_conn->hs_ident, dir_conn->identity_digest, + "QUERY_REJECTED"); + hs_control_desc_event_content(dir_conn->hs_ident, dir_conn->identity_digest, + NULL); +} + +/** Called when we get an unexpected directory fetch status code. */ +static void +client_dir_fetch_unexpected(dir_connection_t *dir_conn, const char *reason, + const int status_code) +{ + tor_assert(dir_conn); + + log_warn(LD_REND, "Fetching v3 hidden service descriptor failed: " + "http status %d (%s) response unexpected from HSDir " + "server '%s:%d'. Retrying at another directory.", + status_code, escaped(reason), TO_CONN(dir_conn)->address, + TO_CONN(dir_conn)->port); + /* Fire control port FAILED event. */ + hs_control_desc_event_failed(dir_conn->hs_ident, dir_conn->identity_digest, + "UNEXPECTED"); + hs_control_desc_event_content(dir_conn->hs_ident, dir_conn->identity_digest, + NULL); +} + /* ========== */ /* Public API */ /* ========== */ @@ -1711,103 +1894,42 @@ hs_config_client_authorization(const or_options_t *options, return ret; } -/** This is called when a descriptor has arrived following a fetch request and - * has been stored in the client cache. Every entry connection that matches - * the service identity key in the ident will get attached to the hidden - * service circuit. */ -void -hs_client_desc_has_arrived(const hs_ident_dir_conn_t *ident) -{ - time_t now = time(NULL); - smartlist_t *entry_conns = NULL; - - tor_assert(ident); - - entry_conns = find_entry_conns(&ident->identity_pk); - - SMARTLIST_FOREACH_BEGIN(entry_conns, entry_connection_t *, entry_conn) { - const hs_descriptor_t *desc; - edge_connection_t *edge_conn = ENTRY_TO_EDGE_CONN(entry_conn); - - /* We were just called because we stored the descriptor for this service - * so not finding a descriptor means we have a bigger problem. */ - desc = hs_cache_lookup_as_client(&ident->identity_pk); - if (BUG(desc == NULL)) { - goto end; - } - - if (!hs_client_any_intro_points_usable(&ident->identity_pk, desc)) { - log_info(LD_REND, "Hidden service descriptor is unusable. " - "Closing streams."); - connection_mark_unattached_ap(entry_conn, - END_STREAM_REASON_RESOLVEFAILED); - /* We are unable to use the descriptor so remove the directory request - * from the cache so the next connection can try again. */ - note_connection_attempt_succeeded(edge_conn->hs_ident); - continue; - } - - log_info(LD_REND, "Descriptor has arrived. Launching circuits."); - - /* Mark connection as waiting for a circuit since we do have a usable - * descriptor now. */ - mark_conn_as_waiting_for_circuit(&edge_conn->base_, now); - } SMARTLIST_FOREACH_END(entry_conn); - - end: - /* We don't have ownership of the objects in this list. */ - smartlist_free(entry_conns); -} - -/** This is called when a descriptor fetch was not found. Every entry - * connection that matches the requested onion service, its extended error - * code will be set accordingly. */ -void -hs_client_desc_not_found(const hs_ident_dir_conn_t *ident) -{ - smartlist_t *entry_conns; - - tor_assert(ident); - - entry_conns = find_entry_conns(&ident->identity_pk); - - SMARTLIST_FOREACH_BEGIN(entry_conns, entry_connection_t *, entry_conn) { - /* Descriptor was not found. We'll flag the socks request with the - * extended error code. If it is supported, it will be sent back. */ - entry_conn->socks_request->socks_extended_error_code = - SOCKS5_HS_NOT_FOUND; - } SMARTLIST_FOREACH_END(entry_conn); - - /* We don't have ownership of the objects in this list. */ - smartlist_free(entry_conns); -} - -/* This is called when a descriptor fetch was successful but the descriptor - * couldn't be decrypted due to missing or bad client authorization. */ +/** Called when a descriptor directory fetch is done. + * + * Act accordingly on all entry connections depending on the HTTP status code + * we got. In case of an error, the SOCKS error is set (if ExtendedErrors is + * set). + * + * The reason is a human readable string returned by the directory server + * which can describe the status of the request. The body is the response + * content, on 200 code it is the descriptor itself. Finally, the status_code + * is the HTTP code returned by the directory server. */ void -hs_client_desc_missing_bad_client_auth(const hs_ident_dir_conn_t *ident, - hs_desc_decode_status_t status) +hs_client_dir_fetch_done(dir_connection_t *dir_conn, const char *reason, + const char *body, const int status_code) { smartlist_t *entry_conns; - tor_assert(ident); + tor_assert(dir_conn); + tor_assert(body); - entry_conns = find_entry_conns(&ident->identity_pk); + /* Get all related entry connections. */ + entry_conns = find_entry_conns(&dir_conn->hs_ident->identity_pk); - SMARTLIST_FOREACH_BEGIN(entry_conns, entry_connection_t *, entry_conn) { - socks5_reply_status_t code; - if (status == HS_DESC_DECODE_BAD_CLIENT_AUTH) { - code = SOCKS5_HS_BAD_CLIENT_AUTH; - } else if (status == HS_DESC_DECODE_NEED_CLIENT_AUTH) { - code = SOCKS5_HS_MISSING_CLIENT_AUTH; - } else { - /* We should not be called with another type of status. Recover by - * sending a generic error. */ - tor_assert_nonfatal_unreached(); - code = HS_DESC_DECODE_GENERIC_ERROR; - } - entry_conn->socks_request->socks_extended_error_code = code; - } SMARTLIST_FOREACH_END(entry_conn); + switch (status_code) { + case 200: + client_dir_fetch_200(dir_conn, entry_conns, body); + break; + case 404: + client_dir_fetch_404(dir_conn, entry_conns); + break; + case 400: + client_dir_fetch_400(dir_conn, reason); + break; + default: + client_dir_fetch_unexpected(dir_conn, reason, status_code); + break; + } /* We don't have ownership of the objects in this list. */ smartlist_free(entry_conns); diff --git a/src/feature/hs/hs_client.h b/src/feature/hs/hs_client.h index 616d31a01..09bbe51df 100644 --- a/src/feature/hs/hs_client.h +++ b/src/feature/hs/hs_client.h @@ -72,10 +72,8 @@ int hs_client_receive_rendezvous2(origin_circuit_t *circ, const uint8_t *payload, size_t payload_len); -void hs_client_desc_has_arrived(const hs_ident_dir_conn_t *ident); -void hs_client_desc_not_found(const hs_ident_dir_conn_t *ident); -void hs_client_desc_missing_bad_client_auth(const hs_ident_dir_conn_t *ident, - hs_desc_decode_status_t status); +void hs_client_dir_fetch_done(dir_connection_t *dir_conn, const char *reason, + const char *body, const int status_code); extend_info_t *hs_client_get_random_intro_from_edge( const edge_connection_t *edge_conn); diff --git a/src/test/test_hs_client.c b/src/test/test_hs_client.c index e5735ba02..96faa9bb7 100644 --- a/src/test/test_hs_client.c +++ b/src/test/test_hs_client.c @@ -825,6 +825,7 @@ test_desc_has_arrived_cleanup(void *arg) ed25519_keypair_t signing_kp; entry_connection_t *socks1 = NULL, *socks2 = NULL; hs_ident_dir_conn_t hs_dir_ident; + dir_connection_t *dir_conn = NULL; (void) arg; @@ -881,9 +882,11 @@ test_desc_has_arrived_cleanup(void *arg) * SOCKS connection to be ended with a resolved failed. */ hs_ident_dir_conn_init(&signing_kp.pubkey, &desc->plaintext_data.blinded_pubkey, &hs_dir_ident); - hs_client_desc_has_arrived(&hs_dir_ident); + dir_conn = dir_connection_new(AF_INET); + dir_conn->hs_ident = hs_ident_dir_conn_dup(&hs_dir_ident); + hs_client_dir_fetch_done(dir_conn, "A reason", desc_str, 200); + connection_free_minimal(TO_CONN(dir_conn)); tt_int_op(socks1->edge_.end_reason, OP_EQ, END_STREAM_REASON_RESOLVEFAILED); - /* XXX: MUST work with OP_EQ. */ tt_int_op(socks2->edge_.end_reason, OP_EQ, END_STREAM_REASON_RESOLVEFAILED); /* Now let say tor cleans up the intro state cache which resets all intro @@ -892,7 +895,6 @@ test_desc_has_arrived_cleanup(void *arg) /* Retrying all SOCKS which should basically do nothing since we don't have * any pending SOCKS connection in AP_CONN_STATE_RENDDESC_WAIT state. */ - /* XXX: BUG() is triggered here, shouldn't if socks2 wasn't alive. */ retry_all_socks_conn_waiting_for_desc(); done:

1 0

[tor/master] hs-v3: Set extended error when descriptor is not found
by asn＠torproject.org 18 Nov '19

18 Nov '19

commit fb1d2120212bef100f2b5e90d27b8c251280cb0c Author: David Goulet <dgoulet(a)torproject.org> Date: Tue May 28 13:36:34 2019 -0400 hs-v3: Set extended error when descriptor is not found Part of #30382 Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/feature/dirclient/dirclient.c | 1 + src/feature/hs/hs_client.c | 24 ++++++++++++++++++++++++ src/feature/hs/hs_client.h | 1 + 3 files changed, 26 insertions(+) diff --git a/src/feature/dirclient/dirclient.c b/src/feature/dirclient/dirclient.c index 3b506317b..64205a44e 100644 --- a/src/feature/dirclient/dirclient.c +++ b/src/feature/dirclient/dirclient.c @@ -2761,6 +2761,7 @@ handle_response_fetch_hsdesc_v3(dir_connection_t *conn, "NOT_FOUND"); hs_control_desc_event_content(conn->hs_ident, conn->identity_digest, NULL); + hs_client_desc_not_found(conn->hs_ident); break; case 400: log_warn(LD_REND, "Fetching v3 hidden service descriptor failed: " diff --git a/src/feature/hs/hs_client.c b/src/feature/hs/hs_client.c index 491c52a04..0e4df73b9 100644 --- a/src/feature/hs/hs_client.c +++ b/src/feature/hs/hs_client.c @@ -42,6 +42,7 @@ #include "core/or/entry_connection_st.h" #include "core/or/extend_info_st.h" #include "core/or/origin_circuit_st.h" +#include "core/or/socks_request_st.h" /** Client-side authorizations for hidden services; map of service identity * public key to hs_client_service_authorization_t *. */ @@ -1758,6 +1759,29 @@ hs_client_desc_has_arrived(const hs_ident_dir_conn_t *ident) smartlist_free(entry_conns); } +/** This is called when a descriptor fetch was not found. Every entry + * connection that matches the requested onion service, its extended error + * code will be set accordingly. */ +void +hs_client_desc_not_found(const hs_ident_dir_conn_t *ident) +{ + smartlist_t *entry_conns; + + tor_assert(ident); + + entry_conns = find_entry_conns(&ident->identity_pk); + + SMARTLIST_FOREACH_BEGIN(entry_conns, entry_connection_t *, entry_conn) { + /* Descriptor was not found. We'll flag the socks request with the + * extended error code. If it is supported, it will be sent back. */ + entry_conn->socks_request->socks_extended_error_code = + SOCKS5_HS_NOT_FOUND; + } SMARTLIST_FOREACH_END(entry_conn); + + /* We don't have ownership of the objects in this list. */ + smartlist_free(entry_conns); +} + /** Return a newly allocated extend_info_t for a randomly chosen introduction * point for the given edge connection identifier ident. Return NULL if we * can't pick any usable introduction points. */ diff --git a/src/feature/hs/hs_client.h b/src/feature/hs/hs_client.h index 0e22a2e04..6bd6e5748 100644 --- a/src/feature/hs/hs_client.h +++ b/src/feature/hs/hs_client.h @@ -73,6 +73,7 @@ int hs_client_receive_rendezvous2(origin_circuit_t *circ, size_t payload_len); void hs_client_desc_has_arrived(const hs_ident_dir_conn_t *ident); +void hs_client_desc_not_found(const hs_ident_dir_conn_t *ident); extend_info_t *hs_client_get_random_intro_from_edge( const edge_connection_t *edge_conn);

1 0

[tor/master] hs-v3: Return descriptor decoding status when storing as client
by asn＠torproject.org 18 Nov '19

18 Nov '19

commit 7bba8bf72f20aa2ac71ac381a7fdf66fadbd473f Author: David Goulet <dgoulet(a)torproject.org> Date: Tue May 28 13:32:15 2019 -0400 hs-v3: Return descriptor decoding status when storing as client This will allow us to callback into the HS subsytem depending on the decoding status and return an extended SOCKS5 error code depending on the decoding issue. This is how we'll be able to tell the SocksPort connection if we are missing or have bad client authorization for a service. Part of #30382 Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/feature/dirclient/dirclient.c | 5 ++++- src/feature/hs/hs_cache.c | 34 +++++++++++++++++++++++++++------- src/feature/hs/hs_cache.h | 4 ++-- src/test/test_hs_client.c | 11 ++++++----- 4 files changed, 39 insertions(+), 15 deletions(-) diff --git a/src/feature/dirclient/dirclient.c b/src/feature/dirclient/dirclient.c index 1ea50fd35..3b506317b 100644 --- a/src/feature/dirclient/dirclient.c +++ b/src/feature/dirclient/dirclient.c @@ -2722,6 +2722,7 @@ handle_response_fetch_hsdesc_v3(dir_connection_t *conn, const char *reason = args->reason; const char *body = args->body; const size_t body_len = args->body_len; + hs_desc_decode_status_t decode_status; tor_assert(conn->hs_ident); @@ -2731,7 +2732,9 @@ handle_response_fetch_hsdesc_v3(dir_connection_t *conn, switch (status_code) { case 200: /* We got something: Try storing it in the cache. */ - if (hs_cache_store_as_client(body, &conn->hs_ident->identity_pk) < 0) { + decode_status = hs_cache_store_as_client(body, + &conn->hs_ident->identity_pk); + if (decode_status != HS_DESC_DECODE_OK) { log_info(LD_REND, "Failed to store hidden service descriptor"); /* Fire control port FAILED event. */ hs_control_desc_event_failed(conn->hs_ident, conn->identity_digest, diff --git a/src/feature/hs/hs_cache.c b/src/feature/hs/hs_cache.c index 39c226746..59b05d8f0 100644 --- a/src/feature/hs/hs_cache.c +++ b/src/feature/hs/hs_cache.c @@ -395,7 +395,8 @@ lookup_v3_desc_as_client(const uint8_t *key) * hs_cache_client_descriptor_t object. In case of error, return NULL. */ static hs_cache_client_descriptor_t * cache_client_desc_new(const char *desc_str, - const ed25519_public_key_t *service_identity_pk) + const ed25519_public_key_t *service_identity_pk, + hs_desc_decode_status_t *decode_status_out) { hs_desc_decode_status_t ret; hs_descriptor_t *desc = NULL; @@ -435,6 +436,9 @@ cache_client_desc_new(const char *desc_str, client_desc->encoded_desc = tor_strdup(desc_str); end: + if (decode_status_out) { + *decode_status_out = ret; + } return client_desc; } @@ -796,19 +800,34 @@ hs_cache_lookup_as_client(const ed25519_public_key_t *key) return NULL; } -/** Public API: Given an encoded descriptor, store it in the client HS - * cache. Return -1 on error, 0 on success .*/ -int +/** Public API: Given an encoded descriptor, store it in the client HS cache. + * Return a decode status which changes how we handle the SOCKS connection + * depending on its value: + * + * HS_DESC_DECODE_OK: Returned on success. Descriptor was properly decoded + * and is now stored. + * + * HS_DESC_DECODE_NEED_CLIENT_AUTH: Client authorization is needed but the + * descriptor was still stored. + * + * HS_DESC_DECODE_BAD_CLIENT_AUTH: Client authorization for this descriptor + * was not usable but the descriptor was + * still stored. + * + * Any other codes means indicate where the error occured and the descriptor + * was not stored. */ +hs_desc_decode_status_t hs_cache_store_as_client(const char *desc_str, const ed25519_public_key_t *identity_pk) { + hs_desc_decode_status_t ret; hs_cache_client_descriptor_t *client_desc = NULL; tor_assert(desc_str); tor_assert(identity_pk); /* Create client cache descriptor object */ - client_desc = cache_client_desc_new(desc_str, identity_pk); + client_desc = cache_client_desc_new(desc_str, identity_pk, &ret); if (!client_desc) { log_warn(LD_GENERAL, "HSDesc parsing failed!"); log_debug(LD_GENERAL, "Failed to parse HSDesc: %s.", escaped(desc_str)); @@ -817,14 +836,15 @@ hs_cache_store_as_client(const char *desc_str, /* Push it to the cache */ if (cache_store_as_client(client_desc) < 0) { + ret = HS_DESC_DECODE_GENERIC_ERROR; goto err; } - return 0; + return ret; err: cache_client_desc_free(client_desc); - return -1; + return ret; } /** Clean all client caches using the current time now. */ diff --git a/src/feature/hs/hs_cache.h b/src/feature/hs/hs_cache.h index 5df7e54fc..e7729f204 100644 --- a/src/feature/hs/hs_cache.h +++ b/src/feature/hs/hs_cache.h @@ -83,8 +83,8 @@ const hs_descriptor_t * hs_cache_lookup_as_client(const struct ed25519_public_key_t *key); const char * hs_cache_lookup_encoded_as_client(const struct ed25519_public_key_t *key); -int hs_cache_store_as_client(const char *desc_str, - const struct ed25519_public_key_t *identity_pk); +hs_desc_decode_status_t hs_cache_store_as_client(const char *desc_str, + const struct ed25519_public_key_t *identity_pk); void hs_cache_clean_as_client(time_t now); void hs_cache_purge_as_client(void); diff --git a/src/test/test_hs_client.c b/src/test/test_hs_client.c index b777dafdf..e5735ba02 100644 --- a/src/test/test_hs_client.c +++ b/src/test/test_hs_client.c @@ -393,7 +393,8 @@ test_client_pick_intro(void *arg) tt_assert(encoded); /* store it */ - hs_cache_store_as_client(encoded, &service_kp.pubkey); + ret = hs_cache_store_as_client(encoded, &service_kp.pubkey); + tt_int_op(ret, OP_EQ, HS_DESC_DECODE_OK); /* fetch it to make sure it works */ const hs_descriptor_t *fetched_desc = @@ -852,7 +853,7 @@ test_desc_has_arrived_cleanup(void *arg) /* Store in the client cache. */ ret = hs_cache_store_as_client(desc_str, &signing_kp.pubkey); - tt_int_op(ret, OP_EQ, 0); + tt_int_op(ret, OP_EQ, HS_DESC_DECODE_OK); cached_desc = hs_cache_lookup_as_client(&signing_kp.pubkey); tt_assert(cached_desc); hs_helper_desc_equal(desc, cached_desc); @@ -953,7 +954,7 @@ test_close_intro_circuits_new_desc(void *arg) /* Store it */ ret = hs_cache_store_as_client(encoded, &service_kp.pubkey); - tt_int_op(ret, OP_EQ, 0); + tt_int_op(ret, OP_EQ, HS_DESC_DECODE_OK); tor_free(encoded); tt_assert(hs_cache_lookup_as_client(&service_kp.pubkey)); } @@ -988,8 +989,8 @@ test_close_intro_circuits_new_desc(void *arg) tt_int_op(ret, OP_EQ, 0); tt_assert(encoded); - hs_cache_store_as_client(encoded, &service_kp.pubkey); - tt_int_op(ret, OP_EQ, 0); + ret = hs_cache_store_as_client(encoded, &service_kp.pubkey); + tt_int_op(ret, OP_EQ, HS_DESC_DECODE_OK); tor_free(encoded); tt_assert(hs_cache_lookup_as_client(&service_kp.pubkey)); }

1 0

[tor/master] hs-v3: Set extended error if .onion is invalid
by asn＠torproject.org 18 Nov '19

18 Nov '19

commit 80f241907c1b3d69784de98e9ff3f96d5c37f51b Author: David Goulet <dgoulet(a)torproject.org> Date: Thu Oct 17 15:15:14 2019 -0400 hs-v3: Set extended error if .onion is invalid In order to achieve this, the parse_extended_hostname() had to be refactored to return either success or failure and setting the hostname type in the given parameter. The reason for that is so it can detect invalid onion addresses that is having a ".onion", the right length but just not passing validation. That way, we can send back the prop304 ExtendedError "X'F1' Onion Service Descriptor Is Invalid" to notify the SOCKS connection of the invalid onion address. Part of #30382 Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/core/or/connection_edge.c | 169 +++++++++++++++++++++++++----------------- src/core/or/connection_edge.h | 18 +++-- src/test/test_hs_common.c | 45 ++++++++--- 3 files changed, 146 insertions(+), 86 deletions(-) diff --git a/src/core/or/connection_edge.c b/src/core/or/connection_edge.c index f0d5752dd..4b4bcff2f 100644 --- a/src/core/or/connection_edge.c +++ b/src/core/or/connection_edge.c @@ -1553,6 +1553,102 @@ consider_plaintext_ports(entry_connection_t *conn, uint16_t port) return 0; } +/** Parse the given hostname in address. Returns true if the parsing was + * successful and type_out contains the type of the hostname. Else, false is + * returned which means it was not recognized and type_out is set to + * BAD_HOSTNAME. + * + * The possible recognized forms are (where true is returned): + * + * If address is of the form "y.onion" with a well-formed handle y: + * Put a NUL after y, lower-case it, and return ONION_V2_HOSTNAME or + * ONION_V3_HOSTNAME depending on the HS version. + * + * If address is of the form "x.y.onion" with a well-formed handle x: + * Drop "x.", put a NUL after y, lower-case it, and return + * ONION_V2_HOSTNAME or ONION_V3_HOSTNAME depending on the HS version. + * + * If address is of the form "y.onion" with a badly-formed handle y: + * Return BAD_HOSTNAME and log a message. + * + * If address is of the form "y.exit": + * Put a NUL after y and return EXIT_HOSTNAME. + * + * Otherwise: + * Return NORMAL_HOSTNAME and change nothing. + */ +STATIC bool +parse_extended_hostname(char *address, hostname_type_t *type_out) +{ + char *s; + char *q; + char query[HS_SERVICE_ADDR_LEN_BASE32+1]; + + s = strrchr(address,'.'); + if (!s) { + *type_out = NORMAL_HOSTNAME; /* no dot, thus normal */ + goto success; + } + if (!strcmp(s+1,"exit")) { + *s = 0; /* NUL-terminate it */ + *type_out = EXIT_HOSTNAME; /* .exit */ + goto success; + } + if (strcmp(s+1,"onion")) { + *type_out = NORMAL_HOSTNAME; /* neither .exit nor .onion, thus normal */ + goto success; + } + + /* so it is .onion */ + *s = 0; /* NUL-terminate it */ + /* locate a 'sub-domain' component, in order to remove it */ + q = strrchr(address, '.'); + if (q == address) { + *type_out = BAD_HOSTNAME; + goto failed; /* reject sub-domain, as DNS does */ + } + q = (NULL == q) ? address : q + 1; + if (strlcpy(query, q, HS_SERVICE_ADDR_LEN_BASE32+1) >= + HS_SERVICE_ADDR_LEN_BASE32+1) { + *type_out = BAD_HOSTNAME; + goto failed; + } + if (q != address) { + memmove(address, q, strlen(q) + 1 /* also get \0 */); + } + /* v2 onion address check. */ + if (strlen(query) == REND_SERVICE_ID_LEN_BASE32) { + *type_out = ONION_V2_HOSTNAME; + if (rend_valid_v2_service_id(query)) { + goto success; + } + goto failed; + } + + /* v3 onion address check. */ + if (strlen(query) == HS_SERVICE_ADDR_LEN_BASE32) { + *type_out = ONION_V3_HOSTNAME; + if (hs_address_is_valid(query)) { + goto success; + } + goto failed; + } + + /* Reaching this point, nothing was recognized. */ + *type_out = BAD_HOSTNAME; + goto failed; + + success: + return true; + failed: + /* otherwise, return to previous state and return 0 */ + *s = '.'; + log_warn(LD_APP, "Invalid %shostname %s; rejecting", + (*type_out == (ONION_V2_HOSTNAME || ONION_V3_HOSTNAME) ? "onion " : ""), + safe_str_client(address)); + return false; +} + /** How many times do we try connecting with an exit configured via * TrackHostExits before concluding that it won't work any more and trying a * different one? */ @@ -2020,16 +2116,15 @@ connection_ap_handshake_rewrite_and_attach(entry_connection_t *conn, const int automap = rr.automap; const addressmap_entry_source_t exit_source = rr.exit_source; - /* Now, we parse the address to see if it's an .onion or .exit or - * other special address. - */ - const hostname_type_t addresstype = parse_extended_hostname(socks->address); - /* Now see whether the hostname is bogus. This could happen because of an * onion hostname whose format we don't recognize. */ - if (addresstype == BAD_HOSTNAME) { + hostname_type_t addresstype; + if (!parse_extended_hostname(socks->address, &addresstype)) { control_event_client_status(LOG_WARN, "SOCKS_BAD_HOSTNAME HOSTNAME=%s", escaped(socks->address)); + if (addresstype == ONION_V3_HOSTNAME) { + conn->socks_request->socks_extended_error_code = SOCKS5_HS_IS_INVALID; + } connection_mark_unattached_ap(conn, END_STREAM_REASON_TORPROTOCOL); return -1; } @@ -4312,68 +4407,6 @@ connection_ap_can_use_exit(const entry_connection_t *conn, return 1; } -/** If address is of the form "y.onion" with a well-formed handle y: - * Put a NUL after y, lower-case it, and return ONION_V2_HOSTNAME or - * ONION_V3_HOSTNAME depending on the HS version. - * - * If address is of the form "x.y.onion" with a well-formed handle x: - * Drop "x.", put a NUL after y, lower-case it, and return - * ONION_V2_HOSTNAME or ONION_V3_HOSTNAME depending on the HS version. - * - * If address is of the form "y.onion" with a badly-formed handle y: - * Return BAD_HOSTNAME and log a message. - * - * If address is of the form "y.exit": - * Put a NUL after y and return EXIT_HOSTNAME. - * - * Otherwise: - * Return NORMAL_HOSTNAME and change nothing. - */ -hostname_type_t -parse_extended_hostname(char *address) -{ - char *s; - char *q; - char query[HS_SERVICE_ADDR_LEN_BASE32+1]; - - s = strrchr(address,'.'); - if (!s) - return NORMAL_HOSTNAME; /* no dot, thus normal */ - if (!strcmp(s+1,"exit")) { - *s = 0; /* NUL-terminate it */ - return EXIT_HOSTNAME; /* .exit */ - } - if (strcmp(s+1,"onion")) - return NORMAL_HOSTNAME; /* neither .exit nor .onion, thus normal */ - - /* so it is .onion */ - *s = 0; /* NUL-terminate it */ - /* locate a 'sub-domain' component, in order to remove it */ - q = strrchr(address, '.'); - if (q == address) { - goto failed; /* reject sub-domain, as DNS does */ - } - q = (NULL == q) ? address : q + 1; - if (strlcpy(query, q, HS_SERVICE_ADDR_LEN_BASE32+1) >= - HS_SERVICE_ADDR_LEN_BASE32+1) - goto failed; - if (q != address) { - memmove(address, q, strlen(q) + 1 /* also get \0 */); - } - if (rend_valid_v2_service_id(query)) { - return ONION_V2_HOSTNAME; /* success */ - } - if (hs_address_is_valid(query)) { - return ONION_V3_HOSTNAME; - } - failed: - /* otherwise, return to previous state and return 0 */ - *s = '.'; - log_warn(LD_APP, "Invalid onion hostname %s; rejecting", - safe_str_client(address)); - return BAD_HOSTNAME; -} - /** Return true iff the (possibly NULL) alen-byte chunk of memory at * a is equal to the (possibly NULL) blen-byte chunk of memory * at b. */ diff --git a/src/core/or/connection_edge.h b/src/core/or/connection_edge.h index e82b6bd76..cda087b16 100644 --- a/src/core/or/connection_edge.h +++ b/src/core/or/connection_edge.h @@ -71,6 +71,15 @@ entry_connection_t *EDGE_TO_ENTRY_CONN(edge_connection_t *); #define connection_mark_unattached_ap(conn, endreason) \ connection_mark_unattached_ap_((conn), (endreason), __LINE__, SHORT_FILE__) +/** Possible return values for parse_extended_hostname. */ +typedef enum hostname_type_t { + BAD_HOSTNAME, + EXIT_HOSTNAME, + NORMAL_HOSTNAME, + ONION_V2_HOSTNAME, + ONION_V3_HOSTNAME, +} hostname_type_t; + MOCK_DECL(void,connection_mark_unattached_ap_, (entry_connection_t *conn, int endreason, int line, const char *file)); @@ -155,13 +164,6 @@ int connection_ap_handshake_rewrite_and_attach(entry_connection_t *conn, origin_circuit_t *circ, crypt_path_t *cpath); -/** Possible return values for parse_extended_hostname. */ -typedef enum hostname_type_t { - NORMAL_HOSTNAME, ONION_V2_HOSTNAME, ONION_V3_HOSTNAME, - EXIT_HOSTNAME, BAD_HOSTNAME -} hostname_type_t; -hostname_type_t parse_extended_hostname(char *address); - #if defined(HAVE_NET_IF_H) && defined(HAVE_NET_PFVAR_H) int get_pf_socket(void); #endif @@ -219,6 +221,8 @@ void half_edge_free_(struct half_edge_t *he); #ifdef CONNECTION_EDGE_PRIVATE +STATIC bool parse_extended_hostname(char *address, hostname_type_t *type_out); + /** A parsed BEGIN or BEGIN_DIR cell */ typedef struct begin_cell_t { /** The address the client has asked us to connect to, or NULL if this is diff --git a/src/test/test_hs_common.c b/src/test/test_hs_common.c index de3f7e04f..9b15b3d1e 100644 --- a/src/test/test_hs_common.c +++ b/src/test/test_hs_common.c @@ -6,6 +6,7 @@ * \brief Test hidden service common functionalities. */ +#define CONNECTION_EDGE_PRIVATE #define HS_COMMON_PRIVATE #define HS_CLIENT_PRIVATE #define HS_SERVICE_PRIVATE @@ -778,6 +779,7 @@ static void test_parse_extended_hostname(void *arg) { (void) arg; + hostname_type_t type; char address1[] = "fooaddress.onion"; char address2[] = "aaaaaaaaaaaaaaaa.onion"; @@ -788,21 +790,42 @@ test_parse_extended_hostname(void *arg) char address7[] = ".abcdefghijklmnop.onion"; char address8[] = "www.25njqamcweflpvkl73j4szahhihoc4xt3ktcgjnpaingr5yhkenl5sid.onion"; + char address9[] = + "www.15njqamcweflpvkl73j4szahhihoc4xt3ktcgjnpaingr5yhkenl5sid.onion"; - tt_assert(BAD_HOSTNAME == parse_extended_hostname(address1)); - tt_assert(ONION_V2_HOSTNAME == parse_extended_hostname(address2)); - tt_str_op(address2,OP_EQ, "aaaaaaaaaaaaaaaa"); - tt_assert(EXIT_HOSTNAME == parse_extended_hostname(address3)); - tt_assert(NORMAL_HOSTNAME == parse_extended_hostname(address4)); - tt_assert(ONION_V2_HOSTNAME == parse_extended_hostname(address5)); - tt_str_op(address5,OP_EQ, "abcdefghijklmnop"); - tt_assert(ONION_V2_HOSTNAME == parse_extended_hostname(address6)); - tt_str_op(address6,OP_EQ, "abcdefghijklmnop"); - tt_assert(BAD_HOSTNAME == parse_extended_hostname(address7)); - tt_assert(ONION_V3_HOSTNAME == parse_extended_hostname(address8)); + tt_assert(!parse_extended_hostname(address1, &type)); + tt_int_op(type, OP_EQ, BAD_HOSTNAME); + + tt_assert(parse_extended_hostname(address2, &type)); + tt_int_op(type, OP_EQ, ONION_V2_HOSTNAME); + tt_str_op(address2, OP_EQ, "aaaaaaaaaaaaaaaa"); + + tt_assert(parse_extended_hostname(address3, &type)); + tt_int_op(type, OP_EQ, EXIT_HOSTNAME); + + tt_assert(parse_extended_hostname(address4, &type)); + tt_int_op(type, OP_EQ, NORMAL_HOSTNAME); + + tt_assert(parse_extended_hostname(address5, &type)); + tt_int_op(type, OP_EQ, ONION_V2_HOSTNAME); + tt_str_op(address5, OP_EQ, "abcdefghijklmnop"); + + tt_assert(parse_extended_hostname(address6, &type)); + tt_int_op(type, OP_EQ, ONION_V2_HOSTNAME); + tt_str_op(address6, OP_EQ, "abcdefghijklmnop"); + + tt_assert(!parse_extended_hostname(address7, &type)); + tt_int_op(type, OP_EQ, BAD_HOSTNAME); + + tt_assert(parse_extended_hostname(address8, &type)); + tt_int_op(type, OP_EQ, ONION_V3_HOSTNAME); tt_str_op(address8, OP_EQ, "25njqamcweflpvkl73j4szahhihoc4xt3ktcgjnpaingr5yhkenl5sid"); + /* Invalid v3 address. */ + tt_assert(!parse_extended_hostname(address9, &type)); + tt_int_op(type, OP_EQ, ONION_V3_HOSTNAME); + done: ; }

1 0

[tor/master] hs-v3: Improve documentation of hs_cache_client_descriptor_t
by asn＠torproject.org 18 Nov '19

18 Nov '19

commit 68a004d147bd4bd245563b7b7ed2f97e1f333dd4 Author: David Goulet <dgoulet(a)torproject.org> Date: Thu Oct 17 09:50:23 2019 -0400 hs-v3: Improve documentation of hs_cache_client_descriptor_t Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/feature/hs/hs_cache.h | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/feature/hs/hs_cache.h b/src/feature/hs/hs_cache.h index 4fd9ac544..a56e2cc6c 100644 --- a/src/feature/hs/hs_cache.h +++ b/src/feature/hs/hs_cache.h @@ -114,8 +114,10 @@ typedef struct hs_cache_client_descriptor_t { * using the next blinded key of the service. */ time_t expiration_ts; - /** The cached descriptor, this object is the owner. It can't be NULL. A - * cache object without a valid descriptor is not possible. */ + /** The cached decoded descriptor, this object is the owner. This can be + * NULL if the descriptor couldn't be decoded due to missing or bad client + * authorization. It can be decoded later from the encoded_desc object if + * the proper client authorization is given tor. */ hs_descriptor_t *desc; /** Encoded descriptor in string form. Can't be NULL. */

1 0

[tor/master] test: Unit test for the hs cache decrypt on new auth
by asn＠torproject.org 18 Nov '19

18 Nov '19

commit 3892ac7c718d25315d779b5c29ffae58c70a5dce Author: David Goulet <dgoulet(a)torproject.org> Date: Thu Jun 6 09:36:02 2019 -0400 test: Unit test for the hs cache decrypt on new auth Part of #30382 Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/feature/hs/hs_client.c | 6 ++++ src/feature/hs/hs_client.h | 1 + src/test/hs_test_helpers.c | 49 +++++++++++++++++++++++++++ src/test/hs_test_helpers.h | 7 ++++ src/test/test_hs_cache.c | 82 +++++++++++++++++++++++++++++++++++++++++++++- 5 files changed, 144 insertions(+), 1 deletion(-) diff --git a/src/feature/hs/hs_client.c b/src/feature/hs/hs_client.c index 2716dca03..1e7adabdc 100644 --- a/src/feature/hs/hs_client.c +++ b/src/feature/hs/hs_client.c @@ -2140,4 +2140,10 @@ get_hs_client_auths_map(void) return client_auths; } +STATIC void +set_hs_client_auths_map(digest256map_t *map) +{ + client_auths = map; +} + #endif /* defined(TOR_UNIT_TESTS) */ diff --git a/src/feature/hs/hs_client.h b/src/feature/hs/hs_client.h index 09bbe51df..699ce6051 100644 --- a/src/feature/hs/hs_client.h +++ b/src/feature/hs/hs_client.h @@ -115,6 +115,7 @@ STATIC void retry_all_socks_conn_waiting_for_desc(void); #ifdef TOR_UNIT_TESTS STATIC digest256map_t *get_hs_client_auths_map(void); +STATIC void set_hs_client_auths_map(digest256map_t *map); #endif /* defined(TOR_UNIT_TESTS) */ diff --git a/src/test/hs_test_helpers.c b/src/test/hs_test_helpers.c index 0a21fe576..e7f5ec90b 100644 --- a/src/test/hs_test_helpers.c +++ b/src/test/hs_test_helpers.c @@ -1,12 +1,16 @@ /* Copyright (c) 2017-2019, The Tor Project, Inc. */ /* See LICENSE for licensing information */ +#define HS_CLIENT_PRIVATE + #include "core/or/or.h" #include "lib/crypt_ops/crypto_ed25519.h" #include "test/test.h" #include "feature/nodelist/torcert.h" +#include "feature/hs/hs_client.h" #include "feature/hs/hs_common.h" +#include "feature/hs/hs_service.h" #include "test/hs_test_helpers.h" hs_desc_intro_point_t * @@ -207,6 +211,35 @@ hs_helper_build_hs_desc_no_ip(const ed25519_keypair_t *signing_kp) return hs_helper_build_hs_desc_impl(1, signing_kp); } +hs_descriptor_t * +hs_helper_build_hs_desc_with_client_auth( + const uint8_t *descriptor_cookie, + const curve25519_public_key_t *client_pk, + const ed25519_keypair_t *signing_kp) +{ + curve25519_keypair_t auth_ephemeral_kp; + hs_descriptor_t *desc = hs_helper_build_hs_desc_impl(0, signing_kp); + hs_desc_authorized_client_t *desc_client; + + /* The number of client authorized auth has tobe a multiple of + * HS_DESC_AUTH_CLIENT_MULTIPLE so remove one that we'll replace. */ + desc_client = smartlist_get(desc->superencrypted_data.clients, 0); + smartlist_remove(desc->superencrypted_data.clients, desc_client); + hs_desc_authorized_client_free(desc_client); + + desc_client = tor_malloc_zero(sizeof(hs_desc_authorized_client_t)); + + curve25519_keypair_generate(&auth_ephemeral_kp, 0); + memcpy(&desc->superencrypted_data.auth_ephemeral_pubkey, + &auth_ephemeral_kp.pubkey, sizeof(curve25519_public_key_t)); + + hs_desc_build_authorized_client(desc->subcredential, client_pk, + &auth_ephemeral_kp.seckey, + descriptor_cookie, desc_client); + smartlist_add(desc->superencrypted_data.clients, desc_client); + return desc; +} + void hs_helper_desc_equal(const hs_descriptor_t *desc1, const hs_descriptor_t *desc2) @@ -353,3 +386,19 @@ hs_helper_desc_equal(const hs_descriptor_t *desc1, ; } +void +hs_helper_add_client_auth(const ed25519_public_key_t *service_pk, + const curve25519_secret_key_t *client_sk) +{ + digest256map_t *client_auths = get_hs_client_auths_map(); + if (client_auths == NULL) { + client_auths = digest256map_new(); + set_hs_client_auths_map(client_auths); + } + + hs_client_service_authorization_t *auth = + tor_malloc_zero(sizeof(hs_client_service_authorization_t)); + memcpy(&auth->enc_seckey, client_sk, sizeof(curve25519_secret_key_t)); + hs_build_address(service_pk, HS_VERSION_THREE, auth->onion_address); + digest256map_set(client_auths, service_pk->pubkey, auth); +} diff --git a/src/test/hs_test_helpers.h b/src/test/hs_test_helpers.h index 9662a83ba..be11a4735 100644 --- a/src/test/hs_test_helpers.h +++ b/src/test/hs_test_helpers.h @@ -15,11 +15,18 @@ hs_descriptor_t *hs_helper_build_hs_desc_no_ip( const ed25519_keypair_t *signing_kp); hs_descriptor_t *hs_helper_build_hs_desc_with_ip( const ed25519_keypair_t *signing_kp); +hs_descriptor_t *hs_helper_build_hs_desc_with_client_auth( + const uint8_t *descriptor_cookie, + const curve25519_public_key_t *client_pk, + const ed25519_keypair_t *signing_kp); void hs_helper_desc_equal(const hs_descriptor_t *desc1, const hs_descriptor_t *desc2); void hs_helper_get_subcred_from_identity_keypair(ed25519_keypair_t *signing_kp, uint8_t *subcred_out); +void hs_helper_add_client_auth(const ed25519_public_key_t *service_pk, + const curve25519_secret_key_t *client_sk); + #endif /* !defined(TOR_HS_TEST_HELPERS_H) */ diff --git a/src/test/test_hs_cache.c b/src/test/test_hs_cache.c index 88cd4079e..c39a4b644 100644 --- a/src/test/test_hs_cache.c +++ b/src/test/test_hs_cache.c @@ -20,9 +20,10 @@ #include "feature/nodelist/networkstatus.h" #include "core/mainloop/connection.h" #include "core/proto/proto_http.h" -#include "lib/crypt_ops/crypto_format.h" #include "core/or/circuitlist.h" #include "core/or/channel.h" +#include "lib/crypt_ops/crypto_format.h" +#include "lib/crypt_ops/crypto_rand.h" #include "core/or/edge_connection_st.h" #include "core/or/or_circuit_st.h" @@ -567,6 +568,83 @@ test_client_cache(void *arg) } } +/** Test that we can store HS descriptors in the client HS cache. */ +static void +test_client_cache_decrypt(void *arg) +{ + int ret; + char *desc_encoded = NULL; + uint8_t descriptor_cookie[HS_DESC_DESCRIPTOR_COOKIE_LEN]; + curve25519_keypair_t client_kp; + ed25519_keypair_t service_kp; + hs_descriptor_t *desc = NULL; + const hs_descriptor_t *search_desc; + const char *search_desc_encoded; + + (void) arg; + + /* Initialize HSDir cache subsystem */ + hs_init(); + + MOCK(networkstatus_get_live_consensus, + mock_networkstatus_get_live_consensus); + + /* Set consensus time */ + parse_rfc1123_time("Sat, 26 Oct 1985 13:00:00 UTC", + &mock_ns.valid_after); + parse_rfc1123_time("Sat, 26 Oct 1985 14:00:00 UTC", + &mock_ns.fresh_until); + parse_rfc1123_time("Sat, 26 Oct 1985 16:00:00 UTC", + &mock_ns.valid_until); + + /* Generate a valid descriptor with normal values. */ + { + ret = ed25519_keypair_generate(&service_kp, 0); + tt_int_op(ret, OP_EQ, 0); + ret = curve25519_keypair_generate(&client_kp, 0); + tt_int_op(ret, OP_EQ, 0); + crypto_rand((char *) descriptor_cookie, sizeof(descriptor_cookie)); + + desc = hs_helper_build_hs_desc_with_client_auth(descriptor_cookie, + &client_kp.pubkey, + &service_kp); + tt_assert(desc); + ret = hs_desc_encode_descriptor(desc, &service_kp, descriptor_cookie, + &desc_encoded); + tt_int_op(ret, OP_EQ, 0); + } + + /* Put it in the cache. Should not be decrypted since the client + * authorization creds were not added to the global map. */ + ret = hs_cache_store_as_client(desc_encoded, &service_kp.pubkey); + tt_int_op(ret, OP_EQ, HS_DESC_DECODE_NEED_CLIENT_AUTH); + + /* We should not be able to decrypt anything. */ + ret = hs_cache_client_new_auth_parse(&service_kp.pubkey); + tt_int_op(ret, OP_EQ, false); + + /* Add client auth to global map. */ + hs_helper_add_client_auth(&service_kp.pubkey, &client_kp.seckey); + + /* We should not be able to decrypt anything. */ + ret = hs_cache_client_new_auth_parse(&service_kp.pubkey); + tt_int_op(ret, OP_EQ, true); + + /* Lookup the cache to make sure it is usable and there. */ + search_desc = hs_cache_lookup_as_client(&service_kp.pubkey); + tt_assert(search_desc); + search_desc_encoded = hs_cache_lookup_encoded_as_client(&service_kp.pubkey); + tt_mem_op(search_desc_encoded, OP_EQ, desc_encoded, strlen(desc_encoded)); + + done: + hs_descriptor_free(desc); + tor_free(desc_encoded); + + hs_free_all(); + + UNMOCK(networkstatus_get_live_consensus); +} + struct testcase_t hs_cache[] = { /* Encoding tests. */ { "directory", test_directory, TT_FORK, @@ -579,6 +657,8 @@ struct testcase_t hs_cache[] = { NULL, NULL }, { "client_cache", test_client_cache, TT_FORK, NULL, NULL }, + { "client_cache_decrypt", test_client_cache_decrypt, TT_FORK, + NULL, NULL }, END_OF_TESTCASES };

1 0

[tor/master] test: Unit test for the SOCKS5 HS client auth errors
by asn＠torproject.org 18 Nov '19

18 Nov '19

commit c0dd5324b35ee158dc392556b67b5a098de2874a Author: David Goulet <dgoulet(a)torproject.org> Date: Thu Jun 6 08:43:48 2019 -0400 test: Unit test for the SOCKS5 HS client auth errors Part of #30382 Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/test/test_hs_client.c | 135 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 135 insertions(+) diff --git a/src/test/test_hs_client.c b/src/test/test_hs_client.c index 96faa9bb7..cf3763427 100644 --- a/src/test/test_hs_client.c +++ b/src/test/test_hs_client.c @@ -25,6 +25,7 @@ #include "app/config/config.h" #include "lib/crypt_ops/crypto_cipher.h" #include "lib/crypt_ops/crypto_dh.h" +#include "lib/crypt_ops/crypto_rand.h" #include "core/or/channeltls.h" #include "feature/dircommon/directory.h" #include "core/mainloop/mainloop.h" @@ -92,6 +93,24 @@ helper_config_client(const char *conf, int validate_only) return ret; } +static void +helper_add_random_client_auth(const ed25519_public_key_t *service_pk) +{ + char *conf = NULL; +#define conf_fmt "ClientOnionAuthDir %s\n" + tor_asprintf(&conf, conf_fmt, get_fname("auth_keys")); +#undef conf_fmt + helper_config_client(conf, 0); + tor_free(conf); + + digest256map_t *client_auths = get_hs_client_auths_map(); + hs_client_service_authorization_t *auth = + tor_malloc_zero(sizeof(hs_client_service_authorization_t)); + curve25519_secret_key_generate(&auth->enc_seckey, 0); + hs_build_address(service_pk, HS_VERSION_THREE, auth->onion_address); + digest256map_set(client_auths, service_pk->pubkey, auth); +} + /* Test helper function: Setup a circuit and a stream with the same hidden * service destination, and put them in circ_out and * conn_out. Make the stream wait for circuits to be established to the @@ -551,6 +570,17 @@ mock_connection_mark_unattached_ap_(entry_connection_t *conn, int endreason, } static void +mock_connection_mark_unattached_ap_no_close(entry_connection_t *conn, + int endreason, int line, + const char *file) +{ + (void) conn; + (void) endreason; + (void) line; + (void) file; +} + +static void test_descriptor_fetch(void *arg) { int ret; @@ -1094,6 +1124,108 @@ test_close_intro_circuits_cache_clean(void *arg) UNMOCK(networkstatus_get_live_consensus); } +static void +test_socks_hs_errors(void *arg) +{ + int ret; + char *desc_encoded = NULL; + ed25519_keypair_t service_kp; + ed25519_keypair_t signing_kp; + entry_connection_t *socks_conn = NULL; + dir_connection_t *dir_conn = NULL; + hs_descriptor_t *desc = NULL; + uint8_t descriptor_cookie[HS_DESC_DESCRIPTOR_COOKIE_LEN]; + + (void) arg; + + MOCK(networkstatus_get_live_consensus, + mock_networkstatus_get_live_consensus); + MOCK(connection_mark_unattached_ap_, + mock_connection_mark_unattached_ap_no_close); + MOCK(read_file_to_str, mock_read_file_to_str); + MOCK(tor_listdir, mock_tor_listdir); + MOCK(check_private_dir, mock_check_private_dir); + + /* Set consensus time */ + parse_rfc1123_time("Sat, 26 Oct 1985 13:00:00 UTC", + &mock_ns.valid_after); + parse_rfc1123_time("Sat, 26 Oct 1985 14:00:00 UTC", + &mock_ns.fresh_until); + parse_rfc1123_time("Sat, 26 Oct 1985 16:00:00 UTC", + &mock_ns.valid_until); + + hs_init(); + + ret = ed25519_keypair_generate(&service_kp, 0); + tt_int_op(ret, OP_EQ, 0); + ret = ed25519_keypair_generate(&signing_kp, 0); + tt_int_op(ret, OP_EQ, 0); + + socks_conn = helper_build_socks_connection(&service_kp.pubkey, + AP_CONN_STATE_RENDDESC_WAIT); + tt_assert(socks_conn); + + /* Create directory connection. */ + dir_conn = dir_connection_new(AF_INET); + dir_conn->hs_ident = tor_malloc_zero(sizeof(hs_ident_dir_conn_t)); + TO_CONN(dir_conn)->purpose = DIR_PURPOSE_FETCH_HSDESC; + ed25519_pubkey_copy(&dir_conn->hs_ident->identity_pk, &service_kp.pubkey); + + /* Encode descriptor so we can decode it. */ + desc = hs_helper_build_hs_desc_with_ip(&service_kp); + tt_assert(desc); + + crypto_rand((char *) descriptor_cookie, sizeof(descriptor_cookie)); + ret = hs_desc_encode_descriptor(desc, &service_kp, descriptor_cookie, + &desc_encoded); + tt_int_op(ret, OP_EQ, 0); + tt_assert(desc_encoded); + + /* Try decoding. Point this to an existing descriptor. The following should + * fail thus the desc_out should be set to NULL. */ + hs_descriptor_t *desc_out = desc; + ret = hs_client_decode_descriptor(desc_encoded, &service_kp.pubkey, + &desc_out); + tt_int_op(ret, OP_EQ, HS_DESC_DECODE_NEED_CLIENT_AUTH); + tt_assert(desc_out == NULL); + + /* The caching will fail to decrypt because the descriptor_cookie used above + * is not known to the HS subsystem. This will lead to a missing client + * auth. */ + 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_MISSING_CLIENT_AUTH); + + /* Add in the global client auth list bad creds for this service. */ + helper_add_random_client_auth(&service_kp.pubkey); + + ret = hs_client_decode_descriptor(desc_encoded, &service_kp.pubkey, + &desc_out); + tt_int_op(ret, OP_EQ, HS_DESC_DECODE_BAD_CLIENT_AUTH); + tt_assert(desc_out == NULL); + + /* Simmulate a fetch done again. This should replace the cached descriptor + * and signal a bad client authorization. */ + 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_BAD_CLIENT_AUTH); + + done: + connection_free_minimal(ENTRY_TO_CONN(socks_conn)); + connection_free_minimal(TO_CONN(dir_conn)); + hs_descriptor_free(desc); + tor_free(desc_encoded); + + hs_free_all(); + + UNMOCK(networkstatus_get_live_consensus); + UNMOCK(connection_mark_unattached_ap_); + UNMOCK(read_file_to_str); + UNMOCK(tor_listdir); + UNMOCK(check_private_dir); +} + struct testcase_t hs_client_tests[] = { { "e2e_rend_circuit_setup_legacy", test_e2e_rend_circuit_setup_legacy, TT_FORK, NULL, NULL }, @@ -1116,5 +1248,8 @@ struct testcase_t hs_client_tests[] = { { "close_intro_circuits_cache_clean", test_close_intro_circuits_cache_clean, TT_FORK, NULL, NULL }, + /* SOCKS5 Extended Error Code. */ + { "socks_hs_errors", test_socks_hs_errors, TT_FORK, NULL, NULL }, + END_OF_TESTCASES };

1 0

[tor/master] doc: Manpage entry for ExtendedErrors of prop304
by asn＠torproject.org 18 Nov '19

18 Nov '19

commit d60ed5a6a2586bdf4feb266e44b7db4703d3c4a6 Author: David Goulet <dgoulet(a)torproject.org> Date: Thu Oct 17 10:28:18 2019 -0400 doc: Manpage entry for ExtendedErrors of prop304 Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- doc/tor.1.txt | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/doc/tor.1.txt b/doc/tor.1.txt index 06bf022b1..2b09da673 100644 --- a/doc/tor.1.txt +++ b/doc/tor.1.txt @@ -1374,6 +1374,44 @@ The following options are useful only for clients (that is, if one. You can disable this behavior, so that Tor will select "No authentication" when IsolateSOCKSAuth is disabled, or when this option is set. + **ExtendedErrors**;; + Return extended error code in the SOCKS reply. So far, the possible + errors are: + + X'F0' Onion Service Descriptor Can Not be Found + + The requested onion service descriptor can't be found on the + hashring and thus not reachable by the client. + + X'F1' Onion Service Descriptor Is Invalid + + The requested onion service descriptor can't be parsed or + signature validation failed. + + 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. + + X'F3' Onion Service Rendezvous Failed + + Client failed to rendezvous with the service which means that the + client is unable to finalize the connection. + + X'F4' Onion Service Missing Client Authorization + + Client was able to download the requested onion service descriptor + but is unable to decrypt its content because it is missing client + authorization information. + + X'F5' Onion Service Wrong Client Authorization + + Client was able to download the requested onion service descriptor + but is unable to decrypt its content using the client + authorization information it has. This means the client access + were revoked. // Anchor only for formatting, not visible in the man page. [[SocksPortFlagsMisc]]::

1 0

[tor/master] hs-v3: Function to re-parse unencrypted descriptor
by asn＠torproject.org 18 Nov '19

18 Nov '19

commit 48a9f8a63f4d605e9bee5e7131b1f6db1ec95295 Author: David Goulet <dgoulet(a)torproject.org> Date: Fri May 31 09:27:36 2019 -0400 hs-v3: Function to re-parse unencrypted descriptor We now keep descriptor that we can't decode due to missing client authorization in the cache. This new function is used when new client authorization are added and to tell the client cache to retry decoding. Part of #30382 Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/feature/hs/hs_cache.c | 28 ++++++++++++++++++++++++++++ src/feature/hs/hs_cache.h | 2 ++ 2 files changed, 30 insertions(+) diff --git a/src/feature/hs/hs_cache.c b/src/feature/hs/hs_cache.c index 59b05d8f0..49d5ade41 100644 --- a/src/feature/hs/hs_cache.c +++ b/src/feature/hs/hs_cache.c @@ -942,6 +942,34 @@ hs_cache_client_intro_state_purge(void) "cache purged."); } +/* This is called when new client authorization was added to the global state. + * It attemps to decode the descriptor of the given service identity key. + * + * Return true if decoding was successful else false. */ +bool +hs_cache_client_new_auth_parse(const ed25519_public_key_t *service_pk) +{ + bool ret = false; + hs_cache_client_descriptor_t *cached_desc = NULL; + + tor_assert(service_pk); + + cached_desc = lookup_v3_desc_as_client(service_pk->pubkey); + if (cached_desc == NULL || cached_desc->desc != NULL) { + /* No entry for that service or the descriptor is already decoded. */ + goto end; + } + + /* Attempt a decode. If we are successful, inform the caller. */ + if (hs_client_decode_descriptor(cached_desc->encoded_desc, service_pk, + &cached_desc->desc) == HS_DESC_DECODE_OK) { + ret = true; + } + + end: + return ret; +} + /**************** Generics *********************************/ /** Do a round of OOM cleanup on all directory caches. Return the amount of diff --git a/src/feature/hs/hs_cache.h b/src/feature/hs/hs_cache.h index e7729f204..4fd9ac544 100644 --- a/src/feature/hs/hs_cache.h +++ b/src/feature/hs/hs_cache.h @@ -99,6 +99,8 @@ const hs_cache_intro_state_t *hs_cache_client_intro_state_find( void hs_cache_client_intro_state_clean(time_t now); void hs_cache_client_intro_state_purge(void); +bool hs_cache_client_new_auth_parse(const ed25519_public_key_t *service_pk); + #ifdef HS_CACHE_PRIVATE #include "lib/crypt_ops/crypto_ed25519.h"

1 0

[tor/master] Merge branch 'tor-github/pr/1423'
by asn＠torproject.org 18 Nov '19

18 Nov '19

commit d28b6792cb99f25d42607eb46985ac4553013dd8 Merge: c34fb3413 d60ed5a6a Author: George Kadianakis <desnacked(a)riseup.net> Date: Mon Nov 18 19:06:53 2019 +0200 Merge branch 'tor-github/pr/1423' doc/tor.1.txt | 38 ++++ src/app/config/config.c | 6 +- src/core/mainloop/connection.c | 2 + src/core/or/connection_edge.c | 179 +++++++++++-------- src/core/or/connection_edge.h | 18 +- src/core/or/entry_port_cfg_st.h | 3 + src/core/or/socks_request_st.h | 7 + src/feature/dirclient/dirclient.c | 57 +----- src/feature/hs/hs_cache.c | 101 +++++++++-- src/feature/hs/hs_cache.h | 12 +- src/feature/hs/hs_client.c | 362 +++++++++++++++++++++++++++++--------- src/feature/hs/hs_client.h | 6 +- src/feature/hs/hs_descriptor.c | 77 ++++---- src/feature/hs/hs_descriptor.h | 25 +++ src/lib/net/socks5_status.h | 9 + src/test/hs_test_helpers.c | 49 ++++++ src/test/hs_test_helpers.h | 7 + src/test/test_hs_cache.c | 86 ++++++++- src/test/test_hs_client.c | 154 +++++++++++++++- src/test/test_hs_common.c | 45 +++-- src/test/test_hs_descriptor.c | 30 ++-- 21 files changed, 964 insertions(+), 309 deletions(-)

1 0