The Tor Project issueshttps://gitlab.torproject.org/groups/tpo/-/issues2021-09-16T14:36:00Zhttps://gitlab.torproject.org/tpo/core/tor/-/issues/7479Replace more linked lists with queue.h implementations2021-09-16T14:36:00ZNick MathewsonReplace more linked lists with queue.h implementationsWe've got linked lists scattered through our source. Now that we have a queue.h implementation, it would be neat to use that instead.
This is something we can (and should!) do piecemeal; let's not try to do it all with one big patch.We've got linked lists scattered through our source. Now that we have a queue.h implementation, it would be neat to use that instead.
This is something we can (and should!) do piecemeal; let's not try to do it all with one big patch.https://gitlab.torproject.org/tpo/core/tor/-/issues/6236Remove duplicate code between parse_{c,s}method_line2021-09-16T14:36:00ZGeorge 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/tpo/core/tor/-/issues/4991Use standard windows-detection macros2021-09-16T14:35:59ZNick MathewsonUse standard windows-detection macrosIn our code, we mostly use MS_WINDOWS to detect windows; we sometimes use WIN32 to detect windows; and we sometimes use _WIN32.
_WIN32 is standard; WIN32 is obsolete; MS_WINDOWS is our own silliness. Let's be consistent and standard a...In our code, we mostly use MS_WINDOWS to detect windows; we sometimes use WIN32 to detect windows; and we sometimes use _WIN32.
_WIN32 is standard; WIN32 is obsolete; MS_WINDOWS is our own silliness. Let's be consistent and standard and just use WIN32.Tor: unspecifiedhttps://gitlab.torproject.org/tpo/core/tor/-/issues/4282Extract common "make private stats dir" code from rephist.c, geoip.c2021-09-16T14:35:59ZNick MathewsonExtract common "make private stats dir" code from rephist.c, geoip.cIn several places in rephist.c and geoip.c, we do:
```
/* Try to write to disk. */
statsdir = get_datadir_fname("stats");
if (check_private_dir(statsdir, CPD_CREATE, get_options()->User) < 0) {
log_warn(LD_HIST, "Unable to crea...In several places in rephist.c and geoip.c, we do:
```
/* Try to write to disk. */
statsdir = get_datadir_fname("stats");
if (check_private_dir(statsdir, CPD_CREATE, get_options()->User) < 0) {
log_warn(LD_HIST, "Unable to create stats/ directory!");
goto done;
}
filename = get_datadir_fname2("stats", "buffer-stats");
if (write_str_to_file(filename, str, 0) < 0)
log_warn(LD_HIST, "Unable to write buffer stats to disk!");
```
Duplicated code is stupid. Some or all of this should get extracted into a new function or two. I'd suggest a function to "create the datadir and a subdirectory of the datadir as needed" and another function to write a file to a subdirectory of the datadir, creating that subdirectory as needed.Tor: 0.2.5.x-finalhttps://gitlab.torproject.org/tpo/core/tor/-/issues/9971for_discovery option in add_an_entry_guard() is confusingly named2021-09-16T14:35:28ZRoger Dingledinefor_discovery option in add_an_entry_guard() is confusingly namedIn legacy/trac#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 ...In legacy/trac#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/tpo/core/tor/-/issues/17225Merge NS_EXPIRY_SLOP and REASONABLY_LIVE_TIME2021-09-16T14:35:00ZteorMerge NS_EXPIRY_SLOP and REASONABLY_LIVE_TIMEThese are now the same value (24 hours) so they can be merged into a common header, and the relevant comments removed.These are now the same value (24 hours) so they can be merged into a common header, and the relevant comments removed.https://gitlab.torproject.org/tpo/core/tor/-/issues/17224Refactor common parts of parse_dir_authority_line and parse_dir_fallback_line2021-09-16T14:35:00ZteorRefactor common parts of parse_dir_authority_line and parse_dir_fallback_lineThere's a lot of common code in parse_dir_authority_line and parse_dir_fallback_line that could be refactored into a shared function.
What needs to be done is:
* identify common code
* create a common loop that parses the key=value pair...There's a lot of common code in parse_dir_authority_line and parse_dir_fallback_line that could be refactored into a shared function.
What needs to be done is:
* identify common code
* create a common loop that parses the key=value pairs it understands, deletes them, and leaves the rest alone
* put that loop in its own function that takes pointers (or pointer pointers?), returns values in those that are included, and returns NULL in those that aren't.
* check this loop's output from both functions to ensure they each get what they need, then do the specific bits on the remaining arguments
* call a common function to parse the IPv4:Port at the start of the line, then delete it
* call a common function to parse the hex digest, which should be the only remaining thinghttps://gitlab.torproject.org/tpo/core/tor/-/issues/16134The various stream lists tied to the circuit structures should use tor_queue.h2021-09-16T14:35:00ZYawning AngelThe various stream lists tied to the circuit structures should use tor_queue.hOffshoot of legacy/trac#16052.
Every list that uses `edge_connection_t.next_stream`, should be converted to a `TOR_SLIST` for maintainability/sanity. Carved off into a separate ticket because legacy/trac#16052 will probably get a 0.2.6...Offshoot of legacy/trac#16052.
Every list that uses `edge_connection_t.next_stream`, should be converted to a `TOR_SLIST` for maintainability/sanity. Carved off into a separate ticket because legacy/trac#16052 will probably get a 0.2.6 backport, while this should not as it is rather intrusive (if mechanical).Tor: unspecifiedYawning AngelYawning Angelhttps://gitlab.torproject.org/tpo/core/tor/-/issues/15214networkstatus_compute_consensus() is unreasonably large and should be refactored2021-09-16T14:35:00ZAndrea Shepardnetworkstatus_compute_consensus() is unreasonably large and should be refactoredI count 874 lines (while auditing strlcat()/strlcpy() calls). Did Minicode raise our function call ration from 25 to 20 per 100 kloc the week this was written?I count 874 lines (while auditing strlcat()/strlcpy() calls). Did Minicode raise our function call ration from 25 to 20 per 100 kloc the week this was written?Tor: unspecifiedhttps://gitlab.torproject.org/tpo/core/tor/-/issues/15015tor --verify-config should not bind to ports2021-09-16T14:35:00Zcypherpunkstor --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/tpo/core/tor/-/issues/14828Multiple hidden services can share a pk_digest/service_id.2021-09-16T14:35:00ZYawning AngelMultiple hidden services can share a pk_digest/service_id.This may be a duplicate, it's past my bed time, so I don't have time to check.
The current rendservice code's duplication check doesn't enforce uniqueness of `pk_digest` and `service_id`. It probably should do so for both things, since...This may be a duplicate, it's past my bed time, so I don't have time to check.
The current rendservice code's duplication check doesn't enforce uniqueness of `pk_digest` and `service_id`. It probably should do so for both things, since I can't think of a reason why this would ever be well defined, or desirable behavior.
The trivial fix would be to add a pair of checks to `rendservice.c:rend_service_load_keys(s)`, that log on LD_CONFIG, and return an error if a collision is detected.https://gitlab.torproject.org/tpo/core/tor/-/issues/13828Refactor rend_cache_store_v2_desc_as_dir and rend_cache_store_v2_desc_as_clie...2021-09-16T14:34:59ZRoger 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/tpo/core/tor/-/issues/13827Cell handling code duplication in channel.c2021-09-16T14:34:59Zrl1987Cell handling code duplication in channel.cIn 336c856e52d211aad6b40d29986264f3277a1327 :
* `channel_get_var_cell_handler()` and `channel_get_cell_handler() `looks very similar
* `channel_write_cell()`, `channel_write_packed_cell()` and `channel_write_var_cell()` is mostly dupl...In 336c856e52d211aad6b40d29986264f3277a1327 :
* `channel_get_var_cell_handler()` and `channel_get_cell_handler() `looks very similar
* `channel_write_cell()`, `channel_write_packed_cell()` and `channel_write_var_cell()` is mostly duplicated code.
* so are `channel_queue_cell()` and `channel_queue_var_cell()`
We should refactor to reduce the code duplication.Tor: 0.3.0.x-finalhttps://gitlab.torproject.org/tpo/core/tor/-/issues/17985Refactor common parts of entry_is_live and entry_guard_set_status2021-09-16T14:34:35ZteorRefactor common parts of entry_is_live and entry_guard_set_statusentry_is_live and entry_guard_set_status look very similar, and return almost identical messages.
We could refactor out the common code (even if we have to use macros).
This would also avoid them getting out of sync.entry_is_live and entry_guard_set_status look very similar, and return almost identical messages.
We could refactor out the common code (even if we have to use macros).
This would also avoid them getting out of sync.Tor: unspecifiedhttps://gitlab.torproject.org/tpo/core/tor/-/issues/17882Remove needless *_support_ntor()2021-09-16T14:34:35ZNick MathewsonRemove needless *_support_ntor()While reviewing legacy/trac#7144, I noticed that circuit_cpath_supports_ntor() is pointless: We no longer allow TAP-only relays on the network, IIUC.
Assuming that I've got that right, we can rip out some code.While reviewing legacy/trac#7144, I noticed that circuit_cpath_supports_ntor() is pointless: We no longer allow TAP-only relays on the network, IIUC.
Assuming that I've got that right, we can rip out some code.Tor: unspecifiedhttps://gitlab.torproject.org/tpo/core/tor/-/issues/17867Remove addresses and ports from dir_server_t and just use the ones in fake_st...2021-09-16T14:34:35ZteorRemove addresses and ports from dir_server_t and just use the ones in fake_statusWe put copies of an AuthDir / FallbackDir's addresses and ports in dir_server_t, and dir_server_t.fake_status. This is just asking for mistakes initialising them.
We could refactor the code so that we always use the addresses and ports ...We put copies of an AuthDir / FallbackDir's addresses and ports in dir_server_t, and dir_server_t.fake_status. This is just asking for mistakes initialising them.
We could refactor the code so that we always use the addresses and ports in fake_status.
This is not particularly urgent, because dir_server_t is read-only.https://gitlab.torproject.org/tpo/core/tor/-/issues/17847Unify router_pick_directory_server_impl and router_pick_trusteddirserver_impl2021-09-16T14:34:35ZteorUnify router_pick_directory_server_impl and router_pick_trusteddirserver_implrouter_pick_directory_server_impl and router_pick_trusteddirserver_impl are very similar.
I wonder if we could refactor them into a single function?
(Even if we have to use a macro to do it.)router_pick_directory_server_impl and router_pick_trusteddirserver_impl are very similar.
I wonder if we could refactor them into a single function?
(Even if we have to use a macro to do it.)https://gitlab.torproject.org/tpo/core/tor/-/issues/17739Refactor clock skew warning code to avoid duplication2021-09-16T14:34:35ZteorRefactor clock skew warning code to avoid duplicationThe following functions contain very similar clock skew code:
* connection_dir_client_reached_eof
* channel_tls_process_netinfo_cell
* or_state_load
We should unify this code to reduce redundancy and increase consistency.The following functions contain very similar clock skew code:
* connection_dir_client_reached_eof
* channel_tls_process_netinfo_cell
* or_state_load
We should unify this code to reduce redundancy and increase consistency.Tor: 0.2.8.x-finalhttps://gitlab.torproject.org/tpo/core/tor/-/issues/17608Refactor accept/reject * redundancy checks out of policies_parse_exit_policy_...2021-09-16T14:34:34ZteorRefactor accept/reject * redundancy checks out of policies_parse_exit_policy_internalpolicies_parse_exit_policy_internal would be a lot easier to read if the code that implements `found_final_effective_entry` was in its own function.policies_parse_exit_policy_internal would be a lot easier to read if the code that implements `found_final_effective_entry` was in its own function.Tor: 0.2.8.x-finalhttps://gitlab.torproject.org/tpo/core/tor/-/issues/18918Clarify directory and ORPort checking functions2021-09-16T14:34:12ZteorClarify directory and ORPort checking functionsThe OR and dir checking functions in router.c are somewhat confusing. We could document them better, or modify their names, or restructure.
In legacy/trac#18616, we added:
* decide_to_advertise_begindir
We already had:
* check_whether_...The OR and dir checking functions in router.c are somewhat confusing. We could document them better, or modify their names, or restructure.
In legacy/trac#18616, we added:
* decide_to_advertise_begindir
We already had:
* check_whether_orport_reachable
* check_whether_dirport_reachable
* router_has_bandwidth_to_be_dirserver
* router_should_be_directory_server
* dir_server_mode
* decide_to_advertise_dirport
* consider_testing_reachabilityTor: 0.3.4.x-final