commit 936c93e562deaba62f0d32f7e7fda770c5604318 Author: Taylor Yu <catalyst@torproject.org> Date: Sat Dec 15 22:14:46 2018 -0600 Hook up control_event_bootstrap() to btrack_orconn Replace a few invocations of control_event_bootstrap() with calls from the bootstrap tracker subsystem. This mostly leaves behavior unchanged. The actual behavior changes come in the next commit. Part of ticket 27167. --- src/core/include.am | 2 + src/core/or/circuitbuild.c | 2 - src/core/or/connection_or.c | 2 - src/feature/control/btrack_orconn.c | 13 +++- src/feature/control/btrack_orconn_cevent.c | 102 +++++++++++++++++++++++++++++ src/feature/control/btrack_orconn_cevent.h | 17 +++++ src/feature/control/control.h | 1 - src/feature/control/control_bootstrap.c | 11 ---- src/feature/nodelist/nodelist.c | 1 - src/test/test_controller_events.c | 4 +- 10 files changed, 133 insertions(+), 22 deletions(-) diff --git a/src/core/include.am b/src/core/include.am index 93a8bca5d..5e69cb9ad 100644 --- a/src/core/include.am +++ b/src/core/include.am @@ -66,6 +66,7 @@ LIBTOR_APP_A_SOURCES = \ src/feature/control/btrack.c \ src/feature/control/btrack_circuit.c \ src/feature/control/btrack_orconn.c \ + src/feature/control/btrack_orconn_cevent.c \ src/feature/control/btrack_orconn_maps.c \ src/feature/control/control.c \ src/feature/control/control_bootstrap.c \ @@ -280,6 +281,7 @@ noinst_HEADERS += \ src/feature/client/transports.h \ src/feature/control/btrack_circuit.h \ src/feature/control/btrack_orconn.h \ + src/feature/control/btrack_orconn_cevent.h \ src/feature/control/btrack_orconn_maps.h \ src/feature/control/btrack_sys.h \ src/feature/control/control.h \ diff --git a/src/core/or/circuitbuild.c b/src/core/or/circuitbuild.c index 3de8da9cd..b89ec09a9 100644 --- a/src/core/or/circuitbuild.c +++ b/src/core/or/circuitbuild.c @@ -583,8 +583,6 @@ circuit_handle_first_hop(origin_circuit_t *circ) circ->base_.n_hop = extend_info_dup(firsthop->extend_info); if (should_launch) { - if (circ->build_state->onehop_tunnel) - control_event_bootstrap(BOOTSTRAP_STATUS_CONN_DIR, 0); n_chan = channel_connect_for_circuit( &firsthop->extend_info->addr, firsthop->extend_info->port, diff --git a/src/core/or/connection_or.c b/src/core/or/connection_or.c index a12ea55c4..5d0874869 100644 --- a/src/core/or/connection_or.c +++ b/src/core/or/connection_or.c @@ -751,8 +751,6 @@ connection_or_finished_connecting(or_connection_t *or_conn) log_debug(LD_HANDSHAKE,"OR connect() to router at %s:%u finished.", conn->address,conn->port); - control_event_bootstrap(BOOTSTRAP_STATUS_HANDSHAKE, 0); - control_event_boot_first_orconn(); if (proxy_type != PROXY_NONE) { /* start proxy handshake */ diff --git a/src/feature/control/btrack_orconn.c b/src/feature/control/btrack_orconn.c index 28f8509fe..0fbf52100 100644 --- a/src/feature/control/btrack_orconn.c +++ b/src/feature/control/btrack_orconn.c @@ -27,6 +27,10 @@ * might be a one-hop circuit for directory lookups, or it might be a * connection for an application circuit because we already have * enough directory info to build an application circuit. + * + * We call functions in btrack_orconn_cevent.c to generate the actual + * controller events, because some of the state decoding we need to do + * is complicated. **/ #include <stdbool.h> @@ -38,6 +42,7 @@ #include "core/or/ocirc_event.h" #include "core/or/orconn_event.h" #include "feature/control/btrack_orconn.h" +#include "feature/control/btrack_orconn_cevent.h" #include "feature/control/btrack_orconn_maps.h" #include "lib/log/log.h" @@ -86,9 +91,10 @@ bto_update_bests(const bt_orconn_t *bto) { tor_assert(bto->is_orig); - bto_update_best(bto, &best_any, "ANY"); - if (!bto->is_onehop) - bto_update_best(bto, &best_ap, "AP"); + if (bto_update_best(bto, &best_any, "ANY")) + bto_cevent_anyconn(bto); + if (!bto->is_onehop && bto_update_best(bto, &best_ap, "AP")) + bto_cevent_apconn(bto); } /** Reset cached "best" values */ @@ -196,4 +202,5 @@ btrack_orconn_fini(void) { bto_clear_maps(); bto_reset_bests(); + bto_cevent_reset(); } diff --git a/src/feature/control/btrack_orconn_cevent.c b/src/feature/control/btrack_orconn_cevent.c new file mode 100644 index 000000000..fbbd34aca --- /dev/null +++ b/src/feature/control/btrack_orconn_cevent.c @@ -0,0 +1,102 @@ +/* Copyright (c) 2007-2018, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + +/** + * \file btrack_orconn_cevent.c + * \brief Emit bootstrap status events for OR connections + **/ + +#include <stdbool.h> + +#include "core/or/or.h" + +#define BTRACK_ORCONN_PRIVATE + +#include "core/or/orconn_event.h" +#include "feature/control/btrack_orconn.h" +#include "feature/control/btrack_orconn_cevent.h" +#include "feature/control/control.h" + +/** + * Have we completed our first OR connection? + * + * Block display of application circuit progress until we do, to avoid + * some misleading behavior of jumping to high progress. + **/ +static bool bto_first_orconn = false; + +/** + * Emit control events when we have updated our idea of the best state + * that any OR connection has reached. + **/ +void +bto_cevent_anyconn(const bt_orconn_t *bto) +{ + switch (bto->state) { + case OR_CONN_STATE_CONNECTING: + case OR_CONN_STATE_PROXY_HANDSHAKING: + /* XXX This isn't quite right, because this isn't necessarily a + directory server we're talking to, but we'll improve the + bootstrap tags and messages later */ + control_event_bootstrap(BOOTSTRAP_STATUS_CONN_DIR, 0); + break; + case OR_CONN_STATE_TLS_HANDSHAKING: + /* Here we should report a connection completed (TCP or proxied), + if we had the states */ + break; + case OR_CONN_STATE_TLS_CLIENT_RENEGOTIATING: + case OR_CONN_STATE_OR_HANDSHAKING_V2: + case OR_CONN_STATE_OR_HANDSHAKING_V3: + /* XXX Again, this isn't quite right, because it's not necessarily + a directory server we're talking to. */ + control_event_bootstrap(BOOTSTRAP_STATUS_HANDSHAKE_DIR, 0); + break; + case OR_CONN_STATE_OPEN: + /* Unblock directory progress display */ + control_event_boot_first_orconn(); + /* Unblock apconn progress display */ + bto_first_orconn = true; + break; + default: + break; + } +} + +/** + * Emit control events when we have updated our idea of the best state + * that any application circuit OR connection has reached. + **/ +void +bto_cevent_apconn(const bt_orconn_t *bto) +{ + if (!bto_first_orconn) + return; + + switch (bto->state) { + case OR_CONN_STATE_CONNECTING: + case OR_CONN_STATE_PROXY_HANDSHAKING: + control_event_bootstrap(BOOTSTRAP_STATUS_CONN_OR, 0); + break; + case OR_CONN_STATE_TLS_HANDSHAKING: + /* Here we should report a connection completed (TCP or proxied) */ + break; + case OR_CONN_STATE_TLS_CLIENT_RENEGOTIATING: + case OR_CONN_STATE_OR_HANDSHAKING_V2: + case OR_CONN_STATE_OR_HANDSHAKING_V3: + control_event_bootstrap(BOOTSTRAP_STATUS_HANDSHAKE_OR, 0); + break; + case OR_CONN_STATE_OPEN: + /* XXX Not quite right, because it implictly reports the next + state, but we'll improve it later. */ + control_event_bootstrap(BOOTSTRAP_STATUS_CIRCUIT_CREATE, 0); + default: + break; + } +} + +/** Forget that we completed our first OR connection */ +void +bto_cevent_reset(void) +{ + bto_first_orconn = false; +} diff --git a/src/feature/control/btrack_orconn_cevent.h b/src/feature/control/btrack_orconn_cevent.h new file mode 100644 index 000000000..165ff69cd --- /dev/null +++ b/src/feature/control/btrack_orconn_cevent.h @@ -0,0 +1,17 @@ +/* Copyright (c) 2007-2018, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + +/** + * \file btrack_orconn_cevent.h + * \brief Header file for btrack_orconn_cevent.c + **/ + +#ifndef TOR_BTRACK_ORCONN_CEVENT_H + +#include "feature/control/btrack_orconn.h" + +void bto_cevent_anyconn(const bt_orconn_t *); +void bto_cevent_apconn(const bt_orconn_t *); +void bto_cevent_reset(void); + +#endif /* defined(TOR_BTRACK_ORCONN_CEVENT_H) */ diff --git a/src/feature/control/control.h b/src/feature/control/control.h index 68c9a6bed..8180c4b60 100644 --- a/src/feature/control/control.h +++ b/src/feature/control/control.h @@ -52,7 +52,6 @@ typedef enum { BOOTSTRAP_STATUS_UNDEF=-1, BOOTSTRAP_STATUS_STARTING=0, BOOTSTRAP_STATUS_CONN_DIR=5, - BOOTSTRAP_STATUS_HANDSHAKE=-2, BOOTSTRAP_STATUS_HANDSHAKE_DIR=10, BOOTSTRAP_STATUS_ONEHOP_CREATE=15, BOOTSTRAP_STATUS_REQUESTING_STATUS=20, diff --git a/src/feature/control/control_bootstrap.c b/src/feature/control/control_bootstrap.c index 0756e208e..0478f0783 100644 --- a/src/feature/control/control_bootstrap.c +++ b/src/feature/control/control_bootstrap.c @@ -34,7 +34,6 @@ static const struct { { BOOTSTRAP_STATUS_UNDEF, "undef", "Undefined" }, { BOOTSTRAP_STATUS_STARTING, "starting", "Starting" }, { BOOTSTRAP_STATUS_CONN_DIR, "conn_dir", "Connecting to directory server" }, - { BOOTSTRAP_STATUS_HANDSHAKE, "status_handshake", "Finishing handshake" }, { BOOTSTRAP_STATUS_HANDSHAKE_DIR, "handshake_dir", "Finishing handshake with directory server" }, { BOOTSTRAP_STATUS_ONEHOP_CREATE, "onehop_create", @@ -151,16 +150,6 @@ control_event_bootstrap(bootstrap_status_t status, int progress) if (bootstrap_percent == BOOTSTRAP_STATUS_DONE) return; /* already bootstrapped; nothing to be done here. */ - /* special case for handshaking status, since our TLS handshaking code - * can't distinguish what the connection is going to be for. */ - if (status == BOOTSTRAP_STATUS_HANDSHAKE) { - if (bootstrap_percent < BOOTSTRAP_STATUS_CONN_OR) { - status = BOOTSTRAP_STATUS_HANDSHAKE_DIR; - } else { - status = BOOTSTRAP_STATUS_HANDSHAKE_OR; - } - } - if (status <= bootstrap_percent) { /* If there's no new progress, return early. */ if (!progress || progress <= bootstrap_percent) diff --git a/src/feature/nodelist/nodelist.c b/src/feature/nodelist/nodelist.c index f93ecd5bf..e1d5e4d3f 100644 --- a/src/feature/nodelist/nodelist.c +++ b/src/feature/nodelist/nodelist.c @@ -2633,7 +2633,6 @@ update_router_have_minimum_dir_info(void) /* If paths have just become available in this update. */ if (res && !have_min_dir_info) { control_event_client_status(LOG_NOTICE, "ENOUGH_DIR_INFO"); - control_event_boot_dir(BOOTSTRAP_STATUS_CONN_OR, 0); log_info(LD_DIR, "We now have enough directory information to build circuits."); } diff --git a/src/test/test_controller_events.c b/src/test/test_controller_events.c index 4c404876b..63013390e 100644 --- a/src/test/test_controller_events.c +++ b/src/test/test_controller_events.c @@ -353,7 +353,7 @@ test_cntev_dirboot_defer_desc(void *arg) assert_bootmsg("0 TAG=starting"); control_event_bootstrap(BOOTSTRAP_STATUS_CONN_DIR, 0); assert_bootmsg("5 TAG=conn_dir"); - control_event_bootstrap(BOOTSTRAP_STATUS_HANDSHAKE, 0); + control_event_bootstrap(BOOTSTRAP_STATUS_HANDSHAKE_DIR, 0); assert_bootmsg("10 TAG=handshake_dir"); /* The deferred event should appear */ control_event_boot_first_orconn(); @@ -378,7 +378,7 @@ test_cntev_dirboot_defer_orconn(void *arg) assert_bootmsg("0 TAG=starting"); control_event_bootstrap(BOOTSTRAP_STATUS_CONN_DIR, 0); assert_bootmsg("5 TAG=conn_dir"); - control_event_bootstrap(BOOTSTRAP_STATUS_HANDSHAKE, 0); + control_event_bootstrap(BOOTSTRAP_STATUS_HANDSHAKE_DIR, 0); assert_bootmsg("10 TAG=handshake_dir"); /* The deferred event should appear */ control_event_boot_first_orconn();