Trac issueshttps://gitlab.torproject.org/legacy/trac/-/issues2020-06-13T14:58:00Zhttps://gitlab.torproject.org/legacy/trac/-/issues/19179Refactor functions that handle 'packages' in consensus/votes2020-06-13T14:58:00ZGeorge KadianakisRefactor functions that handle 'packages' in consensus/votesThis is a side issue of #18840.
The code managing packages for consensuses and votes seems to be of particularly low quality.
See compute_consensus_package_lines() doing ad-hoc parsing. And see validate_recommended_package_line() doing...This is a side issue of #18840.
The code managing packages for consensuses and votes seems to be of particularly low quality.
See compute_consensus_package_lines() doing ad-hoc parsing. And see validate_recommended_package_line() doing more ad-hoc parsing and having wrong return value patterns. Fortunately, both of them are weakly tested in the unittests.
Maybe we should refactor and add more testing for these funcs?Tor: unspecifiedhttps://gitlab.torproject.org/legacy/trac/-/issues/19130Seg fault in round_int64_to_next_multiple_of()2020-06-13T14:57:39ZRoger DingledineSeg fault in round_int64_to_next_multiple_of()On moria1, git 249f3a16
```
#0 0x00007f8412479625 in raise () from /lib64/libc.so.6
#1 0x00007f841247ae05 in abort () from /lib64/libc.so.6
#2 0x00007f8413caee1d in __subvdi3 ()
#3 0x00007f8413c5ddc0 in round_int64_to_next_multiple_o...On moria1, git 249f3a16
```
#0 0x00007f8412479625 in raise () from /lib64/libc.so.6
#1 0x00007f841247ae05 in abort () from /lib64/libc.so.6
#2 0x00007f8413caee1d in __subvdi3 ()
#3 0x00007f8413c5ddc0 in round_int64_to_next_multiple_of (number=72742,
divisor=1024) at src/common/util.c:523
#4 0x00007f8413b7d004 in rep_hist_format_hs_stats (now=1463682482)
at src/or/rephist.c:3074
#5 rep_hist_hs_stats_write (now=1463682482) at src/or/rephist.c:3122
#6 0x00007f8413b4b3f8 in write_stats_file_callback (now=1463682482,
options=0x7f84146c6880) at src/or/main.c:1761
#7 0x00007f8413b60ef0 in periodic_event_dispatch (fd=<value optimized out>,
what=<value optimized out>, data=0x7f8413f4b260) at src/or/periodic.c:52
#8 0x00007f841323cb44 in event_base_loop () from /usr/lib64/libevent-1.4.so.2
#9 0x00007f8413b48e2a in run_main_loop_once () at src/or/main.c:2548
#10 run_main_loop_until_done () at src/or/main.c:2592
#11 do_main_loop () at src/or/main.c:2520
#12 0x00007f8413b49ec5 in tor_main (argc=<value optimized out>,
argv=<value optimized out>) at src/or/main.c:3647
#13 0x00007f8413b45ec9 in main (argc=<value optimized out>,
argv=<value optimized out>) at src/or/tor_main.c:30
(gdb) up
#1 0x00007f841247ae05 in abort () from /lib64/libc.so.6
(gdb) up
#2 0x00007f8413caee1d in __subvdi3 ()
(gdb) up
#3 0x00007f8413c5ddc0 in round_int64_to_next_multiple_of (number=72742,
divisor=1024) at src/common/util.c:523
523 if (INT64_MAX - divisor + 1 < number)
```
Looks like it's in the "add noise when reporting the number of onion addresses it's seen" area.
I do have a couple of local patches applied to moria1, but "surely" they aren't interacting with this bug.Tor: 0.2.9.x-finalNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/legacy/trac/-/issues/19116Add scripts to chutney so it works on a tor binary2020-06-13T13:29:44ZteorAdd scripts to chutney so it works on a tor binaryIf chutney was self-contained and could run tests on a plain tor binary, it would be easier to do jenkins tests using chutney.
To make chutney work, we use parts of the tor git tree, including some Makefile targets to select the tests w...If chutney was self-contained and could run tests on a plain tor binary, it would be easier to do jenkins tests using chutney.
To make chutney work, we use parts of the tor git tree, including some Makefile targets to select the tests we run, and src/test/test-network.sh.
If we move these scripts to the chutney tree, we can run a set of chutney targets anywhere, without requiring a tor build tree.teorteorhttps://gitlab.torproject.org/legacy/trac/-/issues/19033Fuzz out of bounds reads during nodelist processing2020-06-13T14:57:18ZteorFuzz out of bounds reads during nodelist processingWe want to make sure we fixed all the issues with nodelist processing in #19032.
arma says this could be SponsorS or SponsorU.We want to make sure we fixed all the issues with nodelist processing in #19032.
arma says this could be SponsorS or SponsorU.Tor: unspecifiedhttps://gitlab.torproject.org/legacy/trac/-/issues/18902Avoid variable shadowing in Tor2020-06-13T14:56:45ZteorAvoid 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/legacy/trac/-/issues/18897Narrow scan-build checkers to those that have an acceptably low false positiv...2020-06-13T14:56:43ZNick MathewsonNarrow scan-build checkers to those that have an acceptably low false positive rate.Our scan-build.sh script was written to use as many checkers as possible. But the false positive rate is unacceptably high -- to the point that we haven't routinized its use. We should instead try to get the false positive rate to the ...Our scan-build.sh script was written to use as many checkers as possible. But the false positive rate is unacceptably high -- to the point that we haven't routinized its use. We should instead try to get the false positive rate to the point where we are willing to run it daily and actually look at its output.Tor: unspecifiedhttps://gitlab.torproject.org/legacy/trac/-/issues/18895--enable-expensive-hardening has hard-to-debug failures when run-time librari...2020-06-13T14:56:42ZNick Mathewson--enable-expensive-hardening has hard-to-debug failures when run-time libraries aren't installedIf libabsan or libubsan or libclang_rt.asan or libclang_rt.ubsan is missing, and you try to build with --enable-expensive-hardening, then the autoconf scripts will generate an orconfig.h file that doesn't allow Tor to compile.
What we s...If libabsan or libubsan or libclang_rt.asan or libclang_rt.ubsan is missing, and you try to build with --enable-expensive-hardening, then the autoconf scripts will generate an orconfig.h file that doesn't allow Tor to compile.
What we should probably do here is warn about the missing libraries when --enable-expensive-hardening is in use but the runtime libraries are not present.Tor: 0.2.9.x-finalNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/legacy/trac/-/issues/18827Allow specifying the Chutney data directory2020-06-13T13:29:42ZanonymAllow specifying the Chutney data directoryWith "Chutney data directory" I am referring to the `net` sub-directory. I find it preferable to be able to store such cruft outside of the source tree. See the attached patch.With "Chutney data directory" I am referring to the `net` sub-directory. I find it preferable to be able to store such cruft outside of the source tree. See the attached patch.anonymanonymhttps://gitlab.torproject.org/legacy/trac/-/issues/18826Allow setting the listen address for all Chutney hosts2020-06-13T13:29:42ZanonymAllow setting the listen address for all Chutney hostsIn Tails' automated test suite we have plans to use Chutney to run a simulated Tor network (for increased performance and, in particular, robustness). Of course, we do not want to run that Chutney instance in the system under testing (i....In Tails' automated test suite we have plans to use Chutney to run a simulated Tor network (for increased performance and, in particular, robustness). Of course, we do not want to run that Chutney instance in the system under testing (i.e. the Tails instance) but on the host orchestrating the tests. In order to avoid having to forward traffic via firewall rules, we'd like it to be possible to make the Chutney hosts listen on some other IP address than localhost.
The attached patch allows setting the address all Chutney hosts will listen on via the `CHUTNEY_LISTEN_ADDRESS` environment variable.anonymanonymhttps://gitlab.torproject.org/legacy/trac/-/issues/18809Handle linked connections better during bootstrap2020-06-13T14:57:32ZteorHandle linked connections better during bootstrapbegindir connections advance directly to the CLIENT_SENDING state, even before their TLS connection is created.
So we need to modify the logic from #4483 that checks for consensuses that are downloading:
* is it a direct connection in s...begindir connections advance directly to the CLIENT_SENDING state, even before their TLS connection is created.
So we need to modify the logic from #4483 that checks for consensuses that are downloading:
* is it a direct connection in state after connecting, or
* a linked connection that's attached, or
* a linked connection that's in a state after sending
And then do the standard "extra consensus download check" when we're about to link a directory connection to a TLS connection.
Or arma may have a better idea.
I am not sure whether we should fix this in 0.2.8, because the current code works, but it's not optimal when too many TLS connections hang.
Tagging it just in case.Tor: 0.2.8.x-finalteorteorhttps://gitlab.torproject.org/legacy/trac/-/issues/18685Fire a`STATUS_SERVER` event when the hibernation state changes.2020-06-13T14:55:53ZYawning AngelFire a`STATUS_SERVER` event when the hibernation state changes.Most of the hibernation related information is already exposed in a useful manner via `GETINFO`. As far as I can tell the only really other useful thing to do here is to fire an event whenever we enter/exit hibernation so that controlle...Most of the hibernation related information is already exposed in a useful manner via `GETINFO`. As far as I can tell the only really other useful thing to do here is to fire an event whenever we enter/exit hibernation so that controllers don't have to poll continuously via `GETINFO`.Tor: 0.2.9.x-finalhttps://gitlab.torproject.org/legacy/trac/-/issues/18673Memory leak with TestingEnableCellStatsEvent2020-06-13T14:55:52ZNick MathewsonMemory leak with TestingEnableCellStatsEventRunning chutney with asan, I see:
```
=================================================================
==13895==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 1040 byte(s) in 65 object(s) allocated from:
#0 0x55db7087d...Running chutney with asan, I see:
```
=================================================================
==13895==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 1040 byte(s) in 65 object(s) allocated from:
#0 0x55db7087dacb in __interceptor_malloc llvm.src/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:40:3
#1 0x55db70d23dd2 in tor_malloc_ /home/nickm/src/tor/src/common/util.c:171:12
#2 0x55db70cf4847 in smartlist_new /home/nickm/src/tor/src/common/container.c:34:21
#3 0x55db7093e34d in channel_flush_from_first_active_circuit /home/nickm/src/tor/src/or/relay.c:2652:38
#4 0x55db70a6bd22 in channel_flush_some_cells /home/nickm/src/tor/src/or/channel.c:2215:30
#5 0x55db70a35810 in scheduler_run /home/nickm/src/tor/src/or/scheduler.c:421:13
#6 0x55db70a35060 in scheduler_evt_callback /home/nickm/src/tor/src/or/scheduler.c:234:3
#7 0x7f8baa215178 in event_base_loop (/lib64/libevent-2.0.so.5+0x10178)
Indirect leak of 37376 byte(s) in 23 object(s) allocated from:
#0 0x55db7087de1e in realloc llvm.src/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:61:3
#1 0x55db70d242dc in tor_realloc_ /home/nickm/src/tor/src/common/util.c:259:12
#2 0x55db70d24550 in tor_reallocarray_ /home/nickm/src/tor/src/common/util.c:280:10
#3 0x55db70cf509f in smartlist_ensure_capacity /home/nickm/src/tor/src/common/container.c:87:16
#4 0x55db70cf4df1 in smartlist_add /home/nickm/src/tor/src/common/container.c:101:3
#5 0x55db7093e3ac in channel_flush_from_first_active_circuit /home/nickm/src/tor/src/or/relay.c:2653:9
#6 0x55db70a6bd22 in channel_flush_some_cells /home/nickm/src/tor/src/or/channel.c:2215:30
#7 0x55db70a35810 in scheduler_run /home/nickm/src/tor/src/or/scheduler.c:421:13
#8 0x55db70a35060 in scheduler_evt_callback /home/nickm/src/tor/src/or/scheduler.c:234:3
#9 0x7f8baa215178 in event_base_loop (/lib64/libevent-2.0.so.5+0x10178)
Indirect leak of 18636 byte(s) in 4659 object(s) allocated from:
#0 0x55db7087dacb in __interceptor_malloc llvm.src/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:40:3
#1 0x55db70d23dd2 in tor_malloc_ /home/nickm/src/tor/src/common/util.c:171:12
#2 0x55db70d23f6f in tor_malloc_zero_ /home/nickm/src/tor/src/common/util.c:197:18
#3 0x55db7093e195 in channel_flush_from_first_active_circuit /home/nickm/src/tor/src/or/relay.c:2645:11
#4 0x55db70a6bd22 in channel_flush_some_cells /home/nickm/src/tor/src/or/channel.c:2215:30
#5 0x55db70a35810 in scheduler_run /home/nickm/src/tor/src/or/scheduler.c:421:13
#6 0x55db70a35060 in scheduler_evt_callback /home/nickm/src/tor/src/or/scheduler.c:234:3
#7 0x7f8baa215178 in event_base_loop (/lib64/libevent-2.0.so.5+0x10178)
Indirect leak of 5376 byte(s) in 42 object(s) allocated from:
#0 0x55db7087dacb in __interceptor_malloc llvm.src/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:40:3
#1 0x55db70d23dd2 in tor_malloc_ /home/nickm/src/tor/src/common/util.c:171:12
#2 0x55db70d240a4 in tor_malloc_zero_ /home/nickm/src/tor/src/common/util.c:197:18
#3 0x55db70d240a4 in tor_calloc_ /home/nickm/src/tor/src/common/util.c:235
#4 0x55db70cf48f7 in smartlist_new /home/nickm/src/tor/src/common/container.c:37:14
#5 0x55db7093e34d in channel_flush_from_first_active_circuit /home/nickm/src/tor/src/or/relay.c:2652:38
#6 0x55db70a6bd22 in channel_flush_some_cells /home/nickm/src/tor/src/or/channel.c:2215:30
#7 0x55db70a35810 in scheduler_run /home/nickm/src/tor/src/or/scheduler.c:421:13
#8 0x55db70a35060 in scheduler_evt_callback /home/nickm/src/tor/src/or/scheduler.c:234:3
#9 0x7f8baa215178 in event_base_loop (/lib64/libevent-2.0.so.5+0x10178)
SUMMARY: AddressSanitizer: 62428 byte(s) leaked in 4789 allocation(s).
```
Now, this only seems to happen with TestingEnableCellStatsEvent enabled, so it's not a showstopper on the real network, but it's no fun. We should fix this leak.Tor: 0.2.8.x-finalhttps://gitlab.torproject.org/legacy/trac/-/issues/18672Make test-network and test-stem begin with asan turned on2020-06-13T14:55:50ZNick MathewsonMake test-network and test-stem begin with asan turned onWhen I `--enable-expensive-hardening`, there are are some memory leaks in the helper commands that prevent the test scripts from starting.When I `--enable-expensive-hardening`, there are are some memory leaks in the helper commands that prevent the test scripts from starting.Tor: 0.2.8.x-finalhttps://gitlab.torproject.org/legacy/trac/-/issues/18668Numerous WSAStartup warnings in unit tests on windows2020-06-13T14:55:50ZNick MathewsonNumerous WSAStartup warnings in unit tests on windowsHead on over to Jenkins, and look at the windows builder output. It's complaining a lot that we aren't calling WSAStartup. We should fix that.
It's not causing bugs or preventing the tests from passing, but it sure is ugly.Head on over to Jenkins, and look at the windows builder output. It's complaining a lot that we aren't calling WSAStartup. We should fix that.
It's not causing bugs or preventing the tests from passing, but it sure is ugly.Tor: 0.2.9.x-finalNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/legacy/trac/-/issues/18665Make test_util_time pieces pass on windows2020-06-13T14:55:49ZNick MathewsonMake test_util_time pieces pass on windowsThere are some pieces in test_util_time that I disabled to make the tests pass. Presumably these never worked on Windows; but we should re-enable them and make them work.There are some pieces in test_util_time that I disabled to make the tests pass. Presumably these never worked on Windows; but we should re-enable them and make them work.Tor: unspecifiedhttps://gitlab.torproject.org/legacy/trac/-/issues/18618Write a tool to move functions around from module to module2020-06-13T14:55:27ZNick MathewsonWrite a tool to move functions around from module to moduleIt's annoying to move code between modules, especially when doing a _bulk_ move, since you can't easily verify the move, and you can't easily review the move decision without reviewing huge patches.
I'm working on a tool to fix that by ...It's annoying to move code between modules, especially when doing a _bulk_ move, since you can't easily verify the move, and you can't easily review the move decision without reviewing huge patches.
I'm working on a tool to fix that by doing a two step annotate-then-move process.Tor: 0.2.9.x-finalNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/legacy/trac/-/issues/18617Write a tool to check for modularity violations in Tor's codebase2020-06-13T14:55:26ZNick MathewsonWrite a tool to check for modularity violations in Tor's codebaseIt's easy to find out which C files look at symbols from which other C files. If we combine that with the ability to know which C files belong to which modules, and which functions should move to _other_ C files, we'll be in good shape ...It's easy to find out which C files look at symbols from which other C files. If we combine that with the ability to know which C files belong to which modules, and which functions should move to _other_ C files, we'll be in good shape for a low-impact redividing of our source code into well-considered modules, to further simplify our modulewise callgraph.Tor: 0.2.9.x-finalNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/legacy/trac/-/issues/18491Stop logging identifying bridge info at 'notice'2020-06-13T14:55:05ZLinus Nordberglinus@torproject.orgStop logging identifying bridge info at 'notice'A client running with bridges logs "new bridge descriptor '$NICKNAME'" and the bridge IP address at level 'notice'.
If the bridge has both an IPv4 and an IPv6 ORPort, there's an additional log line, this too at the 'notice' level.
The...A client running with bridges logs "new bridge descriptor '$NICKNAME'" and the bridge IP address at level 'notice'.
If the bridge has both an IPv4 and an IPv6 ORPort, there's an additional log line, this too at the 'notice' level.
There might be more.Tor: unspecifiedhttps://gitlab.torproject.org/legacy/trac/-/issues/18363Tor could use a publish/subscribe abstraction2020-06-13T14:57:12ZNick MathewsonTor could use a publish/subscribe abstractionMany places in our codebase, we have a function "foo_has_occurred()" that is called whenever foo happens, and which dispatches into many other semi-unrelated modules. These functions tend to be fairly huge modularity violations, and cre...Many places in our codebase, we have a function "foo_has_occurred()" that is called whenever foo happens, and which dispatches into many other semi-unrelated modules. These functions tend to be fairly huge modularity violations, and create many surprising edges in our module callgraph. Using a publish/subscribe pattern would be the usual way to enforce these divisions.Tor: 0.2.9.x-finalNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/legacy/trac/-/issues/18362Tor could use a generic 'handle' implementation.2020-06-13T14:54:38ZNick MathewsonTor could use a generic 'handle' implementation.Frequently we want to have one object have a pointer to another, but we don't want to have the first object own the second. In these cases, we need to do one of the following ugly C dances:
* We make sure that the pointed-to object n...Frequently we want to have one object have a pointer to another, but we don't want to have the first object own the second. In these cases, we need to do one of the following ugly C dances:
* We make sure that the pointed-to object never outlives the pointing object.
* We make sure that when the pointed-to object is freed, the pointer to it is set to NULL.
* Instead of using a pointer, we use some kind of unique identifier, and look up the pointed-to object in a hash table.
The first two options are error-prone, and the third is slower than regular pointer access.
Instead of these choices, we could use a 'handle' pattern to create a standard way to look up objects indirectly; we could use some of the tricks from a usual 'weak reference' implementation. Ideally, we could write the interface in such a way as to permit more than one possible implementation.
The branch `weakref` in my public repository has some janky progress towards a solution here.Tor: 0.2.9.x-finalNick MathewsonNick Mathewson