[tor-commits] [tor/master] hs-v3: Refactor descriptor dir fetch done code
asn at torproject.org
asn at torproject.org
Mon Nov 18 17:12:15 UTC 2019
commit fbc18c8989bae0e2acb5d903f2bd5b0648bad828
Author: David Goulet <dgoulet at 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 at 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:
More information about the tor-commits
mailing list