The Tor Project issueshttps://gitlab.torproject.org/groups/tpo/-/issues2021-09-16T14:33:04Zhttps://gitlab.torproject.org/tpo/core/tor/-/issues/20887DIRCACHE_MIN_MEM_MB does not stringify on FreeBSD, we should use %d instead2021-09-16T14:33:04ZteorDIRCACHE_MIN_MEM_MB does not stringify on FreeBSD, we should use %d insteadA FreeBSD relay operator reports the message:
```
[WARN] Being a directory cache (default) with less than DIRCACHE_MIN_MB_BANDWIDTH MB of memory is not recommended and may consume most of the available resources, consider disabling this ...A FreeBSD relay operator reports the message:
```
[WARN] Being a directory cache (default) with less than DIRCACHE_MIN_MB_BANDWIDTH MB of memory is not recommended and may consume most of the available resources, consider disabling this functionality by setting the DirCache option to 0
```
So we should print the macro value the same way we do every other value, using "%d" in the string, and passing it as an argument.
(This macro changed name in legacy/trac#20684.)Tor: 0.3.4.x-finalhttps://gitlab.torproject.org/tpo/core/tor/-/issues/19871Crypto operation counters are unused and obsolete2021-09-16T14:33:30ZGeorge KadianakisCrypto operation counters are unused and obsoleteNick told me that the crypto operation counters in `rephist.c` are considered unused and obsolete.
I'm talking about the counters at `note_crypto_pk_op()` etc.
Removing code is always good, so if these things are unused we might as wel...Nick told me that the crypto operation counters in `rephist.c` are considered unused and obsolete.
I'm talking about the counters at `note_crypto_pk_op()` etc.
Removing code is always good, so if these things are unused we might as well remove them.Tor: 0.3.2.x-finalIsis LovecruftIsis Lovecrufthttps://gitlab.torproject.org/tpo/core/tor/-/issues/19926BUG warning in connection_ap_attach_pending: waiting for rendezvous desc :*2021-09-16T14:33:30ZcypherpunksBUG warning in connection_ap_attach_pending: waiting for rendezvous desc :*connection_ap_attach_pending: Bug: 0x613000b8d680 is no longer in circuit_wait. Its current state is waiting for rendezvous desc. Why is it on pending_entry_connections? (on Tor 0.2.9.1-alpha f6c7e131a1ceb178)
Yeah, why?connection_ap_attach_pending: Bug: 0x613000b8d680 is no longer in circuit_wait. Its current state is waiting for rendezvous desc. Why is it on pending_entry_connections? (on Tor 0.2.9.1-alpha f6c7e131a1ceb178)
Yeah, why?https://gitlab.torproject.org/tpo/core/tor/-/issues/19979Use OpenSSL 1.1.0 HKDF in Tor when available.2021-09-16T14:33:30ZNick MathewsonUse OpenSSL 1.1.0 HKDF in Tor when available.OpenSSL 1.1.0 now includes HKDF support. We should consider using their implementation instead of ours when it's available.OpenSSL 1.1.0 now includes HKDF support. We should consider using their implementation instead of ours when it's available.Tor: 0.3.5.x-finalrl1987rl1987https://gitlab.torproject.org/tpo/core/tor/-/issues/18106Rename fascist_firewall_* functions to reachable_address_*2021-09-16T14:34:11ZteorRename fascist_firewall_* functions to reachable_address_*In legacy/trac#17840 and legacy/trac#9067 / legacy/trac#9068, we unified ReachableAddresses and ClientUseIPv4/ClientUseIPv6/UseBridges. We'd previously unified FascistFirewall, FirewallPorts and ReachableAddresses.
This means that the f...In legacy/trac#17840 and legacy/trac#9067 / legacy/trac#9068, we unified ReachableAddresses and ClientUseIPv4/ClientUseIPv6/UseBridges. We'd previously unified FascistFirewall, FirewallPorts and ReachableAddresses.
This means that the functions called fascist_firewall_* should probably be renamed to be more descriptive.
I think reachable_address_* is one option, but let's follow typical naming standards and not repeat words in names:
* fascist_firewall_choose_address_dir_server ->
* reachable_address_choose_address_dir_server ->
* reachable_address_choose_dir_server
Let's also use this as an opportunity to disambiguate fascist_firewall_choose_address_impl & fascist_firewall_choose_address_base.Tor: 0.4.5.x-freezeNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/tpo/core/tor/-/issues/18124We never use interface names on Windows, avoid retrieving them2021-09-16T14:34:11ZteorWe never use interface names on Windows, avoid retrieving themWe're scanning local interfaces more often in the tor codebase, so it would be a good idea to optimise get_interface_addresses_win32().
We never use the interface names, so let's avoid retrieving them by passing GAA_FLAG_SKIP_FRIENDLY_N...We're scanning local interfaces more often in the tor codebase, so it would be a good idea to optimise get_interface_addresses_win32().
We never use the interface names, so let's avoid retrieving them by passing GAA_FLAG_SKIP_FRIENDLY_NAME in FLAGS.
Reference documentation:
https://msdn.microsoft.com/en-us/library/windows/desktop/aa365915%28v=vs.85%29.aspxhttps://gitlab.torproject.org/tpo/core/tor/-/issues/18529Fix duplicate check for "only allow internal addresses if we are on a network...2021-09-16T14:34:12ZNick MathewsonFix duplicate check for "only allow internal addresses if we are on a network with nonstandard authorities"We have this code in config.c:
```
if (tor_addr_is_internal(&myaddr, 0)) {
/* make sure we're ok with publishing an internal IP */
if (!options->DirAuthorities && !options->AlternateDirAuthority) {
/* if they are using th...We have this code in config.c:
```
if (tor_addr_is_internal(&myaddr, 0)) {
/* make sure we're ok with publishing an internal IP */
if (!options->DirAuthorities && !options->AlternateDirAuthority) {
/* if they are using the default authorities, disallow internal IPs
* always. */
log_fn(warn_severity, LD_CONFIG,
"Address '%s' resolves to private IP address '%s'. "
"Tor servers that use the default DirAuthorities must have "
"public IP addresses.", hostname, addr_string);
tor_free(addr_string);
return -1;
}
...
```
And we now have this code in router.c (since legacy/trac#17153):
```
/* Like IPv4, if the relay is configured using the default
* authorities, disallow internal IPs. Otherwise, allow them. */
const int default_auth = (!options->DirAuthorities &&
!options->AlternateDirAuthority);
if (! tor_addr_is_internal(&p->addr, 0) || ! default_auth) {
ipv6_orport = p;
break;
...
```
These two checks are similar and I'd prefer that they be merged when possible.Tor: 0.2.9.x-finalhttps://gitlab.torproject.org/tpo/core/tor/-/issues/18721Define accessors for connection_t's address fields2021-09-16T14:34:12ZteorDefine accessors for connection_t's address fieldsIn legacy/trac#18460, we noted that connection_t's address fields are confusing.
we should open a ticket to add comments explaining the real story of what's going on above in the code, and that we also open a ticket to define a few ac...In legacy/trac#18460, we noted that connection_t's address fields are confusing.
we should open a ticket to add comments explaining the real story of what's going on above in the code, and that we also open a ticket to define a few accessor functions to provide tor_addr_t and string representations of the final target address, proxy-or-final, or whatever other things we actually need.Tor: unspecifiedhttps://gitlab.torproject.org/tpo/core/tor/-/issues/18902Avoid variable shadowing in Tor2021-09-16T14:34:12ZteorAvoid variable shadowing in TorTor sure does shadow a lot of local and global variables with its block-level variable declarations.
I spot-checked a few of these and they look harmless.
Still, it would be great if we removed them by renaming the variables in the inn...Tor sure does shadow a lot of local and global variables with its block-level variable declarations.
I spot-checked a few of these and they look harmless.
Still, it would be great if we removed them by renaming the variables in the innermost scope. This would avoid confusion, and remove the possibility of bugs where the declaration is removed, and the identifier references the outer scope.Tor: 0.2.9.x-finalNick MathewsonNick Mathewsonhttps://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-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/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/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/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/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/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/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/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/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/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-finalrl1987rl1987