The Tor Project issueshttps://gitlab.torproject.org/groups/tpo/-/issues2022-02-07T19:38:04Zhttps://gitlab.torproject.org/tpo/core/tor/-/issues/31609Make CIRCUIT_IS_ORIGIN() look at the base magic number2022-02-07T19:38:04ZDavid Gouletdgoulet@torproject.orgMake CIRCUIT_IS_ORIGIN() look at the base magic numberCurrently, `CIRCUIT_IS_ORIGIN()` actually looks at the purpose, not the base magic number:
```
#define CIRCUIT_IS_ORIGIN(c) (CIRCUIT_PURPOSE_IS_ORIGIN((c)->purpose))
```
We should move it to look at the `magic` like `CIRCUIT_IS_ORCIRC(...Currently, `CIRCUIT_IS_ORIGIN()` actually looks at the purpose, not the base magic number:
```
#define CIRCUIT_IS_ORIGIN(c) (CIRCUIT_PURPOSE_IS_ORIGIN((c)->purpose))
```
We should move it to look at the `magic` like `CIRCUIT_IS_ORCIRC()` is doing.
The reason is because I was adding tracing events to the circuit subsystem and I kept having state transition event with a circuit global identifier of 0 which can't be because that value is set just after allocation.
But at that point, the purpose has not been set so `CIRCUIT_IS_ORIGIN()` wasn't returning true.
Furthermore, this made me discover another issue documented in legacy/trac#31608 where if we do make this change, we _must_ fix this ticket else we have a NULL deref.https://gitlab.torproject.org/tpo/core/tor/-/issues/31589hs-v3: Simplify decrypt_desc_layer interface2021-09-16T14:22:37ZGeorge Kadianakishs-v3: Simplify decrypt_desc_layer interfaceHere is how `decrypt_desc_layer` is called:
```
superencrypted_len = decrypt_desc_layer(desc,
desc->plaintext_data.superencrypted_blob,
desc->plaintext_data.superencrypt...Here is how `decrypt_desc_layer` is called:
```
superencrypted_len = decrypt_desc_layer(desc,
desc->plaintext_data.superencrypted_blob,
desc->plaintext_data.superencrypted_blob_size,
NULL, 1, &superencrypted_plaintext);
```
```
encrypted_len = decrypt_desc_layer(desc,
desc->superencrypted_data.encrypted_blob,
desc->superencrypted_data.encrypted_blob_size,
descriptor_cookie, 0, &encrypted_plaintext);
```
There is no point in passing `desc->superencrypted_data.encrypted_blob` and `desc->superencrypted_data.encrypted_blob_size` since we are already passing the whole `desc` and `is_superencrypted_layer` which should be enough to figure out which fields to use.
We could either of the following two things:
- Ditch `desc` as an argument and pass `desc->plaintext_data.blinded_pubkey` explicitly.
- Ditch `encrypted_blob` and `encrypted_blob_size` as arguments and get them off desc.
I prefer the first, but I'm fine with either, since it will make the interface cleaner.Tor: 0.4.2.x-finalhttps://gitlab.torproject.org/tpo/core/tor/-/issues/31579Space out the arguments to the cell functions in rend_process_relay_cell()2020-06-27T13:49:31ZNeel Chauhanneel@neelc.orgSpace out the arguments to the cell functions in rend_process_relay_cell()In the `switch (command) {` statement in `rend_process_relay_cell()`, arguments to the functions corresponding to cells aren't spaced, like:
```
r = hs_intro_received_establish_intro(or_circ,payload,length);
```In the `switch (command) {` statement in `rend_process_relay_cell()`, arguments to the functions corresponding to cells aren't spaced, like:
```
r = hs_intro_received_establish_intro(or_circ,payload,length);
```Tor: 0.4.2.x-finalNeel Chauhanneel@neelc.orgNeel Chauhanneel@neelc.orghttps://gitlab.torproject.org/tpo/core/tor/-/issues/31545CID 1452819: nul-terminated string handling, possibly spurious2020-06-27T13:49:33ZteorCID 1452819: nul-terminated string handling, possibly spuriousBug introduced by legacy/trac#21003, copying sponsors and tags.
```
/src/feature/nodelist/describe.c: 77 in format_node_description()
71 }
72 if (addr32h && has_addr) {
73 memcpy(cp, " and ", 5);
74 cp += 5;
...Bug introduced by legacy/trac#21003, copying sponsors and tags.
```
/src/feature/nodelist/describe.c: 77 in format_node_description()
71 }
72 if (addr32h && has_addr) {
73 memcpy(cp, " and ", 5);
74 cp += 5;
75 }
76 if (has_addr) {
CID 1452819: (STRING_NULL)
Passing unterminated string "cp" to "tor_addr_to_str", which expects a null-terminated string.
77 tor_addr_to_str(cp, addr, TOR_ADDR_BUF_LEN, 1);
78 }
79
80 return buf;
81 }
82
/src/feature/nodelist/describe.c: 70 in format_node_description()
64 cp += 4;
65 }
66 if (addr32h) {
67 struct in_addr in;
68 in.s_addr = htonl(addr32h);
69 tor_inet_ntoa(&in, cp, INET_NTOA_BUF_LEN);
CID 1452819: (STRING_NULL)
Passing unterminated string "cp" to "strlen", which expects a null-terminated string.
70 cp += strlen(cp);
71 }
72 if (addr32h && has_addr) {
73 memcpy(cp, " and ", 5);
74 cp += 5;
75 }
```
I think the best fix for this issue is using strncpy() rather than memcpy().Tor: 0.4.2.x-finalteorteorhttps://gitlab.torproject.org/tpo/core/tor/-/issues/31478CodeStructure.md is not markdown compliant2021-07-22T16:19:44ZTracCodeStructure.md is not markdown compliantHello,
While going through the Tor project documentation, I saw a TODO in the file `doc/HACKING/CodeStructure.md` which is currently using a malformed markdown.
I will re-format the documentation in this file if that's fine for you.
*...Hello,
While going through the Tor project documentation, I saw a TODO in the file `doc/HACKING/CodeStructure.md` which is currently using a malformed markdown.
I will re-format the documentation in this file if that's fine for you.
**Trac**:
**Username**: aveuillerTor: 0.4.2.x-finalhttps://gitlab.torproject.org/tpo/core/tor/-/issues/31477Practracker integration tests for headers and includes2021-09-16T14:22:37ZNick MathewsonPractracker integration tests for headers and includesWe added new practracker features in legacy/trac#31175 and legacy/trac#31176, but those were written before practracker had integration tests (legacy/trac#31263). We should add them to the practracker tests.We added new practracker features in legacy/trac#31175 and legacy/trac#31176, but those were written before practracker had integration tests (legacy/trac#31263). We should add them to the practracker tests.Tor: 0.4.2.x-finalNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/tpo/core/tor/-/issues/31476Practracker: document new features2021-09-16T14:22:37ZNick MathewsonPractracker: document new featuresOn 31176, asn notes:
>We should update the file-level comment of practracker.py to mention that we are now checking for include dependecies too. Same goes for the header of exceptions.txt .
>
>And maybe it's time to make a small README ...On 31176, asn notes:
>We should update the file-level comment of practracker.py to mention that we are now checking for include dependecies too. Same goes for the header of exceptions.txt .
>
>And maybe it's time to make a small README file to specify all the metrics that practracker is currently looking for, before they become too many?
This is a good ideaTor: 0.4.2.x-finalNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/tpo/core/tor/-/issues/31338Practracker --list-overbroad produces confusing output when there is an excep...2020-06-27T13:49:42ZteorPractracker --list-overbroad produces confusing output when there is an exceptionconnection_control_process_inbuf() is 115 lines long, but the exceptions file says it should be 113.
This line is spurious and should not appear in the output:
```
problem function-size /src/feature/control/control.c:connection_control_...connection_control_process_inbuf() is 115 lines long, but the exceptions file says it should be 113.
This line is spurious and should not appear in the output:
```
problem function-size /src/feature/control/control.c:connection_control_process_inbuf() 113 -> 0
```
Ideally, practracker should log a message that there are no over-broad exceptions, but there are violations.
Here is the full output:
```
$ scripts/maint/practracker/practracker.py --list-overbroad
problem function-size /src/feature/control/control.c:connection_control_process_inbuf() 115
FAILURE: practracker found 1 new problem(s) in the code: see warnings above.
Please fix the problems if you can, and update the exceptions file
(./scripts/maint/practracker/./exceptions.txt) if you can't.
See doc/HACKING/HelpfulTools.md for more information on using practracker.
You can disable this message by setting the TOR_DISABLE_PRACTRACKER environment
variable.
problem function-size /src/feature/control/control.c:connection_control_process_inbuf() 113 -> 0
Exit 1
```Tor: 0.4.2.x-finalNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/tpo/core/tor/-/issues/31311practracker: do not include unwanted files in distribution2021-09-16T14:23:38ZNick Mathewsonpractracker: do not include unwanted files in distributionRight now, "make dist" grabs the entire contents of the practracker directory, including temporary files. That's not a great idea.Right now, "make dist" grabs the entire contents of the practracker directory, including temporary files. That's not a great idea.Tor: 0.4.1.x-finalNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/tpo/core/tor/-/issues/31309Add envvar to pass options to practracker2021-09-16T14:23:38ZNick MathewsonAdd envvar to pass options to practrackerOn legacy/trac#30752, catalyst notes:
>The Makefile should provide a way to pass command line options in to practracker from the environment, so CI won't have to run practracker by bypassing the Makefile. This can be useful if we want to...On legacy/trac#30752, catalyst notes:
>The Makefile should provide a way to pass command line options in to practracker from the environment, so CI won't have to run practracker by bypassing the Makefile. This can be useful if we want to run with less-friendly options during cron builds, for example.
I agree.Tor: 0.4.2.x-finalNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/tpo/core/tor/-/issues/31304Run practracker_tests.py as part of make check2021-09-16T14:23:38ZteorRun practracker_tests.py as part of make checkRunning these tests in CI will help avoid future bugs like:
The practracker_tests.py unit test file called a function by its old
name.
This change does not block legacy/trac#29746, but it should be done in 0.4.2.Running these tests in CI will help avoid future bugs like:
The practracker_tests.py unit test file called a function by its old
name.
This change does not block legacy/trac#29746, but it should be done in 0.4.2.Tor: 0.4.2.x-finalNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/tpo/core/tor/-/issues/31263More tests for practracker2021-09-16T14:23:38ZNick MathewsonMore tests for practrackerI think some integration tests here would make things easier to hack on.I think some integration tests here would make things easier to hack on.Tor: 0.4.2.x-finalNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/tpo/core/tor/-/issues/31202Refactor practracker's issue-listing code to return a generator2021-09-16T14:23:38ZNick MathewsonRefactor practracker's issue-listing code to return a generatorRight now, this issue-listing code exposes the problems with "print()". Instead, it should yield them as a generator, so that the invoking code can decide what to do with them.Right now, this issue-listing code exposes the problems with "print()". Instead, it should yield them as a generator, so that the invoking code can decide what to do with them.Tor: 0.4.2.x-finalNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/tpo/core/tor/-/issues/31176Teach practracker about .may_include files2021-09-16T14:23:38ZNick MathewsonTeach practracker about .may_include filesWe would like to introduce a second category of .may_include rules: those that should only apply on an advisory basis. We would treat violations of these rules as a best practices violation rather than an error. It would allow us to st...We would like to introduce a second category of .may_include rules: those that should only apply on an advisory basis. We would treat violations of these rules as a best practices violation rather than an error. It would allow us to start ratcheting down the number of layering violations.Tor: 0.4.2.x-finalNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/tpo/core/tor/-/issues/31156Add support of TBytes keyword to torrc for AccountingMax setting (and maybe o...2020-07-30T20:37:37ZTracAdd support of TBytes keyword to torrc for AccountingMax setting (and maybe others)Happening on Debian 9 with Tor 0.2.9.16 from the standard apt repo.
(Sorry, if this is already resolved in newer versions. In that case just close and ignore.)
Syslog output:
```
Jul 14 10:50:38 systemd[1]: Starting Anonymizing overla...Happening on Debian 9 with Tor 0.2.9.16 from the standard apt repo.
(Sorry, if this is already resolved in newer versions. In that case just close and ignore.)
Syslog output:
```
Jul 14 10:50:38 systemd[1]: Starting Anonymizing overlay network for TCP...
Jul 14 10:50:39 tor[530]: Jul 14 10:50:39.082 [notice] Tor 0.2.9.16 (git-9ef571339967c1e5) running on Linux with Libevent 2.0.21-stable, OpenSSL 1.1.0k and Zlib 1.2.8.
Jul 14 10:50:39 tor[530]: Jul 14 10:50:39.082 [notice] Tor can't help you if you use it wrong! Learn how to be safe at https://www.torproject.org/download/download#warning
Jul 14 10:50:39 tor[530]: Jul 14 10:50:39.082 [notice] Read configuration file "/usr/share/tor/tor-service-defaults-torrc".
Jul 14 10:50:39 tor[530]: Jul 14 10:50:39.082 [notice] Read configuration file "/etc/tor/torrc".
Jul 14 10:50:39 tor[530]: Jul 14 10:50:39.085 [warn] Unknown unit 'TBytes'.
Jul 14 10:50:39 tor[530]: Jul 14 10:50:39.085 [warn] Failed to parse/validate config: Value 'AccountingMax 5 TBytes' is malformed or out of bounds.
Jul 14 10:50:39 tor[530]: Jul 14 10:50:39.085 [err] Reading config failed--see warnings above.
Jul 14 10:50:39 systemd[1]: tor@default.service: Control process exited, code=exited status=1
Jul 14 10:50:39 systemd[1]: Failed to start Anonymizing overlay network for TCP.
```
Additionally, please note that /var/tor/log didn't give any hint of this, instead it went to syslog. Suboptimal.
**Trac**:
**Username**: tlahttps://gitlab.torproject.org/tpo/core/tor/-/issues/30920Detect uint64 overflow in config_parse_units()2020-06-27T13:49:58ZNick MathewsonDetect uint64 overflow in config_parse_units()The config_parse_units function uses 64-bit arithmetic, but does not detect 64-bit overflow. This means that values like "20000000 TB", which should be rejected, are instead mis-parsed.
Since this function is only used for configuratio...The config_parse_units function uses 64-bit arithmetic, but does not detect 64-bit overflow. This means that values like "20000000 TB", which should be rejected, are instead mis-parsed.
Since this function is only used for configuration parsing, it's not a huge issue, but it should be simple enough to resolve this.
Found while working on 30893.Tor: 0.4.3.x-finalhttps://gitlab.torproject.org/tpo/core/tor/-/issues/30782Cache outdated fetched descriptors on directory authorities2020-06-27T13:50:03ZteorCache outdated fetched descriptors on directory authoritiesWhen tor starts fetching a descriptor due to a consensus, then replaces its consensus before the descriptor arrives, the descriptor is unrecognised.
Directory caches keep the descriptor in the old descriptor cache, but clients and autho...When tor starts fetching a descriptor due to a consensus, then replaces its consensus before the descriptor arrives, the descriptor is unrecognised.
Directory caches keep the descriptor in the old descriptor cache, but clients and authorities drop it.
But I think authorities should keep the descriptor, in case it is used in a future vote or consensus.
This is a bug on tor 0.1.1.13-alpha.Tor: 0.4.2.x-finalteorteorhttps://gitlab.torproject.org/tpo/core/tor/-/issues/30780Return a distinct was_router_added_t when formatting annotations fails2021-09-16T14:23:38ZteorReturn a distinct was_router_added_t when formatting annotations failsThere's a note in dirserv_add_multiple_descriptors() that this is bad.
But no-one ever fixed it.There's a note in dirserv_add_multiple_descriptors() that this is bad.
But no-one ever fixed it.Tor: 0.4.2.x-finalteorteorhttps://gitlab.torproject.org/tpo/core/tor/-/issues/30642Add an ed25519-identity file to the data directory2022-03-31T17:09:18ZteorAdd an ed25519-identity file to the data directoryRelays print their RSA fingerprint to a "fingerprint" file in their data directory.
We need an equivalent file for base-64 encoded ed25519 public keys.Relays print their RSA fingerprint to a "fingerprint" file in their data directory.
We need an equivalent file for base-64 encoded ed25519 public keys.David Gouletdgoulet@torproject.orgDavid Gouletdgoulet@torproject.orghttps://gitlab.torproject.org/tpo/core/tor/-/issues/30630Put CI URLs in ReleasingTor.md2021-07-22T16:19:44ZteorPut CI URLs in ReleasingTor.mdarma asked us where the CI builders are.
The list is in ReleasingTor.md and on the wiki, but only the wiki has the URLs:
https://trac.torproject.org/projects/tor/wiki/org/teams/NetworkTeam/CIFailures#CIBuildersarma asked us where the CI builders are.
The list is in ReleasingTor.md and on the wiki, but only the wiki has the URLs:
https://trac.torproject.org/projects/tor/wiki/org/teams/NetworkTeam/CIFailures#CIBuildersTor: 0.4.1.x-final