Tor issueshttps://gitlab.torproject.org/tpo/core/tor/-/issues2022-07-07T00:48:30Zhttps://gitlab.torproject.org/tpo/core/tor/-/issues/40034resolved_addr_set_suggested() cannot succeed2022-07-07T00:48:30ZNick Mathewsonresolved_addr_set_suggested() cannot succeedThis code will always trigger the BUG:
```
if (BUG(tor_addr_family(addr) != AF_INET ||
tor_addr_family(addr) != AF_INET6)) {
return;
}
```
Maybe this should be `&&`, not `||`?
Possibly related to #40032, possibly not....This code will always trigger the BUG:
```
if (BUG(tor_addr_family(addr) != AF_INET ||
tor_addr_family(addr) != AF_INET6)) {
return;
}
```
Maybe this should be `&&`, not `||`?
Possibly related to #40032, possibly not.
Found via a compiler warning on travis.Tor: 0.4.5.x-freezeDavid Gouletdgoulet@torproject.orgDavid Gouletdgoulet@torproject.orghttps://gitlab.torproject.org/tpo/core/tor/-/issues/40017Inbuf for outgoing SOCKS 4 proxy not cleared before reading from OR connection2022-07-07T00:48:30ZoparaInbuf for outgoing SOCKS 4 proxy not cleared before reading from OR connectionWhen configured to use an outgoing SOCKS 4 proxy (as set with `Socks4Proxy ip:port`), tor does not clear the connection buffer after the SOCKS handshake and before reading bytes from the OR connection. A proxy can send extra characters a...When configured to use an outgoing SOCKS 4 proxy (as set with `Socks4Proxy ip:port`), tor does not clear the connection buffer after the SOCKS handshake and before reading bytes from the OR connection. A proxy can send extra characters at the end of the handshake which will be stored on the OR connection inbuf as if they were authenticated bytes from the first hop.
This is not actually exploitable because of a check in `connection_or_process_inbuf()`, which will close the connection if it sees that there are bytes on the inbuf while the connection is in the TLS handshaking state. This check was originally designed to protect relays and not clients, but luckily it actually protects the client in this case as well.
```C
/* This check was necessary with 0.2.2, when the TLS_SERVER_RENEGOTIATING
* check would otherwise just let data accumulate. It serves no purpose
* in 0.2.3.
*
* XXXX Remove this check once we verify that the above paragraph is
* 100% true. */
if (buf_datalen(conn->base_.inbuf) > MAX_OR_INBUF_WHEN_NONOPEN) {
log_fn(LOG_PROTOCOL_WARN, LD_NET, "Accumulated too much data (%d bytes) "
"on nonopen OR connection %s %s:%u in state %s; closing.",
(int)buf_datalen(conn->base_.inbuf),
connection_or_nonopen_was_started_here(conn) ? "to" : "from",
conn->base_.address, conn->base_.port,
conn_state_to_string(conn->base_.type, conn->base_.state));
connection_or_close_for_error(conn, 0);
ret = -1;
}
```
I think that the comments here should be changed to show that this check is still important, and it would be good to also clear the inbuf when transitioning out of the `OR_CONN_STATE_PROXY_HANDSHAKING` state. The best solution would probably be to not use a single buffer for both authenticated and unauthenticated data, but this would probably take more work. I have also not looked at the other proxy types, but the SOCKS 5 proxy looks okay at first glance.
If you remove the check above (as mentioned in the comment), a SOCKS 4 proxy that appends for example `\x00\x00\x07\x00\x00\x01` (a VERSIONS cell) to the proxy reply will cause the tor client to log:
```
channel_tls_process_versions_cell(): Couldn't find a version in common between my version list and the list in the VERSIONS cell; closing connection.
```
This only causes the client to drop the connection (so the client can't connect to the tor network), but as the proxy would be able to bypass the TLS authentication for the first cells on a connection (*if the check above were removed*) and make the client think that it received cells that the first hop didn't send, there could potentially be other interesting things you could do with other cell types.Tor: 0.4.5.x-stableNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/tpo/core/tor/-/issues/34446TestingTorNetwork should not set AssumeReachable 12021-07-09T17:22:52ZNick MathewsonTestingTorNetwork should not set AssumeReachable 1For S55 we are trying to get self-tests and remote-tests working with IPv6. To make sure we've succeed, we need to try them with Chutney. But to try them with chutney, we need them to be enabled.
With legacy/trac#33583 we removed Assu...For S55 we are trying to get self-tests and remote-tests working with IPv6. To make sure we've succeed, we need to try them with Chutney. But to try them with chutney, we need them to be enabled.
With legacy/trac#33583 we removed AssumeReachable from the chutney configurations. But that didn't actually have any effect, since `AssumeReachable 1` is implicit in `TestingTorNetwork 1`.Nick MathewsonNick Mathewsonhttps://gitlab.torproject.org/tpo/core/tor/-/issues/34400control: HSFETCH command fails to validate v2 addresses2022-07-07T00:49:02ZDavid Gouletdgoulet@torproject.orgcontrol: HSFETCH command fails to validate v2 addressesIn `handle_control_hsfetch()`:
```
} else if (strcmpstart(arg1, v2_str) == 0 &&
rend_valid_descriptor_id(arg1 + v2_str_len) &&
base32_decode(digest, sizeof(digest), arg1 + v2_str_len,
...In `handle_control_hsfetch()`:
```
} else if (strcmpstart(arg1, v2_str) == 0 &&
rend_valid_descriptor_id(arg1 + v2_str_len) &&
base32_decode(digest, sizeof(digest), arg1 + v2_str_len,
REND_DESC_ID_V2_LEN_BASE32) ==
REND_DESC_ID_V2_LEN_BASE32) {
```
The above snippet is how we validate the v2 address for the `HSFETCH` command. The `base32_decode()` function returns the number of bytes _decoded_ and thus it should returns on success `sizeof(digest)`, not the total length of the base32 address (20 vs 32).
Issue was introduced in commit `a517daa56f5848d25ba79617a1a7b82ed2b0a7c0` meaning since 0.4.1.1-alpha (ticket legacy/trac#28913).
I noticed this because I recently updated the bad HSDirscanner Tor to use our latest and it broke the scanner.Tor: 0.4.2.x-finalhttps://gitlab.torproject.org/tpo/core/tor/-/issues/34384AppVeyor builds are failing2022-07-07T00:49:01ZAlexander Færøyahf@torproject.orgAppVeyor builds are failingOur AppVeyor builds are failing. It looks like the issue is related to us not updating Pacman before we install our dependencies.Our AppVeyor builds are failing. It looks like the issue is related to us not updating Pacman before we install our dependencies.Tor: unspecifiedhttps://gitlab.torproject.org/tpo/core/tor/-/issues/34382Don't require M_SYSCALL in sandbox.c2022-07-07T00:49:01ZNick MathewsonDon't require M_SYSCALL in sandbox.cIn sandbox.c, we have a platform-dependent M_SYSCALL macro that is used to extract the syscall from a ucontext_t pointer.
But we only use this value for debugging. Perhaps instead we should make it optional, so that platforms where we ...In sandbox.c, we have a platform-dependent M_SYSCALL macro that is used to extract the syscall from a ucontext_t pointer.
But we only use this value for debugging. Perhaps instead we should make it optional, so that platforms where we don't have it defined can still build with m_syscall.
See also legacy/trac#32904 and legacy/trac#30987Tor: 0.4.4.x-finalAlexander Færøyahf@torproject.orgAlexander Færøyahf@torproject.orghttps://gitlab.torproject.org/tpo/core/tor/-/issues/34381Shellcheck warnings for no-longer-existent contrib scripts2022-07-07T00:49:01ZcShellcheck warnings for no-longer-existent contrib scripts```
% shellcheck --version
ShellCheck - shell script analysis tool
version: 0.7.1
```
Running `make check` results in a few warnings for both `contrib/dist/suse/tor.sh` and `contrib/dist/tor.sh`, namely SC2034 (unused variables) and SC2...```
% shellcheck --version
ShellCheck - shell script analysis tool
version: 0.7.1
```
Running `make check` results in a few warnings for both `contrib/dist/suse/tor.sh` and `contrib/dist/tor.sh`, namely SC2034 (unused variables) and SC2039 (POSIX incompatibilities). If shellcheck is right, then I can gladly go ahead and address these warnings in a PR.Tor: 0.4.4.x-finalhttps://gitlab.torproject.org/tpo/core/tor/-/issues/34376Space out src/feature/stats/rephist.c2022-07-07T00:49:01ZNeel Chauhanneel@neelc.orgSpace out src/feature/stats/rephist.chttps://gitlab.torproject.org/tpo/core/tor/-/issues/34375Remove 0.4.1 from git-list-tor-branches.sh and add 0.4.42022-07-07T00:49:01ZNick MathewsonRemove 0.4.1 from git-list-tor-branches.sh and add 0.4.4Now that 0.4.1 is EOL, we can remove it from git-list-tor-branches.Now that 0.4.1 is EOL, we can remove it from git-list-tor-branches.Tor: 0.4.4.x-finalNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/tpo/core/tor/-/issues/34303Find out why onion service measurements have gotten slower2022-07-07T00:49:01ZKarsten LoesingFind out why onion service measurements have gotten slowerToday I changed the ["Time to download files over Tor" graph](https://metrics.torproject.org/torperf.html) to include version 3 onion service measurements.
By chance, I also looked at the [onion server measurements](https://metrics.torp...Today I changed the ["Time to download files over Tor" graph](https://metrics.torproject.org/torperf.html) to include version 3 onion service measurements.
By chance, I also looked at the [onion server measurements](https://metrics.torproject.org/torperf.html?start=2020-02-25&end=2020-05-25&server=onion&filesize=50kb) and found that the onion service measurements made by op-nl2, op-us2, and op-hk2 have gotten much slower as compared to their op-nl, op-us, and op-hk predecessors.
I'm 95% certain that this is not a bug in the graphs.
My current best guess is that something in `tor` has changed. I'd like to set up a small number of experimental OnionPerf instances all in the same place but with different `tor` versions. Any suggestions on locations (Amsterdam, Florida, Hong Kong) or `tor` versions?
This is also relevant to Sponsor 59 in order to make sure that our current measurements are going to be a baseline for future experiments. Classifying as potential defect.Tor: 0.3.5.x-finalKarsten LoesingKarsten Loesinghttps://gitlab.torproject.org/tpo/core/tor/-/issues/34299man page has wrong default for MinUptimeHidServDirectoryV22022-07-07T00:49:01ZRoger Dingledineman page has wrong default for MinUptimeHidServDirectoryV2In legacy/trac#14149 (Tor 0.2.6.3-alpha) we changed the default for MinUptimeHidServDirectoryV2 to 96 hours, to reflect that the directory authorities had already changed it manually. We changed the spec too. But we forgot to change the ...In legacy/trac#14149 (Tor 0.2.6.3-alpha) we changed the default for MinUptimeHidServDirectoryV2 to 96 hours, to reflect that the directory authorities had already changed it manually. We changed the spec too. But we forgot to change the man page.Tor: 0.4.3.x-finalhttps://gitlab.torproject.org/tpo/core/tor/-/issues/34255Doxygen warnings on 0.4.32022-07-07T00:49:01ZNick MathewsonDoxygen warnings on 0.4.3There are some doxygen warnings about unclosed or unbalanced groups.There are some doxygen warnings about unclosed or unbalanced groups.Tor: 0.4.3.x-finalNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/tpo/core/tor/-/issues/34254Jenkins fails with hs_service.c:3118:3: error: comparison of unsigned express...2022-07-07T00:49:01ZGeorge KadianakisJenkins fails with hs_service.c:3118:3: error: comparison of unsigned expression < 0 is always false```
11:23:10 ../tor/src/feature/hs/hs_service.c: In function 'log_cant_upload_desc':
11:23:10 ../tor/src/feature/hs/hs_service.c:3118:3: error: comparison of unsigned expression < 0 is always false [-Werror=type-limits]
```
https://jenk...```
11:23:10 ../tor/src/feature/hs/hs_service.c: In function 'log_cant_upload_desc':
11:23:10 ../tor/src/feature/hs/hs_service.c:3118:3: error: comparison of unsigned expression < 0 is always false [-Werror=type-limits]
```
https://jenkins.torproject.org/job/tor-ci-windows-master/659/consoleFull
Seems to be a by-product of legacy/trac#33400.Tor: 0.4.4.x-finalGeorge KadianakisGeorge Kadianakishttps://gitlab.torproject.org/tpo/core/tor/-/issues/34251Fix edge case handling in Rust protover is supported2021-09-16T14:21:52ZteorFix edge case handling in Rust protover is supportedTor's Rust FFI for protocol_list_supports_protocol_or_later() returns true for the empty protocol list.
In C, the function returns false, but this behaviour is undocumented.
This bug doesn't affect protocol_list_supports_protocol() in ...Tor's Rust FFI for protocol_list_supports_protocol_or_later() returns true for the empty protocol list.
In C, the function returns false, but this behaviour is undocumented.
This bug doesn't affect protocol_list_supports_protocol() in Rust, because the Rust error checks are done in a different order.
I'll add a quick fix to legacy/trac#33222, but someone else will need to do the backport. We might want to do the Rust error checks in the same order, too.https://gitlab.torproject.org/tpo/core/tor/-/issues/34249Make sure the C and Rust protovers can't get out of sync2022-07-07T00:49:01ZteorMake sure the C and Rust protovers can't get out of syncThere is a recurring bug, where we modify the C protover, but forget the Rust protover. (See legacy/trac#34248, legacy/trac#33285, legacy/trac#29631 for similar issues.)
We could fix the underlying issue by fetching the string from a co...There is a recurring bug, where we modify the C protover, but forget the Rust protover. (See legacy/trac#34248, legacy/trac#33285, legacy/trac#29631 for similar issues.)
We could fix the underlying issue by fetching the string from a common location, using C's `#include` or Rust's `include_str!()`.
Then we could test that C and Rust are the same by putting a copy of the protover string in the unit tests, and making sure that it matches the currently supported protocol versions.
This fix and test will be important for proposal 318, because it will modify both protocol version implementations:
https://github.com/torproject/torspec/blob/master/proposals/318-limit-protovers.mdTor: 0.4.4.x-finalNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/tpo/core/tor/-/issues/34248Declare HSIntro=5 in Tor's rust protover implementation2021-09-16T14:21:52ZteorDeclare HSIntro=5 in Tor's rust protover implementationMy protover tests for legacy/trac#33222 fail in Rust, because Tor's rust protover doesn't declare HSIntro=5.
I'll do a quick fix in legacy/trac#33222, but I want to leave the backport to David.My protover tests for legacy/trac#33222 fail in Rust, because Tor's rust protover doesn't declare HSIntro=5.
I'll do a quick fix in legacy/trac#33222, but I want to leave the backport to David.https://gitlab.torproject.org/tpo/core/tor/-/issues/34245Declare variables in for loops in rend_service_dump_stats()2022-07-07T00:49:01ZNeel Chauhanneel@neelc.orgDeclare variables in for loops in rend_service_dump_stats()Tor: 0.4.4.x-finalNeel Chauhanneel@neelc.orgNeel Chauhanneel@neelc.orghttps://gitlab.torproject.org/tpo/core/tor/-/issues/34238Space out some function calls in parse_short_policy()2022-07-07T00:49:01ZNeel Chauhanneel@neelc.orgSpace out some function calls in parse_short_policy()Tor: 0.4.4.x-finalNeel Chauhanneel@neelc.orgNeel Chauhanneel@neelc.orghttps://gitlab.torproject.org/tpo/core/tor/-/issues/34237Fix spacing in if statement in tor_version_parse()2022-07-07T00:49:00ZNeel Chauhanneel@neelc.orgFix spacing in if statement in tor_version_parse()This if statement in tor_version_parse():
```
if ( hexlen == 0 || (hexlen % 2) == 1)
```
should be:
```
if (hexlen == 0 || (hexlen % 2) == 1)
```This if statement in tor_version_parse():
```
if ( hexlen == 0 || (hexlen % 2) == 1)
```
should be:
```
if (hexlen == 0 || (hexlen % 2) == 1)
```Tor: 0.4.4.x-finalNeel Chauhanneel@neelc.orgNeel Chauhanneel@neelc.orghttps://gitlab.torproject.org/tpo/core/tor/-/issues/34233Fix use of == in configure.ac2022-07-07T00:49:00ZNick MathewsonFix use of == in configure.acA user points out that we're now using == in configure.ac, which isn't portable.
We'll need to backport a fix everywhere that we backported our legacy/trac#34078 fix.A user points out that we're now using == in configure.ac, which isn't portable.
We'll need to backport a fix everywhere that we backported our legacy/trac#34078 fix.Tor: 0.4.3.x-finalNick MathewsonNick Mathewson