Trac issueshttps://gitlab.torproject.org/legacy/trac/-/issues2020-06-13T15:41:46Zhttps://gitlab.torproject.org/legacy/trac/-/issues/30629We seem to be reading some freed events on exit2020-06-13T15:41:46ZRoger DingledineWe seem to be reading some freed events on exitRun your Tor master as a client under valgrind:
```
$ valgrind --leak-check=full src/app/tor
```
and wait for it to bootstrap to 100%. Then ctrl-c it.
On exit, valgrind will give you a pile of complaints like
```
==4119== Invalid read o...Run your Tor master as a client under valgrind:
```
$ valgrind --leak-check=full src/app/tor
```
and wait for it to bootstrap to 100%. Then ctrl-c it.
On exit, valgrind will give you a pile of complaints like
```
==4119== Invalid read of size 8
==4119== at 0x4C1DB9C: ??? (in /usr/lib/x86_64-linux-gnu/libevent-2.1.so.6.0.2)
==4119== by 0x4C21A78: event_free (in /usr/lib/x86_64-linux-gnu/libevent-2.1.so.6.0.2)
==4119== by 0x2ADA19: tor_event_free_ (compat_libevent.c:76)
==4119== by 0x2ADA19: mainloop_event_free_ (compat_libevent.c:461)
==4119== by 0x17748B: tor_mainloop_free_all (mainloop.c:2523)
==4119== by 0x1665FB: subsystems_shutdown_downto (subsysmgr.c:185)
==4119== by 0x165FB4: tor_free_all (shutdown.c:162)
==4119== by 0x164B54: tor_run_main (main.c:1360)
==4119== by 0x1620F9: tor_main (tor_api.c:164)
==4119== by 0x161CB8: main (tor_main.c:32)
==4119== Address 0x5489ec0 is 432 bytes inside a block of size 664 free'd
==4119== at 0x48369AB: free (vg_replace_malloc.c:530)
==4119== by 0x2ADB20: tor_libevent_free_all (compat_libevent.c:490)
==4119== by 0x165FAF: tor_free_all (shutdown.c:160)
==4119== by 0x164B54: tor_run_main (main.c:1360)
==4119== by 0x1620F9: tor_main (tor_api.c:164)
==4119== by 0x161CB8: main (tor_main.c:32)
==4119== Block was alloc'd at
==4119== at 0x483577F: malloc (vg_replace_malloc.c:299)
==4119== by 0x310F47: tor_malloc_ (malloc.c:45)
==4119== by 0x4C1E9B3: event_mm_calloc_ (in /usr/lib/x86_64-linux-gnu/libevent-2.1.so.6.0.2)
==4119== by 0x4C224D9: event_base_new_with_config (in /usr/lib/x86_64-linux-gnu/libevent-2.1.so.6.0.2)
==4119== by 0x2AD284: tor_libevent_initialize (compat_libevent.c:158)
==4119== by 0x28E879: init_libevent (config.c:8031)
==4119== by 0x28E879: options_act_reversible (config.c:1466)
==4119== by 0x28E879: set_options (config.c:934)
==4119== by 0x290721: options_init_from_string (config.c:5529)
==4119== by 0x290CA9: options_init_from_torrc (config.c:5293)
==4119== by 0x1632A6: tor_init (main.c:619)
==4119== by 0x163B13: tor_run_main (main.c:1297)
==4119== by 0x1620F9: tor_main (tor_api.c:164)
==4119== by 0x161CB8: main (tor_main.c:32)
```
maint-0.4.0 does not have this bug, and tor-0.4.1.1-alpha does.
A git bisect brought me to commit 6eb1b8da0ab2, which is about periodic events so it looks promising. It's from #30293.Tor: 0.4.1.x-finalNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/legacy/trac/-/issues/30539Add an "autostyle" target to apply all of our automatic restyling things.2020-06-13T15:41:34ZNick MathewsonAdd an "autostyle" target to apply all of our automatic restyling things.Let's have a nice simple automatic styling target to run all of our code cleanup tools.Let's have a nice simple automatic styling target to run all of our code cleanup tools.Tor: 0.4.1.x-finalNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/legacy/trac/-/issues/30475hs_service.c: compile-time warning with GCC 9.1.12020-06-13T15:41:29ZNick Mathewsonhs_service.c: compile-time warning with GCC 9.1.1I tried building with GCC 9.1.1 for the first time, and got various warnings.
```
make[1]: Entering directory '/home/nickm/src/tor-035'
CC src/feature/hs/hs_service.o
In file included from ./src/lib/crypt_ops/crypto_rsa.h:21,
...I tried building with GCC 9.1.1 for the first time, and got various warnings.
```
make[1]: Entering directory '/home/nickm/src/tor-035'
CC src/feature/hs/hs_service.o
In file included from ./src/lib/crypt_ops/crypto_rsa.h:21,
from ./src/core/or/or.h:32,
from src/feature/hs/hs_service.c:11:
In function ‘load_client_keys’,
inlined from ‘load_service_keys’ at src/feature/hs/hs_service.c:1090:7,
inlined from ‘hs_service_load_all_keys’ at src/feature/hs/hs_service.c:4010:9:
./src/lib/log/log.h:244:3: error: ‘%s’ directive argument is null [-Werror=format-overflow=]
244 | log_fn_(LOG_WARN, domain, __FUNCTION__, args, ##__VA_ARGS__)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/feature/hs/hs_service.c:1267:7: note: in expansion of macro ‘log_warn’
1267 | log_warn(LD_REND, "Client authorization file %s can't be read. "
| ^~~~~~~~
src/feature/hs/hs_service.c: In function ‘hs_service_load_all_keys’:
src/feature/hs/hs_service.c:1267:52: note: format string is defined here
1267 | log_warn(LD_REND, "Client authorization file %s can't be read. "
| ^~
cc1: all warnings being treated as errors
make[1]: *** [Makefile:9877: src/feature/hs/hs_service.o] Error 1
CC src/feature/hs/core_libtor_app_testing_a-hs_service.o
In file included from ./src/lib/crypt_ops/crypto_rsa.h:21,
from ./src/core/or/or.h:32,
from src/feature/hs/hs_service.c:11:
In function ‘load_client_keys’,
inlined from ‘load_service_keys’ at src/feature/hs/hs_service.c:1090:7,
inlined from ‘hs_service_load_all_keys’ at src/feature/hs/hs_service.c:4010:9:
./src/lib/log/log.h:244:3: error: ‘%s’ directive argument is null [-Werror=format-overflow=]
244 | log_fn_(LOG_WARN, domain, __FUNCTION__, args, ##__VA_ARGS__)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/feature/hs/hs_service.c:1267:7: note: in expansion of macro ‘log_warn’
1267 | log_warn(LD_REND, "Client authorization file %s can't be read. "
| ^~~~~~~~
src/feature/hs/hs_service.c: In function ‘hs_service_load_all_keys’:
src/feature/hs/hs_service.c:1267:52: note: format string is defined here
1267 | log_warn(LD_REND, "Client authorization file %s can't be read. "
| ^~
cc1: all warnings being treated as errors
make[1]: *** [Makefile:11111: src/feature/hs/core_libtor_app_testing_a-hs_service.o] Error 1
make[1]: Target 'all-am' not remade because of errors.
make[1]: Leaving directory '/home/nickm/src/tor-035'
make: *** [Makefile:5788: all] Error 2
[1016]$
```
It looks like this is a real bug: when there's something wrong with the client authorization file, we first free and null the file, and only log its contents afterwards.
This appears to affect 0.3.5 and later.Tor: 0.3.5.x-finalNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/legacy/trac/-/issues/30309Rename tor_mem_is_zero to fast_mem_is_zero; use it consistently2020-06-13T15:41:04ZNick MathewsonRename tor_mem_is_zero to fast_mem_is_zero; use it consistentlyBy convention, tor_memeq() means "constant time" and fast_memeq() means "variable-time." But tor_mem_is_zero is variable-time, whereas its constant-time variant is safe_mem_is_zero.
I'm fine with the "safe_" name, but we should rename ...By convention, tor_memeq() means "constant time" and fast_memeq() means "variable-time." But tor_mem_is_zero is variable-time, whereas its constant-time variant is safe_mem_is_zero.
I'm fine with the "safe_" name, but we should rename tor_mem_is_zero() to indicate that it is the fast one, not the safe one. We should also audit its users, particularly tor_digest{256,}_is_zero()Tor: 0.4.1.x-finalhttps://gitlab.torproject.org/legacy/trac/-/issues/30308Use tor_mem_is_zero to check for broken node identity in node_is_a_configured...2020-06-13T15:41:03ZNick MathewsonUse tor_mem_is_zero to check for broken node identity in node_is_a_configured_bridge()In node_is_a_configured_bridge(), we use `BUG(tor_digest_is_zero(...))` to check for a broken node_t whose digest is not set. But that's overkill: tor_digest_is_zero() uses the slow constant-time tor_memeq(), and this is a case where we...In node_is_a_configured_bridge(), we use `BUG(tor_digest_is_zero(...))` to check for a broken node_t whose digest is not set. But that's overkill: tor_digest_is_zero() uses the slow constant-time tor_memeq(), and this is a case where we will hit a BUG().Tor: 0.4.1.x-finalNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/legacy/trac/-/issues/30294Move dirauth periodic events into dirauth module.2020-06-13T15:41:01ZNick MathewsonMove dirauth periodic events into dirauth module.Tor: 0.4.1.x-finalNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/legacy/trac/-/issues/30189ALL_BUGS_ARE_FATAL build failures in 0.3.4 and later.2020-06-13T15:40:46ZNick MathewsonALL_BUGS_ARE_FATAL build failures in 0.3.4 and later.Found while reviewing #30179:
We don't include stdlib.h in util_bugs.h, which is sometimes a problem, since the macros declared there can call abort().
This problem is particularly bad for the nonfatal assertions, since they don't call...Found while reviewing #30179:
We don't include stdlib.h in util_bugs.h, which is sometimes a problem, since the macros declared there can call abort().
This problem is particularly bad for the nonfatal assertions, since they don't call abort() unless ALL_BUGS_ARE_FATAL is defined, which it only rarely is.Tor: 0.3.5.x-finalNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/legacy/trac/-/issues/30150Fix coverity warnings in tests2020-06-13T15:40:39ZNick MathewsonFix coverity warnings in testsIt's not really a problem if the unit tests could potentially leak memory or something, but in the interest of code quality, we should fix all the coverity warnings here too.It's not really a problem if the unit tests could potentially leak memory or something, but in the interest of code quality, we should fix all the coverity warnings here too.Tor: 0.4.1.x-finalNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/legacy/trac/-/issues/30149Fix "medium" and "low" impact coverity false positives outside of tests.2020-06-13T15:40:39ZNick MathewsonFix "medium" and "low" impact coverity false positives outside of tests.Tor: 0.4.1.x-finalNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/legacy/trac/-/issues/30147Fix "high-impact" coverity false-positive warnings outside of tests.2020-06-13T15:40:38ZNick MathewsonFix "high-impact" coverity false-positive warnings outside of tests.Let's fix all the current "high impact" coverity issues that are outside of the unit tests. (For other issues, see siblings of this ticket.)Let's fix all the current "high impact" coverity issues that are outside of the unit tests. (For other issues, see siblings of this ticket.)Tor: 0.4.1.x-finalNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/legacy/trac/-/issues/30004when reloading a consensus with a CRLF, log at notice.2020-06-13T15:40:13ZNick Mathewsonwhen reloading a consensus with a CRLF, log at notice.Tor: 0.4.0.x-finalNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/legacy/trac/-/issues/29984Controller protocol parser can't find empty initial line2020-06-13T15:40:05ZNick MathewsonController protocol parser can't find empty initial lineWe've apparently run into this in a few places before: The code in control.c that extracts the command from the command line will discard the rest of the line too, if the line is empty.
This means that the control.c code can't distingui...We've apparently run into this in a few places before: The code in control.c that extracts the command from the command line will discard the rest of the line too, if the line is empty.
This means that the control.c code can't distinguish these cases:
```
+FOO A
B
C
.
```
and
```
+FOO
A
B
C
```Tor: 0.4.1.x-finalNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/legacy/trac/-/issues/29756Check for correct header macro guards.2020-06-13T15:39:21ZNick MathewsonCheck for correct header macro guards.We should probably make sure that our headers all have correct guard macros. When we forget to do this, we usually get sad about it.We should probably make sure that our headers all have correct guard macros. When we forget to do this, we usually get sad about it.Tor: 0.4.1.x-finalNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/legacy/trac/-/issues/29747shellcheck errors on the new git scripts2020-06-13T15:39:19ZGeorge Kadianakisshellcheck errors on the new git scriptsThe scripts added by #29391 are triggering shellcheck errors on travis:
https://travis-ci.org/asn-d6/tor/jobs/505198755#L3149
CI seems to be partially broken because of that.The scripts added by #29391 are triggering shellcheck errors on travis:
https://travis-ci.org/asn-d6/tor/jobs/505198755#L3149
CI seems to be partially broken because of that.Tor: 0.4.1.x-finalNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/legacy/trac/-/issues/29746Improve Tor best practices tracker2020-06-13T15:43:55ZGeorge KadianakisImprove Tor best practices trackerVarious improvements could be done to the best-practices tracker (practracker #29221). Here are a few from Nick's last review:
https://github.com/torproject/tor/pull/742#discussion_r262564420
https://github.com/torproject/tor/pull/742#d...Various improvements could be done to the best-practices tracker (practracker #29221). Here are a few from Nick's last review:
https://github.com/torproject/tor/pull/742#discussion_r262564420
https://github.com/torproject/tor/pull/742#discussion_r262565153
~~https://github.com/torproject/tor/pull/742#discussion_r262566041~~
https://github.com/torproject/tor/pull/742#discussion_r262566501
https://github.com/torproject/tor/pull/742#discussion_r262567882
This could also be a fine starting volunteer project.Tor: 0.4.2.x-finalNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/legacy/trac/-/issues/29542Stop using weak_rng.2020-06-13T15:38:25ZNick MathewsonStop using weak_rng.We have crypto_fast_rng now, we should be able to use it everywhere that we were relying on our goofy old prng.We have crypto_fast_rng now, we should be able to use it everywhere that we were relying on our goofy old prng.Tor: 0.4.1.x-finalNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/legacy/trac/-/issues/29536prng: Add a global fast PRNG object2020-06-13T15:38:22ZDavid Gouletdgoulet@torproject.orgprng: Add a global fast PRNG objectprop289 needs to have a fast PRNG allocated early in order to avoid creating a fast prng object every time we want to add random to a cell.prop289 needs to have a fast PRNG allocated early in order to avoid creating a fast prng object every time we want to add random to a cell.Tor: 0.4.1.x-finalNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/legacy/trac/-/issues/29357add an ActiveOnStartup config option2020-06-13T15:37:57ZMark Smithadd an ActiveOnStartup config optionWhen Tor Browser starts tor, it does not make sense to start in dormant mode (the browser is an interactive application which people will typically immediately begin using to surf the web). It would be very helpful if tor had an `ActiveO...When Tor Browser starts tor, it does not make sense to start in dormant mode (the browser is an interactive application which people will typically immediately begin using to surf the web). It would be very helpful if tor had an `ActiveOnStartup` Boolean config option which would ensure that tor starts in active mode.
Related: #29045Tor: 0.4.0.x-finalNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/legacy/trac/-/issues/2916932-bit warnings in test_circuitpadding.c2020-06-13T15:37:12ZNick Mathewson32-bit warnings in test_circuitpadding.cUgh, Jenkins is failing. Time to fix it. Here's an example:
```
16:52:06 src/test/test_circuitpadding.c:1864:43: error: implicit conversion loses integer precision: 'circpad_time_t' (aka 'unsigned long long') to 'long' [-Werror,-Wshort...Ugh, Jenkins is failing. Time to fix it. Here's an example:
```
16:52:06 src/test/test_circuitpadding.c:1864:43: error: implicit conversion loses integer precision: 'circpad_time_t' (aka 'unsigned long long') to 'long' [-Werror,-Wshorten-64-to-32]
16:52:06 tt_int_op(client_side->padding_info[0]->padding_scheduled_at_usec,
16:52:06 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~
```Tor: 0.4.0.x-finalNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/legacy/trac/-/issues/29109Run stochastic tests faster, log better message when they fail2020-06-13T15:36:52ZNick MathewsonRun stochastic tests faster, log better message when they failI've got a patch to improve the stochastic tests in https://github.com/torproject/tor/pull/656 . I'd like to merge it before the alpha release, if possible to avoid a bunch of wasted cycles and spurious bug reports.I've got a patch to improve the stochastic tests in https://github.com/torproject/tor/pull/656 . I'd like to merge it before the alpha release, if possible to avoid a bunch of wasted cycles and spurious bug reports.Tor: 0.4.0.x-finalNick MathewsonNick Mathewson