commit 85542ee5a0b37bf5b572b9beeda3cb8038ecd88e Author: Taylor Yu catalyst@torproject.org Date: Mon Dec 17 09:31:16 2018 -0600

The big bootstrap phase redefinition

Redefine the set of bootstrap phases to allow display of finer-grained progress in the early connection stages of connecting to a relay.

This includes adding intermediate phases for proxy and PT connections.

Also add a separate new phase to indicate obtaining enough directory info to build circuits so we can report that independently of actually initiating an ORCONN to build the first application circuit. Previously, we would claim to be connecting to a relay when we had merely finished obtaining directory info.

Part of ticket 27167. --- src/feature/control/btrack_orconn_cevent.c | 87 ++++++++++++++++++++++++------ src/feature/control/control.h | 41 +++++++++++--- src/feature/control/control_bootstrap.c | 48 ++++++++++++++--- src/feature/nodelist/nodelist.c | 3 +- src/test/test_controller_events.c | 20 +++---- 5 files changed, 159 insertions(+), 40 deletions(-)

diff --git a/src/feature/control/btrack_orconn_cevent.c b/src/feature/control/btrack_orconn_cevent.c index fbbd34aca..c7970dca4 100644 --- a/src/feature/control/btrack_orconn_cevent.c +++ b/src/feature/control/btrack_orconn_cevent.c @@ -4,6 +4,11 @@ /** * \file btrack_orconn_cevent.c * \brief Emit bootstrap status events for OR connections + * + * We do some decoding of the raw OR_CONN_STATE_* values. For + * example, OR_CONN_STATE_CONNECTING means the first TCP connect() + * completing, regardless of whether it's directly to a relay instead + * of a proxy or a PT. **/

#include <stdbool.h> @@ -25,33 +30,68 @@ **/ static bool bto_first_orconn = false;

+/** Is the ORCONN using a pluggable transport? */ +static bool +using_pt(const bt_orconn_t *bto) +{ + return bto->proxy_type == PROXY_PLUGGABLE; +} + +/** Is the ORCONN using a non-PT proxy? */ +static bool +using_proxy(const bt_orconn_t *bto) +{ + switch (bto->proxy_type) { + case PROXY_CONNECT: + case PROXY_SOCKS4: + case PROXY_SOCKS5: + return true; + default: + return false; + } +} + /** * Emit control events when we have updated our idea of the best state * that any OR connection has reached. + * + * Do some decoding of the ORCONN states depending on whether a PT or + * a proxy is in use. **/ void bto_cevent_anyconn(const bt_orconn_t *bto) { switch (bto->state) { case OR_CONN_STATE_CONNECTING: + /* Exactly what kind of thing we're connecting to isn't + * information we directly get from the states in connection_or.c, + * so decode it here. */ + if (using_pt(bto)) + control_event_bootstrap(BOOTSTRAP_STATUS_CONN_PT, 0); + else if (using_proxy(bto)) + control_event_bootstrap(BOOTSTRAP_STATUS_CONN_PROXY, 0); + else + control_event_bootstrap(BOOTSTRAP_STATUS_CONN, 0); + break; 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); + /* Similarly, starting a proxy handshake means the TCP connect() + * succeeded to the proxy. Let's be specific about what kind of + * proxy. */ + if (using_pt(bto)) + control_event_bootstrap(BOOTSTRAP_STATUS_CONN_DONE_PT, 0); + else if (using_proxy(bto)) + control_event_bootstrap(BOOTSTRAP_STATUS_CONN_DONE_PROXY, 0); break; case OR_CONN_STATE_TLS_HANDSHAKING: - /* Here we should report a connection completed (TCP or proxied), - if we had the states */ + control_event_bootstrap(BOOTSTRAP_STATUS_CONN_DONE, 0); 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); + control_event_bootstrap(BOOTSTRAP_STATUS_HANDSHAKE, 0); break; case OR_CONN_STATE_OPEN: + control_event_bootstrap(BOOTSTRAP_STATUS_HANDSHAKE_DONE, 0); /* Unblock directory progress display */ control_event_boot_first_orconn(); /* Unblock apconn progress display */ @@ -65,6 +105,9 @@ bto_cevent_anyconn(const bt_orconn_t *bto) /** * Emit control events when we have updated our idea of the best state * that any application circuit OR connection has reached. + * + * Do some decoding of the ORCONN states depending on whether a PT or + * a proxy is in use. **/ void bto_cevent_apconn(const bt_orconn_t *bto) @@ -74,21 +117,35 @@ bto_cevent_apconn(const bt_orconn_t *bto)

switch (bto->state) { case OR_CONN_STATE_CONNECTING: + /* Exactly what kind of thing we're connecting to isn't + * information we directly get from the states in connection_or.c, + * so decode it here. */ + if (using_pt(bto)) + control_event_bootstrap(BOOTSTRAP_STATUS_AP_CONN_PT, 0); + else if (using_proxy(bto)) + control_event_bootstrap(BOOTSTRAP_STATUS_AP_CONN_PROXY, 0); + else + control_event_bootstrap(BOOTSTRAP_STATUS_AP_CONN, 0); + break; case OR_CONN_STATE_PROXY_HANDSHAKING: - control_event_bootstrap(BOOTSTRAP_STATUS_CONN_OR, 0); + /* Similarly, starting a proxy handshake means the TCP connect() + * succeeded to the proxy. Let's be specific about what kind of + * proxy. */ + if (using_pt(bto)) + control_event_bootstrap(BOOTSTRAP_STATUS_AP_CONN_DONE_PT, 0); + else if (using_proxy(bto)) + control_event_bootstrap(BOOTSTRAP_STATUS_AP_CONN_DONE_PROXY, 0); break; case OR_CONN_STATE_TLS_HANDSHAKING: - /* Here we should report a connection completed (TCP or proxied) */ + control_event_bootstrap(BOOTSTRAP_STATUS_AP_CONN_DONE, 0); 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); + control_event_bootstrap(BOOTSTRAP_STATUS_AP_HANDSHAKE, 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); + control_event_bootstrap(BOOTSTRAP_STATUS_AP_HANDSHAKE_DONE, 0); default: break; } diff --git a/src/feature/control/control.h b/src/feature/control/control.h index 8180c4b60..08d8924e2 100644 --- a/src/feature/control/control.h +++ b/src/feature/control/control.h @@ -51,17 +51,42 @@ typedef enum buildtimeout_set_event_t { typedef enum { BOOTSTRAP_STATUS_UNDEF=-1, BOOTSTRAP_STATUS_STARTING=0, - BOOTSTRAP_STATUS_CONN_DIR=5, - BOOTSTRAP_STATUS_HANDSHAKE_DIR=10, - BOOTSTRAP_STATUS_ONEHOP_CREATE=15, - BOOTSTRAP_STATUS_REQUESTING_STATUS=20, - BOOTSTRAP_STATUS_LOADING_STATUS=25, + + /* Initial connection to any relay */ + + BOOTSTRAP_STATUS_CONN_PT=1, + BOOTSTRAP_STATUS_CONN_DONE_PT=2, + BOOTSTRAP_STATUS_CONN_PROXY=3, + BOOTSTRAP_STATUS_CONN_DONE_PROXY=4, + BOOTSTRAP_STATUS_CONN=5, + BOOTSTRAP_STATUS_CONN_DONE=10, + BOOTSTRAP_STATUS_HANDSHAKE=14, + BOOTSTRAP_STATUS_HANDSHAKE_DONE=15, + + /* Loading directory info */ + + BOOTSTRAP_STATUS_ONEHOP_CREATE=20, + BOOTSTRAP_STATUS_REQUESTING_STATUS=25, + BOOTSTRAP_STATUS_LOADING_STATUS=30, BOOTSTRAP_STATUS_LOADING_KEYS=40, BOOTSTRAP_STATUS_REQUESTING_DESCRIPTORS=45, BOOTSTRAP_STATUS_LOADING_DESCRIPTORS=50, - BOOTSTRAP_STATUS_CONN_OR=80, - BOOTSTRAP_STATUS_HANDSHAKE_OR=85, - BOOTSTRAP_STATUS_CIRCUIT_CREATE=90, + BOOTSTRAP_STATUS_ENOUGH_DIRINFO=75, + + /* Connecting to a relay for AP circuits */ + + BOOTSTRAP_STATUS_AP_CONN_PT=76, + BOOTSTRAP_STATUS_AP_CONN_DONE_PT=77, + BOOTSTRAP_STATUS_AP_CONN_PROXY=78, + BOOTSTRAP_STATUS_AP_CONN_DONE_PROXY=79, + BOOTSTRAP_STATUS_AP_CONN=80, + BOOTSTRAP_STATUS_AP_CONN_DONE=85, + BOOTSTRAP_STATUS_AP_HANDSHAKE=89, + BOOTSTRAP_STATUS_AP_HANDSHAKE_DONE=90, + + /* Creating AP circuits */ + + BOOTSTRAP_STATUS_CIRCUIT_CREATE=95, BOOTSTRAP_STATUS_DONE=100 } bootstrap_status_t;

diff --git a/src/feature/control/control_bootstrap.c b/src/feature/control/control_bootstrap.c index 0478f0783..49e190dc5 100644 --- a/src/feature/control/control_bootstrap.c +++ b/src/feature/control/control_bootstrap.c @@ -33,9 +33,24 @@ static const struct { } boot_to_str_tab[] = { { BOOTSTRAP_STATUS_UNDEF, "undef", "Undefined" }, { BOOTSTRAP_STATUS_STARTING, "starting", "Starting" }, - { BOOTSTRAP_STATUS_CONN_DIR, "conn_dir", "Connecting to directory server" }, - { BOOTSTRAP_STATUS_HANDSHAKE_DIR, "handshake_dir", - "Finishing handshake with directory server" }, + + /* Initial connection to any relay */ + + { BOOTSTRAP_STATUS_CONN_PT, "conn_pt", "Connecting to pluggable transport" }, + { BOOTSTRAP_STATUS_CONN_DONE_PT, "conn_done_pt", + "Connected to pluggable transport" }, + { BOOTSTRAP_STATUS_CONN_PROXY, "conn_proxy", "Connecting to proxy" }, + { BOOTSTRAP_STATUS_CONN_DONE_PROXY, "conn_done_proxy", + "Connected to proxy" }, + { BOOTSTRAP_STATUS_CONN, "conn", "Connecting to a relay" }, + { BOOTSTRAP_STATUS_CONN_DONE, "conn_done", "Connected to a relay" }, + { BOOTSTRAP_STATUS_HANDSHAKE, "handshake", + "Handshaking with a relay" }, + { BOOTSTRAP_STATUS_HANDSHAKE_DONE, "handshake_done", + "Handshake with a relay done" }, + + /* Loading directory info */ + { BOOTSTRAP_STATUS_ONEHOP_CREATE, "onehop_create", "Establishing an encrypted directory connection" }, { BOOTSTRAP_STATUS_REQUESTING_STATUS, "requesting_status", @@ -48,9 +63,30 @@ static const struct { "Asking for relay descriptors" }, { BOOTSTRAP_STATUS_LOADING_DESCRIPTORS, "loading_descriptors", "Loading relay descriptors" }, - { BOOTSTRAP_STATUS_CONN_OR, "conn_or", "Connecting to the Tor network" }, - { BOOTSTRAP_STATUS_HANDSHAKE_OR, "handshake_or", - "Finishing handshake with first hop" }, + { BOOTSTRAP_STATUS_ENOUGH_DIRINFO, "enough_dirinfo", + "Loaded enough directory info to build circuits" }, + + /* Connecting to a relay for AP circuits */ + + { BOOTSTRAP_STATUS_AP_CONN_PT, "ap_conn_pt", + "Connecting to pluggable transport to build circuits" }, + { BOOTSTRAP_STATUS_AP_CONN_DONE_PT, "ap_conn_done_pt", + "Connected to pluggable transport to build circuits" }, + { BOOTSTRAP_STATUS_AP_CONN_PROXY, "ap_conn_proxy", + "Connecting to proxy " }, + { BOOTSTRAP_STATUS_AP_CONN_DONE_PROXY, "ap_conn_done_proxy", + "Connected to proxy to build circuits" }, + { BOOTSTRAP_STATUS_AP_CONN, "ap_conn", + "Connecting to a relay to build circuits" }, + { BOOTSTRAP_STATUS_AP_CONN_DONE, "ap_conn_done", + "Connected to a relay to build circuits" }, + { BOOTSTRAP_STATUS_AP_HANDSHAKE, "ap_handshake", + "Finishing handshake with a relay to build circuits" }, + { BOOTSTRAP_STATUS_AP_HANDSHAKE_DONE, "ap_handshake_done", + "Handshake fininshed with a relay to build circuits" }, + + /* Creating AP circuits */ + { BOOTSTRAP_STATUS_CIRCUIT_CREATE, "circuit_create", "Establishing a Tor circuit" }, { BOOTSTRAP_STATUS_DONE, "done", "Done" }, diff --git a/src/feature/nodelist/nodelist.c b/src/feature/nodelist/nodelist.c index e1d5e4d3f..d94e73f48 100644 --- a/src/feature/nodelist/nodelist.c +++ b/src/feature/nodelist/nodelist.c @@ -2546,7 +2546,7 @@ count_loading_descriptors_progress(void) if (fraction > 1.0) return 0; /* it's not the number of descriptors holding us back */ return BOOTSTRAP_STATUS_LOADING_DESCRIPTORS + (int) - (fraction*(BOOTSTRAP_STATUS_CONN_OR-1 - + (fraction*(BOOTSTRAP_STATUS_ENOUGH_DIRINFO-1 - BOOTSTRAP_STATUS_LOADING_DESCRIPTORS)); }

@@ -2633,6 +2633,7 @@ 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_ENOUGH_DIRINFO, 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 63013390e..dc2bb77e9 100644 --- a/src/test/test_controller_events.c +++ b/src/test/test_controller_events.c @@ -351,10 +351,10 @@ test_cntev_dirboot_defer_desc(void *arg) /* This event should get deferred */ control_event_boot_dir(BOOTSTRAP_STATUS_REQUESTING_DESCRIPTORS, 0); 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_DIR, 0); - assert_bootmsg("10 TAG=handshake_dir"); + control_event_bootstrap(BOOTSTRAP_STATUS_CONN, 0); + assert_bootmsg("5 TAG=conn"); + control_event_bootstrap(BOOTSTRAP_STATUS_HANDSHAKE, 0); + assert_bootmsg("14 TAG=handshake"); /* The deferred event should appear */ control_event_boot_first_orconn(); assert_bootmsg("45 TAG=requesting_descriptors"); @@ -374,15 +374,15 @@ test_cntev_dirboot_defer_orconn(void *arg) control_event_bootstrap(BOOTSTRAP_STATUS_STARTING, 0); assert_bootmsg("0 TAG=starting"); /* This event should get deferred */ - control_event_boot_dir(BOOTSTRAP_STATUS_CONN_OR, 0); + control_event_boot_dir(BOOTSTRAP_STATUS_ENOUGH_DIRINFO, 0); 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_DIR, 0); - assert_bootmsg("10 TAG=handshake_dir"); + control_event_bootstrap(BOOTSTRAP_STATUS_CONN, 0); + assert_bootmsg("5 TAG=conn"); + control_event_bootstrap(BOOTSTRAP_STATUS_HANDSHAKE, 0); + assert_bootmsg("14 TAG=handshake"); /* The deferred event should appear */ control_event_boot_first_orconn(); - assert_bootmsg("80 TAG=conn_or"); + assert_bootmsg("75 TAG=enough_dirinfo"); done: tor_free(saved_event_str); UNMOCK(queue_control_event_string);