From 308dde0c386158971a66c6e035b3f1ad67019eb5 Mon Sep 17 00:00:00 2001 From: Taylor Yu Date: Thu, 13 Dec 2018 17:18:49 -0600 Subject: [PATCH 01/10] Remove unused old_state var in connection_or.c connection_or_change_state() saved an old_state to pass to channel_tls_handle_state_change_on_orconn(), which promptly cast it to void. Remove this unused variable and parameter. --- src/core/or/channeltls.c | 3 --- src/core/or/channeltls.h | 1 - src/core/or/connection_or.c | 6 +----- 3 files changed, 1 insertion(+), 9 deletions(-) diff --git a/src/core/or/channeltls.c b/src/core/or/channeltls.c index 8f407d5e153..cf33feec015 100644 --- a/src/core/or/channeltls.c +++ b/src/core/or/channeltls.c @@ -950,7 +950,6 @@ channel_tls_listener_describe_transport_method(channel_listener_t *chan_l) void channel_tls_handle_state_change_on_orconn(channel_tls_t *chan, or_connection_t *conn, - uint8_t old_state, uint8_t state) { channel_t *base_chan; @@ -959,8 +958,6 @@ channel_tls_handle_state_change_on_orconn(channel_tls_t *chan, tor_assert(conn); tor_assert(conn->chan == chan); tor_assert(chan->conn == conn); - /* Shut the compiler up without triggering -Wtautological-compare */ - (void)old_state; base_chan = TLS_CHAN_TO_BASE(chan); diff --git a/src/core/or/channeltls.h b/src/core/or/channeltls.h index 12715450b92..2ec7fe54532 100644 --- a/src/core/or/channeltls.h +++ b/src/core/or/channeltls.h @@ -49,7 +49,6 @@ channel_tls_t * channel_tls_from_base(channel_t *chan); void channel_tls_handle_cell(cell_t *cell, or_connection_t *conn); void channel_tls_handle_state_change_on_orconn(channel_tls_t *chan, or_connection_t *conn, - uint8_t old_state, uint8_t state); void channel_tls_handle_var_cell(var_cell_t *var_cell, or_connection_t *conn); diff --git a/src/core/or/connection_or.c b/src/core/or/connection_or.c index 962ec51c366..04ab4fe4a95 100644 --- a/src/core/or/connection_or.c +++ b/src/core/or/connection_or.c @@ -408,16 +408,12 @@ connection_or_report_broken_states(int severity, int domain) static void connection_or_change_state(or_connection_t *conn, uint8_t state) { - uint8_t old_state; - tor_assert(conn); - old_state = conn->base_.state; conn->base_.state = state; if (conn->chan) - channel_tls_handle_state_change_on_orconn(conn->chan, conn, - old_state, state); + channel_tls_handle_state_change_on_orconn(conn->chan, conn, state); } /** Return the number of circuits using an or_connection_t; this used to From 271b50f54abac7af44e3e54589ff965d3cdac816 Mon Sep 17 00:00:00 2001 From: Taylor Yu Date: Sun, 16 Dec 2018 16:05:58 -0600 Subject: [PATCH 02/10] Add ORCONN event pubsub system Add a publish-subscribe subsystem to publish messages about changes to OR connections. connection_or_change_state() in connection_or.c and control_event_or_conn_event() in control.c publish messages to this subsystem via helper functions. Move state constants from connection_or.h to orconn_state.h so that subscribers don't have to include all of connection_or.h to take actions based on changes in OR connection state. Move event constants from control.h for similar reasons. Part of ticket 27167. --- src/app/main/subsystem_list.c | 2 + src/core/include.am | 3 + src/core/mainloop/connection.c | 2 +- src/core/or/connection_or.c | 73 +++++++++++++++++--- src/core/or/connection_or.h | 30 ++------- src/core/or/orconn_event.c | 81 ++++++++++++++++++++++ src/core/or/orconn_event.h | 120 +++++++++++++++++++++++++++++++++ src/core/or/orconn_event_sys.h | 12 ++++ src/feature/control/control.h | 12 +--- src/feature/relay/ext_orport.c | 2 +- 10 files changed, 288 insertions(+), 49 deletions(-) create mode 100644 src/core/or/orconn_event.c create mode 100644 src/core/or/orconn_event.h create mode 100644 src/core/or/orconn_event_sys.h diff --git a/src/app/main/subsystem_list.c b/src/app/main/subsystem_list.c index 8640329e92d..18587380968 100644 --- a/src/app/main/subsystem_list.c +++ b/src/app/main/subsystem_list.c @@ -8,6 +8,7 @@ #include "lib/cc/compat_compiler.h" #include "lib/cc/torint.h" +#include "core/or/orconn_event_sys.h" #include "lib/compress/compress_sys.h" #include "lib/crypt_ops/crypto_sys.h" #include "lib/err/torerr_sys.h" @@ -35,6 +36,7 @@ const subsys_fns_t *tor_subsystems[] = { &sys_compress, /* -70 */ &sys_crypto, /* -60 */ &sys_tortls, /* -50 */ + &sys_orconn_event, /* -40 */ }; const unsigned n_tor_subsystems = ARRAY_LENGTH(tor_subsystems); diff --git a/src/core/include.am b/src/core/include.am index 3cd6e83ed54..a1fae73609d 100644 --- a/src/core/include.am +++ b/src/core/include.am @@ -39,6 +39,7 @@ LIBTOR_APP_A_SOURCES = \ src/core/or/connection_or.c \ src/core/or/dos.c \ src/core/or/onion.c \ + src/core/or/orconn_event.c \ src/core/or/policies.c \ src/core/or/protover.c \ src/core/or/protover_rust.c \ @@ -238,6 +239,8 @@ noinst_HEADERS += \ src/core/or/listener_connection_st.h \ src/core/or/onion.h \ src/core/or/or.h \ + src/core/or/orconn_event.h \ + src/core/or/orconn_event_sys.h \ src/core/or/or_circuit_st.h \ src/core/or/or_connection_st.h \ src/core/or/or_handshake_certs_st.h \ diff --git a/src/core/mainloop/connection.c b/src/core/mainloop/connection.c index 601c872c98e..6cc61227026 100644 --- a/src/core/mainloop/connection.c +++ b/src/core/mainloop/connection.c @@ -1875,7 +1875,7 @@ connection_init_accepted_conn(connection_t *conn, /* Initiate Extended ORPort authentication. */ return connection_ext_or_start_auth(TO_OR_CONN(conn)); case CONN_TYPE_OR: - control_event_or_conn_status(TO_OR_CONN(conn), OR_CONN_EVENT_NEW, 0); + connection_or_event_status(TO_OR_CONN(conn), OR_CONN_EVENT_NEW, 0); rv = connection_tls_start_handshake(TO_OR_CONN(conn), 1); if (rv < 0) { connection_or_close_for_error(TO_OR_CONN(conn), 0); diff --git a/src/core/or/connection_or.c b/src/core/or/connection_or.c index 04ab4fe4a95..a12ea55c467 100644 --- a/src/core/or/connection_or.c +++ b/src/core/or/connection_or.c @@ -29,6 +29,7 @@ */ #define TOR_CHANNEL_INTERNAL_ #define CONNECTION_OR_PRIVATE +#define ORCONN_EVENT_PRIVATE #include "core/or/channel.h" #include "core/or/channeltls.h" #include "core/or/circuitbuild.h" @@ -79,6 +80,8 @@ #include "lib/tls/tortls.h" #include "lib/tls/x509.h" +#include "core/or/orconn_event.h" + static int connection_tls_finish_handshake(or_connection_t *conn); static int connection_or_launch_v3_or_handshake(or_connection_t *conn); static int connection_or_process_cells_from_inbuf(or_connection_t *conn); @@ -401,6 +404,49 @@ connection_or_report_broken_states(int severity, int domain) smartlist_free(items); } +/** + * Helper function to publish an OR connection status event + * + * Publishes a messages to subscribers of ORCONN messages, and sends + * the control event. + **/ +void +connection_or_event_status(or_connection_t *conn, or_conn_status_event_t tp, + int reason) +{ + orconn_event_msg_t msg; + + msg.type = ORCONN_MSGTYPE_STATUS; + msg.u.status.gid = conn->base_.global_identifier; + msg.u.status.status = tp; + msg.u.status.reason = reason; + orconn_event_publish(&msg); + control_event_or_conn_status(conn, tp, reason); +} + +/** + * Helper function to publish a state change message + * + * connection_or_change_state() calls this to notify subscribers about + * a change of an OR connection state. + **/ +static void +connection_or_state_publish(const or_connection_t *conn, uint8_t state) +{ + orconn_event_msg_t msg; + + msg.type = ORCONN_MSGTYPE_STATE; + msg.u.state.gid = conn->base_.global_identifier; + msg.u.state.proxy_type = conn->proxy_type; + msg.u.state.state = state; + if (conn->chan) { + msg.u.state.chan = TLS_CHAN_TO_BASE(conn->chan)->global_identifier; + } else { + msg.u.state.chan = 0; + } + orconn_event_publish(&msg); +} + /** Call this to change or_connection_t states, so the owning channel_tls_t can * be notified. */ @@ -412,6 +458,7 @@ connection_or_change_state(or_connection_t *conn, uint8_t state) conn->base_.state = state; + connection_or_state_publish(conn, state); if (conn->chan) channel_tls_handle_state_change_on_orconn(conn->chan, conn, state); } @@ -755,8 +802,8 @@ connection_or_about_to_close(or_connection_t *or_conn) entry_guard_chan_failed(TLS_CHAN_TO_BASE(or_conn->chan)); if (conn->state >= OR_CONN_STATE_TLS_HANDSHAKING) { int reason = tls_error_to_orconn_end_reason(or_conn->tls_error); - control_event_or_conn_status(or_conn, OR_CONN_EVENT_FAILED, - reason); + connection_or_event_status(or_conn, OR_CONN_EVENT_FAILED, + reason); if (!authdir_mode_tests_reachability(options)) control_event_bootstrap_prob_or( orconn_end_reason_to_control_string(reason), @@ -766,10 +813,10 @@ connection_or_about_to_close(or_connection_t *or_conn) } else if (conn->hold_open_until_flushed) { /* We only set hold_open_until_flushed when we're intentionally * closing a connection. */ - control_event_or_conn_status(or_conn, OR_CONN_EVENT_CLOSED, + connection_or_event_status(or_conn, OR_CONN_EVENT_CLOSED, tls_error_to_orconn_end_reason(or_conn->tls_error)); } else if (!tor_digest_is_zero(or_conn->identity_digest)) { - control_event_or_conn_status(or_conn, OR_CONN_EVENT_CLOSED, + connection_or_event_status(or_conn, OR_CONN_EVENT_CLOSED, tls_error_to_orconn_end_reason(or_conn->tls_error)); } } @@ -1361,7 +1408,7 @@ void connection_or_connect_failed(or_connection_t *conn, int reason, const char *msg) { - control_event_or_conn_status(conn, OR_CONN_EVENT_FAILED, reason); + connection_or_event_status(conn, OR_CONN_EVENT_FAILED, reason); if (!authdir_mode_tests_reachability(get_options())) control_event_bootstrap_prob_or(msg, reason, conn); note_or_connect_failed(conn); @@ -1468,9 +1515,6 @@ connection_or_connect, (const tor_addr_t *_addr, uint16_t port, return NULL; } - connection_or_change_state(conn, OR_CONN_STATE_CONNECTING); - control_event_or_conn_status(conn, OR_CONN_EVENT_LAUNCHED, 0); - conn->is_outgoing = 1; /* If we are using a proxy server, find it and use it. */ @@ -1482,7 +1526,14 @@ connection_or_connect, (const tor_addr_t *_addr, uint16_t port, port = proxy_port; conn->base_.proxy_state = PROXY_INFANT; } + connection_or_change_state(conn, OR_CONN_STATE_CONNECTING); + connection_or_event_status(conn, OR_CONN_EVENT_LAUNCHED, 0); } else { + /* This duplication of state change calls is necessary in case we + * run into an error condition below */ + connection_or_change_state(conn, OR_CONN_STATE_CONNECTING); + connection_or_event_status(conn, OR_CONN_EVENT_LAUNCHED, 0); + /* get_proxy_addrport() might fail if we have a Bridge line that references a transport, but no ClientTransportPlugin lines defining its transport proxy. If this is the case, let's try to @@ -1978,8 +2029,8 @@ connection_or_client_learned_peer_id(or_connection_t *conn, /* Tell the new guard API about the channel failure */ entry_guard_chan_failed(TLS_CHAN_TO_BASE(conn->chan)); - control_event_or_conn_status(conn, OR_CONN_EVENT_FAILED, - END_OR_CONN_REASON_OR_IDENTITY); + connection_or_event_status(conn, OR_CONN_EVENT_FAILED, + END_OR_CONN_REASON_OR_IDENTITY); if (!authdir_mode_tests_reachability(options)) control_event_bootstrap_prob_or( "Unexpected identity in router certificate", @@ -2219,7 +2270,7 @@ int connection_or_set_state_open(or_connection_t *conn) { connection_or_change_state(conn, OR_CONN_STATE_OPEN); - control_event_or_conn_status(conn, OR_CONN_EVENT_CONNECTED, 0); + connection_or_event_status(conn, OR_CONN_EVENT_CONNECTED, 0); /* Link protocol 3 appeared in Tor 0.2.3.6-alpha, so any connection * that uses an earlier link protocol should not be treated as a relay. */ diff --git a/src/core/or/connection_or.h b/src/core/or/connection_or.h index d4bcdd93e50..5f4856d51f7 100644 --- a/src/core/or/connection_or.h +++ b/src/core/or/connection_or.h @@ -17,32 +17,7 @@ struct ed25519_keypair_t; or_connection_t *TO_OR_CONN(connection_t *); -#define OR_CONN_STATE_MIN_ 1 -/** State for a connection to an OR: waiting for connect() to finish. */ -#define OR_CONN_STATE_CONNECTING 1 -/** State for a connection to an OR: waiting for proxy handshake to complete */ -#define OR_CONN_STATE_PROXY_HANDSHAKING 2 -/** State for an OR connection client: SSL is handshaking, not done - * yet. */ -#define OR_CONN_STATE_TLS_HANDSHAKING 3 -/** State for a connection to an OR: We're doing a second SSL handshake for - * renegotiation purposes. (V2 handshake only.) */ -#define OR_CONN_STATE_TLS_CLIENT_RENEGOTIATING 4 -/** State for a connection at an OR: We're waiting for the client to - * renegotiate (to indicate a v2 handshake) or send a versions cell (to - * indicate a v3 handshake) */ -#define OR_CONN_STATE_TLS_SERVER_RENEGOTIATING 5 -/** State for an OR connection: We're done with our SSL handshake, we've done - * renegotiation, but we haven't yet negotiated link protocol versions and - * sent a netinfo cell. */ -#define OR_CONN_STATE_OR_HANDSHAKING_V2 6 -/** State for an OR connection: We're done with our SSL handshake, but we - * haven't yet negotiated link protocol versions, done a V3 handshake, and - * sent a netinfo cell. */ -#define OR_CONN_STATE_OR_HANDSHAKING_V3 7 -/** State for an OR connection: Ready to send/receive cells. */ -#define OR_CONN_STATE_OPEN 8 -#define OR_CONN_STATE_MAX_ 8 +#include "core/or/orconn_event.h" void connection_or_clear_identity(or_connection_t *conn); void connection_or_clear_identity_map(void); @@ -81,6 +56,9 @@ MOCK_DECL(void,connection_or_close_for_error, void connection_or_report_broken_states(int severity, int domain); +void connection_or_event_status(or_connection_t *conn, + or_conn_status_event_t tp, int reason); + MOCK_DECL(int,connection_tls_start_handshake,(or_connection_t *conn, int receiving)); int connection_tls_continue_handshake(or_connection_t *conn); diff --git a/src/core/or/orconn_event.c b/src/core/or/orconn_event.c new file mode 100644 index 00000000000..11f5ed96620 --- /dev/null +++ b/src/core/or/orconn_event.c @@ -0,0 +1,81 @@ +/* Copyright (c) 2007-2018, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + +/** + * \file orconn_event.c + * \brief Publish state change messages for OR connections + * + * Implements a basic publish-subscribe framework for messages about + * the state of OR connections. The publisher calls the subscriber + * callback functions synchronously. + * + * Although the synchronous calls might not simplify the call graph, + * this approach improves data isolation because the publisher doesn't + * need knowledge about the internals of subscribing subsystems. It + * also avoids race conditions that might occur in asynchronous + * frameworks. + **/ + +#include "core/or/or.h" +#include "lib/subsys/subsys.h" + +#define ORCONN_EVENT_PRIVATE +#include "core/or/orconn_event.h" +#include "core/or/orconn_event_sys.h" + +/** List of subscribers */ +static smartlist_t *orconn_event_rcvrs; + +/** Initialize subscriber list */ +static int +orconn_event_init(void) +{ + orconn_event_rcvrs = smartlist_new(); + return 0; +} + +/** Free subscriber list */ +static void +orconn_event_fini(void) +{ + smartlist_free(orconn_event_rcvrs); +} + +/** + * Subscribe to messages about OR connection events + * + * Register a callback function to receive messages about ORCONNs. + * The publisher calls this function synchronously. + **/ +void +orconn_event_subscribe(orconn_event_rcvr_t fn) +{ + tor_assert(fn); + /* Don't duplicate subscriptions. */ + if (smartlist_contains(orconn_event_rcvrs, fn)) + return; + + smartlist_add(orconn_event_rcvrs, fn); +} + +/** + * Publish a message about OR connection events + * + * This calls the subscriber receiver function synchronously. + **/ +void +orconn_event_publish(const orconn_event_msg_t *msg) +{ + SMARTLIST_FOREACH_BEGIN(orconn_event_rcvrs, orconn_event_rcvr_t, fn) { + tor_assert(fn); + (*fn)(msg); + } SMARTLIST_FOREACH_END(fn); +} + +const subsys_fns_t sys_orconn_event = { + .name = "orconn_event", + .supported = true, + .level = -40, + .initialize = orconn_event_init, + .shutdown = orconn_event_fini, +}; diff --git a/src/core/or/orconn_event.h b/src/core/or/orconn_event.h new file mode 100644 index 00000000000..4c999e53be5 --- /dev/null +++ b/src/core/or/orconn_event.h @@ -0,0 +1,120 @@ +/* Copyright (c) 2001 Matej Pfajfar. + * Copyright (c) 2001-2004, Roger Dingledine. + * Copyright (c) 2004-2006, Roger Dingledine, Nick Mathewson. + * Copyright (c) 2007-2018, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + +/** + * \file orconn_event.h + * \brief Header file for orconn_event.c + * + * The OR_CONN_STATE_* symbols are here to make it easier for + * subscribers to make decisions based on the messages that they + * receive. + **/ + +#ifndef TOR_ORCONN_EVENT_H +#define TOR_ORCONN_EVENT_H + +/** + * @name States of OR connections + * + * These must be in a partial ordering such that usually no OR + * connection will transition from a higher-numbered state to a + * lower-numbered one. Code such as bto_update_best() depends on this + * ordering to determine the best state it's seen so far. + * @{ */ +#define OR_CONN_STATE_MIN_ 1 +/** State for a connection to an OR: waiting for connect() to finish. */ +#define OR_CONN_STATE_CONNECTING 1 +/** State for a connection to an OR: waiting for proxy handshake to complete */ +#define OR_CONN_STATE_PROXY_HANDSHAKING 2 +/** State for an OR connection client: SSL is handshaking, not done + * yet. */ +#define OR_CONN_STATE_TLS_HANDSHAKING 3 +/** State for a connection to an OR: We're doing a second SSL handshake for + * renegotiation purposes. (V2 handshake only.) */ +#define OR_CONN_STATE_TLS_CLIENT_RENEGOTIATING 4 +/** State for a connection at an OR: We're waiting for the client to + * renegotiate (to indicate a v2 handshake) or send a versions cell (to + * indicate a v3 handshake) */ +#define OR_CONN_STATE_TLS_SERVER_RENEGOTIATING 5 +/** State for an OR connection: We're done with our SSL handshake, we've done + * renegotiation, but we haven't yet negotiated link protocol versions and + * sent a netinfo cell. */ +#define OR_CONN_STATE_OR_HANDSHAKING_V2 6 +/** State for an OR connection: We're done with our SSL handshake, but we + * haven't yet negotiated link protocol versions, done a V3 handshake, and + * sent a netinfo cell. */ +#define OR_CONN_STATE_OR_HANDSHAKING_V3 7 +/** State for an OR connection: Ready to send/receive cells. */ +#define OR_CONN_STATE_OPEN 8 +#define OR_CONN_STATE_MAX_ 8 +/** @} */ + +/** Used to indicate the type of an OR connection event passed to the + * controller. The various types are defined in control-spec.txt */ +typedef enum or_conn_status_event_t { + OR_CONN_EVENT_LAUNCHED = 0, + OR_CONN_EVENT_CONNECTED = 1, + OR_CONN_EVENT_FAILED = 2, + OR_CONN_EVENT_CLOSED = 3, + OR_CONN_EVENT_NEW = 4, +} or_conn_status_event_t; + +/** Discriminant values for orconn event message */ +typedef enum orconn_msgtype_t { + ORCONN_MSGTYPE_STATE, + ORCONN_MSGTYPE_STATUS, +} orconn_msgtype_t; + +/** + * Message for orconn state update + * + * This contains information about internal state changes of + * or_connection_t objects. The chan and proxy_type fields are + * additional information that a subscriber may need to make + * decisions. + **/ +typedef struct orconn_state_msg_t { + uint64_t gid; /**< connection's global ID */ + uint64_t chan; /**< associated channel ID */ + int proxy_type; /**< connection's proxy type */ + uint8_t state; /**< new connection state */ +} orconn_state_msg_t; + +/** + * Message for orconn status event + * + * This contains information that ends up in ORCONN control protocol + * events. + **/ +typedef struct orconn_status_msg_t { + uint64_t gid; /**< connection's global ID */ + int status; /**< or_conn_status_event_t */ + int reason; /**< reason */ +} orconn_status_msg_t; + +/** Discriminated union for the actual message */ +typedef struct orconn_event_msg_t { + int type; + union { + orconn_state_msg_t state; + orconn_status_msg_t status; + } u; +} orconn_event_msg_t; + +/** + * Receiver function pointer for OR subscribers + * + * This function gets called synchronously by the publisher. + **/ +typedef void (*orconn_event_rcvr_t)(const orconn_event_msg_t *); + +void orconn_event_subscribe(orconn_event_rcvr_t); + +#ifdef ORCONN_EVENT_PRIVATE +void orconn_event_publish(const orconn_event_msg_t *); +#endif + +#endif /* defined(TOR_ORCONN_EVENT_H) */ diff --git a/src/core/or/orconn_event_sys.h b/src/core/or/orconn_event_sys.h new file mode 100644 index 00000000000..7639023386a --- /dev/null +++ b/src/core/or/orconn_event_sys.h @@ -0,0 +1,12 @@ +/* Copyright (c) 2007-2018, The Tor Project, Inc. */ + +/** + * \file orconn_event_sys.h + * \brief Declare subsystem object for the OR connection event module. + **/ +#ifndef TOR_ORCONN_EVENT_SYS_H +#define TOR_ORCONN_EVENT_SYS_H + +extern const struct subsys_fns_t sys_orconn_event; + +#endif /* defined(TOR_ORCONN_SYS_H) */ diff --git a/src/feature/control/control.h b/src/feature/control/control.h index d78ce4d87ca..d3245fcaf1d 100644 --- a/src/feature/control/control.h +++ b/src/feature/control/control.h @@ -29,6 +29,8 @@ typedef enum circuit_status_minor_event_t { CIRC_MINOR_EVENT_CANNIBALIZED, } circuit_status_minor_event_t; +#include "core/or/orconn_event.h" + /** Used to indicate the type of a stream event passed to the controller. * The various types are defined in control-spec.txt */ typedef enum stream_status_event_t { @@ -43,16 +45,6 @@ typedef enum stream_status_event_t { STREAM_EVENT_REMAP = 8 } stream_status_event_t; -/** Used to indicate the type of an OR connection event passed to the - * controller. The various types are defined in control-spec.txt */ -typedef enum or_conn_status_event_t { - OR_CONN_EVENT_LAUNCHED = 0, - OR_CONN_EVENT_CONNECTED = 1, - OR_CONN_EVENT_FAILED = 2, - OR_CONN_EVENT_CLOSED = 3, - OR_CONN_EVENT_NEW = 4, -} or_conn_status_event_t; - /** Used to indicate the type of a buildtime event */ typedef enum buildtimeout_set_event_t { BUILDTIMEOUT_SET_EVENT_COMPUTED = 0, diff --git a/src/feature/relay/ext_orport.c b/src/feature/relay/ext_orport.c index 3607bdede4c..0a649f2743b 100644 --- a/src/feature/relay/ext_orport.c +++ b/src/feature/relay/ext_orport.c @@ -90,7 +90,7 @@ connection_ext_or_transition(or_connection_t *conn) conn->base_.type = CONN_TYPE_OR; TO_CONN(conn)->state = 0; // set the state to a neutral value - control_event_or_conn_status(conn, OR_CONN_EVENT_NEW, 0); + connection_or_event_status(conn, OR_CONN_EVENT_NEW, 0); connection_tls_start_handshake(conn, 1); } From a0b4fa1f167f9b013ee1a8326e325ec3f97e4700 Mon Sep 17 00:00:00 2001 From: Taylor Yu Date: Sun, 16 Dec 2018 17:01:25 -0600 Subject: [PATCH 03/10] Add origin circuit event pubsub system Add a publish-subscribe subsystem to publish messages about changes to origin circuits. Functions in circuitbuild.c and circuitlist.c publish messages to this subsystem. Move circuit event constants out of control.h so that subscribers don't have to include all of control.h to take actions based on messages they receive. Part of ticket 27167. --- src/app/main/subsystem_list.c | 2 + src/core/include.am | 3 ++ src/core/or/circuitbuild.c | 30 +++++++++++- src/core/or/circuitlist.c | 57 +++++++++++++++++++++- src/core/or/circuitlist.h | 3 ++ src/core/or/circuitstats.c | 6 +-- src/core/or/circuituse.c | 2 +- src/core/or/ocirc_event.c | 84 +++++++++++++++++++++++++++++++++ src/core/or/ocirc_event.h | 89 +++++++++++++++++++++++++++++++++++ src/core/or/ocirc_event_sys.h | 13 +++++ src/feature/control/control.c | 5 +- src/feature/control/control.h | 10 +--- 12 files changed, 287 insertions(+), 17 deletions(-) create mode 100644 src/core/or/ocirc_event.c create mode 100644 src/core/or/ocirc_event.h create mode 100644 src/core/or/ocirc_event_sys.h diff --git a/src/app/main/subsystem_list.c b/src/app/main/subsystem_list.c index 18587380968..ef9b8142d97 100644 --- a/src/app/main/subsystem_list.c +++ b/src/app/main/subsystem_list.c @@ -8,6 +8,7 @@ #include "lib/cc/compat_compiler.h" #include "lib/cc/torint.h" +#include "core/or/ocirc_event_sys.h" #include "core/or/orconn_event_sys.h" #include "lib/compress/compress_sys.h" #include "lib/crypt_ops/crypto_sys.h" @@ -37,6 +38,7 @@ const subsys_fns_t *tor_subsystems[] = { &sys_crypto, /* -60 */ &sys_tortls, /* -50 */ &sys_orconn_event, /* -40 */ + &sys_ocirc_event, /* -39 */ }; const unsigned n_tor_subsystems = ARRAY_LENGTH(tor_subsystems); diff --git a/src/core/include.am b/src/core/include.am index a1fae73609d..e171e4a6f1c 100644 --- a/src/core/include.am +++ b/src/core/include.am @@ -39,6 +39,7 @@ LIBTOR_APP_A_SOURCES = \ src/core/or/connection_or.c \ src/core/or/dos.c \ src/core/or/onion.c \ + src/core/or/ocirc_event.c \ src/core/or/orconn_event.c \ src/core/or/policies.c \ src/core/or/protover.c \ @@ -245,6 +246,8 @@ noinst_HEADERS += \ src/core/or/or_connection_st.h \ src/core/or/or_handshake_certs_st.h \ src/core/or/or_handshake_state_st.h \ + src/core/or/ocirc_event.h \ + src/core/or/ocirc_event_sys.h \ src/core/or/origin_circuit_st.h \ src/core/or/policies.h \ src/core/or/port_cfg_st.h \ diff --git a/src/core/or/circuitbuild.c b/src/core/or/circuitbuild.c index d3744dc1c17..3de8da9cde6 100644 --- a/src/core/or/circuitbuild.c +++ b/src/core/or/circuitbuild.c @@ -26,6 +26,7 @@ **/ #define CIRCUITBUILD_PRIVATE +#define OCIRC_EVENT_PRIVATE #include "core/or/or.h" #include "app/config/config.h" @@ -46,6 +47,7 @@ #include "core/or/connection_edge.h" #include "core/or/connection_or.h" #include "core/or/onion.h" +#include "core/or/ocirc_event.h" #include "core/or/policies.h" #include "core/or/relay.h" #include "feature/client/bridges.h" @@ -492,7 +494,7 @@ circuit_establish_circuit(uint8_t purpose, extend_info_t *exit_ei, int flags) return NULL; } - control_event_circuit_status(circ, CIRC_EVENT_LAUNCHED, 0); + circuit_event_status(circ, CIRC_EVENT_LAUNCHED, 0); if ((err_reason = circuit_handle_first_hop(circ)) < 0) { circuit_mark_for_close(TO_CIRCUIT(circ), -err_reason); @@ -508,6 +510,28 @@ origin_circuit_get_guard_state(origin_circuit_t *circ) return circ->guard_state; } +/** + * Helper function to publish a channel association message + * + * circuit_handle_first_hop() calls this to notify subscribers about a + * channel launch event, which associates a circuit with a channel. + * This doesn't always correspond to an assignment of the circuit's + * n_chan field, because that seems to be only for fully-open + * channels. + **/ +static void +circuit_chan_publish(const origin_circuit_t *circ, const channel_t *chan) +{ + ocirc_event_msg_t msg; + + msg.type = OCIRC_MSGTYPE_CHAN; + msg.u.chan.gid = circ->global_identifier; + msg.u.chan.chan = chan->global_identifier; + msg.u.chan.onehop = circ->build_state->onehop_tunnel; + + ocirc_event_publish(&msg); +} + /** Start establishing the first hop of our circuit. Figure out what * OR we should connect to, and if necessary start the connection to * it. If we're already connected, then send the 'create' cell. @@ -570,6 +594,7 @@ circuit_handle_first_hop(origin_circuit_t *circ) log_info(LD_CIRC,"connect to firsthop failed. Closing."); return -END_CIRC_REASON_CONNECTFAILED; } + circuit_chan_publish(circ, n_chan); } log_debug(LD_CIRC,"connecting in progress (or finished). Good."); @@ -581,6 +606,7 @@ circuit_handle_first_hop(origin_circuit_t *circ) } else { /* it's already open. use it. */ tor_assert(!circ->base_.n_hop); circ->base_.n_chan = n_chan; + circuit_chan_publish(circ, n_chan); log_debug(LD_CIRC,"Conn open. Delivering first onion skin."); if ((err_reason = circuit_send_next_onion_skin(circ)) < 0) { log_info(LD_CIRC,"circuit_send_next_onion_skin failed."); @@ -1416,7 +1442,7 @@ circuit_finish_handshake(origin_circuit_t *circ, hop->state = CPATH_STATE_OPEN; log_info(LD_CIRC,"Finished building circuit hop:"); circuit_log_path(LOG_INFO,LD_CIRC,circ); - control_event_circuit_status(circ, CIRC_EVENT_EXTENDED, 0); + circuit_event_status(circ, CIRC_EVENT_EXTENDED, 0); return 0; } diff --git a/src/core/or/circuitlist.c b/src/core/or/circuitlist.c index 0aa21000a18..c4b5f7ee3e4 100644 --- a/src/core/or/circuitlist.c +++ b/src/core/or/circuitlist.c @@ -51,6 +51,7 @@ * logic, which was originally circuit-focused. **/ #define CIRCUITLIST_PRIVATE +#define OCIRC_EVENT_PRIVATE #include "lib/cc/torint.h" /* TOR_PRIuSZ */ #include "core/or/or.h" @@ -96,6 +97,9 @@ #include "lib/compress/compress_zstd.h" #include "lib/buf/buffers.h" +#define OCIRC_EVENT_PRIVATE +#include "core/or/ocirc_event.h" + #include "ht.h" #include "core/or/cpath_build_state_st.h" @@ -481,6 +485,56 @@ circuit_set_n_circid_chan(circuit_t *circ, circid_t id, } } +/** + * Helper function to publish a message about events on an origin circuit + * + * Publishes a message to subscribers of origin circuit events, and + * sends the control event. + **/ +int +circuit_event_status(origin_circuit_t *circ, circuit_status_event_t tp, + int reason_code) +{ + ocirc_event_msg_t msg; + + tor_assert(circ); + + msg.type = OCIRC_MSGTYPE_CEVENT; + msg.u.cevent.gid = circ->global_identifier; + msg.u.cevent.evtype = tp; + msg.u.cevent.reason = reason_code; + msg.u.cevent.onehop = circ->build_state->onehop_tunnel; + + ocirc_event_publish(&msg); + return control_event_circuit_status(circ, tp, reason_code); +} + +/** + * Helper function to publish a state change message + * + * circuit_set_state() calls this to notify subscribers about a change + * of the state of an origin circuit. + **/ +static void +circuit_state_publish(const circuit_t *circ) +{ + ocirc_event_msg_t msg; + const origin_circuit_t *ocirc; + + if (!CIRCUIT_IS_ORIGIN(circ)) + return; + ocirc = CONST_TO_ORIGIN_CIRCUIT(circ); + /* Only inbound OR circuits can be in this state, not origin circuits. */ + tor_assert(circ->state != CIRCUIT_STATE_ONIONSKIN_PENDING); + + msg.type = OCIRC_MSGTYPE_STATE; + msg.u.state.gid = ocirc->global_identifier; + msg.u.state.state = circ->state; + msg.u.state.onehop = ocirc->build_state->onehop_tunnel; + + ocirc_event_publish(&msg); +} + /** Change the state of circ to state, adding it to or removing * it from lists as appropriate. */ void @@ -510,6 +564,7 @@ circuit_set_state(circuit_t *circ, uint8_t state) if (state == CIRCUIT_STATE_GUARD_WAIT || state == CIRCUIT_STATE_OPEN) tor_assert(!circ->n_chan_create_cell); circ->state = state; + circuit_state_publish(circ); } /** Append to out all circuits in state CHAN_WAIT waiting for @@ -2270,7 +2325,7 @@ circuit_about_to_free(circuit_t *circ) smartlist_remove(circuits_pending_other_guards, circ); } if (CIRCUIT_IS_ORIGIN(circ)) { - control_event_circuit_status(TO_ORIGIN_CIRCUIT(circ), + circuit_event_status(TO_ORIGIN_CIRCUIT(circ), (circ->state == CIRCUIT_STATE_OPEN || circ->state == CIRCUIT_STATE_GUARD_WAIT) ? CIRC_EVENT_CLOSED:CIRC_EVENT_FAILED, diff --git a/src/core/or/circuitlist.h b/src/core/or/circuitlist.h index cb89d1820d8..37d37a089d6 100644 --- a/src/core/or/circuitlist.h +++ b/src/core/or/circuitlist.h @@ -14,6 +14,7 @@ #include "lib/testsupport/testsupport.h" #include "feature/hs/hs_ident.h" +#include "core/or/ocirc_event.h" /** Circuit state: I'm the origin, still haven't done all my handshakes. */ #define CIRCUIT_STATE_BUILDING 0 @@ -184,6 +185,8 @@ void channel_mark_circid_unusable(channel_t *chan, circid_t id); void channel_mark_circid_usable(channel_t *chan, circid_t id); time_t circuit_id_when_marked_unusable_on_channel(circid_t circ_id, channel_t *chan); +int circuit_event_status(origin_circuit_t *circ, circuit_status_event_t tp, + int reason_code); void circuit_set_state(circuit_t *circ, uint8_t state); void circuit_close_all_marked(void); int32_t circuit_initial_package_window(void); diff --git a/src/core/or/circuitstats.c b/src/core/or/circuitstats.c index 0429f2c86ee..61d5e18a458 100644 --- a/src/core/or/circuitstats.c +++ b/src/core/or/circuitstats.c @@ -639,9 +639,9 @@ circuit_build_times_rewind_history(circuit_build_times_t *cbt, int n) void circuit_build_times_mark_circ_as_measurement_only(origin_circuit_t *circ) { - control_event_circuit_status(circ, - CIRC_EVENT_FAILED, - END_CIRC_REASON_TIMEOUT); + circuit_event_status(circ, + CIRC_EVENT_FAILED, + END_CIRC_REASON_TIMEOUT); circuit_change_purpose(TO_CIRCUIT(circ), CIRCUIT_PURPOSE_C_MEASURE_TIMEOUT); /* Record this event to check for too many timeouts diff --git a/src/core/or/circuituse.c b/src/core/or/circuituse.c index 088358a4d68..e230ad10058 100644 --- a/src/core/or/circuituse.c +++ b/src/core/or/circuituse.c @@ -1664,7 +1664,7 @@ circuit_testing_failed(origin_circuit_t *circ, int at_last_hop) void circuit_has_opened(origin_circuit_t *circ) { - control_event_circuit_status(circ, CIRC_EVENT_BUILT, 0); + circuit_event_status(circ, CIRC_EVENT_BUILT, 0); /* Remember that this circuit has finished building. Now if we start * it building again later (e.g. by extending it), we will know not diff --git a/src/core/or/ocirc_event.c b/src/core/or/ocirc_event.c new file mode 100644 index 00000000000..f9f8af279f3 --- /dev/null +++ b/src/core/or/ocirc_event.c @@ -0,0 +1,84 @@ +/* Copyright (c) 2007-2018, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + +/** + * \file ocirc_event.c + * \brief Publish state change messages for origin circuits + * + * Implements a basic publish-subscribe framework for messages about + * the state of origin circuits. The publisher calls the subscriber + * callback functions synchronously. + * + * Although the synchronous calls might not simplify the call graph, + * this approach improves data isolation because the publisher doesn't + * need knowledge about the internals of subscribing subsystems. It + * also avoids race conditions that might occur in asynchronous + * frameworks. + **/ + +#include "core/or/or.h" + +#define OCIRC_EVENT_PRIVATE + +#include "core/or/cpath_build_state_st.h" +#include "core/or/ocirc_event.h" +#include "core/or/ocirc_event_sys.h" +#include "core/or/origin_circuit_st.h" +#include "lib/subsys/subsys.h" + +/** List of subscribers */ +static smartlist_t *ocirc_event_rcvrs; + +/** Initialize subscriber list */ +static int +ocirc_event_init(void) +{ + ocirc_event_rcvrs = smartlist_new(); + return 0; +} + +/** Free subscriber list */ +static void +ocirc_event_fini(void) +{ + smartlist_free(ocirc_event_rcvrs); +} + +/** + * Subscribe to messages about origin circuit events + * + * Register a callback function to receive messages about origin + * circuits. The publisher calls this function synchronously. + **/ +void +ocirc_event_subscribe(ocirc_event_rcvr_t fn) +{ + tor_assert(fn); + /* Don't duplicate subscriptions. */ + if (smartlist_contains(ocirc_event_rcvrs, fn)) + return; + + smartlist_add(ocirc_event_rcvrs, fn); +} + +/** + * Publish a message about OR connection events + * + * This calls the subscriber receiver function synchronously. + **/ +void +ocirc_event_publish(const ocirc_event_msg_t *msg) +{ + SMARTLIST_FOREACH_BEGIN(ocirc_event_rcvrs, ocirc_event_rcvr_t, fn) { + tor_assert(fn); + (*fn)(msg); + } SMARTLIST_FOREACH_END(fn); +} + +const subsys_fns_t sys_ocirc_event = { + .name = "ocirc_event", + .supported = true, + .level = -39, + .initialize = ocirc_event_init, + .shutdown = ocirc_event_fini, +}; diff --git a/src/core/or/ocirc_event.h b/src/core/or/ocirc_event.h new file mode 100644 index 00000000000..19a237d7dfc --- /dev/null +++ b/src/core/or/ocirc_event.h @@ -0,0 +1,89 @@ +/* Copyright (c) 2007-2018, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + +/** + * \file ocirc_event.h + * \brief Header file for ocirc_event.c + **/ + +#ifndef TOR_OCIRC_EVENT_H +#define TOR_OCIRC_EVENT_H + +#include + +#include "lib/cc/torint.h" + +/** Used to indicate the type of a circuit event passed to the controller. + * The various types are defined in control-spec.txt */ +typedef enum circuit_status_event_t { + CIRC_EVENT_LAUNCHED = 0, + CIRC_EVENT_BUILT = 1, + CIRC_EVENT_EXTENDED = 2, + CIRC_EVENT_FAILED = 3, + CIRC_EVENT_CLOSED = 4, +} circuit_status_event_t; + +/** Message for origin circuit state update */ +typedef struct ocirc_state_msg_t { + uint32_t gid; /**< global ID (only origin circuits have them) */ + int state; /**< new circuit state */ + bool onehop; /**< one-hop circuit? */ +} ocirc_state_msg_t; + +/** + * Message when a channel gets associated to a circuit. + * + * This doesn't always correspond to something in circuitbuild.c + * setting the n_chan field in the circuit. For some reason, if + * circuit_handle_first_hop() launches a new circuit, it doesn't set + * the n_chan field. + */ +typedef struct ocirc_chan_msg_t { + uint32_t gid; /**< global ID */ + uint64_t chan; /**< channel ID */ + bool onehop; /**< one-hop circuit? */ +} ocirc_chan_msg_t; + +/** + * Message for origin circuit status event + * + * This contains information that ends up in CIRC control protocol events. + */ +typedef struct ocirc_cevent_msg_t { + uint32_t gid; /**< global ID */ + int evtype; /**< event type */ + int reason; /**< reason */ + bool onehop; /**< one-hop circuit? */ +} ocirc_cevent_msg_t; + +/** Discriminant values for origin circuit event message */ +typedef enum ocirc_msgtype_t { + OCIRC_MSGTYPE_STATE, + OCIRC_MSGTYPE_CHAN, + OCIRC_MSGTYPE_CEVENT, +} ocirc_msgtype_t; + +/** Discriminated union for the actual message */ +typedef struct ocirc_event_msg_t { + int type; + union { + ocirc_state_msg_t state; + ocirc_chan_msg_t chan; + ocirc_cevent_msg_t cevent; + } u; +} ocirc_event_msg_t; + +/** + * Receiver function pointer for origin circuit subscribers + * + * This function gets called synchronously by the publisher. + **/ +typedef void (*ocirc_event_rcvr_t)(const ocirc_event_msg_t *); + +void ocirc_event_subscribe(ocirc_event_rcvr_t fn); + +#ifdef OCIRC_EVENT_PRIVATE +void ocirc_event_publish(const ocirc_event_msg_t *msg); +#endif + +#endif /* defined(TOR_OCIRC_EVENT_STATE_H) */ diff --git a/src/core/or/ocirc_event_sys.h b/src/core/or/ocirc_event_sys.h new file mode 100644 index 00000000000..0bc135ffafc --- /dev/null +++ b/src/core/or/ocirc_event_sys.h @@ -0,0 +1,13 @@ +/* Copyright (c) 2007-2018, The Tor Project, Inc. */ + +/** + * \file ocirc_event_sys.h + * \brief Declare subsystem object for the origin circuit event module. + **/ + +#ifndef TOR_OCIRC_EVENT_SYS_H +#define TOR_OCIRC_EVENT_SYS_H + +extern const struct subsys_fns_t sys_ocirc_event; + +#endif /* defined(TOR_OCIRC_EVENT_H) */ diff --git a/src/feature/control/control.c b/src/feature/control/control.c index 4ef550c9193..da62c949813 100644 --- a/src/feature/control/control.c +++ b/src/feature/control/control.c @@ -34,6 +34,7 @@ **/ #define CONTROL_PRIVATE +#define OCIRC_EVENT_PRIVATE #include "core/or/or.h" #include "app/config/config.h" @@ -50,6 +51,7 @@ #include "core/or/command.h" #include "core/or/connection_edge.h" #include "core/or/connection_or.h" +#include "core/or/ocirc_event.h" #include "core/or/policies.h" #include "core/or/reasons.h" #include "core/or/versions.h" @@ -3749,7 +3751,7 @@ handle_control_extendcircuit(control_connection_t *conn, uint32_t len, connection_printf_to_buf(conn, "250 EXTENDED %lu\r\n", (unsigned long)circ->global_identifier); if (zero_circ) /* send a 'launched' event, for completeness */ - control_event_circuit_status(circ, CIRC_EVENT_LAUNCHED, 0); + circuit_event_status(circ, CIRC_EVENT_LAUNCHED, 0); done: SMARTLIST_FOREACH(router_nicknames, char *, n, tor_free(n)); smartlist_free(router_nicknames); @@ -5625,6 +5627,7 @@ control_event_circuit_status(origin_circuit_t *circ, circuit_status_event_t tp, { const char *status; char reasons[64] = ""; + if (!EVENT_IS_INTERESTING(EVENT_CIRCUIT_STATUS)) return 0; tor_assert(circ); diff --git a/src/feature/control/control.h b/src/feature/control/control.h index d3245fcaf1d..68c9a6bed19 100644 --- a/src/feature/control/control.h +++ b/src/feature/control/control.h @@ -12,15 +12,7 @@ #ifndef TOR_CONTROL_H #define TOR_CONTROL_H -/** Used to indicate the type of a circuit event passed to the controller. - * The various types are defined in control-spec.txt */ -typedef enum circuit_status_event_t { - CIRC_EVENT_LAUNCHED = 0, - CIRC_EVENT_BUILT = 1, - CIRC_EVENT_EXTENDED = 2, - CIRC_EVENT_FAILED = 3, - CIRC_EVENT_CLOSED = 4, -} circuit_status_event_t; +#include "core/or/ocirc_event.h" /** Used to indicate the type of a CIRC_MINOR event passed to the controller. * The various types are defined in control-spec.txt . */ From b0f974633ac17e07c3ab93ecea9337506bb8d583 Mon Sep 17 00:00:00 2001 From: Taylor Yu Date: Wed, 19 Dec 2018 21:35:26 -0600 Subject: [PATCH 04/10] Add LD_BTRACK log domain for bootstrap tracker Part of ticket 27167. --- doc/tor.1.txt | 2 +- src/lib/log/log.c | 2 +- src/lib/log/log.h | 4 +++- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/doc/tor.1.txt b/doc/tor.1.txt index 974a484541e..4ff789a931b 100644 --- a/doc/tor.1.txt +++ b/doc/tor.1.txt @@ -678,7 +678,7 @@ GENERAL OPTIONS The currently recognized domains are: general, crypto, net, config, fs, protocol, mm, http, app, control, circ, rend, bug, dir, dirserv, or, edge, acct, hist, handshake, heartbeat, channel, sched, guard, consdiff, dos, - process, and pt. + process, pt, and btrack. Domain names are case-insensitive. + + For example, "`Log [handshake]debug [~net,~mm]info notice stdout`" sends diff --git a/src/lib/log/log.c b/src/lib/log/log.c index 2c25ddf1aa9..d032f57adda 100644 --- a/src/lib/log/log.c +++ b/src/lib/log/log.c @@ -1268,7 +1268,7 @@ static const char *domain_list[] = { "GENERAL", "CRYPTO", "NET", "CONFIG", "FS", "PROTOCOL", "MM", "HTTP", "APP", "CONTROL", "CIRC", "REND", "BUG", "DIR", "DIRSERV", "OR", "EDGE", "ACCT", "HIST", "HANDSHAKE", "HEARTBEAT", "CHANNEL", - "SCHED", "GUARD", "CONSDIFF", "DOS", "PROCESS", "PT", NULL + "SCHED", "GUARD", "CONSDIFF", "DOS", "PROCESS", "PT", "BTRACK", NULL }; /** Return a bitmask for the log domain for which domain is the name, diff --git a/src/lib/log/log.h b/src/lib/log/log.h index c8820ee0371..423e58e11bd 100644 --- a/src/lib/log/log.h +++ b/src/lib/log/log.h @@ -111,8 +111,10 @@ #define LD_PROCESS (1u<<26) /** Pluggable Transports. */ #define LD_PT (1u<<27) +/** Bootstrap tracker. */ +#define LD_BTRACK (1u<<28) /** Number of logging domains in the code. */ -#define N_LOGGING_DOMAINS 28 +#define N_LOGGING_DOMAINS 29 /** This log message is not safe to send to a callback-based logger * immediately. Used as a flag, not a log domain. */ From b0ae6a332a0882a670b3a2bacf5c70b69936e4bc Mon Sep 17 00:00:00 2001 From: Taylor Yu Date: Mon, 17 Dec 2018 09:45:09 -0600 Subject: [PATCH 05/10] Add bootstrap tracker subsystem Add a tracker for bootstrap progress, tracking events related to origin circuit and ORCONN states. This uses the ocirc_event and orconn_event publish-subscribe subsystems. Part of ticket 27167. --- src/app/main/subsystem_list.c | 2 + src/core/include.am | 8 + src/feature/control/btrack.c | 53 ++++++ src/feature/control/btrack_circuit.c | 164 +++++++++++++++++ src/feature/control/btrack_circuit.h | 15 ++ src/feature/control/btrack_orconn.c | 196 ++++++++++++++++++++ src/feature/control/btrack_orconn.h | 38 ++++ src/feature/control/btrack_orconn_maps.c | 223 +++++++++++++++++++++++ src/feature/control/btrack_orconn_maps.h | 17 ++ src/feature/control/btrack_sys.h | 14 ++ 10 files changed, 730 insertions(+) create mode 100644 src/feature/control/btrack.c create mode 100644 src/feature/control/btrack_circuit.c create mode 100644 src/feature/control/btrack_circuit.h create mode 100644 src/feature/control/btrack_orconn.c create mode 100644 src/feature/control/btrack_orconn.h create mode 100644 src/feature/control/btrack_orconn_maps.c create mode 100644 src/feature/control/btrack_orconn_maps.h create mode 100644 src/feature/control/btrack_sys.h diff --git a/src/app/main/subsystem_list.c b/src/app/main/subsystem_list.c index ef9b8142d97..2f2586ac15d 100644 --- a/src/app/main/subsystem_list.c +++ b/src/app/main/subsystem_list.c @@ -10,6 +10,7 @@ #include "core/or/ocirc_event_sys.h" #include "core/or/orconn_event_sys.h" +#include "feature/control/btrack_sys.h" #include "lib/compress/compress_sys.h" #include "lib/crypt_ops/crypto_sys.h" #include "lib/err/torerr_sys.h" @@ -39,6 +40,7 @@ const subsys_fns_t *tor_subsystems[] = { &sys_tortls, /* -50 */ &sys_orconn_event, /* -40 */ &sys_ocirc_event, /* -39 */ + &sys_btrack, /* -30 */ }; const unsigned n_tor_subsystems = ARRAY_LENGTH(tor_subsystems); diff --git a/src/core/include.am b/src/core/include.am index e171e4a6f1c..93a8bca5d5b 100644 --- a/src/core/include.am +++ b/src/core/include.am @@ -63,6 +63,10 @@ LIBTOR_APP_A_SOURCES = \ src/feature/client/dnsserv.c \ src/feature/client/entrynodes.c \ src/feature/client/transports.c \ + src/feature/control/btrack.c \ + src/feature/control/btrack_circuit.c \ + src/feature/control/btrack_orconn.c \ + src/feature/control/btrack_orconn_maps.c \ src/feature/control/control.c \ src/feature/control/control_bootstrap.c \ src/feature/control/fmt_serverstatus.c \ @@ -274,6 +278,10 @@ noinst_HEADERS += \ src/feature/client/dnsserv.h \ src/feature/client/entrynodes.h \ src/feature/client/transports.h \ + src/feature/control/btrack_circuit.h \ + src/feature/control/btrack_orconn.h \ + src/feature/control/btrack_orconn_maps.h \ + src/feature/control/btrack_sys.h \ src/feature/control/control.h \ src/feature/control/control_connection_st.h \ src/feature/control/fmt_serverstatus.h \ diff --git a/src/feature/control/btrack.c b/src/feature/control/btrack.c new file mode 100644 index 00000000000..14220faad1c --- /dev/null +++ b/src/feature/control/btrack.c @@ -0,0 +1,53 @@ +/* Copyright (c) 2007-2018, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + +/** + * \file btrack.c + * \brief Bootstrap trackers + * + * Initializes and shuts down the specific bootstrap trackers. These + * trackers help the reporting of bootstrap progress by maintaining + * state information about various subsystems within tor. When the + * correct state changes happen, these trackers emit controller + * events. + * + * These trackers avoid referring directly to the internals of state + * objects of other subsystems. + * + * btrack_circuit.c contains the tracker for origin circuits. + * + * btrack_orconn.c contains the tracker for OR connections. + * + * Eventually there will be a tracker for directory downloads as well. + **/ + +#include "feature/control/btrack_circuit.h" +#include "feature/control/btrack_orconn.h" +#include "feature/control/btrack_sys.h" +#include "lib/subsys/subsys.h" + +static int +btrack_init(void) +{ + if (btrack_orconn_init()) + return -1; + if (btrack_circ_init()) + return -1; + + return 0; +} + +static void +btrack_fini(void) +{ + btrack_orconn_fini(); + btrack_circ_fini(); +} + +const subsys_fns_t sys_btrack = { + .name = "btrack", + .supported = true, + .level = -30, + .initialize = btrack_init, + .shutdown = btrack_fini, +}; diff --git a/src/feature/control/btrack_circuit.c b/src/feature/control/btrack_circuit.c new file mode 100644 index 00000000000..bf09e0b99c5 --- /dev/null +++ b/src/feature/control/btrack_circuit.c @@ -0,0 +1,164 @@ +/* Copyright (c) 2007-2018, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + +/** + * \file btrack_circuit.c + * \brief Bootstrap tracker for origin circuits + * + * Track state changes of origin circuits, as published by the circuit + * subsystem. + **/ + +#include "core/or/or.h" + +#include "core/or/ocirc_event.h" + +#include "feature/control/btrack_circuit.h" +#include "feature/control/control.h" +#include "lib/log/log.h" + +/** Pair of a best origin circuit GID with its state or status */ +typedef struct btc_best_t { + uint32_t gid; + int val; +} btc_best_t; + +/** GID and state of the best origin circuit we've seen so far */ +static btc_best_t best_any_state = { 0, -1 }; +/** GID and state of the best application circuit we've seen so far */ +static btc_best_t best_ap_state = { 0, -1 }; +/** GID and status of the best origin circuit we've seen so far */ +static btc_best_t best_any_evtype = { 0, -1 }; +/** GID and status of the best application circuit we've seen so far */ +static btc_best_t best_ap_evtype = { 0, -1 }; + +/** Reset cached "best" values */ +static void +btc_reset_bests(void) +{ + best_any_state.gid = best_ap_state.gid = 0; + best_any_state.val = best_ap_state.val = -1; + best_any_evtype.gid = best_ap_state.gid = 0; + best_any_evtype.val = best_ap_evtype.val = -1; +} + +/** True if @a state is a "better" origin circuit state than @a best->val */ +static bool +btc_state_better(int state, const btc_best_t *best) +{ + return state > best->val; +} + +/** + * Definine an ordering on circuit status events + * + * The CIRC_EVENT_ constants aren't sorted in a useful order, so this + * array helps to decode them. This approach depends on the statuses + * being nonnegative and dense. + **/ +static int circ_event_order[] = { + [CIRC_EVENT_FAILED] = -1, + [CIRC_EVENT_CLOSED] = -1, + [CIRC_EVENT_LAUNCHED] = 1, + [CIRC_EVENT_EXTENDED] = 2, + [CIRC_EVENT_BUILT] = 3, +}; +#define N_CIRC_EVENT_ORDER \ + (sizeof(circ_event_order) / sizeof(circ_event_order[0])) + +/** True if @a state is a "better" origin circuit event status than @a + best->val */ +static bool +btc_evtype_better(int state, const btc_best_t *best) +{ + if (state < 0) + return false; + if (best->val < 0) + return true; + + tor_assert(state >= 0 && (unsigned)state < N_CIRC_EVENT_ORDER); + tor_assert(best->val >= 0 && (unsigned)best->val < N_CIRC_EVENT_ORDER); + return circ_event_order[state] > circ_event_order[best->val]; +} + +static bool +btc_update_state(const ocirc_state_msg_t *msg, btc_best_t *best, + const char *type) +{ + if (btc_state_better(msg->state, best)) { + log_info(LD_BTRACK, "CIRC BEST_%s state %d->%d gid=%"PRIu32, type, + best->val, msg->state, msg->gid); + best->gid = msg->gid; + best->val = msg->state; + return true; + } + return false; +} + +static bool +btc_update_evtype(const ocirc_cevent_msg_t *msg, btc_best_t *best, + const char *type) +{ + if (btc_evtype_better(msg->evtype, best)) { + log_info(LD_BTRACK, "CIRC BEST_%s evtype %d->%d gid=%"PRIu32, type, + best->val, msg->evtype, msg->gid); + best->gid = msg->gid; + best->val = msg->evtype; + return true; + } + return false; +} + +static void +btc_state_rcvr(const ocirc_state_msg_t *msg) +{ + log_debug(LD_BTRACK, "CIRC gid=%"PRIu32" state=%d onehop=%d", + msg->gid, msg->state, msg->onehop); + + btc_update_state(msg, &best_any_state, "ANY"); + if (msg->onehop) + return; + btc_update_state(msg, &best_ap_state, "AP"); +} + +static void +btc_cevent_rcvr(const ocirc_cevent_msg_t *msg) +{ + log_debug(LD_BTRACK, "CIRC gid=%"PRIu32" evtype=%d reason=%d onehop=%d", + msg->gid, msg->evtype, msg->reason, msg->onehop); + + btc_update_evtype(msg, &best_any_evtype, "ANY"); + if (msg->onehop) + return; + btc_update_evtype(msg, &best_ap_evtype, "AP"); +} + +static void +btc_event_rcvr(const ocirc_event_msg_t *msg) +{ + switch (msg->type) { + case OCIRC_MSGTYPE_STATE: + return btc_state_rcvr(&msg->u.state); + case OCIRC_MSGTYPE_CHAN: + log_debug(LD_BTRACK, "CIRC gid=%"PRIu32" chan=%"PRIu64" onehop=%d", + msg->u.chan.gid, msg->u.chan.chan, msg->u.chan.onehop); + break; + case OCIRC_MSGTYPE_CEVENT: + return btc_cevent_rcvr(&msg->u.cevent); + default: + break; + } +} + +int +btrack_circ_init(void) +{ + ocirc_event_subscribe(btc_event_rcvr); + return 0; +} + +void +btrack_circ_fini(void) +{ + btc_reset_bests(); +} diff --git a/src/feature/control/btrack_circuit.h b/src/feature/control/btrack_circuit.h new file mode 100644 index 00000000000..ab8b8b652ce --- /dev/null +++ b/src/feature/control/btrack_circuit.h @@ -0,0 +1,15 @@ +/* Copyright (c) 2007-2018, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + +/** + * \file btrack_circuit.h + * \brief Header file for btrack_circuit.c + **/ + +#ifndef TOR_BTRACK_CIRCUIT_H +#define TOR_BTRACK_CIRCUIT_H + +int btrack_circ_init(void); +void btrack_circ_fini(void); + +#endif /* defined(TOR_BTRACK_CIRCUIT_H) */ diff --git a/src/feature/control/btrack_orconn.c b/src/feature/control/btrack_orconn.c new file mode 100644 index 00000000000..69344569bfd --- /dev/null +++ b/src/feature/control/btrack_orconn.c @@ -0,0 +1,196 @@ +/* Copyright (c) 2007-2018, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + +/** + * \file btrack_orconn.c + * \brief Bootstrap tracker for OR connections + * + * Track state changes of OR connections, as published by the + * connection subsystem. Also track circuit launch events, because + * they're one of the few ways to discover the association between a + * channel (and OR connection) and a circuit. + * + * We track all OR connections that we receive events for, whether or + * not they're carrying origin circuits. (An OR connection might + * carry origin circuits only after we first find out about that + * connection.) + * + * All origin ORCONN events update the "any" state variables, while + * only application ORCONN events update the "ap" state variables (and + * also update the "any") variables. + * + * We do this because we want to report the first increments of + * connection progress as the earliest bootstrap phases. This results + * in a better user experience because failures here translate into + * zero or very small amounts of displayed progress, instead of + * progress stuck near completion. The first connection to a relay + * 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. + **/ + +#include + +#include "core/or/or.h" + +#define BTRACK_ORCONN_PRIVATE + +#include "core/or/ocirc_event.h" +#include "core/or/orconn_event.h" +#include "feature/control/btrack_orconn.h" +#include "feature/control/btrack_orconn_maps.h" +#include "lib/log/log.h" + +/** Pair of a best ORCONN GID and with its state */ +typedef struct bto_best_t { + uint64_t gid; + int state; +} bto_best_t; + +/** GID and state of the best ORCONN we've seen so far */ +static bto_best_t best_any = { 0, -1 }; +/** GID and state of the best application circuit ORCONN we've seen so far */ +static bto_best_t best_ap = { 0, -1 }; + +/** + * Update a cached state of a best ORCONN progress we've seen so far. + * + * Return true if the new state is better than the old. + **/ +static bool +bto_update_best(const bt_orconn_t *bto, bto_best_t *best, const char *type) +{ + if (bto->state < best->state) + return false; + best->gid = bto->gid; + if (bto->state > best->state) { + log_info(LD_BTRACK, "ORCONN BEST_%s state %d->%d gid=%"PRIu64, type, + best->state, bto->state, bto->gid); + best->state = bto->state; + return true; + } + return false; +} + +/** + * Update cached states of best ORCONN progress we've seen + * + * Only update the application ORCONN state if we know it's carrying + * an application circuit. + **/ +static void +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"); +} + +/** Reset cached "best" values */ +static void +bto_reset_bests(void) +{ + best_any.gid = best_ap.gid = 0; + best_any.state = best_ap.state = -1; +} + +/** + * Update cached states of ORCONNs from the incoming message. This + * message comes from code in connection_or.c. + **/ +static void +bto_state_rcvr(const orconn_state_msg_t *msg) +{ + bt_orconn_t *bto; + + bto = bto_find_or_new(msg->gid, msg->chan); + log_debug(LD_BTRACK, "ORCONN gid=%"PRIu64" chan=%"PRIu64 + " proxy_type=%d state=%d", + msg->gid, msg->chan, msg->proxy_type, msg->state); + bto->proxy_type = msg->proxy_type; + bto->state = msg->state; + if (bto->is_orig) + bto_update_bests(bto); +} + +/** + * Delete a cached ORCONN state if we get an incoming message saying + * the ORCONN is failed or closed. This message comes from code in + * control.c. + **/ +static void +bto_status_rcvr(const orconn_status_msg_t *msg) +{ + switch (msg->status) { + case OR_CONN_EVENT_FAILED: + case OR_CONN_EVENT_CLOSED: + log_info(LD_BTRACK, "ORCONN DELETE gid=%"PRIu64" status=%d reason=%d", + msg->gid, msg->status, msg->reason); + return bto_delete(msg->gid); + default: + break; + } +} + +/** Dispatch to individual ORCONN message handlers */ +static void +bto_event_rcvr(const orconn_event_msg_t *msg) +{ + switch (msg->type) { + case ORCONN_MSGTYPE_STATE: + return bto_state_rcvr(&msg->u.state); + case ORCONN_MSGTYPE_STATUS: + return bto_status_rcvr(&msg->u.status); + default: + tor_assert(false); + } +} + +/** + * Create or update a cached ORCONN state for a newly launched + * connection, including whether it's launched by an origin circuit + * and whether it's a one-hop circuit. + **/ +static void +bto_chan_rcvr(const ocirc_event_msg_t *msg) +{ + bt_orconn_t *bto; + + /* Ignore other kinds of origin circuit events; we don't need them */ + if (msg->type != OCIRC_MSGTYPE_CHAN) + return; + + bto = bto_find_or_new(0, msg->u.chan.chan); + if (!bto->is_orig || (bto->is_onehop && !msg->u.chan.onehop)) { + log_debug(LD_BTRACK, "ORCONN LAUNCH chan=%"PRIu64" onehop=%d", + msg->u.chan.chan, msg->u.chan.onehop); + } + bto->is_orig = true; + if (!msg->u.chan.onehop) + bto->is_onehop = false; + bto_update_bests(bto); +} + +/** + * Initialize the hash maps and subscribe to ORCONN and origin + * circuit events. + **/ +int +btrack_orconn_init(void) +{ + bto_init_maps(); + orconn_event_subscribe(bto_event_rcvr); + ocirc_event_subscribe(bto_chan_rcvr); + + return 0; +} + +/** Clear the hash maps and reset the "best" states */ +void +btrack_orconn_fini(void) +{ + bto_clear_maps(); + bto_reset_bests(); +} diff --git a/src/feature/control/btrack_orconn.h b/src/feature/control/btrack_orconn.h new file mode 100644 index 00000000000..4e514d4b046 --- /dev/null +++ b/src/feature/control/btrack_orconn.h @@ -0,0 +1,38 @@ +/* Copyright (c) 2007-2018, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + +/** + * \file btrack_orconn.h + * \brief Header file for btrack_orconn.c + **/ + +#ifndef TOR_BTRACK_ORCONN_H +#define TOR_BTRACK_ORCONN_H + +#ifdef BTRACK_ORCONN_PRIVATE + +#include "ht.h" + +/** + * Structure for tracking OR connection states + * + * This gets linked into two hash maps: one with connection IDs, and + * another with channel IDs. + **/ +typedef struct bt_orconn_t { + HT_ENTRY(bt_orconn_t) node; /**< Hash map entry indexed by gid */ + HT_ENTRY(bt_orconn_t) chan_node; /**< Hash map entry indexed by channel ID */ + uint64_t gid; /**< Global ID of this ORCONN */ + uint64_t chan; /**< Channel ID, if known */ + int proxy_type; /**< Proxy type */ + uint8_t state; /**< State of this ORCONN */ + bool is_orig; /**< Does this carry an origin circuit? */ + bool is_onehop; /**< Is this for a one-hop circuit? */ +} bt_orconn_t; + +#endif /* defined(BTRACK_ORCONN_PRIVATE) */ + +int btrack_orconn_init(void); +void btrack_orconn_fini(void); + +#endif /* defined(TOR_BTRACK_ORCONN_H) */ diff --git a/src/feature/control/btrack_orconn_maps.c b/src/feature/control/btrack_orconn_maps.c new file mode 100644 index 00000000000..b6bb23804c9 --- /dev/null +++ b/src/feature/control/btrack_orconn_maps.c @@ -0,0 +1,223 @@ +/* Copyright (c) 2007-2018, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + +/** + * \file btrack_orconn_maps.c + * \brief Hash map implementation for btrack_orconn.c + * + * These functions manipulate the hash maps that contain bt_orconn + * objects. + **/ + +#include + +#include "core/or/or.h" + +#include "ht.h" +#include "siphash.h" + +#define BTRACK_ORCONN_PRIVATE + +#include "feature/control/btrack_orconn.h" +#include "feature/control/btrack_orconn_maps.h" +#include "lib/log/log.h" + +static inline unsigned int +bto_gid_hash_(bt_orconn_t *elm) +{ + return (unsigned)siphash24g(&elm->gid, sizeof(elm->gid)); +} + +static inline int +bto_gid_eq_(bt_orconn_t *a, bt_orconn_t *b) +{ + return a->gid == b->gid; +} + +static inline unsigned int +bto_chan_hash_(bt_orconn_t *elm) +{ + return (unsigned)siphash24g(&elm->chan, sizeof(elm->chan)); +} + +static inline int +bto_chan_eq_(bt_orconn_t *a, bt_orconn_t *b) +{ + return a->chan == b->chan; +} + +HT_HEAD(bto_gid_ht, bt_orconn_t); +HT_PROTOTYPE(bto_gid_ht, bt_orconn_t, node, bto_gid_hash_, bto_gid_eq_) +HT_GENERATE2(bto_gid_ht, bt_orconn_t, node, + bto_gid_hash_, bto_gid_eq_, 0.6, + tor_reallocarray_, tor_free_) +static struct bto_gid_ht *bto_gid_map; + +HT_HEAD(bto_chan_ht, bt_orconn_t); +HT_PROTOTYPE(bto_chan_ht, bt_orconn_t, chan_node, bto_chan_hash_, bto_chan_eq_) +HT_GENERATE2(bto_chan_ht, bt_orconn_t, chan_node, + bto_chan_hash_, bto_chan_eq_, 0.6, + tor_reallocarray_, tor_free_) +static struct bto_chan_ht *bto_chan_map; + +/** Clear the GID hash map, freeing any bt_orconn_t objects that become + * unreferenced */ +static void +bto_gid_clear_map(void) +{ + bt_orconn_t **elt, **next, *c; + + for (elt = HT_START(bto_gid_ht, bto_gid_map); + elt; + elt = next) { + c = *elt; + next = HT_NEXT_RMV(bto_gid_ht, bto_gid_map, elt); + + c->gid = 0; + /* Don't delete if chan ID isn't zero: it's still in the chan hash map */ + if (!c->chan) + tor_free(c); + } + HT_CLEAR(bto_gid_ht, bto_gid_map); + tor_free(bto_gid_map); +} + +/** Clear the chan ID hash map, freeing any bt_orconn_t objects that + * become unreferenced */ +static void +bto_chan_clear_map(void) +{ + bt_orconn_t **elt, **next, *c; + + for (elt = HT_START(bto_chan_ht, bto_chan_map); + elt; + elt = next) { + c = *elt; + next = HT_NEXT_RMV(bto_chan_ht, bto_chan_map, elt); + + c->chan = 0; + /* Don't delete if GID isn't zero, it's still in the GID hash map */ + if (!c->gid) + tor_free(c); + } + HT_CLEAR(bto_chan_ht, bto_chan_map); + tor_free(bto_chan_map); +} + +/** Delete a bt_orconn from the hash maps by GID */ +void +bto_delete(uint64_t gid) +{ + bt_orconn_t key, *bto; + + key.gid = gid; + key.chan = 0; + bto = HT_FIND(bto_gid_ht, bto_gid_map, &key); + if (!bto) { + /* The orconn might be unregistered because it's an EXT_OR_CONN? */ + log_debug(LD_BTRACK, "tried to delete unregistered ORCONN gid=%"PRIu64, + gid); + return; + } + HT_REMOVE(bto_gid_ht, bto_gid_map, &key); + if (bto->chan) { + key.chan = bto->chan; + HT_REMOVE(bto_chan_ht, bto_chan_map, &key); + } + tor_free(bto); +} + +/** + * Helper for bto_find_or_new(). + * + * Update GID and chan ID of an existing bt_orconn object if needed, + * given a search key previously used within bto_find_or_new(). + **/ +static bt_orconn_t * +bto_update(bt_orconn_t *bto, const bt_orconn_t *key) +{ + /* ORCONN GIDs shouldn't change once assigned */ + tor_assert(!bto->gid || !key->gid || bto->gid == key->gid); + if (!bto->gid && key->gid) { + /* Got a gid when we didn't already have one; insert into gid map */ + log_debug(LD_BTRACK, "ORCONN chan=%"PRIu64" newgid=%"PRIu64, key->chan, + key->gid); + bto->gid = key->gid; + HT_INSERT(bto_gid_ht, bto_gid_map, bto); + } + /* association of ORCONN with channel shouldn't change */ + tor_assert(!bto->chan || !key->chan || bto->chan == key->chan); + if (!bto->chan && key->chan) { + /* Got a chan when we didn't already have one; insert into chan map */ + log_debug(LD_BTRACK, "ORCONN gid=%"PRIu64" newchan=%"PRIu64, + bto->gid, key->chan); + bto->chan = key->chan; + HT_INSERT(bto_chan_ht, bto_chan_map, bto); + } + return bto; +} + +/** Helper for bto_find_or_new() */ +static bt_orconn_t * +bto_new(const bt_orconn_t *key) +{ + struct bt_orconn_t *bto = tor_malloc(sizeof(*bto)); + + bto->gid = key->gid; + bto->chan = key->chan; + bto->state = 0; + bto->proxy_type = 0; + bto->is_orig = false; + bto->is_onehop = true; + + if (bto->gid) + HT_INSERT(bto_gid_ht, bto_gid_map, bto); + if (bto->chan) + HT_INSERT(bto_chan_ht, bto_chan_map, bto); + + return bto; +} + +/** + * Insert a new bt_orconn with the given GID and chan ID, or update + * the GID and chan ID if one already exists. + * + * Return the found or allocated bt_orconn. + **/ +bt_orconn_t * +bto_find_or_new(uint64_t gid, uint64_t chan) +{ + bt_orconn_t key, *bto = NULL; + + tor_assert(gid || chan); + key.gid = gid; + key.chan = chan; + if (key.gid) + bto = HT_FIND(bto_gid_ht, bto_gid_map, &key); + if (!bto && key.chan) { + /* Not found by GID; look up by chan ID */ + bto = HT_FIND(bto_chan_ht, bto_chan_map, &key); + } + if (bto) + return bto_update(bto, &key); + else + return bto_new(&key); +} + +/** Initialize the hash maps */ +void +bto_init_maps(void) +{ + bto_gid_map = tor_malloc(sizeof(*bto_gid_map)); + HT_INIT(bto_gid_ht, bto_gid_map); + bto_chan_map = tor_malloc(sizeof(*bto_chan_map)); + HT_INIT(bto_chan_ht, bto_chan_map); +} + +/** Clear the hash maps, freeing all associated storage */ +void +bto_clear_maps(void) +{ + bto_gid_clear_map(); + bto_chan_clear_map(); +} diff --git a/src/feature/control/btrack_orconn_maps.h b/src/feature/control/btrack_orconn_maps.h new file mode 100644 index 00000000000..b1c2c7aa08f --- /dev/null +++ b/src/feature/control/btrack_orconn_maps.h @@ -0,0 +1,17 @@ +/* Copyright (c) 2007-2018, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + +/** + * \file btrack_orconn_maps.h + * \brief Header file for btrack_orconn_maps.c + **/ + +#ifndef TOR_BTRACK_ORCONN_MAPS_H + +void bto_delete(uint64_t); +bt_orconn_t *bto_find_or_new(uint64_t, uint64_t); + +void bto_init_maps(void); +void bto_clear_maps(void); + +#endif /* defined(TOR_BTRACK_ORCONN_MAPS_H) */ diff --git a/src/feature/control/btrack_sys.h b/src/feature/control/btrack_sys.h new file mode 100644 index 00000000000..f80cf342e74 --- /dev/null +++ b/src/feature/control/btrack_sys.h @@ -0,0 +1,14 @@ +/* Copyright (c) 2007-2018, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + +/** + * \file btrack_sys.h + * \brief Declare subsystem object for the bootstrap tracker susbystem. + **/ + +#ifndef TOR_BTRACK_SYS_H +#define TOR_BTRACK_SYS_H + +extern const struct subsys_fns_t sys_btrack; + +#endif /* defined(TOR_BTRACK_SYS_H) */ From 7aa65b3ccd6d2b8be36385fb17878ac8ce5d667b Mon Sep 17 00:00:00 2001 From: Taylor Yu Date: Sat, 15 Dec 2018 22:14:46 -0600 Subject: [PATCH 06/10] 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(-) create mode 100644 src/feature/control/btrack_orconn_cevent.c create mode 100644 src/feature/control/btrack_orconn_cevent.h diff --git a/src/core/include.am b/src/core/include.am index 93a8bca5d5b..5e69cb9adaf 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 3de8da9cde6..b89ec09a99e 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 a12ea55c467..5d0874869d2 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 69344569bfd..4829d869c37 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 @@ -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" @@ -83,9 +88,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 */ @@ -193,4 +199,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 00000000000..fbbd34aca5e --- /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 + +#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 00000000000..165ff69cdbe --- /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 68c9a6bed19..8180c4b6030 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 0756e208e00..0478f0783dc 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 f93ecd5bfe0..e1d5e4d3fa9 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 4c404876b03..63013390e47 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(); From 232ba2ab7d5c77eb1c3c37990cba137a0dfa5eaf Mon Sep 17 00:00:00 2001 From: Taylor Yu Date: Mon, 17 Dec 2018 09:31:16 -0600 Subject: [PATCH 07/10] 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 fbbd34aca5e..c7970dca4df 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 @@ -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 8180c4b6030..08d8924e2da 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 0478f0783dc..49e190dc516 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 e1d5e4d3fa9..d94e73f48ff 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 63013390e47..dc2bb77e9b9 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); From 199f603dddc15a96cdd42a1e319468142a2c45c6 Mon Sep 17 00:00:00 2001 From: Taylor Yu Date: Thu, 20 Dec 2018 09:21:16 -0600 Subject: [PATCH 08/10] Add tests for bootstrap tracker Part of ticket 27617. --- src/test/include.am | 1 + src/test/test.c | 1 + src/test/test.h | 1 + src/test/test_btrack.c | 100 ++++++++++++++++++++ src/test/test_controller_events.c | 147 +++++++++++++++++++++++++++++- 5 files changed, 249 insertions(+), 1 deletion(-) create mode 100644 src/test/test_btrack.c diff --git a/src/test/include.am b/src/test/include.am index 648cf5a5414..4725e8cbaa8 100644 --- a/src/test/include.am +++ b/src/test/include.am @@ -94,6 +94,7 @@ src_test_test_SOURCES += \ src/test/test_address.c \ src/test/test_address_set.c \ src/test/test_bridges.c \ + src/test/test_btrack.c \ src/test/test_buffers.c \ src/test/test_bwmgt.c \ src/test/test_cell_formats.c \ diff --git a/src/test/test.c b/src/test/test.c index 85d41d9863b..13e8c717092 100644 --- a/src/test/test.c +++ b/src/test/test.c @@ -857,6 +857,7 @@ struct testgroup_t testgroups[] = { { "consdiffmgr/", consdiffmgr_tests }, { "container/", container_tests }, { "control/", controller_tests }, + { "control/btrack/", btrack_tests }, { "control/event/", controller_event_tests }, { "crypto/", crypto_tests }, { "crypto/ope/", crypto_ope_tests }, diff --git a/src/test/test.h b/src/test/test.h index 1b10c3d12da..9f754469c82 100644 --- a/src/test/test.h +++ b/src/test/test.h @@ -180,6 +180,7 @@ extern struct testcase_t addr_tests[]; extern struct testcase_t address_set_tests[]; extern struct testcase_t address_tests[]; extern struct testcase_t bridges_tests[]; +extern struct testcase_t btrack_tests[]; extern struct testcase_t buffer_tests[]; extern struct testcase_t bwmgt_tests[]; extern struct testcase_t cell_format_tests[]; diff --git a/src/test/test_btrack.c b/src/test/test_btrack.c new file mode 100644 index 00000000000..7b5d108f981 --- /dev/null +++ b/src/test/test_btrack.c @@ -0,0 +1,100 @@ +/* Copyright (c) 2013-2018, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + +#include "core/or/or.h" + +#include "test/test.h" +#include "test/log_test_helpers.h" + +#define OCIRC_EVENT_PRIVATE +#define ORCONN_EVENT_PRIVATE +#include "core/or/ocirc_event.h" +#include "core/or/orconn_event.h" + +static void +test_btrack_launch(void *arg) +{ + orconn_event_msg_t conn; + ocirc_event_msg_t circ; + + (void)arg; + conn.type = ORCONN_MSGTYPE_STATE; + conn.u.state.gid = 1; + conn.u.state.chan = 1; + conn.u.state.proxy_type = PROXY_NONE; + conn.u.state.state = OR_CONN_STATE_CONNECTING; + + setup_full_capture_of_logs(LOG_DEBUG); + orconn_event_publish(&conn); + expect_log_msg_containing("ORCONN gid=1 chan=1 proxy_type=0 state=1"); + expect_no_log_msg_containing("ORCONN BEST_"); + teardown_capture_of_logs(); + + circ.type = OCIRC_MSGTYPE_CHAN; + circ.u.chan.chan = 1; + circ.u.chan.onehop = true; + + setup_full_capture_of_logs(LOG_DEBUG); + ocirc_event_publish(&circ); + expect_log_msg_containing("ORCONN LAUNCH chan=1 onehop=1"); + expect_log_msg_containing("ORCONN BEST_ANY state -1->1 gid=1"); + teardown_capture_of_logs(); + + conn.u.state.gid = 2; + conn.u.state.chan = 2; + + setup_full_capture_of_logs(LOG_DEBUG); + orconn_event_publish(&conn); + expect_log_msg_containing("ORCONN gid=2 chan=2 proxy_type=0 state=1"); + expect_no_log_msg_containing("ORCONN BEST_"); + teardown_capture_of_logs(); + + circ.u.chan.chan = 2; + circ.u.chan.onehop = false; + + setup_full_capture_of_logs(LOG_DEBUG); + ocirc_event_publish(&circ); + expect_log_msg_containing("ORCONN LAUNCH chan=2 onehop=0"); + expect_log_msg_containing("ORCONN BEST_AP state -1->1 gid=2"); + teardown_capture_of_logs(); + + done: + ; +} + +static void +test_btrack_delete(void *arg) +{ + orconn_event_msg_t conn; + + (void)arg; + conn.type = ORCONN_MSGTYPE_STATE; + conn.u.state.gid = 1; + conn.u.state.chan = 1; + conn.u.state.proxy_type = PROXY_NONE; + conn.u.state.state = OR_CONN_STATE_CONNECTING; + + setup_full_capture_of_logs(LOG_DEBUG); + orconn_event_publish(&conn); + expect_log_msg_containing("ORCONN gid=1 chan=1 proxy_type=0"); + teardown_capture_of_logs(); + + conn.type = ORCONN_MSGTYPE_STATUS; + conn.u.status.gid = 1; + conn.u.status.status = OR_CONN_EVENT_CLOSED; + conn.u.status.reason = 0; + + setup_full_capture_of_logs(LOG_DEBUG); + orconn_event_publish(&conn); + expect_log_msg_containing("ORCONN DELETE gid=1 status=3 reason=0"); + teardown_capture_of_logs(); + + done: + ; +} + +struct testcase_t btrack_tests[] = { + { "launch", test_btrack_launch, TT_FORK, 0, NULL }, + { "delete", test_btrack_delete, TT_FORK, 0, NULL }, + END_OF_TESTCASES +}; diff --git a/src/test/test_controller_events.c b/src/test/test_controller_events.c index dc2bb77e9b9..99e1eb7cb0d 100644 --- a/src/test/test_controller_events.c +++ b/src/test/test_controller_events.c @@ -4,10 +4,14 @@ #define CONNECTION_PRIVATE #define TOR_CHANNEL_INTERNAL_ #define CONTROL_PRIVATE +#define OCIRC_EVENT_PRIVATE +#define ORCONN_EVENT_PRIVATE #include "core/or/or.h" #include "core/or/channel.h" #include "core/or/channeltls.h" #include "core/or/circuitlist.h" +#include "core/or/ocirc_event.h" +#include "core/or/orconn_event.h" #include "core/mainloop/connection.h" #include "feature/control/control.h" #include "test/test.h" @@ -388,7 +392,145 @@ test_cntev_dirboot_defer_orconn(void *arg) UNMOCK(queue_control_event_string); } -#define TEST(name, flags) \ +static void +setup_orconn_state(orconn_event_msg_t *msg, uint64_t gid, uint64_t chan, + int proxy_type) +{ + msg->type = ORCONN_MSGTYPE_STATE; + msg->u.state.gid = gid; + msg->u.state.chan = chan; + msg->u.state.proxy_type = proxy_type; +} + +static void +send_orconn_state(orconn_event_msg_t *msg, uint8_t state) +{ + msg->u.state.state = state; + orconn_event_publish(msg); +} + +static void +send_ocirc_chan(uint32_t gid, uint64_t chan, bool onehop) +{ + ocirc_event_msg_t msg; + + msg.type = OCIRC_MSGTYPE_CHAN; + msg.u.chan.gid = gid; + msg.u.chan.chan = chan; + msg.u.chan.onehop = onehop; + ocirc_event_publish(&msg); +} + +static void +test_cntev_orconn_state(void *arg) +{ + orconn_event_msg_t conn; + + (void)arg; + MOCK(queue_control_event_string, mock_queue_control_event_string); + control_testing_set_global_event_mask(EVENT_MASK_(EVENT_STATUS_CLIENT)); + setup_orconn_state(&conn, 1, 1, PROXY_NONE); + + send_orconn_state(&conn, OR_CONN_STATE_CONNECTING); + send_ocirc_chan(1, 1, true); + assert_bootmsg("5 TAG=conn"); + send_orconn_state(&conn, OR_CONN_STATE_TLS_HANDSHAKING); + assert_bootmsg("10 TAG=conn_done"); + send_orconn_state(&conn, OR_CONN_STATE_OR_HANDSHAKING_V3); + assert_bootmsg("14 TAG=handshake"); + send_orconn_state(&conn, OR_CONN_STATE_OPEN); + assert_bootmsg("15 TAG=handshake_done"); + + conn.u.state.gid = 2; + conn.u.state.chan = 2; + send_orconn_state(&conn, OR_CONN_STATE_CONNECTING); + /* It doesn't know it's an origin circuit yet */ + assert_bootmsg("15 TAG=handshake_done"); + send_ocirc_chan(2, 2, false); + assert_bootmsg("80 TAG=ap_conn"); + send_orconn_state(&conn, OR_CONN_STATE_TLS_HANDSHAKING); + assert_bootmsg("85 TAG=ap_conn_done"); + send_orconn_state(&conn, OR_CONN_STATE_OR_HANDSHAKING_V3); + assert_bootmsg("89 TAG=ap_handshake"); + send_orconn_state(&conn, OR_CONN_STATE_OPEN); + assert_bootmsg("90 TAG=ap_handshake_done"); + + done: + tor_free(saved_event_str); + UNMOCK(queue_control_event_string); +} + +static void +test_cntev_orconn_state_pt(void *arg) +{ + orconn_event_msg_t conn; + + (void)arg; + MOCK(queue_control_event_string, mock_queue_control_event_string); + control_testing_set_global_event_mask(EVENT_MASK_(EVENT_STATUS_CLIENT)); + setup_orconn_state(&conn, 1, 1, PROXY_PLUGGABLE); + send_ocirc_chan(1, 1, true); + + send_orconn_state(&conn, OR_CONN_STATE_CONNECTING); + assert_bootmsg("1 TAG=conn_pt"); + send_orconn_state(&conn, OR_CONN_STATE_PROXY_HANDSHAKING); + assert_bootmsg("2 TAG=conn_done_pt"); + send_orconn_state(&conn, OR_CONN_STATE_TLS_HANDSHAKING); + assert_bootmsg("10 TAG=conn_done"); + send_orconn_state(&conn, OR_CONN_STATE_OR_HANDSHAKING_V3); + assert_bootmsg("14 TAG=handshake"); + send_orconn_state(&conn, OR_CONN_STATE_OPEN); + assert_bootmsg("15 TAG=handshake_done"); + + send_ocirc_chan(2, 2, false); + conn.u.state.gid = 2; + conn.u.state.chan = 2; + send_orconn_state(&conn, OR_CONN_STATE_CONNECTING); + assert_bootmsg("76 TAG=ap_conn_pt"); + send_orconn_state(&conn, OR_CONN_STATE_PROXY_HANDSHAKING); + assert_bootmsg("77 TAG=ap_conn_done_pt"); + + done: + tor_free(saved_event_str); + UNMOCK(queue_control_event_string); +} + +static void +test_cntev_orconn_state_proxy(void *arg) +{ + orconn_event_msg_t conn; + + (void)arg; + MOCK(queue_control_event_string, mock_queue_control_event_string); + control_testing_set_global_event_mask(EVENT_MASK_(EVENT_STATUS_CLIENT)); + setup_orconn_state(&conn, 1, 1, PROXY_CONNECT); + send_ocirc_chan(1, 1, true); + + send_orconn_state(&conn, OR_CONN_STATE_CONNECTING); + assert_bootmsg("3 TAG=conn_proxy"); + send_orconn_state(&conn, OR_CONN_STATE_PROXY_HANDSHAKING); + assert_bootmsg("4 TAG=conn_done_proxy"); + send_orconn_state(&conn, OR_CONN_STATE_TLS_HANDSHAKING); + assert_bootmsg("10 TAG=conn_done"); + send_orconn_state(&conn, OR_CONN_STATE_OR_HANDSHAKING_V3); + assert_bootmsg("14 TAG=handshake"); + send_orconn_state(&conn, OR_CONN_STATE_OPEN); + assert_bootmsg("15 TAG=handshake_done"); + + send_ocirc_chan(2, 2, false); + conn.u.state.gid = 2; + conn.u.state.chan = 2; + send_orconn_state(&conn, OR_CONN_STATE_CONNECTING); + assert_bootmsg("78 TAG=ap_conn_proxy"); + send_orconn_state(&conn, OR_CONN_STATE_PROXY_HANDSHAKING); + assert_bootmsg("79 TAG=ap_conn_done_proxy"); + + done: + tor_free(saved_event_str); + UNMOCK(queue_control_event_string); +} + +#define TEST(name, flags) \ { #name, test_cntev_ ## name, flags, 0, NULL } struct testcase_t controller_event_tests[] = { @@ -398,5 +540,8 @@ struct testcase_t controller_event_tests[] = { TEST(event_mask, TT_FORK), TEST(dirboot_defer_desc, TT_FORK), TEST(dirboot_defer_orconn, TT_FORK), + TEST(orconn_state, TT_FORK), + TEST(orconn_state_pt, TT_FORK), + TEST(orconn_state_proxy, TT_FORK), END_OF_TESTCASES }; From 131e8b77afb64ca9a4f66c3ecbe70644f20cf87c Mon Sep 17 00:00:00 2001 From: Taylor Yu Date: Thu, 20 Dec 2018 15:50:34 -0600 Subject: [PATCH 09/10] changes file for ticket27167 --- changes/ticket27167 | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 changes/ticket27167 diff --git a/changes/ticket27167 b/changes/ticket27167 new file mode 100644 index 00000000000..81c66630c8b --- /dev/null +++ b/changes/ticket27167 @@ -0,0 +1,11 @@ + o Major features (bootstrap): + - Report the first connection to a relay as the earliest phases of + bootstrap progress, regardless of whether it's a connection for + building application circuits. This allows finer-grained + reporting of early progress than previously possible with the + improvements of ticket 27169. Closes tickets 27167 and 27103. + Addresses ticket 27308. + - Separately report the intermediate stage of having connected to + a proxy or pluggable transport, versus succesfully using that + proxy or pluggable transport to connect to a relay. Closes + tickets 27100 and 28884. From 9bf3335b7bf013236c7954eaa7005b5dbe1a6a9f Mon Sep 17 00:00:00 2001 From: Taylor Yu Date: Fri, 21 Dec 2018 11:38:58 -0600 Subject: [PATCH 10/10] fixup! Add ORCONN event pubsub system --- src/feature/control/btrack_orconn.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/feature/control/btrack_orconn.c b/src/feature/control/btrack_orconn.c index 4829d869c37..0fbf5210007 100644 --- a/src/feature/control/btrack_orconn.c +++ b/src/feature/control/btrack_orconn.c @@ -67,6 +67,9 @@ bto_update_best(const bt_orconn_t *bto, bto_best_t *best, const char *type) { if (bto->state < best->state) return false; + /* Update even if we won't change best->state, because it's more + * recent information that a particular connection transitioned to + * that state. */ best->gid = bto->gid; if (bto->state > best->state) { log_info(LD_BTRACK, "ORCONN BEST_%s state %d->%d gid=%"PRIu64, type,