Trac issueshttps://gitlab.torproject.org/legacy/trac/-/issues2020-06-13T14:43:36Zhttps://gitlab.torproject.org/legacy/trac/-/issues/15015tor --verify-config should not bind to ports2020-06-13T14:43:36Zcypherpunkstor --verify-config should not bind to portshttps://lists.torproject.org/pipermail/tor-talk/2015-February/036973.html
maybe we can also get rid of the 'requires root privileges' - thing?
https://lists.torproject.org/pipermail/tor-talk/2015-February/036970.htmlhttps://lists.torproject.org/pipermail/tor-talk/2015-February/036973.html
maybe we can also get rid of the 'requires root privileges' - thing?
https://lists.torproject.org/pipermail/tor-talk/2015-February/036970.htmlTor: 0.3.4.x-finalrl1987rl1987https://gitlab.torproject.org/legacy/trac/-/issues/14709Should the hash tables in circuitmux_t have channel ID in them?2020-06-13T14:42:27ZAndrea ShepardShould the hash tables in circuitmux_t have channel ID in them?(per discussion with nickm and skruffy in #tor-dev on 2015-02-02)
Arguably using (channel_id, circuit_id) pairs rather than bare circuit IDs in the hash table in a circuitmux (circuitmux_t.chanid_circid_map) is generality we never actua...(per discussion with nickm and skruffy in #tor-dev on 2015-02-02)
Arguably using (channel_id, circuit_id) pairs rather than bare circuit IDs in the hash table in a circuitmux (circuitmux_t.chanid_circid_map) is generality we never actually used and we should consider whether we can eliminate it and save memory.Tor: unspecifiedhttps://gitlab.torproject.org/legacy/trac/-/issues/14581Looking up bridge by ID may choose the wrong bridge2020-06-13T14:42:19ZMatthew FinkelLooking up bridge by ID may choose the wrong bridgeAs mentioned by arma in #14216 [comment 3](https://trac.torproject.org/projects/tor/ticket/14216#comment:3).
Basically, whenever we try to look up a bridge by ID we may not choose the one we want. If we configured multiple pluggable tra...As mentioned by arma in #14216 [comment 3](https://trac.torproject.org/projects/tor/ticket/14216#comment:3).
Basically, whenever we try to look up a bridge by ID we may not choose the one we want. If we configured multiple pluggable transports for the same bridge, each PT will be associated with the same ID. We usually stop searching when we find a matching ID in the list, but the first match may not be the transport we wanted.
As an example, node_is_a_configured_bridge():
```
int
node_is_a_configured_bridge(const node_t *node)
{
int retval = 0;
smartlist_t *orports = node_get_all_orports(node);
retval = get_configured_bridge_by_orports_digest(node->identity,
orports) != NULL;
```
calls get_configured_bridge_by_orports_digest()
```
static bridge_info_t *
get_configured_bridge_by_orports_digest(const char *digest,
const smartlist_t *orports)
{
if (!bridge_list)
return NULL;
SMARTLIST_FOREACH_BEGIN(bridge_list, bridge_info_t *, bridge)
{
if (tor_digest_is_zero(bridge->identity)) {
SMARTLIST_FOREACH_BEGIN(orports, tor_addr_port_t *, ap)
{
if (tor_addr_compare(&bridge->addr, &ap->addr, CMP_EXACT) == 0 &&
bridge->port == ap->port)
return bridge;
}
SMARTLIST_FOREACH_END(ap);
}
if (digest && tor_memeq(bridge->identity, digest, DIGEST_LEN))
return bridge;
}
```Tor: unspecifiedhttps://gitlab.torproject.org/legacy/trac/-/issues/13828Refactor rend_cache_store_v2_desc_as_dir and rend_cache_store_v2_desc_as_clie...2020-06-13T14:40:40ZRoger DingledineRefactor rend_cache_store_v2_desc_as_dir and rend_cache_store_v2_desc_as_client to avoid duplicate codeThere's a lot of overlapping code here in terms of checking the timestamp on the parsed descriptor, checking if we have a newer one already, creating and linking the new entry, etc. It's not exactly the same but a lot of it sure is simil...There's a lot of overlapping code here in terms of checking the timestamp on the parsed descriptor, checking if we have a newer one already, creating and linking the new entry, etc. It's not exactly the same but a lot of it sure is similar. This is the sort of situation where a subfunction to do the shared behavior might be handy.Tor: unspecifiedhttps://gitlab.torproject.org/legacy/trac/-/issues/11138Improve code handling SOCKS connection in tor daemon2020-06-13T14:39:17ZDavid Gouletdgoulet@torproject.orgImprove code handling SOCKS connection in tor daemonAfter a discussion at the weekly little-t tor meeting on #tor-dev, it appears that the code handling SOCKS4/5 connections is a mess:
```
15:41 < nickm> sad to say, the SOCKS code is a mess right now. It'll probably need significant tes...After a discussion at the weekly little-t tor meeting on #tor-dev, it appears that the code handling SOCKS4/5 connections is a mess:
```
15:41 < nickm> sad to say, the SOCKS code is a mess right now. It'll probably need significant testing and refactoring before we can think about extending it
```
Considering proposal [229](https://gitweb.torproject.org/torspec.git/blob/HEAD:/proposals/229-further-socks5-extensions.txt|) to extend SOCKS5 for Tor use cases, we should work on improving this code base before anything else.
1) Tests, tests and moar tests
2) Refactor
3) Extend
Related issues: #11134, #9221Tor: unspecifiedhttps://gitlab.torproject.org/legacy/trac/-/issues/10481connection_mark_unattached_ap_: checking always true edge_has_sent_end2020-06-13T14:33:33Zcypherpunksconnection_mark_unattached_ap_: checking always true edge_has_sent_end```
ENTRY_TO_EDGE_CONN(conn)->edge_has_sent_end = 1; /* no circ yet */
```
```
if ((edge_conn->on_circuit != NULL || edge_conn->edge_has_sent_end) &&
connection_edge_is_rendezvous_stream(edge_conn)) {
rend_client_note_conn...```
ENTRY_TO_EDGE_CONN(conn)->edge_has_sent_end = 1; /* no circ yet */
```
```
if ((edge_conn->on_circuit != NULL || edge_conn->edge_has_sent_end) &&
connection_edge_is_rendezvous_stream(edge_conn)) {
rend_client_note_connection_attempt_ended(
edge_conn->rend_data->onion_address);
}
```Tor: unspecifiedhttps://gitlab.torproject.org/legacy/trac/-/issues/9971for_discovery option in add_an_entry_guard() is confusingly named2020-06-13T14:32:38ZRoger Dingledinefor_discovery option in add_an_entry_guard() is confusingly namedIn #9946 I added a new argument "for_discovery" to add_an_entry_guard(). Nick prefers "provisional" or "probationary".
In parallel, I think we should probably rename the made_contact field in entry guard t, to be *why* we're remembering...In #9946 I added a new argument "for_discovery" to add_an_entry_guard(). Nick prefers "provisional" or "probationary".
In parallel, I think we should probably rename the made_contact field in entry guard t, to be *why* we're remembering that we've made contact, rather than simply that we have.
And lastly, we should do something about the godawful number of int arguments that add_an_entry_guard() now takes.Tor: 0.2.8.x-finalhttps://gitlab.torproject.org/legacy/trac/-/issues/9241Abstract and decouple path selection from circuit construction2020-06-13T14:30:19ZMike PerryAbstract and decouple path selection from circuit constructionFor #9001, we first need to abstract the existing path selection mechanisms and make sure they are tested and equivalent.For #9001, we first need to abstract the existing path selection mechanisms and make sure they are tested and equivalent.Tor: unspecifiedhttps://gitlab.torproject.org/legacy/trac/-/issues/8486Introduce ExitNode country code per listener2020-06-13T14:28:16ZbastikIntroduce ExitNode country code per listenerBorn out of this [tor-talk reply](https://lists.torproject.org/pipermail/tor-talk/2013-March/027592.html) you could introduce the ExitNode country code per listener.
It could be used for a listener that listens for Thunderbird traffic. ...Born out of this [tor-talk reply](https://lists.torproject.org/pipermail/tor-talk/2013-March/027592.html) you could introduce the ExitNode country code per listener.
It could be used for a listener that listens for Thunderbird traffic. E-mail providers with restrictive policies, which lock you out because the connecting IP is not from a country you are normally in, could be used over Tor with such a per listener ExitNode selection, without having to use the same exit set for browsing.
The summary limits it to the country code, which should enhance the user experience since restrictive service providers should work over Tor, however I would not mind if this feature is similar to the global ExitNode option.
The global option is more flexible and would allow to select only exits that are known to work (e.g. one that are not blacklisted by the service provider). However it might be problematic since clients are bound to certain exits. Depending on how narrow that set is it could degrade anonymity to a high degree.Tor: unspecifiedhttps://gitlab.torproject.org/legacy/trac/-/issues/8197Do something about policies_parse_exit_policy()'s arguments2020-06-13T14:27:22ZRoger DingledineDo something about policies_parse_exit_policy()'s arguments```
r = policies_parse_exit_policy(&line, &policy, 1, 0, 0, 1);
...
test_assert(0 == policies_parse_exit_policy(NULL, &policy2, 1, 1, 0, 1));
```
Quick quiz: which of these booleans means what? And which one isn't a boolean at all? ...```
r = policies_parse_exit_policy(&line, &policy, 1, 0, 0, 1);
...
test_assert(0 == policies_parse_exit_policy(NULL, &policy2, 1, 1, 0, 1));
```
Quick quiz: which of these booleans means what? And which one isn't a boolean at all? :)
(Don't work on this ticket until we merge #5528.)Tor: 0.2.6.x-finalrl1987rl1987https://gitlab.torproject.org/legacy/trac/-/issues/8160Create separate pending counters during circuit construction2020-06-13T14:27:08ZMike PerryCreate separate pending counters during circuit constructionWhile testing the path bias code, I noticed a source of rounding error when we scaled our counts down while circuits are open. I corrected for this by counting the number of open circuits during scaling and subtracting that from our coun...While testing the path bias code, I noticed a source of rounding error when we scaled our counts down while circuits are open. I corrected for this by counting the number of open circuits during scaling and subtracting that from our counts where appropriate, but a better fix might be to actually store separate pending counters that we don't transfer into the official, scaled counters until circuit closure.Tor: unspecifiedhttps://gitlab.torproject.org/legacy/trac/-/issues/8111Refactor our checking of whether we should be reading/writing on a connection...2020-06-13T14:26:57ZNick MathewsonRefactor our checking of whether we should be reading/writing on a connection to use a set of reason-flagsThere are plenty of things that can block reading or writing on a connection: being out of bandwidth, not having anything more to say, having an internal buffer get too full, etc.
There are too many places where one of the things that w...There are plenty of things that can block reading or writing on a connection: being out of bandwidth, not having anything more to say, having an internal buffer get too full, etc.
There are too many places where one of the things that would block read/write becomes false, and so we check all of the *other* potential reasons not to read/write before we re-enable reading/writing.
There's a much better pattern we could use: have a flags variable for "read_blocked" and a flags variable for "write_blocked", each bit of which represents a reason not to be reading or writing. Instead of calling connection_{stop,start}_{reading,writing} directly, we would call {un,}block_{reading,writing}(conn, reason), which would set or clear a bit corresponding to 'reason', and block/unblock reading/writing accordingly.
I haven't yet verified that this is a win; we should audit all the uses of connection_{stop,start}_{reading,writing} reading to see.Tor: unspecifiedMrSquancheeMrSquancheehttps://gitlab.torproject.org/legacy/trac/-/issues/7961Publish transports that bind on IPv6 addresses2020-06-13T14:34:48ZGeorge KadianakisPublish transports that bind on IPv6 addressesCurrently, `pt_get_extra_info_descriptor_string()` uses `router_pick_published_address()` to retrieve our external IP address so that it can write it in our extra-info descriptor along with the TCP port that our transport listens on.
Th...Currently, `pt_get_extra_info_descriptor_string()` uses `router_pick_published_address()` to retrieve our external IP address so that it can write it in our extra-info descriptor along with the TCP port that our transport listens on.
The bad news are that `router_pick_published_address()` only returns IPv4 addresses, and we will probably have to refactor it, or do something like this:
~~ https://gitweb.torproject.org/tor.git/blob/ebf30613ea41bbed3340851e839da9b7db4351c5:/src/or/router.c#l1775 ~~
(wrong commit reference)
for IPv6 addresses.
Not sure if this can get in 0.2.4.x. I guess it depends on how quickly we implement it, and how complex the changes are going to be.Tor: unspecifiedhttps://gitlab.torproject.org/legacy/trac/-/issues/7755circuit_t::timestamp_dirty should be cleaned up2020-06-13T14:25:45ZRobert Ransomcircuit_t::timestamp_dirty should be cleaned up`circuit_t::timestamp_dirty` is used in multiple bizarre ways, which should be reverse-engineered, documented, and cleaned up (probably by replacing it with multiple fields, each with a name which reflects its actual meaning and/or purpo...`circuit_t::timestamp_dirty` is used in multiple bizarre ways, which should be reverse-engineered, documented, and cleaned up (probably by replacing it with multiple fields, each with a name which reflects its actual meaning and/or purpose). See also the #7157 review discussion.
Also, is there a good reason for it to be in `circuit_t`, not `origin_circuit_t`? (Any use of it on `or_circuit_t`s (or even on service-side rendezvous `origin_circuit_t`s) would have different semantics.)Tor: unspecifiedhttps://gitlab.torproject.org/legacy/trac/-/issues/7482Discard nonsense in address.c about v4-mapped addresses2020-06-13T14:25:00ZNick MathewsonDiscard nonsense in address.c about v4-mapped addressesRight now, some but not all places in address.c do strange contortions to treat ::ffff:1.2.3.4 as equivalent to 1.2.3.4. Instead, we should be rejecting these addresses as private, rather than trying to pretend that we support them.Right now, some but not all places in address.c do strange contortions to treat ::ffff:1.2.3.4 as equivalent to 1.2.3.4. Instead, we should be rejecting these addresses as private, rather than trying to pretend that we support them.Tor: unspecifiedhttps://gitlab.torproject.org/legacy/trac/-/issues/7174Refactor node_t and router lists to make it easier to identify bugs2020-06-13T14:23:59ZKarsten LoesingRefactor node_t and router lists to make it easier to identify bugsRoger suspects that refactoring our node_t and router lists may make it easier to identify bugs like #1776. He also wonders whether the shift to microdescs has made things easier (or maybe even made #1776 go away).Roger suspects that refactoring our node_t and router lists may make it easier to identify bugs like #1776. He also wonders whether the shift to microdescs has made things easier (or maybe even made #1776 go away).Tor: unspecifiedhttps://gitlab.torproject.org/legacy/trac/-/issues/6761PDS_NO_EXISTING_SERVERDESC_FETCH is somewhat archaic2020-06-13T14:22:14ZRoger DingledinePDS_NO_EXISTING_SERVERDESC_FETCH is somewhat archaicIn bug #366 we made it so Tor won't open a second dir fetch to an authority if it has one already. Great.
```
rs = router_pick_trusteddirserver(type, pds_flags);
if (rs == NULL && (pds_flags & (PDS_NO_EXISTING_SERVERDESC_...In bug #366 we made it so Tor won't open a second dir fetch to an authority if it has one already. Great.
```
rs = router_pick_trusteddirserver(type, pds_flags);
if (rs == NULL && (pds_flags & (PDS_NO_EXISTING_SERVERDESC_FETCH|
PDS_NO_EXISTING_MICRODESC_FETCH))) {
[...]
log_debug(LD_DIR, "Deferring serverdesc fetch: all authorities "
"are in use.");
```
But we didn't update it to look for begindir conns, so it only applies to direct dir fetches. Ok.
```
if (no_microdesc_fetching) {
if (connection_get_by_type_addr_port_purpose(
CONN_TYPE_DIR, &addr, d->dir_port, DIR_PURPOSE_FETCH_MICRODESC)) {
++n_busy;
continue;
```
So it doesn't apply to clients, only relays. That makes sense, because relays are the ones who typically would contact authorities anyway.
```
int prefer_authority = directory_fetches_from_authorities(options);
```
But when a relay starts up and gets a consensus, it has a line like this nowadays:
```
Sep 04 05:47:40.000 [info] launch_descriptor_downloads(): Launching 33 requests for 3114 routers, 96 at a time
```
33 requests! Surely that's way more than the 8 or so authorities we have. And relays don't use begindir to talk to authorities, since it slows them down too much:
```
int use_begindir = supports_begindir &&
directory_command_should_use_begindir(options, _addr,
or_port, router_purpose, anonymized_connection);
```
Doesn't that mean we hit the "one per authority" limit and drop the rest of those requests?
It turns out that directory_fetches_from_authorities() is false for most relays when they start up:
```
if (server_mode(options) && router_pick_published_address(options, &addr)<0)
return 1; /* we don't know our IP address; ask an authority. */
refuseunknown = ! router_my_exit_policy_is_reject_star() &&
should_refuse_unknown_exits(options);
if (options->DirPort == NULL && !refuseunknown)
return 0;
if (!server_mode(options) || !advertised_server_mode())
return 0;
me = router_get_my_routerinfo();
if (!me || (!me->dir_port && !refuseunknown))
return 0; /* if dirport not advertised, return 0 too */
return 1;
```
So these relays end up asking arbitrary other relays they found in the consensus! Cue Nick's circus music here. Not the best way to get fresh info.
In my case here (and I expect it's a common case), my relay failed the "!advertised_server_mode" check, since it hadn't done its reachability test yet so it hadn't published a descriptor yet.
Maybe this is actually a feature that just-starting-up relays don't fetch descriptors from authorities. It probably doesn't hurt much, and probably helps authority load a bit.
But I don't think it's a feature that we allow multiple descriptor-fetching dir requests in parallel to an authority iff they're begindir requests.Tor: unspecifiedhttps://gitlab.torproject.org/legacy/trac/-/issues/6456Merge parse_client_transport_line() and parse_server_transport_line()2020-06-13T14:21:26ZGeorge KadianakisMerge parse_client_transport_line() and parse_server_transport_line()There is too much code duplication between `parse_client_transport_line()` and `parse_server_transport_line`. We should probably merge them into one function during 0.2.4.x.There is too much code duplication between `parse_client_transport_line()` and `parse_server_transport_line`. We should probably merge them into one function during 0.2.4.x.Tor: 0.2.6.x-finalAndrea ShepardAndrea Shepardhttps://gitlab.torproject.org/legacy/trac/-/issues/6236Remove duplicate code between parse_{c,s}method_line2020-06-13T14:20:39ZGeorge KadianakisRemove duplicate code between parse_{c,s}method_line`parse_{c,s}method_line` share lots of duplicate code. We must find a way to merge the two functions, or hide the duplicate code into functions.`parse_{c,s}method_line` share lots of duplicate code. We must find a way to merge the two functions, or hide the duplicate code into functions.Tor: 0.3.4.x-finalhttps://gitlab.torproject.org/legacy/trac/-/issues/5507option_transition_affects_workers could be split2020-06-13T14:18:34ZNick Mathewsonoption_transition_affects_workers could be splitWe could split option_transition_affects_workers into those changes that require a worker restart, and those that require an init_keys(). This could save us some spurious calls.
Suggested by cypherpunks on #4438.We could split option_transition_affects_workers into those changes that require a worker restart, and those that require an init_keys(). This could save us some spurious calls.
Suggested by cypherpunks on #4438.Tor: unspecified