diff options
| -rw-r--r-- | changes/ticket40579 | 3 | ||||
| -rw-r--r-- | changes/ticket40581 | 4 | ||||
| -rw-r--r-- | src/feature/hs/hs_client.c | 48 | ||||
| -rw-r--r-- | src/feature/hs/hs_metrics.c | 23 |
4 files changed, 64 insertions, 14 deletions
diff --git a/changes/ticket40579 b/changes/ticket40579 new file mode 100644 index 0000000000..e2558c1102 --- /dev/null +++ b/changes/ticket40579 @@ -0,0 +1,3 @@ + o Minor bugfixes (onion service, client): + - Fix a fatal assert due to a guard subsystem recursion triggered by the + onion service client. Fixes bug 40579; bugfix on 0.3.5.1-alpha. diff --git a/changes/ticket40581 b/changes/ticket40581 new file mode 100644 index 0000000000..315215d8ed --- /dev/null +++ b/changes/ticket40581 @@ -0,0 +1,4 @@ + o Minor bugfixes (metrics port, onion service): + - Fix the metrics with a port label to be unique. Before this, all ports of + an onion service would be on the same line which violates the Prometheus + rules of unique labels. Fixes bug 40581; bugfix on 0.4.5.1-alpha. diff --git a/src/feature/hs/hs_client.c b/src/feature/hs/hs_client.c index 4b4e268542..6c9645f0b8 100644 --- a/src/feature/hs/hs_client.c +++ b/src/feature/hs/hs_client.c @@ -38,6 +38,7 @@ #include "lib/crypt_ops/crypto_format.h" #include "lib/crypt_ops/crypto_rand.h" #include "lib/crypt_ops/crypto_util.h" +#include "lib/evloop/compat_libevent.h" #include "core/or/cpath_build_state_st.h" #include "feature/dircommon/dir_connection_st.h" @@ -46,11 +47,30 @@ #include "core/or/origin_circuit_st.h" #include "core/or/socks_request_st.h" +#include "trunnel/hs/cell_introduce1.h" + +/** This event is activated when we are notified that directory information has + * changed. It must be done asynchronous from the call due to possible + * recursion from the caller of that notification. See #40579. */ +static struct mainloop_event_t *dir_info_changed_ev = NULL; + /** Client-side authorizations for hidden services; map of service identity * public key to hs_client_service_authorization_t *. */ static digest256map_t *client_auths = NULL; -#include "trunnel/hs/cell_introduce1.h" +/** Mainloop callback. Scheduled to run when we are notified of a directory + * info change. See hs_client_dir_info_changed(). */ +static void +dir_info_changed_callback(mainloop_event_t *event, void *arg) +{ + (void) event; + (void) arg; + + /* We have possibly reached the minimum directory information or new + * consensus so retry all pending SOCKS connection in + * AP_CONN_STATE_RENDDESC_WAIT state in order to fetch the descriptor. */ + retry_all_socks_conn_waiting_for_desc(); +} /** Return a human-readable string for the client fetch status code. */ static const char * @@ -2584,6 +2604,9 @@ hs_client_free_all(void) /* Purge the hidden service request cache. */ hs_purge_last_hid_serv_requests(); client_service_authorization_free_all(); + + /* This is NULL safe. */ + mainloop_event_free(dir_info_changed_ev); } /** Purge all potentially remotely-detectable state held in the hidden @@ -2609,14 +2632,27 @@ hs_client_purge_state(void) log_info(LD_REND, "Hidden service client state has been purged."); } -/** Called when our directory information has changed. */ +/** Called when our directory information has changed. + * + * The work done in that function has to either be kept within the HS subsystem + * or else scheduled as a mainloop event. In other words, this function can't + * call outside to another subsystem to avoid risking recursion problems. */ void hs_client_dir_info_changed(void) { - /* We have possibly reached the minimum directory information or new - * consensus so retry all pending SOCKS connection in - * AP_CONN_STATE_RENDDESC_WAIT state in order to fetch the descriptor. */ - retry_all_socks_conn_waiting_for_desc(); + /* Make sure the mainloop has been initialized. Code path exist that reaches + * this before it is. */ + if (!tor_libevent_is_initialized()) { + return; + } + + /* Lazily create the event. HS Client subsystem doesn't have an init function + * and so we do it here before activating it. */ + if (!dir_info_changed_ev) { + dir_info_changed_ev = mainloop_event_new(dir_info_changed_callback, NULL); + } + /* Activate it to run immediately. */ + mainloop_event_activate(dir_info_changed_ev); } #ifdef TOR_UNIT_TESTS diff --git a/src/feature/hs/hs_metrics.c b/src/feature/hs/hs_metrics.c index 452bb44419..25e2e62111 100644 --- a/src/feature/hs/hs_metrics.c +++ b/src/feature/hs/hs_metrics.c @@ -59,19 +59,26 @@ init_store(hs_service_t *service) store = service->metrics.store; for (size_t i = 0; i < base_metrics_size; ++i) { - metrics_store_entry_t *entry = - metrics_store_add(store, base_metrics[i].type, base_metrics[i].name, - base_metrics[i].help); - - /* Add labels to the entry. */ - metrics_store_entry_add_label(entry, - format_label("onion", service->onion_address)); + /* Add entries with port as label. We need one metric line per port. */ if (base_metrics[i].port_as_label && service->config.ports) { SMARTLIST_FOREACH_BEGIN(service->config.ports, const rend_service_port_config_t *, p) { + metrics_store_entry_t *entry = + metrics_store_add(store, base_metrics[i].type, base_metrics[i].name, + base_metrics[i].help); + + /* Add labels to the entry. */ + metrics_store_entry_add_label(entry, + format_label("onion", service->onion_address)); metrics_store_entry_add_label(entry, - format_label("port", port_to_str(p->virtual_port))); + format_label("port", port_to_str(p->virtual_port))); } SMARTLIST_FOREACH_END(p); + } else { + metrics_store_entry_t *entry = + metrics_store_add(store, base_metrics[i].type, base_metrics[i].name, + base_metrics[i].help); + metrics_store_entry_add_label(entry, + format_label("onion", service->onion_address)); } } } |