commit a0b4fa1f167f9b013ee1a8326e325ec3f97e4700 Author: Taylor Yu catalyst@torproject.org Date: Sun Dec 16 17:01:25 2018 -0600

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(-)

diff --git a/src/app/main/subsystem_list.c b/src/app/main/subsystem_list.c index 185873809..ef9b8142d 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 a1fae7360..e171e4a6f 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 d3744dc1c..3de8da9cd 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 0aa21000a..c4b5f7ee3 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 <b>circ</b> to <b>state</b>, 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 <b>out</b> 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 cb89d1820..37d37a089 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 0429f2c86..61d5e18a4 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 088358a4d..e230ad100 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 000000000..f9f8af279 --- /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 000000000..19a237d7d --- /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 <stdbool.h> + +#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 000000000..0bc135ffa --- /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 4ef550c91..da62c9498 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 d3245fcaf..68c9a6bed 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 . */