August 2020 - tor-commits - lists.torproject.org

[tor/release-0.4.4] Merge branch 'maint-0.4.4' into release-0.4.4
by asn＠torproject.org 12 Aug '20

12 Aug '20

commit 0b1835be925382d9a508cadb1060e890a39927db Merge: cce39dd915 5a1918d7e7 Author: George Kadianakis <desnacked(a)riseup.net> Date: Wed Aug 12 13:47:59 2020 +0300 Merge branch 'maint-0.4.4' into release-0.4.4 changes/bug40083 | 5 +++++ src/feature/relay/relay_periodic.c | 5 ++++- 2 files changed, 9 insertions(+), 1 deletion(-)

1 0

[tor/release-0.4.4] Merge branch 'bug40083_035' into bug40083_042
by asn＠torproject.org 12 Aug '20

12 Aug '20

commit 08de260682ad32a6410f9d26d58020882d1dd859 Merge: 4e684c8695 62361cc698 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Aug 4 13:10:16 2020 -0400 Merge branch 'bug40083_035' into bug40083_042 Fixes conflicts due to code movement. changes/bug40083 | 5 +++++ src/feature/relay/relay_periodic.c | 5 ++++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --cc src/feature/relay/relay_periodic.c index b48b495895,0000000000..8a93118f11 mode 100644,000000..100644 --- a/src/feature/relay/relay_periodic.c +++ b/src/feature/relay/relay_periodic.c @@@ -1,308 -1,0 +1,311 @@@ +/* Copyright (c) 2001 Matej Pfajfar. + * Copyright (c) 2001-2004, Roger Dingledine. + * Copyright (c) 2004-2006, Roger Dingledine, Nick Mathewson. + * Copyright (c) 2007-2019, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + +/** + * @file relay_periodic.c + * @brief Periodic functions for the relay subsytem + **/ + +#include "orconfig.h" +#include "core/or/or.h" + +#include "core/mainloop/periodic.h" +#include "core/mainloop/cpuworker.h" // XXXX use a pubsub event. +#include "core/mainloop/mainloop.h" +#include "core/mainloop/netstatus.h" +#include "core/or/circuituse.h" // XXXX move have_performed_bandwidth_test + +#include "feature/relay/dns.h" +#include "feature/relay/relay_periodic.h" +#include "feature/relay/router.h" +#include "feature/relay/routerkeys.h" +#include "feature/relay/routermode.h" +#include "feature/relay/selftest.h" +#include "feature/stats/predict_ports.h" + +#include "lib/crypt_ops/crypto_rand.h" + +#include "feature/nodelist/routerinfo_st.h" +#include "feature/control/control_events.h" + +#define DECLARE_EVENT(name, roles, flags) \ + static periodic_event_item_t name ## _event = \ + PERIODIC_EVENT(name, \ + PERIODIC_EVENT_ROLE_##roles, \ + flags) + +#define FL(name) (PERIODIC_EVENT_FLAG_##name) + +/** + * Periodic callback: If we're a server and initializing dns failed, retry. + */ +static int +retry_dns_callback(time_t now, const or_options_t *options) +{ + (void)now; +#define RETRY_DNS_INTERVAL (10*60) + if (server_mode(options) && has_dns_init_failed()) + dns_init(); + return RETRY_DNS_INTERVAL; +} + +DECLARE_EVENT(retry_dns, ROUTER, 0); + +static int dns_honesty_first_time = 1; + +/** + * Periodic event: if we're an exit, see if our DNS server is telling us + * obvious lies. + */ +static int +check_dns_honesty_callback(time_t now, const or_options_t *options) +{ + (void)now; + /* 9. and if we're an exit node, check whether our DNS is telling stories + * to us. */ + if (net_is_disabled() || + ! public_server_mode(options) || + router_my_exit_policy_is_reject_star()) + return PERIODIC_EVENT_NO_UPDATE; + + if (dns_honesty_first_time) { + /* Don't launch right when we start */ + dns_honesty_first_time = 0; + return crypto_rand_int_range(60, 180); + } + + dns_launch_correctness_checks(); + return 12*3600 + crypto_rand_int(12*3600); +} + +DECLARE_EVENT(check_dns_honesty, RELAY, FL(NEED_NET)); + +/* Periodic callback: rotate the onion keys after the period defined by the + * "onion-key-rotation-days" consensus parameter, shut down and restart all + * cpuworkers, and update our descriptor if necessary. + */ +static int +rotate_onion_key_callback(time_t now, const or_options_t *options) +{ + if (server_mode(options)) { + int onion_key_lifetime = get_onion_key_lifetime(); + time_t rotation_time = get_onion_key_set_at()+onion_key_lifetime; + if (rotation_time > now) { + return ONION_KEY_CONSENSUS_CHECK_INTERVAL; + } + + log_info(LD_GENERAL,"Rotating onion key."); + rotate_onion_key(); + cpuworkers_rotate_keyinfo(); + if (router_rebuild_descriptor(1)<0) { + log_info(LD_CONFIG, "Couldn't rebuild router descriptor"); + } + if (advertised_server_mode() && !net_is_disabled()) + router_upload_dir_desc_to_dirservers(0); + return ONION_KEY_CONSENSUS_CHECK_INTERVAL; + } + return PERIODIC_EVENT_NO_UPDATE; +} + +DECLARE_EVENT(rotate_onion_key, ROUTER, 0); + +/** Periodic callback: consider rebuilding or and re-uploading our descriptor + * (if we've passed our internal checks). */ +static int +check_descriptor_callback(time_t now, const or_options_t *options) +{ +/** How often do we check whether part of our router info has changed in a + * way that would require an upload? That includes checking whether our IP + * address has changed. */ +#define CHECK_DESCRIPTOR_INTERVAL (60) + + (void)options; + + /* 2b. Once per minute, regenerate and upload the descriptor if the old + * one is inaccurate. */ + if (!net_is_disabled()) { + check_descriptor_bandwidth_changed(now); + check_descriptor_ipaddress_changed(now); + mark_my_descriptor_dirty_if_too_old(now); + consider_publishable_server(0); + } + + return CHECK_DESCRIPTOR_INTERVAL; +} + +DECLARE_EVENT(check_descriptor, ROUTER, FL(NEED_NET)); + +static int dirport_reachability_count = 0; + +/** + * Periodic callback: check whether we're reachable (as a relay), and + * whether our bandwidth has changed enough that we need to + * publish a new descriptor. + */ +static int +check_for_reachability_bw_callback(time_t now, const or_options_t *options) +{ + /* XXXX This whole thing was stuck in the middle of what is now + * XXXX check_descriptor_callback. I'm not sure it's right. */ ++ /** How often should we consider launching reachability tests in our first ++ * TIMEOUT_UNTIL_UNREACHABILITY_COMPLAINT seconds? */ ++#define EARLY_CHECK_REACHABILITY_INTERVAL (60) + + /* also, check religiously for reachability, if it's within the first + * 20 minutes of our uptime. */ + if (server_mode(options) && + (have_completed_a_circuit() || !any_predicted_circuits(now)) && + !net_is_disabled()) { + if (get_uptime() < TIMEOUT_UNTIL_UNREACHABILITY_COMPLAINT) { + router_do_reachability_checks(1, dirport_reachability_count==0); + if (++dirport_reachability_count > 5) + dirport_reachability_count = 0; - return 1; ++ return EARLY_CHECK_REACHABILITY_INTERVAL; + } else { + /* If we haven't checked for 12 hours and our bandwidth estimate is + * low, do another bandwidth test. This is especially important for + * bridges, since they might go long periods without much use. */ + const routerinfo_t *me = router_get_my_routerinfo(); + static int first_time = 1; + if (!first_time && me && + me->bandwidthcapacity < me->bandwidthrate && + me->bandwidthcapacity < 51200) { + reset_bandwidth_test(); + } + first_time = 0; +#define BANDWIDTH_RECHECK_INTERVAL (12*60*60) + return BANDWIDTH_RECHECK_INTERVAL; + } + } + return CHECK_DESCRIPTOR_INTERVAL; +} + +DECLARE_EVENT(check_for_reachability_bw, ROUTER, FL(NEED_NET)); + +/** + * Callback: Send warnings if Tor doesn't find its ports reachable. + */ +static int +reachability_warnings_callback(time_t now, const or_options_t *options) +{ + (void) now; + + if (get_uptime() < TIMEOUT_UNTIL_UNREACHABILITY_COMPLAINT) { + return (int)(TIMEOUT_UNTIL_UNREACHABILITY_COMPLAINT - get_uptime()); + } + + if (server_mode(options) && + !net_is_disabled() && + have_completed_a_circuit()) { + /* every 20 minutes, check and complain if necessary */ + const routerinfo_t *me = router_get_my_routerinfo(); + if (me && !check_whether_orport_reachable(options)) { + char *address = tor_dup_ip(me->addr); + log_warn(LD_CONFIG,"Your server (%s:%d) has not managed to confirm that " + "its ORPort is reachable. Relays do not publish descriptors " + "until their ORPort and DirPort are reachable. Please check " + "your firewalls, ports, address, /etc/hosts file, etc.", + address, me->or_port); + control_event_server_status(LOG_WARN, + "REACHABILITY_FAILED ORADDRESS=%s:%d", + address, me->or_port); + tor_free(address); + } + + if (me && !check_whether_dirport_reachable(options)) { + char *address = tor_dup_ip(me->addr); + log_warn(LD_CONFIG, + "Your server (%s:%d) has not managed to confirm that its " + "DirPort is reachable. Relays do not publish descriptors " + "until their ORPort and DirPort are reachable. Please check " + "your firewalls, ports, address, /etc/hosts file, etc.", + address, me->dir_port); + control_event_server_status(LOG_WARN, + "REACHABILITY_FAILED DIRADDRESS=%s:%d", + address, me->dir_port); + tor_free(address); + } + } + + return TIMEOUT_UNTIL_UNREACHABILITY_COMPLAINT; +} + +DECLARE_EVENT(reachability_warnings, ROUTER, FL(NEED_NET)); + +/* Periodic callback: Every 30 seconds, check whether it's time to make new + * Ed25519 subkeys. + */ +static int +check_ed_keys_callback(time_t now, const or_options_t *options) +{ + if (server_mode(options)) { + if (should_make_new_ed_keys(options, now)) { + int new_signing_key = load_ed_keys(options, now); + if (new_signing_key < 0 || + generate_ed_link_cert(options, now, new_signing_key > 0)) { + log_err(LD_OR, "Unable to update Ed25519 keys! Exiting."); + tor_shutdown_event_loop_and_exit(1); + } + } + return 30; + } + return PERIODIC_EVENT_NO_UPDATE; +} + +DECLARE_EVENT(check_ed_keys, ROUTER, 0); + +/* Period callback: Check if our old onion keys are still valid after the + * period of time defined by the consensus parameter + * "onion-key-grace-period-days", otherwise expire them by setting them to + * NULL. + */ +static int +check_onion_keys_expiry_time_callback(time_t now, const or_options_t *options) +{ + if (server_mode(options)) { + int onion_key_grace_period = get_onion_key_grace_period(); + time_t expiry_time = get_onion_key_set_at()+onion_key_grace_period; + if (expiry_time > now) { + return ONION_KEY_CONSENSUS_CHECK_INTERVAL; + } + + log_info(LD_GENERAL, "Expiring old onion keys."); + expire_old_onion_keys(); + cpuworkers_rotate_keyinfo(); + return ONION_KEY_CONSENSUS_CHECK_INTERVAL; + } + + return PERIODIC_EVENT_NO_UPDATE; +} + +DECLARE_EVENT(check_onion_keys_expiry_time, ROUTER, 0); + +void +relay_register_periodic_events(void) +{ + periodic_events_register(&retry_dns_event); + periodic_events_register(&check_dns_honesty_event); + periodic_events_register(&rotate_onion_key_event); + periodic_events_register(&check_descriptor_event); + periodic_events_register(&check_for_reachability_bw_event); + periodic_events_register(&reachability_warnings_event); + periodic_events_register(&check_ed_keys_event); + periodic_events_register(&check_onion_keys_expiry_time_event); + + dns_honesty_first_time = 1; + dirport_reachability_count = 0; +} + +/** + * Update our schedule so that we'll check whether we need to update our + * descriptor immediately, rather than after up to CHECK_DESCRIPTOR_INTERVAL + * seconds. + */ +void +reschedule_descriptor_update_check(void) +{ + periodic_event_reschedule(&check_descriptor_event); +}

1 0

[tor/maint-0.4.4] Merge remote-tracking branch 'tor-gitlab/mr/104' into maint-0.4.4
by asn＠torproject.org 12 Aug '20

12 Aug '20

commit 8e9edb93be7771e44352c539abdf0b269ac56fab Merge: 1770d1c216 aef9c0f580 Author: George Kadianakis <desnacked(a)riseup.net> Date: Wed Aug 12 13:23:08 2020 +0300 Merge remote-tracking branch 'tor-gitlab/mr/104' into maint-0.4.4 changes/ticket40081 | 6 ++++++ src/core/or/channel.c | 46 +++++++--------------------------------------- src/core/or/channel.h | 9 +++------ src/core/or/channeltls.c | 32 ++++++++++---------------------- src/core/or/circuitbuild.c | 2 ++ src/test/test_channel.c | 12 +++++------- 6 files changed, 33 insertions(+), 74 deletions(-)

1 0

[tor/master] Remove channel_is_canonical_is_reliable()
by asn＠torproject.org 12 Aug '20

12 Aug '20

commit 435f31aed34e9b58d1e8d9f460e6d2e3c6714fbc Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Aug 3 11:25:37 2020 -0400 Remove channel_is_canonical_is_reliable() This function once served to let circuits continue to be built over version-1 link connections. But such connections are long-obsolete, and it's time to remove this check. Closes #40081. --- changes/ticket40081 | 6 ++++++ src/core/or/channel.c | 44 ++++++-------------------------------------- src/core/or/channel.h | 9 +++------ src/core/or/channeltls.c | 32 ++++++++++---------------------- src/core/or/circuitbuild.c | 2 ++ src/test/test_channel.c | 13 +++++-------- 6 files changed, 32 insertions(+), 74 deletions(-) diff --git a/changes/ticket40081 b/changes/ticket40081 new file mode 100644 index 0000000000..683ae33518 --- /dev/null +++ b/changes/ticket40081 @@ -0,0 +1,6 @@ + o Minor features (security): + - Channels using obsolete versions of the Tor link protocol are no + longer allowed to circumvent address-canonicity checks. + (This is only a minor issue, since such channels have no way to + set ed25519 keys, and therefore should always be rejected.) + Closes ticket 40081. diff --git a/src/core/or/channel.c b/src/core/or/channel.c index 3886906875..b3a2d7122b 100644 --- a/src/core/or/channel.c +++ b/src/core/or/channel.c @@ -772,10 +772,9 @@ channel_check_for_duplicates(void) connections_to_relay++; total_relay_connections++; - if (chan->is_canonical(chan, 0)) total_canonical++; + if (chan->is_canonical(chan)) total_canonical++; - if (!chan->is_canonical_to_peer && chan->is_canonical(chan, 0) - && chan->is_canonical(chan, 1)) { + if (!chan->is_canonical_to_peer && chan->is_canonical(chan)) { total_half_canonical++; } } @@ -2431,21 +2430,9 @@ channel_get_for_extend(const char *rsa_id_digest, continue; } - /* Never return a non-canonical connection using a recent link protocol - * if the address is not what we wanted. - * - * The channel_is_canonical_is_reliable() function asks the lower layer - * if we should trust channel_is_canonical(). The below is from the - * comments of the old circuit_or_get_for_extend() and applies when - * the lower-layer transport is channel_tls_t. - * - * (For old link protocols, we can't rely on is_canonical getting - * set properly if we're talking to the right address, since we might - * have an out-of-date descriptor, and we will get no NETINFO cell to - * tell us about the right address.) - */ + /* Only return canonical connections or connections where the address + * is the address we wanted. */ if (!channel_is_canonical(chan) && - channel_is_canonical_is_reliable(chan) && !channel_matches_target_addr_for_extend(chan, target_addr)) { ++n_noncanonical; continue; @@ -2587,16 +2574,12 @@ channel_dump_statistics, (channel_t *chan, int severity)) /* Handle marks */ tor_log(severity, LD_GENERAL, - " * Channel %"PRIu64 " has these marks: %s %s %s " - "%s %s %s", + " * Channel %"PRIu64 " has these marks: %s %s %s %s %s", (chan->global_identifier), channel_is_bad_for_new_circs(chan) ? "bad_for_new_circs" : "!bad_for_new_circs", channel_is_canonical(chan) ? "canonical" : "!canonical", - channel_is_canonical_is_reliable(chan) ? - "is_canonical_is_reliable" : - "!is_canonical_is_reliable", channel_is_client(chan) ? "client" : "!client", channel_is_local(chan) ? @@ -2955,22 +2938,7 @@ channel_is_canonical(channel_t *chan) tor_assert(chan); tor_assert(chan->is_canonical); - return chan->is_canonical(chan, 0); -} - -/** - * Test if the canonical flag is reliable. - * - * This function asks if the lower layer thinks it's safe to trust the - * result of channel_is_canonical(). - */ -int -channel_is_canonical_is_reliable(channel_t *chan) -{ - tor_assert(chan); - tor_assert(chan->is_canonical); - - return chan->is_canonical(chan, 1); + return chan->is_canonical(chan); } /** diff --git a/src/core/or/channel.h b/src/core/or/channel.h index 97aa000337..78e4d90ea5 100644 --- a/src/core/or/channel.h +++ b/src/core/or/channel.h @@ -351,12 +351,10 @@ struct channel_s { /** Check if the lower layer has queued writes */ int (*has_queued_writes)(channel_t *); /** - * If the second param is zero, ask the lower layer if this is - * 'canonical', for a transport-specific definition of canonical; if - * it is 1, ask if the answer to the preceding query is safe to rely - * on. + * Ask the lower layer if this is 'canonical', for a transport-specific + * definition of canonical. */ - int (*is_canonical)(channel_t *, int); + int (*is_canonical)(channel_t *); /** Check if this channel matches a specified extend_info_t */ int (*matches_extend_info)(channel_t *, extend_info_t *); /** Check if this channel matches a target address when extending */ @@ -733,7 +731,6 @@ int channel_has_queued_writes(channel_t *chan); int channel_is_bad_for_new_circs(channel_t *chan); void channel_mark_bad_for_new_circs(channel_t *chan); int channel_is_canonical(channel_t *chan); -int channel_is_canonical_is_reliable(channel_t *chan); int channel_is_client(const channel_t *chan); int channel_is_local(channel_t *chan); int channel_is_incoming(channel_t *chan); diff --git a/src/core/or/channeltls.c b/src/core/or/channeltls.c index f874e39946..299ab88576 100644 --- a/src/core/or/channeltls.c +++ b/src/core/or/channeltls.c @@ -106,7 +106,7 @@ channel_tls_get_transport_name_method(channel_t *chan, char **transport_out); static const char * channel_tls_get_remote_descr_method(channel_t *chan, int flags); static int channel_tls_has_queued_writes_method(channel_t *chan); -static int channel_tls_is_canonical_method(channel_t *chan, int req); +static int channel_tls_is_canonical_method(channel_t *chan); static int channel_tls_matches_extend_info_method(channel_t *chan, extend_info_t *extend_info); @@ -643,12 +643,11 @@ channel_tls_has_queued_writes_method(channel_t *chan) /** * Tell the upper layer if we're canonical. * - * This implements the is_canonical method for channel_tls_t; if req is zero, - * it returns whether this is a canonical channel, and if it is one it returns - * whether that can be relied upon. + * This implements the is_canonical method for channel_tls_t: + * it returns whether this is a canonical channel. */ static int -channel_tls_is_canonical_method(channel_t *chan, int req) +channel_tls_is_canonical_method(channel_t *chan) { int answer = 0; channel_tls_t *tlschan = BASE_CHAN_TO_TLS(chan); @@ -656,24 +655,13 @@ channel_tls_is_canonical_method(channel_t *chan, int req) tor_assert(tlschan); if (tlschan->conn) { - switch (req) { - case 0: - answer = tlschan->conn->is_canonical; - break; - case 1: - /* - * Is the is_canonical bit reliable? In protocols version 2 and up - * we get the canonical address from a NETINFO cell, but in older - * versions it might be based on an obsolete descriptor. - */ - answer = (tlschan->conn->link_proto >= 2); - break; - default: - /* This shouldn't happen; channel.c is broken if it does */ - tor_assert_nonfatal_unreached_once(); - } + /* If this bit is set to 0, and link_proto is sufficiently old, then we + * can't actually _rely_ on this being a non-canonical channel. + * Nonetheless, we're going to believe that this is a non-canonical + * channel in this case, since nobody should be using these link protocols + * any more. */ + answer = tlschan->conn->is_canonical; } - /* else return 0 for tlschan->conn == NULL */ return answer; } diff --git a/src/core/or/circuitbuild.c b/src/core/or/circuitbuild.c index f3a5791d6c..1a5025cd71 100644 --- a/src/core/or/circuitbuild.c +++ b/src/core/or/circuitbuild.c @@ -707,6 +707,8 @@ circuit_deliver_create_cell(circuit_t *circ, const create_cell_t *create_cell, goto error; } + tor_assert_nonfatal_once(circ->n_chan->is_canonical); + memset(&cell, 0, sizeof(cell_t)); r = relayed ? create_cell_format_relayed(&cell, create_cell) : create_cell_format(&cell, create_cell); diff --git a/src/test/test_channel.c b/src/test/test_channel.c index e55b9b0750..afb7db813c 100644 --- a/src/test/test_channel.c +++ b/src/test/test_channel.c @@ -46,7 +46,6 @@ static int dump_statistics_mock_matches = 0; static int test_close_called = 0; static int test_chan_should_be_canonical = 0; static int test_chan_should_match_target = 0; -static int test_chan_canonical_should_be_reliable = 0; static int test_chan_listener_close_fn_called = 0; static int test_chan_listener_fn_called = 0; @@ -357,14 +356,10 @@ scheduler_release_channel_mock(channel_t *ch) } static int -test_chan_is_canonical(channel_t *chan, int req) +test_chan_is_canonical(channel_t *chan) { tor_assert(chan); - if (req && test_chan_canonical_should_be_reliable) { - return 1; - } - if (test_chan_should_be_canonical) { return 1; } @@ -1381,6 +1376,9 @@ test_channel_for_extend(void *arg) /* Make it older than chan1. */ chan2->timestamp_created = chan1->timestamp_created - 1; + /* Say it's all canonical. */ + test_chan_should_be_canonical = 1; + /* Set channel identities and add it to the channel map. The last one to be * added is made the first one in the list so the lookup will always return * that one first. */ @@ -1475,8 +1473,8 @@ test_channel_for_extend(void *arg) chan2->is_bad_for_new_circs = 0; /* Non canonical channels. */ + test_chan_should_be_canonical = 0; test_chan_should_match_target = 0; - test_chan_canonical_should_be_reliable = 1; ret_chan = channel_get_for_extend(digest, &ed_id, &addr, &msg, &launch); tt_assert(!ret_chan); tt_str_op(msg, OP_EQ, "Connections all too old, or too non-canonical. " @@ -1567,4 +1565,3 @@ struct testcase_t channel_tests[] = { NULL, NULL }, END_OF_TESTCASES }; -

1 0

[tor/maint-0.4.4] Merge branch 'ticket40081_035' into ticket40081_044
by asn＠torproject.org 12 Aug '20

12 Aug '20

commit aef9c0f5809d2978f16f08ada1f3e482bd4dbb6f Merge: 18d2c7c5d7 435f31aed3 Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Aug 3 11:49:09 2020 -0400 Merge branch 'ticket40081_035' into ticket40081_044 Resolves conflicts: src/core/or/channel.c src/test/test_channel.c changes/ticket40081 | 6 ++++++ src/core/or/channel.c | 46 +++++++--------------------------------------- src/core/or/channel.h | 9 +++------ src/core/or/channeltls.c | 32 ++++++++++---------------------- src/core/or/circuitbuild.c | 2 ++ src/test/test_channel.c | 12 +++++------- 6 files changed, 33 insertions(+), 74 deletions(-) diff --cc src/core/or/channel.c index 09986df404,b3a2d7122b..5ed6bb9272 --- a/src/core/or/channel.c +++ b/src/core/or/channel.c @@@ -784,13 -771,10 +784,12 @@@ channel_check_for_duplicates(void connections_to_relay++; total_relay_connections++; + if (is_dirauth) + total_dirauth_connections++; - if (chan->is_canonical(chan, 0)) total_canonical++; + if (chan->is_canonical(chan)) total_canonical++; - if (!chan->is_canonical_to_peer && chan->is_canonical(chan, 0) - && chan->is_canonical(chan, 1)) { + if (!chan->is_canonical_to_peer && chan->is_canonical(chan)) { total_half_canonical++; } } @@@ -2457,21 -2430,10 +2456,9 @@@ channel_get_for_extend,(const char *rsa continue; } - /* If the connection is using a recent link protocol, only return canonical - * connections, when the address is one of the addresses we wanted. - * - * The channel_is_canonical_is_reliable() function asks the lower layer - * if we should trust channel_is_canonical(). It only applies when - * the lower-layer transport is channel_tls_t. - * - * For old link protocols, we can't rely on is_canonical getting - * set properly if we're talking to the right address, since we might - * have an out-of-date descriptor, and we will get no NETINFO cell to - * tell us about the right address. - */ - if (!channel_is_canonical(chan) && - channel_is_canonical_is_reliable(chan) && - !matches_target) { + /* Only return canonical connections or connections where the address + * is the address we wanted. */ - if (!channel_is_canonical(chan) && - !channel_matches_target_addr_for_extend(chan, target_addr)) { ++ if (!channel_is_canonical(chan) && !matches_target) { ++n_noncanonical; continue; } diff --cc src/test/test_channel.c index 849cc497fc,afb7db813c..83b69cc80b --- a/src/test/test_channel.c +++ b/src/test/test_channel.c @@@ -1463,10 -1473,9 +1461,10 @@@ test_channel_for_extend(void *arg chan2->is_bad_for_new_circs = 0; /* Non canonical channels. */ + test_chan_should_be_canonical = 0; test_chan_should_match_target = 0; - test_chan_canonical_should_be_reliable = 1; - ret_chan = channel_get_for_extend(digest, &ed_id, &addr, &msg, &launch); + ret_chan = channel_get_for_extend(digest, &ed_id, &ipv4_addr, &ipv6_addr, + &msg, &launch); tt_assert(!ret_chan); tt_str_op(msg, OP_EQ, "Connections all too old, or too non-canonical. " " Launching a new one.");

1 0

[tor/master] Merge branch 'ticket40081_035' into ticket40081_044
by asn＠torproject.org 12 Aug '20

12 Aug '20

commit aef9c0f5809d2978f16f08ada1f3e482bd4dbb6f Merge: 18d2c7c5d7 435f31aed3 Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Aug 3 11:49:09 2020 -0400 Merge branch 'ticket40081_035' into ticket40081_044 Resolves conflicts: src/core/or/channel.c src/test/test_channel.c changes/ticket40081 | 6 ++++++ src/core/or/channel.c | 46 +++++++--------------------------------------- src/core/or/channel.h | 9 +++------ src/core/or/channeltls.c | 32 ++++++++++---------------------- src/core/or/circuitbuild.c | 2 ++ src/test/test_channel.c | 12 +++++------- 6 files changed, 33 insertions(+), 74 deletions(-) diff --cc src/core/or/channel.c index 09986df404,b3a2d7122b..5ed6bb9272 --- a/src/core/or/channel.c +++ b/src/core/or/channel.c @@@ -784,13 -771,10 +784,12 @@@ channel_check_for_duplicates(void connections_to_relay++; total_relay_connections++; + if (is_dirauth) + total_dirauth_connections++; - if (chan->is_canonical(chan, 0)) total_canonical++; + if (chan->is_canonical(chan)) total_canonical++; - if (!chan->is_canonical_to_peer && chan->is_canonical(chan, 0) - && chan->is_canonical(chan, 1)) { + if (!chan->is_canonical_to_peer && chan->is_canonical(chan)) { total_half_canonical++; } } @@@ -2457,21 -2430,10 +2456,9 @@@ channel_get_for_extend,(const char *rsa continue; } - /* If the connection is using a recent link protocol, only return canonical - * connections, when the address is one of the addresses we wanted. - * - * The channel_is_canonical_is_reliable() function asks the lower layer - * if we should trust channel_is_canonical(). It only applies when - * the lower-layer transport is channel_tls_t. - * - * For old link protocols, we can't rely on is_canonical getting - * set properly if we're talking to the right address, since we might - * have an out-of-date descriptor, and we will get no NETINFO cell to - * tell us about the right address. - */ - if (!channel_is_canonical(chan) && - channel_is_canonical_is_reliable(chan) && - !matches_target) { + /* Only return canonical connections or connections where the address + * is the address we wanted. */ - if (!channel_is_canonical(chan) && - !channel_matches_target_addr_for_extend(chan, target_addr)) { ++ if (!channel_is_canonical(chan) && !matches_target) { ++n_noncanonical; continue; } diff --cc src/test/test_channel.c index 849cc497fc,afb7db813c..83b69cc80b --- a/src/test/test_channel.c +++ b/src/test/test_channel.c @@@ -1463,10 -1473,9 +1461,10 @@@ test_channel_for_extend(void *arg chan2->is_bad_for_new_circs = 0; /* Non canonical channels. */ + test_chan_should_be_canonical = 0; test_chan_should_match_target = 0; - test_chan_canonical_should_be_reliable = 1; - ret_chan = channel_get_for_extend(digest, &ed_id, &addr, &msg, &launch); + ret_chan = channel_get_for_extend(digest, &ed_id, &ipv4_addr, &ipv6_addr, + &msg, &launch); tt_assert(!ret_chan); tt_str_op(msg, OP_EQ, "Connections all too old, or too non-canonical. " " Launching a new one.");

1 0

[tor/maint-0.4.4] Remove channel_is_canonical_is_reliable()
by asn＠torproject.org 12 Aug '20

12 Aug '20

commit 435f31aed34e9b58d1e8d9f460e6d2e3c6714fbc Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Aug 3 11:25:37 2020 -0400 Remove channel_is_canonical_is_reliable() This function once served to let circuits continue to be built over version-1 link connections. But such connections are long-obsolete, and it's time to remove this check. Closes #40081. --- changes/ticket40081 | 6 ++++++ src/core/or/channel.c | 44 ++++++-------------------------------------- src/core/or/channel.h | 9 +++------ src/core/or/channeltls.c | 32 ++++++++++---------------------- src/core/or/circuitbuild.c | 2 ++ src/test/test_channel.c | 13 +++++-------- 6 files changed, 32 insertions(+), 74 deletions(-) diff --git a/changes/ticket40081 b/changes/ticket40081 new file mode 100644 index 0000000000..683ae33518 --- /dev/null +++ b/changes/ticket40081 @@ -0,0 +1,6 @@ + o Minor features (security): + - Channels using obsolete versions of the Tor link protocol are no + longer allowed to circumvent address-canonicity checks. + (This is only a minor issue, since such channels have no way to + set ed25519 keys, and therefore should always be rejected.) + Closes ticket 40081. diff --git a/src/core/or/channel.c b/src/core/or/channel.c index 3886906875..b3a2d7122b 100644 --- a/src/core/or/channel.c +++ b/src/core/or/channel.c @@ -772,10 +772,9 @@ channel_check_for_duplicates(void) connections_to_relay++; total_relay_connections++; - if (chan->is_canonical(chan, 0)) total_canonical++; + if (chan->is_canonical(chan)) total_canonical++; - if (!chan->is_canonical_to_peer && chan->is_canonical(chan, 0) - && chan->is_canonical(chan, 1)) { + if (!chan->is_canonical_to_peer && chan->is_canonical(chan)) { total_half_canonical++; } } @@ -2431,21 +2430,9 @@ channel_get_for_extend(const char *rsa_id_digest, continue; } - /* Never return a non-canonical connection using a recent link protocol - * if the address is not what we wanted. - * - * The channel_is_canonical_is_reliable() function asks the lower layer - * if we should trust channel_is_canonical(). The below is from the - * comments of the old circuit_or_get_for_extend() and applies when - * the lower-layer transport is channel_tls_t. - * - * (For old link protocols, we can't rely on is_canonical getting - * set properly if we're talking to the right address, since we might - * have an out-of-date descriptor, and we will get no NETINFO cell to - * tell us about the right address.) - */ + /* Only return canonical connections or connections where the address + * is the address we wanted. */ if (!channel_is_canonical(chan) && - channel_is_canonical_is_reliable(chan) && !channel_matches_target_addr_for_extend(chan, target_addr)) { ++n_noncanonical; continue; @@ -2587,16 +2574,12 @@ channel_dump_statistics, (channel_t *chan, int severity)) /* Handle marks */ tor_log(severity, LD_GENERAL, - " * Channel %"PRIu64 " has these marks: %s %s %s " - "%s %s %s", + " * Channel %"PRIu64 " has these marks: %s %s %s %s %s", (chan->global_identifier), channel_is_bad_for_new_circs(chan) ? "bad_for_new_circs" : "!bad_for_new_circs", channel_is_canonical(chan) ? "canonical" : "!canonical", - channel_is_canonical_is_reliable(chan) ? - "is_canonical_is_reliable" : - "!is_canonical_is_reliable", channel_is_client(chan) ? "client" : "!client", channel_is_local(chan) ? @@ -2955,22 +2938,7 @@ channel_is_canonical(channel_t *chan) tor_assert(chan); tor_assert(chan->is_canonical); - return chan->is_canonical(chan, 0); -} - -/** - * Test if the canonical flag is reliable. - * - * This function asks if the lower layer thinks it's safe to trust the - * result of channel_is_canonical(). - */ -int -channel_is_canonical_is_reliable(channel_t *chan) -{ - tor_assert(chan); - tor_assert(chan->is_canonical); - - return chan->is_canonical(chan, 1); + return chan->is_canonical(chan); } /** diff --git a/src/core/or/channel.h b/src/core/or/channel.h index 97aa000337..78e4d90ea5 100644 --- a/src/core/or/channel.h +++ b/src/core/or/channel.h @@ -351,12 +351,10 @@ struct channel_s { /** Check if the lower layer has queued writes */ int (*has_queued_writes)(channel_t *); /** - * If the second param is zero, ask the lower layer if this is - * 'canonical', for a transport-specific definition of canonical; if - * it is 1, ask if the answer to the preceding query is safe to rely - * on. + * Ask the lower layer if this is 'canonical', for a transport-specific + * definition of canonical. */ - int (*is_canonical)(channel_t *, int); + int (*is_canonical)(channel_t *); /** Check if this channel matches a specified extend_info_t */ int (*matches_extend_info)(channel_t *, extend_info_t *); /** Check if this channel matches a target address when extending */ @@ -733,7 +731,6 @@ int channel_has_queued_writes(channel_t *chan); int channel_is_bad_for_new_circs(channel_t *chan); void channel_mark_bad_for_new_circs(channel_t *chan); int channel_is_canonical(channel_t *chan); -int channel_is_canonical_is_reliable(channel_t *chan); int channel_is_client(const channel_t *chan); int channel_is_local(channel_t *chan); int channel_is_incoming(channel_t *chan); diff --git a/src/core/or/channeltls.c b/src/core/or/channeltls.c index f874e39946..299ab88576 100644 --- a/src/core/or/channeltls.c +++ b/src/core/or/channeltls.c @@ -106,7 +106,7 @@ channel_tls_get_transport_name_method(channel_t *chan, char **transport_out); static const char * channel_tls_get_remote_descr_method(channel_t *chan, int flags); static int channel_tls_has_queued_writes_method(channel_t *chan); -static int channel_tls_is_canonical_method(channel_t *chan, int req); +static int channel_tls_is_canonical_method(channel_t *chan); static int channel_tls_matches_extend_info_method(channel_t *chan, extend_info_t *extend_info); @@ -643,12 +643,11 @@ channel_tls_has_queued_writes_method(channel_t *chan) /** * Tell the upper layer if we're canonical. * - * This implements the is_canonical method for channel_tls_t; if req is zero, - * it returns whether this is a canonical channel, and if it is one it returns - * whether that can be relied upon. + * This implements the is_canonical method for channel_tls_t: + * it returns whether this is a canonical channel. */ static int -channel_tls_is_canonical_method(channel_t *chan, int req) +channel_tls_is_canonical_method(channel_t *chan) { int answer = 0; channel_tls_t *tlschan = BASE_CHAN_TO_TLS(chan); @@ -656,24 +655,13 @@ channel_tls_is_canonical_method(channel_t *chan, int req) tor_assert(tlschan); if (tlschan->conn) { - switch (req) { - case 0: - answer = tlschan->conn->is_canonical; - break; - case 1: - /* - * Is the is_canonical bit reliable? In protocols version 2 and up - * we get the canonical address from a NETINFO cell, but in older - * versions it might be based on an obsolete descriptor. - */ - answer = (tlschan->conn->link_proto >= 2); - break; - default: - /* This shouldn't happen; channel.c is broken if it does */ - tor_assert_nonfatal_unreached_once(); - } + /* If this bit is set to 0, and link_proto is sufficiently old, then we + * can't actually _rely_ on this being a non-canonical channel. + * Nonetheless, we're going to believe that this is a non-canonical + * channel in this case, since nobody should be using these link protocols + * any more. */ + answer = tlschan->conn->is_canonical; } - /* else return 0 for tlschan->conn == NULL */ return answer; } diff --git a/src/core/or/circuitbuild.c b/src/core/or/circuitbuild.c index f3a5791d6c..1a5025cd71 100644 --- a/src/core/or/circuitbuild.c +++ b/src/core/or/circuitbuild.c @@ -707,6 +707,8 @@ circuit_deliver_create_cell(circuit_t *circ, const create_cell_t *create_cell, goto error; } + tor_assert_nonfatal_once(circ->n_chan->is_canonical); + memset(&cell, 0, sizeof(cell_t)); r = relayed ? create_cell_format_relayed(&cell, create_cell) : create_cell_format(&cell, create_cell); diff --git a/src/test/test_channel.c b/src/test/test_channel.c index e55b9b0750..afb7db813c 100644 --- a/src/test/test_channel.c +++ b/src/test/test_channel.c @@ -46,7 +46,6 @@ static int dump_statistics_mock_matches = 0; static int test_close_called = 0; static int test_chan_should_be_canonical = 0; static int test_chan_should_match_target = 0; -static int test_chan_canonical_should_be_reliable = 0; static int test_chan_listener_close_fn_called = 0; static int test_chan_listener_fn_called = 0; @@ -357,14 +356,10 @@ scheduler_release_channel_mock(channel_t *ch) } static int -test_chan_is_canonical(channel_t *chan, int req) +test_chan_is_canonical(channel_t *chan) { tor_assert(chan); - if (req && test_chan_canonical_should_be_reliable) { - return 1; - } - if (test_chan_should_be_canonical) { return 1; } @@ -1381,6 +1376,9 @@ test_channel_for_extend(void *arg) /* Make it older than chan1. */ chan2->timestamp_created = chan1->timestamp_created - 1; + /* Say it's all canonical. */ + test_chan_should_be_canonical = 1; + /* Set channel identities and add it to the channel map. The last one to be * added is made the first one in the list so the lookup will always return * that one first. */ @@ -1475,8 +1473,8 @@ test_channel_for_extend(void *arg) chan2->is_bad_for_new_circs = 0; /* Non canonical channels. */ + test_chan_should_be_canonical = 0; test_chan_should_match_target = 0; - test_chan_canonical_should_be_reliable = 1; ret_chan = channel_get_for_extend(digest, &ed_id, &addr, &msg, &launch); tt_assert(!ret_chan); tt_str_op(msg, OP_EQ, "Connections all too old, or too non-canonical. " @@ -1567,4 +1565,3 @@ struct testcase_t channel_tests[] = { NULL, NULL }, END_OF_TESTCASES }; -

1 0

[tor/master] Merge branch 'maint-0.4.4'
by asn＠torproject.org 12 Aug '20

12 Aug '20

commit c591a0d2466cdc644fd82519f6f1ec32da764287 Merge: 2f708079cd 8e9edb93be Author: George Kadianakis <desnacked(a)riseup.net> Date: Wed Aug 12 13:23:33 2020 +0300 Merge branch 'maint-0.4.4' changes/ticket40081 | 6 ++++++ src/core/or/channel.c | 46 +++++++--------------------------------------- src/core/or/channel.h | 9 +++------ src/core/or/channeltls.c | 32 ++++++++++---------------------- src/core/or/circuitbuild.c | 2 ++ src/test/test_channel.c | 12 +++++------- 6 files changed, 33 insertions(+), 74 deletions(-) diff --cc src/core/or/channeltls.c index ae60038c34,56552f0095..51b772728e --- a/src/core/or/channeltls.c +++ b/src/core/or/channeltls.c @@@ -103,13 -102,14 +103,13 @@@ static void channel_tls_close_method(ch static const char * channel_tls_describe_transport_method(channel_t *chan); static void channel_tls_free_method(channel_t *chan); static double channel_tls_get_overhead_estimate_method(channel_t *chan); -static int -channel_tls_get_remote_addr_method(channel_t *chan, tor_addr_t *addr_out); +static int channel_tls_get_remote_addr_method(const channel_t *chan, + tor_addr_t *addr_out); static int channel_tls_get_transport_name_method(channel_t *chan, char **transport_out); -static const char * -channel_tls_get_remote_descr_method(channel_t *chan, int flags); +static const char *channel_tls_describe_peer_method(const channel_t *chan); static int channel_tls_has_queued_writes_method(channel_t *chan); - static int channel_tls_is_canonical_method(channel_t *chan, int req); + static int channel_tls_is_canonical_method(channel_t *chan); static int channel_tls_matches_extend_info_method(channel_t *chan, extend_info_t *extend_info);

1 0

[tor/master] Merge remote-tracking branch 'tor-gitlab/mr/104' into maint-0.4.4
by asn＠torproject.org 12 Aug '20

12 Aug '20

commit 8e9edb93be7771e44352c539abdf0b269ac56fab Merge: 1770d1c216 aef9c0f580 Author: George Kadianakis <desnacked(a)riseup.net> Date: Wed Aug 12 13:23:08 2020 +0300 Merge remote-tracking branch 'tor-gitlab/mr/104' into maint-0.4.4 changes/ticket40081 | 6 ++++++ src/core/or/channel.c | 46 +++++++--------------------------------------- src/core/or/channel.h | 9 +++------ src/core/or/channeltls.c | 32 ++++++++++---------------------- src/core/or/circuitbuild.c | 2 ++ src/test/test_channel.c | 12 +++++------- 6 files changed, 33 insertions(+), 74 deletions(-)

1 0

[tor/release-0.4.4] Merge remote-tracking branch 'tor-gitlab/mr/104' into maint-0.4.4
by asn＠torproject.org 12 Aug '20

12 Aug '20

commit 8e9edb93be7771e44352c539abdf0b269ac56fab Merge: 1770d1c216 aef9c0f580 Author: George Kadianakis <desnacked(a)riseup.net> Date: Wed Aug 12 13:23:08 2020 +0300 Merge remote-tracking branch 'tor-gitlab/mr/104' into maint-0.4.4 changes/ticket40081 | 6 ++++++ src/core/or/channel.c | 46 +++++++--------------------------------------- src/core/or/channel.h | 9 +++------ src/core/or/channeltls.c | 32 ++++++++++---------------------- src/core/or/circuitbuild.c | 2 ++ src/test/test_channel.c | 12 +++++------- 6 files changed, 33 insertions(+), 74 deletions(-)

1 0