From eb07166eb8aed3a37fe0e5ade476d7084bc0c5e4 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 18 Feb 2020 12:55:57 -0500 Subject: [PATCH 1/6] Make sure callers can handle NULL ext-or auth cookie filename --- src/app/main/shutdown.c | 3 ++- src/feature/client/transports.c | 6 ++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/app/main/shutdown.c b/src/app/main/shutdown.c index 27d92609eb..aac15246b9 100644 --- a/src/app/main/shutdown.c +++ b/src/app/main/shutdown.c @@ -75,7 +75,8 @@ tor_cleanup(void) /* Remove Extended ORPort cookie authentication file */ { char *cookie_fname = get_ext_or_auth_cookie_file_name(); - tor_remove_file(cookie_fname); + if (cookie_fname) + tor_remove_file(cookie_fname); tor_free(cookie_fname); } if (accounting_is_enabled(options)) diff --git a/src/feature/client/transports.c b/src/feature/client/transports.c index a8ea9781a4..55069bb60a 100644 --- a/src/feature/client/transports.c +++ b/src/feature/client/transports.c @@ -1420,8 +1420,10 @@ create_managed_proxy_environment(const managed_proxy_t *mp) smartlist_add_asprintf(envs, "TOR_PT_EXTENDED_SERVER_PORT=%s", ext_or_addrport_tmp); } - smartlist_add_asprintf(envs, "TOR_PT_AUTH_COOKIE_FILE=%s", - cookie_file_loc); + if (cookie_file_loc) { + smartlist_add_asprintf(envs, "TOR_PT_AUTH_COOKIE_FILE=%s", + cookie_file_loc); + } tor_free(ext_or_addrport_tmp); tor_free(cookie_file_loc); From 8ce15933e5e8d9c65c7cca2389e42ff2da604b77 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 18 Feb 2020 13:07:33 -0500 Subject: [PATCH 2/6] Move some declarations into proto_ext_or.h They are for functions declared in that file. --- src/core/proto/proto_ext_or.h | 7 +++++++ src/feature/relay/ext_orport.h | 6 ------ 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/core/proto/proto_ext_or.h b/src/core/proto/proto_ext_or.h index daac3e3eb7..3408599fb7 100644 --- a/src/core/proto/proto_ext_or.h +++ b/src/core/proto/proto_ext_or.h @@ -24,4 +24,11 @@ struct ext_or_cmd_t { int fetch_ext_or_command_from_buf(struct buf_t *buf, struct ext_or_cmd_t **out); +ext_or_cmd_t *ext_or_cmd_new(uint16_t len); + +#define ext_or_cmd_free(cmd) \ + FREE_AND_NULL(ext_or_cmd_t, ext_or_cmd_free_, (cmd)) + +void ext_or_cmd_free_(ext_or_cmd_t *cmd); + #endif /* !defined(TOR_PROTO_EXT_OR_H) */ diff --git a/src/feature/relay/ext_orport.h b/src/feature/relay/ext_orport.h index dbe89fce18..d44c97be8b 100644 --- a/src/feature/relay/ext_orport.h +++ b/src/feature/relay/ext_orport.h @@ -33,12 +33,6 @@ int connection_ext_or_start_auth(or_connection_t *or_conn); -ext_or_cmd_t *ext_or_cmd_new(uint16_t len); - -#define ext_or_cmd_free(cmd) \ - FREE_AND_NULL(ext_or_cmd_t, ext_or_cmd_free_, (cmd)) - -void ext_or_cmd_free_(ext_or_cmd_t *cmd); void connection_or_set_ext_or_identifier(or_connection_t *conn); void connection_or_remove_from_ext_or_id_map(or_connection_t *conn); void connection_or_clear_ext_or_id_map(void); From 1f958b6020ce40484386a722aa31776a67eb5532 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 18 Feb 2020 13:10:57 -0500 Subject: [PATCH 3/6] Move ext_orport identifier map into ext_orport.c There's no need to move the declarations: those were already in ext_orport.h. This shrinks connection_or.c a little. --- src/core/or/connection_or.c | 69 ---------------------------------- src/feature/relay/ext_orport.c | 69 ++++++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 69 deletions(-) diff --git a/src/core/or/connection_or.c b/src/core/or/connection_or.c index 76bfbf0b30..12dca97b75 100644 --- a/src/core/or/connection_or.c +++ b/src/core/or/connection_or.c @@ -109,10 +109,6 @@ TO_OR_CONN(connection_t *c) return DOWNCAST(or_connection_t, c); } -/** Global map between Extended ORPort identifiers and OR - * connections. */ -static digestmap_t *orconn_ext_or_id_map = NULL; - /** Clear clear conn->identity_digest and update other data * structures as appropriate.*/ void @@ -198,71 +194,6 @@ connection_or_set_identity_digest(or_connection_t *conn, channel_set_identity_digest(chan, rsa_digest, ed_id); } -/** Remove the Extended ORPort identifier of conn from the - * global identifier list. Also, clear the identifier from the - * connection itself. */ -void -connection_or_remove_from_ext_or_id_map(or_connection_t *conn) -{ - or_connection_t *tmp; - if (!orconn_ext_or_id_map) - return; - if (!conn->ext_or_conn_id) - return; - - tmp = digestmap_remove(orconn_ext_or_id_map, conn->ext_or_conn_id); - if (!tor_digest_is_zero(conn->ext_or_conn_id)) - tor_assert(tmp == conn); - - memset(conn->ext_or_conn_id, 0, EXT_OR_CONN_ID_LEN); -} - -/** Return the connection whose ext_or_id is id. Return NULL if no such - * connection is found. */ -or_connection_t * -connection_or_get_by_ext_or_id(const char *id) -{ - if (!orconn_ext_or_id_map) - return NULL; - return digestmap_get(orconn_ext_or_id_map, id); -} - -/** Deallocate the global Extended ORPort identifier list */ -void -connection_or_clear_ext_or_id_map(void) -{ - digestmap_free(orconn_ext_or_id_map, NULL); - orconn_ext_or_id_map = NULL; -} - -/** Creates an Extended ORPort identifier for conn and deposits - * it into the global list of identifiers. */ -void -connection_or_set_ext_or_identifier(or_connection_t *conn) -{ - char random_id[EXT_OR_CONN_ID_LEN]; - or_connection_t *tmp; - - if (!orconn_ext_or_id_map) - orconn_ext_or_id_map = digestmap_new(); - - /* Remove any previous identifiers: */ - if (conn->ext_or_conn_id && !tor_digest_is_zero(conn->ext_or_conn_id)) - connection_or_remove_from_ext_or_id_map(conn); - - do { - crypto_rand(random_id, sizeof(random_id)); - } while (digestmap_get(orconn_ext_or_id_map, random_id)); - - if (!conn->ext_or_conn_id) - conn->ext_or_conn_id = tor_malloc_zero(EXT_OR_CONN_ID_LEN); - - memcpy(conn->ext_or_conn_id, random_id, EXT_OR_CONN_ID_LEN); - - tmp = digestmap_set(orconn_ext_or_id_map, random_id, conn); - tor_assert(!tmp); -} - /**************************************************************/ /** Map from a string describing what a non-open OR connection was doing when diff --git a/src/feature/relay/ext_orport.c b/src/feature/relay/ext_orport.c index ce4e043dd7..eada68347e 100644 --- a/src/feature/relay/ext_orport.c +++ b/src/feature/relay/ext_orport.c @@ -652,6 +652,75 @@ connection_ext_or_start_auth(or_connection_t *or_conn) return 0; } +/** Global map between Extended ORPort identifiers and OR + * connections. */ +static digestmap_t *orconn_ext_or_id_map = NULL; + +/** Remove the Extended ORPort identifier of conn from the + * global identifier list. Also, clear the identifier from the + * connection itself. */ +void +connection_or_remove_from_ext_or_id_map(or_connection_t *conn) +{ + or_connection_t *tmp; + if (!orconn_ext_or_id_map) + return; + if (!conn->ext_or_conn_id) + return; + + tmp = digestmap_remove(orconn_ext_or_id_map, conn->ext_or_conn_id); + if (!tor_digest_is_zero(conn->ext_or_conn_id)) + tor_assert(tmp == conn); + + memset(conn->ext_or_conn_id, 0, EXT_OR_CONN_ID_LEN); +} + +/** Return the connection whose ext_or_id is id. Return NULL if no such + * connection is found. */ +or_connection_t * +connection_or_get_by_ext_or_id(const char *id) +{ + if (!orconn_ext_or_id_map) + return NULL; + return digestmap_get(orconn_ext_or_id_map, id); +} + +/** Deallocate the global Extended ORPort identifier list */ +void +connection_or_clear_ext_or_id_map(void) +{ + digestmap_free(orconn_ext_or_id_map, NULL); + orconn_ext_or_id_map = NULL; +} + +/** Creates an Extended ORPort identifier for conn and deposits + * it into the global list of identifiers. */ +void +connection_or_set_ext_or_identifier(or_connection_t *conn) +{ + char random_id[EXT_OR_CONN_ID_LEN]; + or_connection_t *tmp; + + if (!orconn_ext_or_id_map) + orconn_ext_or_id_map = digestmap_new(); + + /* Remove any previous identifiers: */ + if (conn->ext_or_conn_id && !tor_digest_is_zero(conn->ext_or_conn_id)) + connection_or_remove_from_ext_or_id_map(conn); + + do { + crypto_rand(random_id, sizeof(random_id)); + } while (digestmap_get(orconn_ext_or_id_map, random_id)); + + if (!conn->ext_or_conn_id) + conn->ext_or_conn_id = tor_malloc_zero(EXT_OR_CONN_ID_LEN); + + memcpy(conn->ext_or_conn_id, random_id, EXT_OR_CONN_ID_LEN); + + tmp = digestmap_set(orconn_ext_or_id_map, random_id, conn); + tor_assert(!tmp); +} + /** Free any leftover allocated memory of the ext_orport.c subsystem. */ void ext_orport_free_all(void) From a1a3a4d5854e1d44ab7ca7e2a117753a8284a704 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 18 Feb 2020 13:20:19 -0500 Subject: [PATCH 4/6] Make ext_orport.c relay-mode-only. --- changes/ticket33368 | 3 +++ src/feature/relay/ext_orport.h | 37 ++++++++++++++++++++++++++++++++++ src/feature/relay/include.am | 2 +- 3 files changed, 41 insertions(+), 1 deletion(-) create mode 100644 changes/ticket33368 diff --git a/changes/ticket33368 b/changes/ticket33368 new file mode 100644 index 0000000000..ecc6f66f4e --- /dev/null +++ b/changes/ticket33368 @@ -0,0 +1,3 @@ + o Minor features (client-only compilation): + - Disable more code related to the ext_orport protocol when compiling + without support for relay mode. Closes ticket 33368. diff --git a/src/feature/relay/ext_orport.h b/src/feature/relay/ext_orport.h index d44c97be8b..cf13c5d420 100644 --- a/src/feature/relay/ext_orport.h +++ b/src/feature/relay/ext_orport.h @@ -31,6 +31,8 @@ #define EXT_OR_CONN_STATE_FLUSHING 5 #define EXT_OR_CONN_STATE_MAX_ 5 +#ifdef HAVE_MODULE_RELAY + int connection_ext_or_start_auth(or_connection_t *or_conn); void connection_or_set_ext_or_identifier(or_connection_t *conn); @@ -45,6 +47,41 @@ int init_ext_or_cookie_authentication(int is_enabled); char *get_ext_or_auth_cookie_file_name(void); void ext_orport_free_all(void); +#else /* !defined(HAVE_MODULE_RELAY) */ + +static inline int +connection_ext_or_start_auth(or_connection_t *conn) +{ + (void)conn; + tor_assert_nonfatal_unreached(); + return -1; +} +static inline int +connection_ext_or_finished_flushing(or_connection_t *conn) +{ + (void)conn; + tor_assert_nonfatal_unreached(); + return -1; +} +static inline int +connection_ext_or_process_inbuf(or_connection_t *conn) +{ + (void)conn; + tor_assert_nonfatal_unreached(); + return -1; +} +#define connection_or_set_ext_or_identifier(conn) \ + ((void)(conn)) +#define connection_or_remove_from_ext_or_id_map(conn) \ + ((void)(conn)) +#define connection_or_clear_ext_or_id_map() \ + STMT_NIL + +#define get_ext_or_auth_cookie_file_name() \ + (NULL) + +#endif /* defined(HAVE_MODULE_RELAY) */ + #ifdef EXT_ORPORT_PRIVATE STATIC int connection_write_ext_or_command(connection_t *conn, uint16_t command, diff --git a/src/feature/relay/include.am b/src/feature/relay/include.am index a4c025ae12..1cfc0961b7 100644 --- a/src/feature/relay/include.am +++ b/src/feature/relay/include.am @@ -2,7 +2,6 @@ # Legacy shared relay code: migrate to the relay module over time LIBTOR_APP_A_SOURCES += \ src/feature/relay/dns.c \ - src/feature/relay/ext_orport.c \ src/feature/relay/onion_queue.c \ src/feature/relay/router.c \ src/feature/relay/routerkeys.c \ @@ -12,6 +11,7 @@ LIBTOR_APP_A_SOURCES += \ # ADD_C_FILE: INSERT SOURCES HERE. MODULE_RELAY_SOURCES = \ + src/feature/relay/ext_orport.c \ src/feature/relay/routermode.c \ src/feature/relay/relay_config.c \ src/feature/relay/relay_periodic.c \ From df6191f620f92ce2196955a240c45da63c074fee Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 19 Feb 2020 12:04:31 -0500 Subject: [PATCH 5/6] Make connection_or_get_by_ext_or_id() testing-only Apparently it is only used by the unit tests: tor doesn't want it at all. I've opened a new ticket (33383) to we if we should remove this whole feature. --- src/feature/relay/ext_orport.c | 2 ++ src/feature/relay/ext_orport.h | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/feature/relay/ext_orport.c b/src/feature/relay/ext_orport.c index eada68347e..533400bff5 100644 --- a/src/feature/relay/ext_orport.c +++ b/src/feature/relay/ext_orport.c @@ -675,6 +675,7 @@ connection_or_remove_from_ext_or_id_map(or_connection_t *conn) memset(conn->ext_or_conn_id, 0, EXT_OR_CONN_ID_LEN); } +#ifdef TOR_UNIT_TESTS /** Return the connection whose ext_or_id is id. Return NULL if no such * connection is found. */ or_connection_t * @@ -684,6 +685,7 @@ connection_or_get_by_ext_or_id(const char *id) return NULL; return digestmap_get(orconn_ext_or_id_map, id); } +#endif /** Deallocate the global Extended ORPort identifier list */ void diff --git a/src/feature/relay/ext_orport.h b/src/feature/relay/ext_orport.h index cf13c5d420..b09fe3b570 100644 --- a/src/feature/relay/ext_orport.h +++ b/src/feature/relay/ext_orport.h @@ -38,8 +38,6 @@ int connection_ext_or_start_auth(or_connection_t *or_conn); void connection_or_set_ext_or_identifier(or_connection_t *conn); void connection_or_remove_from_ext_or_id_map(or_connection_t *conn); void connection_or_clear_ext_or_id_map(void); -or_connection_t *connection_or_get_by_ext_or_id(const char *id); - int connection_ext_or_finished_flushing(or_connection_t *conn); int connection_ext_or_process_inbuf(or_connection_t *or_conn); @@ -91,9 +89,11 @@ STATIC int handle_client_auth_nonce(const char *client_nonce, size_t client_nonce_len, char **client_hash_out, char **reply_out, size_t *reply_len_out); + #ifdef TOR_UNIT_TESTS extern uint8_t *ext_or_auth_cookie; extern int ext_or_auth_cookie_is_set; +or_connection_t *connection_or_get_by_ext_or_id(const char *id); #endif #endif /* defined(EXT_ORPORT_PRIVATE) */ From 9b21a5d2549bb1ef1ee8f3c8cc92b33ff411ce36 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 19 Feb 2020 12:12:00 -0500 Subject: [PATCH 6/6] Note module-local functions in ext_orport.h Since these are only used inside the feature/relay module, they don't need stubs. --- src/feature/relay/ext_orport.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/feature/relay/ext_orport.h b/src/feature/relay/ext_orport.h index b09fe3b570..416c358397 100644 --- a/src/feature/relay/ext_orport.h +++ b/src/feature/relay/ext_orport.h @@ -40,9 +40,10 @@ void connection_or_remove_from_ext_or_id_map(or_connection_t *conn); void connection_or_clear_ext_or_id_map(void); int connection_ext_or_finished_flushing(or_connection_t *conn); int connection_ext_or_process_inbuf(or_connection_t *or_conn); - -int init_ext_or_cookie_authentication(int is_enabled); char *get_ext_or_auth_cookie_file_name(void); + +/* (No stub needed for these: they are only called within feature/relay.) */ +int init_ext_or_cookie_authentication(int is_enabled); void ext_orport_free_all(void); #else /* !defined(HAVE_MODULE_RELAY) */