The Tor Project issueshttps://gitlab.torproject.org/groups/tpo/-/issues2020-07-24T13:29:19Zhttps://gitlab.torproject.org/tpo/core/tor/-/issues/6313Many of Tor's complex functions should be refactored2020-07-24T13:29:19ZNick MathewsonMany of Tor's complex functions should be refactoredSee some discussion on legacy/trac#6177.
I'm attaching a list of the most complex functions in Tor (by cyclomatic complexity).
Possibly cyclomatic-complexity-per-line would be another good thing to look at.See some discussion on legacy/trac#6177.
I'm attaching a list of the most complex functions in Tor (by cyclomatic complexity).
Possibly cyclomatic-complexity-per-line would be another good thing to look at.Tor: unspecifiedhttps://gitlab.torproject.org/tpo/core/tor/-/issues/6298Tests should not depend on localhost==127.0.0.1 (Test 'addr/basic' failure in...2020-06-27T14:06:07ZAndrea ShepardTests should not depend on localhost==127.0.0.1 (Test 'addr/basic' failure in tor master)I just built branch 'master' (revision 46434ecf5b6f1ad88deb86fdac044c41ae4b534b) from the tor.git repository with gcc 4.5.3 on x86_64-linux using this configure line:
CC="gcc -m64 -mtune=core2" PKG_CONFIG_LIBDIR=/usr/lib64/pkgconfig PKG...I just built branch 'master' (revision 46434ecf5b6f1ad88deb86fdac044c41ae4b534b) from the tor.git repository with gcc 4.5.3 on x86_64-linux using this configure line:
CC="gcc -m64 -mtune=core2" PKG_CONFIG_LIBDIR=/usr/lib64/pkgconfig PKG_CONFIG_PATH=/usr/share/pkgconfig ./configure --prefix=/usr --libdir=/usr/lib64 --libexecdir=/usr/libexec64 --sysconfdir=/etc --localstatedir=/var --build=x86_64-linux --host=x86_64-linux --disable-linker-hardening --disable-asciidoc
This causes a test suite failure I haven't seen before:
addr/basic:
FAIL test_addr.c:36: assert((u32) == (0x7f000001u)): 2851995905 vs 2130706433
[basic FAILED]
Turns out the DNS in this hotel *resolves localhost to 169.254.1.1*. This is horribly wrong, of course, and I should have had a proper /etc/hosts for it anyway (*lashes self with wet noodle*), but why does the test suite depend on stuff random machines elsewhere on the network do at all to pass? Shouldn't we mock this stuff somehow?Tor: 0.3.1.x-finalNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/tpo/core/tor/-/issues/6238"Removed n/m microdescriptors as old" log message once per hour on notice2020-06-27T14:06:09ZSebastian Hahn"Removed n/m microdescriptors as old" log message once per hour on noticeLooks like we forgot to demote a log message because it only happens after a relay has been running for a couple of days. Probably we should move that message to infoLooks like we forgot to demote a log message because it only happens after a relay has been running for a couple of days. Probably we should move that message to infoTor: 0.2.3.x-finalhttps://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/6176Clean up service IDs2020-06-27T14:06:11ZAndrea ShepardClean up service IDsThere are several occurences in rendservice.c and rendclient.c of service IDs produced by hashing public keys. They should be properly zeroed when functions return/heap is freed.
* rendclient.c:
* lookup_last_hid_serv_request() (li...There are several occurences in rendservice.c and rendclient.c of service IDs produced by hashing public keys. They should be properly zeroed when functions return/heap is freed.
* rendclient.c:
* lookup_last_hid_serv_request() (line 430)
* directory_get_from_hs_dir() (line 539)
* rendservice.c:
* rend_service_intro_has_opened() (line 1562)
* rend_service_intro_established() (line 1680)
* rend_service_rendezvous_has_opened() (line 1721)
* upload_service_descriptor() (line 1981)
* rend_service_set_connection_addr_port() (line 2463)Tor: unspecifiedhttps://gitlab.torproject.org/tpo/core/tor/-/issues/6153Make circ_times static, and abstract most of its accessors.2020-06-27T14:06:13ZNick MathewsonMake circ_times static, and abstract most of its accessors.The circ_times structure is exported as a global from circuitbuild.c. It really shouldn't be. It has 3 kinds of external users FWICT:
* Things that pass it as an argument to circuit_build_*().
* Two functions in circuituse.c that wa...The circ_times structure is exported as a global from circuitbuild.c. It really shouldn't be. It has 3 kinds of external users FWICT:
* Things that pass it as an argument to circuit_build_*().
* Two functions in circuituse.c that want to know the current timeout values.
For the former we could use an accessor function, or we could have the functions in circuitbuild.c use the static object if they receive NULL. For the latter, we could have a function that returns the current timeouts.
Making this change would also let us more completely avoid looking at circ_times when LearnCircuitBuildTimeout is 0.Tor: 0.2.5.x-finalhttps://gitlab.torproject.org/tpo/core/tor/-/issues/6113We say GMT when we should say UTC2020-06-27T14:06:14ZNick MathewsonWe say GMT when we should say UTCIn several places throughout the codebase and specifications, we say "GMT" when we ought to say "UTC". Nearly all of these are trivial to replace, except for our "parse_rfc1123_time" and "format_rfc1123_time" functions, where we apparen...In several places throughout the codebase and specifications, we say "GMT" when we ought to say "UTC". Nearly all of these are trivial to replace, except for our "parse_rfc1123_time" and "format_rfc1123_time" functions, where we apparently depend on the exact string GMT. Everything else can change.Tor: unspecifiedhttps://gitlab.torproject.org/tpo/core/tor/-/issues/5915Write patch to make socks handshakes succeed instantly2022-07-20T16:29:47ZRoger DingledineWrite patch to make socks handshakes succeed instantlyIn legacy/trac#3875 Mike suggests we try out the usability of having the Tor in TBB just immediately succeed at all socks handshakes, and hang up if it gets an end cell rather than a connected cell.
We should whip up a Tor patch that do...In legacy/trac#3875 Mike suggests we try out the usability of having the Tor in TBB just immediately succeed at all socks handshakes, and hang up if it gets an end cell rather than a connected cell.
We should whip up a Tor patch that does this behavior, so somebody can try it out.
(We may or may not want to merge it into mainline Tor branches. I guess it depends how complex the patch is and how successful the tests are.)Tor: unspecifiedhttps://gitlab.torproject.org/tpo/core/tor/-/issues/5847Better error message on GETINFO desc/* when you only have MDs.2020-06-27T14:06:22ZTracBetter error message on GETINFO desc/* when you only have MDs.The "GETINFO desc/id/OR identity" and "GETINFO desc/name/OR nick" options don't seem to work on the Tor control port:
GETINFO desc/id/DD397148A4AB4D43E5E6CB9C5F45E922872CC2D3
552 Unrecognized key "desc/id/DD397148A4AB4D43E5E6CB9C5F45E92...The "GETINFO desc/id/OR identity" and "GETINFO desc/name/OR nick" options don't seem to work on the Tor control port:
GETINFO desc/id/DD397148A4AB4D43E5E6CB9C5F45E922872CC2D3
552 Unrecognized key "desc/id/DD397148A4AB4D43E5E6CB9C5F45E922872CC2D3"
If I replace "desc" with "md" I get the expected response, and they're both described in the spec as taking the same arguments...
**Trac**:
**Username**: mickeycTor: 0.3.2.x-finalhttps://gitlab.torproject.org/tpo/core/tor/-/issues/5823Remove dirreq-v2-* and dirreq-v?-share statistics2020-06-27T14:06:23ZKarsten LoesingRemove dirreq-v2-* and dirreq-v?-share statisticsThe v2 directory protocol is (almost?) dead, and we stopped using dirreq-v?-share lines, because they were highly inaccurate. Should we stop including these lines in extra-info descriptors? If so, I can prepare a patch.The v2 directory protocol is (almost?) dead, and we stopped using dirreq-v?-share lines, because they were highly inaccurate. Should we stop including these lines in extra-info descriptors? If so, I can prepare a patch.Tor: 0.2.4.x-finalKarsten LoesingKarsten Loesinghttps://gitlab.torproject.org/tpo/core/tor/-/issues/5760Safe cookie authentication failure replies do not end with a CRLF2020-06-27T14:06:25ZneenaSafe cookie authentication failure replies do not end with a CRLFOn the control socket, AUTHCHALLENGE's failure replies do not end with a CRLF.
```
+12:29% nc localhost 9100
AUTHCHALLENGE SAFEOHJOIE
513 AUTHCHALLENGE only supports SAFECOOKIE authentication%
```
```
+1:43% nc localhost 9100
AUTHCHALLE...On the control socket, AUTHCHALLENGE's failure replies do not end with a CRLF.
```
+12:29% nc localhost 9100
AUTHCHALLENGE SAFEOHJOIE
513 AUTHCHALLENGE only supports SAFECOOKIE authentication%
```
```
+1:43% nc localhost 9100
AUTHCHALLENGE SAFECOOKIE FOOFA
513 Invalid base16 client nonce%
```
That is all.Tor: 0.2.2.x-finalneenaneenahttps://gitlab.torproject.org/tpo/core/tor/-/issues/5605Why is control_ports_write_to_file() called from options_act_reversible()?2020-06-27T14:06:29ZRoger DingledineWhy is control_ports_write_to_file() called from options_act_reversible()?control_ports_write_to_file() writes out to a file every single time we setconf something. That's a minor issue, but ok.
What's weirder is that we call it from options_act_reversible(). There's a codepath (where the logs fail to initial...control_ports_write_to_file() writes out to a file every single time we setconf something. That's a minor issue, but ok.
What's weirder is that we call it from options_act_reversible(). There's a codepath (where the logs fail to initialize) where we end up refusing the new options but the control ports file remains written with the new options.
I think the better answer is to call it from options_act(). Also, it'd be slightly cleaner to pass 'options' in to the function.
Optimistically marking as easy (what could go wrong ;).Tor: 0.2.5.x-finalhttps://gitlab.torproject.org/tpo/core/tor/-/issues/5584Raise awareness of safer logging2020-06-27T14:06:31ZbastikRaise awareness of safer loggingCurrent releases of Tor give out a notice like:
[Notice] Tor can't help you if you use it wrong! Learn how to
be safe at (...)
Whenever logging is enabled Tor could give out a notice like:
[Notice] Please log safely. Don't log unless it...Current releases of Tor give out a notice like:
[Notice] Tor can't help you if you use it wrong! Learn how to
be safe at (...)
Whenever logging is enabled Tor could give out a notice like:
[Notice] Please log safely. Don't log unless it's serves an important reason. Please overwrite the log afterwards.
It's for clients and relays.Tor: 0.2.5.x-finalhttps://gitlab.torproject.org/tpo/core/tor/-/issues/5583Add option to overwrite logs2020-06-27T14:06:31ZbastikAdd option to overwrite logsAdd an option to make Tor overwrite logs, instead of appending them.
For instance whenever Tor gets restarted and this option is enabled the log does not get appended.*
This should enable operators to log without the risk of month old ...Add an option to make Tor overwrite logs, instead of appending them.
For instance whenever Tor gets restarted and this option is enabled the log does not get appended.*
This should enable operators to log without the risk of month old logs. Even when not enabled by default, what I was looking for in the first place, as I had not understood why logs are appending, it might help to have this option available.
The same applies to the client, once I was asked to provide logs from my client, and got told afterwards to remove the logging entry from torrc. When that does not happen, the client is logging for eternity and therefor provides an usage history due to the fact that the log is appending.
I couldn't pick "Tor" for "Component"; for me it's client and relay related.
*Other ways like, logging for a 24 hour period, keep this period and delete the lines that are older, might bring more problems than it's worth.Tor: 0.2.6.x-finalhttps://gitlab.torproject.org/tpo/core/tor/-/issues/5558tor_vsscanf() returns -1 if '%%' doesn't match a '%'.2020-06-27T14:06:32ZGeorge Kadianakistor_vsscanf() returns -1 if '%%' doesn't match a '%'.`tor_vsscanf()` returns -1 if '%%' doesn't match a '%', but `tor_vsscanf()` is supposed to return -1 only on malformed patterns.
`tor_vsscanf()` should return `n_matched` in this case.
```
} else if (*pattern == '%') {
if...`tor_vsscanf()` returns -1 if '%%' doesn't match a '%', but `tor_vsscanf()` is supposed to return -1 only on malformed patterns.
`tor_vsscanf()` should return `n_matched` in this case.
```
} else if (*pattern == '%') {
if (*buf != '%')
return -1;
++buf;
++pattern;
```Tor: 0.2.3.x-finalhttps://gitlab.torproject.org/tpo/core/tor/-/issues/5528router->address is redundant with router->addr2020-06-27T14:06:34ZRoger Dingledinerouter->address is redundant with router->addrIt used to be more, but these days address is always an IP address, and I believe it always matches router->addr.
So it's only around to save us the trouble of making a string out of ri->addr when we want one. Maybe that's not worth it ...It used to be more, but these days address is always an IP address, and I believe it always matches router->addr.
So it's only around to save us the trouble of making a string out of ri->addr when we want one. Maybe that's not worth it anymore.
Here are the places where it's set:
In router_parse_entry_from_string():
```
router->address = tor_strdup(tok->args[1]);
if (!tor_inet_aton(router->address, &in)) {
log_warn(LD_DIR,"Router address is not an IP address.");
goto err;
}
router->addr = ntohl(in.s_addr);
```
In rewrite_node_address_for_bridge():
```
ri->addr = tor_addr_to_ipv4h(&bridge->addr);
tor_free(ri->address);
ri->address = tor_dup_ip(ri->addr);
```
In router_rebuild_descriptor():
```
ri->address = tor_dup_ip(addr);
ri->nickname = tor_strdup(options->Nickname);
ri->addr = addr;
```
I think these three might be all the places.Tor: 0.2.5.x-finalhttps://gitlab.torproject.org/tpo/core/tor/-/issues/5526Expose current Accounting calculation2020-06-27T14:06:34ZTracExpose current Accounting calculationThere should be some way to to have a Tor relay configured with AccountingMax to write the current accounting state to the log file, either on command or periodically.
What I want to see is the bandwidth consumption for the current acco...There should be some way to to have a Tor relay configured with AccountingMax to write the current accounting state to the log file, either on command or periodically.
What I want to see is the bandwidth consumption for the current accounting period and the number of days/hours until the accounting period ends.
**Trac**:
**Username**: tmpname0901Tor: 0.2.5.x-finalhttps://gitlab.torproject.org/tpo/core/tor/-/issues/5505torify.in can turn back into torify2020-06-27T14:06:35ZNick Mathewsontorify.in can turn back into torifyOnce we had torify.in get preprocessed so that we could have a LOCALSTATEDIR reference in it or something. But we no longer need to set up an explicit tsocks configuration, now that we've removed torify's support for tsocks ... so we no...Once we had torify.in get preprocessed so that we could have a LOCALSTATEDIR reference in it or something. But we no longer need to set up an explicit tsocks configuration, now that we've removed torify's support for tsocks ... so we no longer need to preprocess torify.in into torify.
We should therefore:
* Rename torify.in to torify
* Remove torify from .gitignore
* Make torify, not torify.in get distributed
* Remove torify from the list of things that made by the aclocal substitutionTor: unspecifiedhttps://gitlab.torproject.org/tpo/core/tor/-/issues/5452Tor should complain if the user has configured a too-short CBT2020-06-27T14:06:37ZRobert RansomTor should complain if the user has configured a too-short CBTThe reporter of legacy/trac#5363 set the following torrc lines, and Tor got flaky:
```
LearnCircuitBuildTimeout 0
CircuitBuildTimeout 5
```
Perhaps Tor should warn the user if he/she/it has specified a non-adaptive circuit-build timeout...The reporter of legacy/trac#5363 set the following torrc lines, and Tor got flaky:
```
LearnCircuitBuildTimeout 0
CircuitBuildTimeout 5
```
Perhaps Tor should warn the user if he/she/it has specified a non-adaptive circuit-build timeout and Tor is failing to build too many circuits.Tor: 0.2.3.x-finalhttps://gitlab.torproject.org/tpo/core/tor/-/issues/5394warn users about their clock if they see a consensus from the future2020-06-27T14:06:39ZRoger Dingledinewarn users about their clock if they see a consensus from the futureIn router_set_networkstatus_v2() we check if the v2 ns is from the future, and warn if so:
```
if (ns->published_on > now + NETWORKSTATUS_ALLOW_SKEW) {
char dbuf[64];
long delta = now - ns->published_on;
format_time_interva...In router_set_networkstatus_v2() we check if the v2 ns is from the future, and warn if so:
```
if (ns->published_on > now + NETWORKSTATUS_ALLOW_SKEW) {
char dbuf[64];
long delta = now - ns->published_on;
format_time_interval(dbuf, sizeof(dbuf), delta);
log_warn(LD_GENERAL, "Network status from %s was published %s in the "
"future (%s GMT). Check your time and date settings! "
"Not caching.",
source_desc, dbuf, published);
control_event_general_status(LOG_WARN,
"CLOCK_SKEW MIN_SKEW=%ld SOURCE=NETWORKSTATUS:%s:%d",
delta, ns->source_address, ns->source_dirport);
skewed = 1;
}
```
We should do something similar for v3 consensuses.
Also, we might want to use a smaller value than '24 hours' for skew tolerance, since there's really no realistic way a consensus will validly appear from even the near future.Tor: 0.2.4.x-final