January 2021 - tor-commits - lists.torproject.org

[tor/release-0.4.5] Introduce a new bridge_has_invalid_transport() function.
by nickm＠torproject.org 21 Jan '21

21 Jan '21

commit 71fd30b75ad028dfce69563792547f06fdf9d3c2 Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Jan 21 13:17:16 2021 -0500 Introduce a new bridge_has_invalid_transport() function. In addition to simplifying callsites a little, this function gives correct behavior for bridges without a configured transport. --- src/feature/client/bridges.c | 13 ++++++++++++- src/feature/client/bridges.h | 1 + src/feature/client/entrynodes.c | 2 +- 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/feature/client/bridges.c b/src/feature/client/bridges.c index 11b2ffd62d..96c3497c6f 100644 --- a/src/feature/client/bridges.c +++ b/src/feature/client/bridges.c @@ -175,6 +175,17 @@ bridget_get_transport_name(const bridge_info_t *bridge) return bridge->transport_name; } +/** + * Return true if @a bridge has a transport name for which we don't actually + * know a transport. + */ +bool +bridge_has_invalid_transport(const bridge_info_t *bridge) +{ + const char *tname = bridget_get_transport_name(bridge); + return tname && transport_get_by_name(tname) == NULL; +} + /** If we have a bridge configured whose digest matches <b>digest</b>, or a * bridge with no known digest whose address matches any of the * tor_addr_port_t's in <b>orports</b>, return that bridge. Else return @@ -656,7 +667,7 @@ launch_direct_bridge_descriptor_fetch(bridge_info_t *bridge) DIR_PURPOSE_FETCH_SERVERDESC)) return; /* it's already on the way */ - if (transport_get_by_name(bridget_get_transport_name(bridge)) == NULL) { + if (bridge_has_invalid_transport(bridge)) { download_status_mark_impossible(&bridge->fetch_status); log_warn(LD_CONFIG, "Can't use bridge at %s: there is no configured " "transport called \"%s\".", diff --git a/src/feature/client/bridges.h b/src/feature/client/bridges.h index 1b090e8649..f5ecc1b76d 100644 --- a/src/feature/client/bridges.h +++ b/src/feature/client/bridges.h @@ -24,6 +24,7 @@ const smartlist_t *bridge_list_get(void); const uint8_t *bridge_get_rsa_id_digest(const bridge_info_t *bridge); const tor_addr_port_t * bridge_get_addr_port(const bridge_info_t *bridge); const char *bridget_get_transport_name(const bridge_info_t *bridge); +bool bridge_has_invalid_transport(const bridge_info_t *bridge); bridge_info_t *get_configured_bridge_by_addr_port_digest( const tor_addr_t *addr, uint16_t port, diff --git a/src/feature/client/entrynodes.c b/src/feature/client/entrynodes.c index 2676df6aae..232216c521 100644 --- a/src/feature/client/entrynodes.c +++ b/src/feature/client/entrynodes.c @@ -804,7 +804,7 @@ get_sampled_guard_for_bridge(guard_selection_t *gs, entry_guard_t *guard; if (BUG(!addrport)) return NULL; // LCOV_EXCL_LINE - if (!transport_get_by_name(bridget_get_transport_name(bridge))) { + if (bridge_has_invalid_transport(bridge)) { return NULL; } guard = get_sampled_guard_by_bridge_addr(gs, addrport);

1 0

[tor/release-0.4.5] Merge branch 'maint-0.4.5' into release-0.4.5
by nickm＠torproject.org 21 Jan '21

21 Jan '21

commit 397fc7a1682a8d3a57998fa95aba60ad3d87fd15 Merge: 906d1b24f0 71fd30b75a Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Jan 21 13:19:39 2021 -0500 Merge branch 'maint-0.4.5' into release-0.4.5 changes/ticket40106 | 6 ++++++ src/app/config/config.c | 17 ----------------- src/feature/client/bridges.c | 20 ++++++++++++++++++++ src/feature/client/bridges.h | 1 + src/feature/client/entrynodes.c | 3 +++ 5 files changed, 30 insertions(+), 17 deletions(-)

1 0

[tor/maint-0.4.5] bridge: Don't initiate connection without a transport
by nickm＠torproject.org 21 Jan '21

21 Jan '21

commit 09c6d0324626ffa349c7eed66d9ede92ecd71583 Author: David Goulet <dgoulet(a)torproject.org> Date: Wed Jan 20 10:31:30 2021 -0500 bridge: Don't initiate connection without a transport Don't pick the bridge as the guard or launch descriptor fetch if no transport is found. Fixes #40106 Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/feature/client/bridges.c | 9 +++++++++ src/feature/client/entrynodes.c | 3 +++ 2 files changed, 12 insertions(+) diff --git a/src/feature/client/bridges.c b/src/feature/client/bridges.c index 8e2bb01661..11b2ffd62d 100644 --- a/src/feature/client/bridges.c +++ b/src/feature/client/bridges.c @@ -656,6 +656,15 @@ launch_direct_bridge_descriptor_fetch(bridge_info_t *bridge) DIR_PURPOSE_FETCH_SERVERDESC)) return; /* it's already on the way */ + if (transport_get_by_name(bridget_get_transport_name(bridge)) == NULL) { + download_status_mark_impossible(&bridge->fetch_status); + log_warn(LD_CONFIG, "Can't use bridge at %s: there is no configured " + "transport called \"%s\".", + safe_str_client(fmt_and_decorate_addr(&bridge->addr)), + bridget_get_transport_name(bridge)); + return; /* Can't use this bridge; it has not */ + } + if (routerset_contains_bridge(options->ExcludeNodes, bridge)) { download_status_mark_impossible(&bridge->fetch_status); log_warn(LD_APP, "Not using bridge at %s: it is in ExcludeNodes.", diff --git a/src/feature/client/entrynodes.c b/src/feature/client/entrynodes.c index 82866ea668..2676df6aae 100644 --- a/src/feature/client/entrynodes.c +++ b/src/feature/client/entrynodes.c @@ -804,6 +804,9 @@ get_sampled_guard_for_bridge(guard_selection_t *gs, entry_guard_t *guard; if (BUG(!addrport)) return NULL; // LCOV_EXCL_LINE + if (!transport_get_by_name(bridget_get_transport_name(bridge))) { + return NULL; + } guard = get_sampled_guard_by_bridge_addr(gs, addrport); if (! guard || (id && tor_memneq(id, guard->identity, DIGEST_LEN))) return NULL;

1 0

[tor/maint-0.4.5] config: Remove Bridge <-> ClientTransportPlugin validation
by nickm＠torproject.org 21 Jan '21

21 Jan '21

commit 7692f443d4ba5c79c8acb74991b614685345c406 Author: David Goulet <dgoulet(a)torproject.org> Date: Wed Jan 20 11:24:47 2021 -0500 config: Remove Bridge <-> ClientTransportPlugin validation This validation was only done if DisableNetwork was off because we would use the global list of transports/bridges and DisableNetwork would not populate it. This was a problem for any user using DisableNetwork which includes Tor Browser and thus leading to the Bug() warning. Without a more in depth refactoring, we can't do this validation without the global list. The previous commit makes it that any connection to a bridge without a transport won't happen thus we keep the security feature of not connecting to a bridge without its corresponding transport. Related to #40106 Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- changes/ticket40106 | 9 +++++---- src/app/config/config.c | 17 ----------------- 2 files changed, 5 insertions(+), 21 deletions(-) diff --git a/changes/ticket40106 b/changes/ticket40106 index d007cf535e..3f344d478f 100644 --- a/changes/ticket40106 +++ b/changes/ticket40106 @@ -1,5 +1,6 @@ o Minor bugfixes (config, bridge): - - Really fix the case where torrc has a missing ClientTransportPlugin but - configured with a Bridge line and UseBridges. Previously, we failed to - also look at the managed proxy list and thus it would fail for the - "exec" case. Fixes bug 40106; bugfix on 0.4.5.1-alpha. + - Don't initiate a connection to a bridge without a corresponding + transport. Fixes bug 40106; bugfix on 0.4.5.1-alpha. + - This also reverts an earlier fix we did for this that would validate + configuration to avoid such situation but turns out it wouldn't work for + a DisableNetwork thus the new approach. diff --git a/src/app/config/config.c b/src/app/config/config.c index 7db5e5cfa8..c7799ec1a2 100644 --- a/src/app/config/config.c +++ b/src/app/config/config.c @@ -2189,23 +2189,6 @@ options_act,(const or_options_t *old_options)) } } - /* Validate that we actually have a configured transport for a Bridge line - * that has one. This is done here because we require the bridge and - * transport to be added to the global list before doing the validation. - * - * In an ideal world, pt_parse_transport_line() would actually return a - * transport_t object so we could inspect it and thus do this step at - * validation time. */ - SMARTLIST_FOREACH_BEGIN(bridge_list_get(), const bridge_info_t *, bi) { - const char *bi_transport_name = bridget_get_transport_name(bi); - if (bi_transport_name && (!transport_get_by_name(bi_transport_name) && - !managed_proxy_has_transport(bi_transport_name))) { - log_warn(LD_CONFIG, "Bridge line with transport %s is missing a " - "ClientTransportPlugin line", bi_transport_name); - return -1; - } - } SMARTLIST_FOREACH_END(bi); - if (options_act_server_transport(old_options) < 0) return -1;

1 0

[tor/maint-0.4.5] Merge remote-tracking branch 'tor-gitlab/mr/268' into maint-0.4.5
by nickm＠torproject.org 21 Jan '21

21 Jan '21

commit 9390e2bf833ea1e7ef0354744b8497a5bed270d0 Merge: 7b102d53e3 7692f443d4 Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Jan 21 13:10:16 2021 -0500 Merge remote-tracking branch 'tor-gitlab/mr/268' into maint-0.4.5 changes/ticket40106 | 9 +++++---- src/app/config/config.c | 17 ----------------- src/feature/client/bridges.c | 9 +++++++++ src/feature/client/entrynodes.c | 3 +++ 4 files changed, 17 insertions(+), 21 deletions(-)

1 0

[tor/maint-0.4.5] Remove unnecessary non-fatal assertion.
by dgoulet＠torproject.org 21 Jan '21

21 Jan '21

commit 13cf96445377ba97090c759901d2574cdc82ca3c Author: Alexander Færøy <ahf(a)torproject.org> Date: Tue Jan 19 16:59:13 2021 +0000 Remove unnecessary non-fatal assertion. This patch removes a call to `tor_assert_nonfatal()` if `extend_info_from_node()` returns NULL. This is unnecessary as we already handle the case where `info` is NULL in the next `if (!info) { ... }` block in the code. See: tor#32666. --- changes/bug32666 | 3 +++ src/core/or/circuitbuild.c | 1 - 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/changes/bug32666 b/changes/bug32666 new file mode 100644 index 0000000000..d487b22148 --- /dev/null +++ b/changes/bug32666 @@ -0,0 +1,3 @@ + o Minor bugfixes (onion services): + - Avoid a non-fatal assertion in certain edge-cases when establishing a + circuit to onion service. Fixes bug 32666; bugfix on 0.3.0.3-alpha. diff --git a/src/core/or/circuitbuild.c b/src/core/or/circuitbuild.c index ea32a5bc57..c0c918abe4 100644 --- a/src/core/or/circuitbuild.c +++ b/src/core/or/circuitbuild.c @@ -2456,7 +2456,6 @@ onion_extend_cpath(origin_circuit_t *circ) choose_good_middle_server(purpose, state, circ->cpath, cur_len); if (r) { info = extend_info_from_node(r, 0); - tor_assert_nonfatal(info); } }

1 0

[tor/maint-0.4.5] Merge branch 'tor-gitlab/mr/265' into maint-0.4.5
by dgoulet＠torproject.org 21 Jan '21

21 Jan '21

commit 7b102d53e3ff3c4b2a8f9fe6d583f7b2273ed001 Merge: 18654b629f 13cf964453 Author: David Goulet <dgoulet(a)torproject.org> Date: Thu Jan 21 12:27:27 2021 -0500 Merge branch 'tor-gitlab/mr/265' into maint-0.4.5 changes/bug32666 | 3 +++ src/core/or/circuitbuild.c | 1 - 2 files changed, 3 insertions(+), 1 deletion(-)

1 0

[tor/master] Merge branch 'tor-gitlab/mr/265' into maint-0.4.5
by dgoulet＠torproject.org 21 Jan '21

21 Jan '21

commit 7b102d53e3ff3c4b2a8f9fe6d583f7b2273ed001 Merge: 18654b629f 13cf964453 Author: David Goulet <dgoulet(a)torproject.org> Date: Thu Jan 21 12:27:27 2021 -0500 Merge branch 'tor-gitlab/mr/265' into maint-0.4.5 changes/bug32666 | 3 +++ src/core/or/circuitbuild.c | 1 - 2 files changed, 3 insertions(+), 1 deletion(-)

1 0

[tor/master] Merge branch 'maint-0.4.5'
by dgoulet＠torproject.org 21 Jan '21

21 Jan '21

commit 8a2786072008f18ccd5bace4f014057d06345af0 Merge: 2c4355fcec 7b102d53e3 Author: David Goulet <dgoulet(a)torproject.org> Date: Thu Jan 21 12:27:34 2021 -0500 Merge branch 'maint-0.4.5' changes/bug32666 | 3 +++ src/core/or/circuitbuild.c | 1 - 2 files changed, 3 insertions(+), 1 deletion(-)

1 0

[tor/master] Remove unnecessary non-fatal assertion.
by dgoulet＠torproject.org 21 Jan '21

21 Jan '21

commit 13cf96445377ba97090c759901d2574cdc82ca3c Author: Alexander Færøy <ahf(a)torproject.org> Date: Tue Jan 19 16:59:13 2021 +0000 Remove unnecessary non-fatal assertion. This patch removes a call to `tor_assert_nonfatal()` if `extend_info_from_node()` returns NULL. This is unnecessary as we already handle the case where `info` is NULL in the next `if (!info) { ... }` block in the code. See: tor#32666. --- changes/bug32666 | 3 +++ src/core/or/circuitbuild.c | 1 - 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/changes/bug32666 b/changes/bug32666 new file mode 100644 index 0000000000..d487b22148 --- /dev/null +++ b/changes/bug32666 @@ -0,0 +1,3 @@ + o Minor bugfixes (onion services): + - Avoid a non-fatal assertion in certain edge-cases when establishing a + circuit to onion service. Fixes bug 32666; bugfix on 0.3.0.3-alpha. diff --git a/src/core/or/circuitbuild.c b/src/core/or/circuitbuild.c index ea32a5bc57..c0c918abe4 100644 --- a/src/core/or/circuitbuild.c +++ b/src/core/or/circuitbuild.c @@ -2456,7 +2456,6 @@ onion_extend_cpath(origin_circuit_t *circ) choose_good_middle_server(purpose, state, circ->cpath, cur_len); if (r) { info = extend_info_from_node(r, 0); - tor_assert_nonfatal(info); } }

1 0