Tor issueshttps://gitlab.torproject.org/tpo/core/tor/-/issues2020-06-27T13:49:32Zhttps://gitlab.torproject.org/tpo/core/tor/-/issues/31554Restrict "make test-stem" to tests that actually use tor2020-06-27T13:49:32ZteorRestrict "make test-stem" to tests that actually use torIn legacy/trac#30694, we restricted the travis stem job to tests that actually use tor.
But we should lower that change to "make test-stem".
Gaba, this is sponsor 27 can, because it makes refactoring easier to test.In legacy/trac#30694, we restricted the travis stem job to tests that actually use tor.
But we should lower that change to "make test-stem".
Gaba, this is sponsor 27 can, because it makes refactoring easier to test.Tor: 0.3.5.x-finalteorteorhttps://gitlab.torproject.org/tpo/core/tor/-/issues/31548hs-v3: Service can pick more than HiddenServiceNumIntroductionPoints intro po...2021-06-23T17:24:15ZDavid Gouletdgoulet@torproject.orghs-v3: Service can pick more than HiddenServiceNumIntroductionPoints intro pointsDuring my testing of legacy/trac#30200, I ended up with service descriptor with 4 intro points even though `HiddenServiceNumIntroductionPoints` is set to 3 (default).
Further investigation confirmed this by adding a log in the `decode_i...During my testing of legacy/trac#30200, I ended up with service descriptor with 4 intro points even though `HiddenServiceNumIntroductionPoints` is set to 3 (default).
Further investigation confirmed this by adding a log in the `decode_intro_points()` function which showed me 4 intro points.
I haven't found out why but one feature of HS is that we launch `HiddenServiceNumIntroductionPoints` + 2 intro circuits in parallel and the first one to finish are picked.
It appears that more than the defined value can finish at the same time and will be picked.Tor: 0.3.5.x-finalDavid Gouletdgoulet@torproject.orgDavid Gouletdgoulet@torproject.orghttps://gitlab.torproject.org/tpo/core/tor/-/issues/31408torrc : ClientOnionAuthDir after include directives breaks client to v2 services2021-06-23T17:24:15ZTractorrc : ClientOnionAuthDir after include directives breaks client to v2 servicesIf I append these two statements to torrc, in this order :
ClientOnionAuthDir /etc/tor/auth/
%include /etc/tor/torrc.d/
and restart tor.service, I can connect to 1 × v3 and 4 × v2 services, within seconds.
But if I reverse their order...If I append these two statements to torrc, in this order :
ClientOnionAuthDir /etc/tor/auth/
%include /etc/tor/torrc.d/
and restart tor.service, I can connect to 1 × v3 and 4 × v2 services, within seconds.
But if I reverse their order ( %include first ), I can only connect to the v3 service -- all other connections will eventually time out.
In man, I missed to find any recommandation about ordering these statements.
( this is on Debian Stretch with torproject's stretch repo )
Thanks!
**Trac**:
**Username**: xahoTor: 0.3.5.x-finalGeorge KadianakisGeorge Kadianakishttps://gitlab.torproject.org/tpo/core/tor/-/issues/31008Typographical error on tor man pages help command2020-06-29T15:24:29ZAadi BajpaiTypographical error on tor man pages help commandman tor gives the following output as the first command:
```
-h, -help
Display a short help message and exit.
```
Except it's --help and not -help.
-help doesn't work, I found it weird so I tested it just in case and it doesn't.
New ...man tor gives the following output as the first command:
```
-h, -help
Display a short help message and exit.
```
Except it's --help and not -help.
-help doesn't work, I found it weird so I tested it just in case and it doesn't.
New users may get confused if they don't know the -h/--help convention.Tor: 0.3.5.x-finalhttps://gitlab.torproject.org/tpo/core/tor/-/issues/30916assert in dimap_add_entry()2020-07-14T22:24:14ZDavid Gouletdgoulet@torproject.orgassert in dimap_add_entry()From tor-relays@:
https://lists.torproject.org/pipermail/tor-relays/2019-June/017402.html
The stack trace is:
```
Jun 18 13:33:31.000 [notice] Bootstrapped 0%: Starting
Jun 18 13:33:32.000 [notice] Starting with guard context "defau...From tor-relays@:
https://lists.torproject.org/pipermail/tor-relays/2019-June/017402.html
The stack trace is:
```
Jun 18 13:33:31.000 [notice] Bootstrapped 0%: Starting
Jun 18 13:33:32.000 [notice] Starting with guard context "default"
============================================================ T= 1560854012
INTERNAL ERROR: Raw assertion failed at ../src/lib/ctime/di_ops.c:179: !
old_val/usr/bin/tor(dump_stack_symbols_to_error_fds+0x33)[0x55a17b410943]
/usr/bin/tor(tor_raw_assertion_failed_msg_+0x86)[0x55a17b410fd6]
/usr/bin/tor(dimap_add_entry+0xa0)[0x55a17b411ba0]
/usr/bin/tor(construct_ntor_key_map+0x69)[0x55a17b357969]
/usr/bin/tor(server_onion_keys_new+0x4d)[0x55a17b39f4dd]
/usr/bin/tor(+0x66e27)[0x55a17b287e27]
/usr/bin/tor(threadpool_new+0x18b)[0x55a17b3b3f0b]
/usr/bin/tor(cpu_init+0x9d)[0x55a17b28828d]
/usr/bin/tor(run_tor_main_loop+0x136)[0x55a17b27a496]
/usr/bin/tor(tor_run_main+0x1215)[0x55a17b27b935]
/usr/bin/tor(tor_main+0x3a)[0x55a17b278a8a]
/usr/bin/tor(main+0x19)[0x55a17b278609]
/lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf1)[0x7ffb901ba2e1]
/usr/bin/tor(_start+0x2a)[0x55a17b27865a]
Jun 18 13:33:33.000 [notice] Tor 0.3.5.8 opening log file.
```
It appears that tor tried to add the same value in the `di_digest256_map_t` twice.
Logs indicate 0.3.5.8Tor: 0.3.5.x-finalNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/tpo/core/tor/-/issues/30744Allow failures in the Travis test-stem job2020-06-27T13:50:06ZteorAllow failures in the Travis test-stem jobTor: 0.3.5.x-finalteorteorhttps://gitlab.torproject.org/tpo/core/tor/-/issues/30713Disable or allow_fail test_rebind.py in macOS Travis2020-06-27T13:50:07ZTaylor YuDisable or allow_fail test_rebind.py in macOS Travistest_rebind.py is having spurious failures often on macOS. We should either allow_fail the macOS builds, or disable test_rebind.py.test_rebind.py is having spurious failures often on macOS. We should either allow_fail the macOS builds, or disable test_rebind.py.Tor: 0.3.5.x-finalteorteorhttps://gitlab.torproject.org/tpo/core/tor/-/issues/30475hs_service.c: compile-time warning with GCC 9.1.12020-06-27T13:50:13ZNick 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/tpo/core/tor/-/issues/30454hs-v3: INTRODUCE1 trunnel code doensn't handle HS_INTRO_ACK_STATUS_CANT_RELAY2021-06-23T17:24:34ZDavid Gouletdgoulet@torproject.orghs-v3: INTRODUCE1 trunnel code doensn't handle HS_INTRO_ACK_STATUS_CANT_RELAYI'm not sure how it happens because this code got in almost at the same time but the introduction ACK status code are not synchronized with what trunnel can parse which can lead to the intro point hard asserting() if it ever tries to sen...I'm not sure how it happens because this code got in almost at the same time but the introduction ACK status code are not synchronized with what trunnel can parse which can lead to the intro point hard asserting() if it ever tries to send the status code: `HS_INTRO_ACK_STATUS_CANT_RELAY = 0x0003`.
See `cell_introduce1.trunnel`, it does not handle status code `0x0003`:
```
u16 status IN [0x0000, 0x0001, 0x0002];
```
Fortunately for us, it can NOT happen with the current code. The only call site is here:
```
/* Relay the cell to the service on its intro circuit with an INTRODUCE2
* cell which is the same exact payload. */
if (relay_send_command_from_edge(CONTROL_CELL_ID, TO_CIRCUIT(service_circ),
RELAY_COMMAND_INTRODUCE2,
(char *) request, request_len, NULL)) {
log_warn(LD_PROTOCOL, "Unable to send INTRODUCE2 cell to the service.");
/* Inform the client that we can't relay the cell. */
status = HS_INTRO_ACK_STATUS_CANT_RELAY;
goto send_ack;
}
```
... and turns out that `relay_send_command_from_edge()` can *NEVER* return anything else than 0 (we've audited all the currently supported versions, they all behave the same there).
This prevents tor from asserting in `send_introduce_ack_cell()` with:
```
/* A wrong status is a very bad code flow error as this value is controlled
* by the code in this file and not an external input. This means we use a
* code that is not known by the trunnel ABI. */
tor_assert(ret == 0);
```
There are a series of things to fix here.
1. The status code should be defined within the trunnel file and ONLY that ABI should be used. In short, `hs_intro_ack_status_t` should disappear in favor of the one that needs to be defined with trunnel. Side node, same must be done for: `hs_intro_auth_key_type_t`
2. Because clients won't be able to know what `HS_INTRO_ACK_STATUS_CANT_RELAY` is, we should remove it at once for now since it was/is never used.
3. We should add a default case to the trunnel definition so if we ever extend this later, tor will not explode or simply fail to parse the cell.
4. We should conduct an audit of the `tor_assert()` in the HS code and replace them with non fatal ones if possible.
We got very lucky here because else it would have been an easy remote relay crash so (4) is very important. One lesson is also to *NEVER* duplicate any values defined in a trunnel file. Always use the defined ABI of that trunnel spec.
End note, this was found by legacy/trac#15516 branch which re-used this error code and during testing, the intro point relay exploded.
All this got into 0.3.0.1-alpha.Tor: 0.3.5.x-finalDavid Gouletdgoulet@torproject.orgDavid Gouletdgoulet@torproject.orghttps://gitlab.torproject.org/tpo/core/tor/-/issues/30452List which compile-time modules are enabled2020-06-27T13:50:14ZNick MathewsonList which compile-time modules are enabledWe should have a way to find out which compile-time modules Tor was built with.We should have a way to find out which compile-time modules Tor was built with.Tor: 0.3.5.x-finalNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/tpo/core/tor/-/issues/30344conn_read_callback is called on connections that are marked for closed2020-07-14T22:24:00ZRob Jansenconn_read_callback is called on connections that are marked for closedThere is a bug causing busy loops in Libevent and infinite loops in the Shadow simulator. In my Shadow simulations, I have observed that `conn_read_callback` is getting called on a connection that is marked for close.
This is similar to...There is a bug causing busy loops in Libevent and infinite loops in the Shadow simulator. In my Shadow simulations, I have observed that `conn_read_callback` is getting called on a connection that is marked for close.
This is similar to legacy/trac#5263, and as described there, I believe it is a bug for `conn_read_callback` to be called on sockets that are marked for close.
The bug is problematic in Shadow when the marked connection also wants to flush, but attempting to write the outbuf inside `conn_close_if_marked` fails because the write would block (because the kernel write buffer is already full). Because it still wants to flush, the connection does not get closed, but the connection remains readable causing Libevent to continue looping on `conn_read_callback` until the socket buffer can actually write. This wastes CPU resources in Tor, and causes an infinite loop in Shadow because Shadow's discrete-event time does not advance during this loop.
I found the bug on 0.3.5.8, and it is probably present at least since then.
I think the conn should not be waiting for read events when it is marked. I'm not sure where in the code this assumption is first violated, but the following patch fixed the issue in my Shadow simulations:
```
diff --git a/src/core/mainloop/mainloop.c b/src/core/mainloop/mainloop.c
index 6e2b300..e82c77a 100644
--- a/src/core/mainloop/mainloop.c
+++ b/src/core/mainloop/mainloop.c
@@ -894,6 +894,21 @@ conn_read_callback(evutil_socket_t fd, short event, void *_conn)
}
assert_connection_ok(conn, time(NULL));
+ if (conn->marked_for_close && connection_is_reading(conn)) {
+ /* Libevent says we can read, but we are marked so we will never try
+ * to read again. We will try to close the connection below inside of
+ * close_closeable_connections(), but let's make sure not to cause
+ * Libevent to spin on conn_read_callback() while we wait for the
+ * socket to let us flush to it.*/
+ connection_stop_reading(conn);
+
+ /* Now, if we still have data to flush, then make sure Libevent tells
+ * us when the conn will allow us to write the bytes. */
+ if (connection_wants_to_flush(conn) && !connection_is_writing(conn)) {
+ connection_start_writing(conn);
+ }
+ }
+
if (smartlist_len(closeable_connection_lst))
close_closeable_connections();
}
```Tor: 0.3.5.x-finalGeorge KadianakisGeorge Kadianakishttps://gitlab.torproject.org/tpo/core/tor/-/issues/30316Vote's 'bandwidth-file-headers' is in wrong order2020-06-27T13:50:17ZDamian JohnsonVote's 'bandwidth-file-headers' is in wrong orderHi network team. Moria1's present vote places its "bandwidth-file-headers" in the wrong location.
Tor's dir-spec says: "**Status documents contain a preamble, an authority section, a list of router status entries, and one or more footer...Hi network team. Moria1's present vote places its "bandwidth-file-headers" in the wrong location.
Tor's dir-spec says: "**Status documents contain a preamble, an authority section, a list of router status entries, and one or more footer signature, in that order.**"
The trouble is that our bandwidth-file-headers field is specified as being part of the preamble, whereas moria1 places it **after** its authority section. Header fields appear in the following order...
* flag-thresholds (preamble)
* params (preamble)
* dir-source (authority section)
* contact (authority section)
* shared-rand-participate (authority section)
* shared-rand-commit (authority section, multiple)
* shared-rand-previous-value (authority section)
* shared-rand-current-value (authority section)
* **bandwidth-file-headers (preamble)**
* dir-key-certificate-version (key certificate)
* ... etc...
As a result Stem does not parse this field: legacy/trac#30282Tor: 0.3.5.x-finalNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/tpo/core/tor/-/issues/30234Get a stacktrace from tor processes launched by stem2020-06-27T13:50:21ZteorGet a stacktrace from tor processes launched by stemWe need to show the stem tor log at the end of the build.
We might also need to propagate the USR1 and abort signals to tor.We need to show the stem tor log at the end of the build.
We might also need to propagate the USR1 and abort signals to tor.Tor: 0.3.5.x-finalteorteorhttps://gitlab.torproject.org/tpo/core/tor/-/issues/30190Do not warn about compatible OpenSSL upgrades2020-06-27T13:50:22ZteorDo not warn about compatible OpenSSL upgradesFrom https://github.com/torproject/tor/pull/951
When releasing OpenSSL patch-level maintenance updates,
we do not want to rebuild binaries using it.
And since they guarantee ABI stability, we do not have to.
Without this patch, warning...From https://github.com/torproject/tor/pull/951
When releasing OpenSSL patch-level maintenance updates,
we do not want to rebuild binaries using it.
And since they guarantee ABI stability, we do not have to.
Without this patch, warning messages were produced
that confused users:
https://bugzilla.opensuse.org/show_bug.cgi?id=1129411Tor: 0.3.5.x-finalhttps://gitlab.torproject.org/tpo/core/tor/-/issues/30189ALL_BUGS_ARE_FATAL build failures in 0.3.4 and later.2020-06-27T13:50:22ZNick MathewsonALL_BUGS_ARE_FATAL build failures in 0.3.4 and later.Found while reviewing legacy/trac#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...Found while reviewing legacy/trac#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/tpo/core/tor/-/issues/30148Fix infrequent, unlikely memory leak on failure to create keys directory2020-06-27T13:50:24ZNick MathewsonFix infrequent, unlikely memory leak on failure to create keys directoryIn load_ed_keys(), if we can't create the key directory, we leak some memory.In load_ed_keys(), if we can't create the key directory, we leak some memory.Tor: 0.3.5.x-finalNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/tpo/core/tor/-/issues/30117Support stem's backtrace signals in Travis2020-06-27T13:50:25ZteorSupport stem's backtrace signals in TravisIn legacy/trac#29437, we're trying to track down a stem hang in our CI.
In legacy/trac#30012, I added backtrace support to stem.
But we need that backtrace support in our CI to see why stem is hanging.In legacy/trac#29437, we're trying to track down a stem hang in our CI.
In legacy/trac#30012, I added backtrace support to stem.
But we need that backtrace support in our CI to see why stem is hanging.Tor: 0.3.5.x-finalteorteorhttps://gitlab.torproject.org/tpo/core/tor/-/issues/30040Double-free bug on huge bandwidth file in some platforms2021-08-23T15:16:06ZGeorge KadianakisDouble-free bug on huge bandwidth file in some platformsHere is a very situational double-free bug reported in hackerone from bug hunter paldium. It's a low-severity item since bandwidth files are considered trusted input, and anyone who controls a bandwidth file can cause worse disasters tha...Here is a very situational double-free bug reported in hackerone from bug hunter paldium. It's a low-severity item since bandwidth files are considered trusted input, and anyone who controls a bandwidth file can cause worse disasters than double-frees. Also it only applies on very specific platforms that none of our dirauths use.
```
Details:
The function compat_getdelim_ is used for tor_getline if tor is compiled
on a system that lacks getline and getdelim. These systems should be
very rare, considering that getdelim is POSIX.
If this system is further a 32 bit architecture, it is possible to
trigger a double free with huge files.
If bufsiz has been already increased to 2 GB, the next chunk would
be 4 GB in size, which wraps around to 0 due to 32 bit limitations.
A realloc(*buf, 0) could be imagined as "free(*buf); return malloc(0);"
which therefore could return NULL. The code in question considers
that an error, but will keep the value of *buf pointing to already
freed memory.
The caller of tor_getline() would free the pointer again, therefore
leading to a double free.
This code can only be triggered in dirserv_read_measured_bandwidths
with a huge measured bandwith list file on a system that actually
allows to reach 2 GB of space through realloc.
It is not possible to trigger this on Linux with glibc or other major
*BSD systems even on unit tests, because these systems cannot reach
so much memory due to memory fragmentation.
This patch is effectively based on the penetration test report of
cure53 for curl available at https://cure53.de/pentest-report_curl.pdf
and explained under section "CRL-01-007 Double-free in aprintf() via
unsafe size_t multiplication (Medium)".
## Impact
Successfully triggering a double free can corrupt the heap
which might allow more sophisticated attacks within the
tor application.
```Tor: 0.3.5.x-finalhttps://gitlab.torproject.org/tpo/core/tor/-/issues/30011Kill test-stem if takes more than 9.5 minutes2020-06-27T13:50:30ZteorKill test-stem if takes more than 9.5 minutesLet's kill test-stem if it times out.Let's kill test-stem if it times out.Tor: 0.3.5.x-finalteorteorhttps://gitlab.torproject.org/tpo/core/tor/-/issues/29922util/time test failure on Jenkins2020-06-27T13:50:33ZAlexander Færøyahf@torproject.orgutil/time test failure on JenkinsOur 'util/time' test fails on our tor-ci-mingwcross-master-test builder with:
```
util/time: Mar 27 13:29:43.773 [warn] tor_gmtime_r(): Bug: gmtime(-1) failed with error Invalid argument: Rounding up to 1970 (on Tor 0.4.1.0-alpha-dev )
...Our 'util/time' test fails on our tor-ci-mingwcross-master-test builder with:
```
util/time: Mar 27 13:29:43.773 [warn] tor_gmtime_r(): Bug: gmtime(-1) failed with error Invalid argument: Rounding up to 1970 (on Tor 0.4.1.0-alpha-dev )
[time FAILED]
```
See: https://jenkins.torproject.org/job/tor-ci-mingwcross-master-test/ARCHITECTURE=amd64,SUITE=stretch/lastBuild/consoleTor: 0.3.5.x-finalNick MathewsonNick Mathewson