We found this in #40076, after we started using buf_move_all() in
more places. Fixes bug #40076; bugfix on 0.3.3.1-alpha. As far as
I know, the crash only affects master, but I think this warrants a
backport, "just in case".
Without this fix, if an PT forgets to send a USERADDR command, that
results in a connection getting treated as local for the purposes of
rate-limiting.
If the PT _does_ use USERADDR, we still believe it.
Closes ticket 33747.
Previously we tolerated up to 1.5 connections for every relay we
were connected to, and didn't warn if we had fewer than 5
connections total.
Now we tolerate up to 1.5 connections per relay, and up to 4
connections per authority, and we don't warn at all when we have
fewer than 25 connections total.
Fixes bug 33880, which seems to have been provoked by our #17592
change in 0.3.5.
This commit makes it that if the ORPort is set with a single port, it will
bind to both global listen IPv4 and IPv6 addresses.
To pin an "ORPort <PORT>" to be IPv4 or IPv6, the IPv4Only/IPv6Only flags are
honored thus this will _only_ bind on IPv6 for that port value:
ORPort 9050 IPv6Only
Results in: [::]:9050
ORPort 9051 IPv4Only
Results in: [0.0.0.0]:9051
Attempting to configure an explicit IPv4 address with IPv6Only flag is an
error and vice versa.
Closes#33246
Signed-off-by: David Goulet <dgoulet@torproject.org>
The need for casting negative syscall arguments depends on the
glibc version. This affects the rules for the openat syscall which
uses the constant AT_FDCWD that is defined as a negative number.
This commit adds logic to only apply the cast when necessary, on
glibc versions from 2.27 onwards.
Different versions of glibc use either open or openat for the
opendir function. This commit adds logic to use the correct rule
for each glibc version, namely:
- Until 2.14 open is used
- From 2.15 to to 2.21 openat is used
- From 2.22 to 2.26 open is used
- From 2.27 onwards openat is used
The need for casting negative syscall arguments depends on the
glibc version. This affects the rules for the openat syscall which
uses the constant AT_FDCWD that is defined as a negative number.
This commit adds logic to only apply the cast when necessary, on
glibc versions from 2.27 onwards.
Instead of replacing connection_t.{addr,port} with a canonical
orport, and tracking the truth in real_addr, we now leave
connection_t.addr alone, and put the canonical address in
canonical_orport.
Closes#40042Closes#33898
(This is not fully general yet: we only pick randomly among
_supported_ addresses, and each extendinfo contains at most one IPv4
address and at most one IPv6 address, no matter what the extend cell
had.)
This change will help dual-stack relays do IPv6 reachability tests,
in theory, by having them sometimes do IPv4 connections and
sometimes do ipv6 connections.
Closes ticket 33220.
Create a doc/tracing/ directory to contain a top level README.md which is the
previously named Tracing.md and add the EventsCircuit.md which describes the
circuit subsystem tracing events in depth.
Closes#40036
Signed-off-by: David Goulet <dgoulet@torproject.org>
This patch ensures that we strip "\r" characters on both Windows as well
as Unix when we read text files. This should prevent the issue where
some Tor state files have been moved from a Windows machine, and thus
contains CRLF line ending, to a Unix machine where only \n is needed.
We add a test-case to ensure that we handle this properly on all our
platforms.
See: https://bugs.torproject.org/tpo/core/tor/33781
This changes a LOT of code but in the end, behavior is the same.
Unfortunately, many functions had to be changed to accomodate but in majority
of cases, to become simpler.
Functions are also removed specifically those that were there to convert an
IPv4 as a host format to a tor_addr_t. Those are not needed anymore.
The IPv4 address field has been standardized to "ipv4_addr", the ORPort to
"ipv4_orport" (currently IPv6 uses ipv6_orport) and DirPort to "ipv4_dirport".
This is related to Sponsor 55 work that adds IPv6 support for relays and this
work is needed in order to have a common interface between IPv4 and IPv6.
Closes#40043.
Signed-off-by: David Goulet <dgoulet@torproject.org>
check-cocci is still a good idea -- perhaps as a cron job? But
doing it as part of our regular tests has just been confusing,
especially to volunteers who shouldn't have to become coccinelle
experts in order to get their patches through our CI.
Closes#40030.
When receiving an introduction NACK, the client either decides to close or
re-extend the circuit to another intro point.
In order to do this, the service descriptor needs to exists but it is possible
that it gets removed from the cache between the establishement of the
introduction circuit and the reception of the (N)ACK.
For that reason, the BUG(desc == NULL) is removed because it is a possible
normal use case. Tor recovers gracefully already.
Fixes#34087
Signed-off-by: David Goulet <dgoulet@torproject.org>
This commit modifies the behavior of `parse_extended_address` in such a way
that if it fails, it will always return a `BAD_HOSTNAME` value, which is then
used to return the 0xF6 extended error code. This way, in any case that is
not a valid v2 address, we return the 0xF6 error code, which is the expected
behavior.
Signed-off-by: David Goulet <dgoulet@torproject.org>
This reverts commit bf2a399fc0.
Don't set by default the prefer IPv6 feature on client ports because it breaks
the torsocks use case. The SOCKS resolve command is lacking a mechanism to ask
for a specific address family (v4 or v6) thus prioritizing IPv6 when an IPv4
address is asked on the resolve SOCKS interface resulting in a failure.
Tor Browser explicitly set PreferIPv6 so this should not affect the majority
of our users.
Closes#33796
Signed-off-by: David Goulet <dgoulet@torproject.org>
If no Address statement are found in the configuration file, attempt to learn
our address by looking at the ORPort address if any. Specifying an address is
optional so if we can't find one, it is fine, we move on to the next discovery
mechanism.
Note that specifying a hostname on the ORPort is not yet supported at this
commit.
Closes#33236
Signed-off-by: David Goulet <dgoulet@torproject.org>
For some reasons, Appveyor started to use the stdio printf format for 64 bit
values (PRIu64, ...). Mingw doesn't like that so force it to use the Windows
specific macros by setting D__USE_MINGW_ANSI_STDIO=0.
Fixes#40026
This patch fixes an out-of-bound memory read in
`tor_tls_cert_matches_key()` when Tor is compiled to use Mozilla's NSS
instead of OpenSSL.
The NSS library stores some length fields in bits instead of bytes, but
the comparison function found in `SECITEM_ItemsAreEqual()` needs the
length to be encoded in bytes. This means that for a 140-byte,
DER-encoded, SubjectPublicKeyInfo struct (with a 1024-bit RSA public key
in it), we would ask `SECITEM_ItemsAreEqual()` to compare the first 1120
bytes instead of 140 (140bytes * 8bits = 1120bits).
This patch fixes the issue by converting from bits to bytes before
calling `SECITEM_ItemsAreEqual()` and convert the `len`-fields back to
bits before we leave the function.
This patch is part of the fix for TROVE-2020-001.
See: https://bugs.torproject.org/33119
If at least one service is configured as a version 2, a log warning is emitted
once and only once.
Closes#40003
Signed-off-by: David Goulet <dgoulet@torproject.org>
The find_my_address() function now prioritize the local interface over the
local hostname when guessing the IP address.
See proposal 312, section 3.2.1, general case:
https://gitweb.torproject.org/torspec.git/tree/proposals/312-relay-auto-ipv6-addr.txt#n359
The entire unit tests had to be refactored to make this possible. Instead of
hot patching it, it has been rewritten to cover all possible cases and the
test interface has been changed to accomodate both IPv4 and IPv6 in order for
them to be tested identically.
Closes#33238
Signed-off-by: David Goulet <dgoulet@torproject.org>
Authorities currently add themselves to the trusted dir servers list,
but if they have an IPv6 ORPort, they leave it out.
This commit makes authorities add their own IPv6 ORPort to the trusted
dir servers list.
Closes ticket 32822.
This is not actually a bug! It can happen for a bunch of reasons,
which all boil down to "trying to add an extrainfo for which we no
longer have the corresponding routerinfo".
Fixes#16016; bugfix on 0.2.6.3-alpha.
Previously we had two chains of logic for reachability tests: one
for launching them, and one for telling the user that we had
launched them. Now, we simply have the launch code inform the user:
this way, we can't get out of sync.
Closes ticket 34137.
AssumeReachable should only be about whether a relay thinks that it
is reachable itself. But previously, we've had it also turn off
reachability checking of _other_ relays on authorities.
(Technically, reachability tests were still run, but we would ignore
the results.)
With this patch, there is a new AuthDirTestReachability option
(default 1) that controls whether authorities run reachability
tests.
Making this change allows us to have test networks where authorities
set AssumeReachable without disabling their reachability testing
entirely.
Closes ticket #34445.
These parameters do not suppress checks, but they tell relays that
it's okay to publish even when those checks fail.
I have chosen lowercase hyphenated names, since these seem to be
more common in networkstatus params.
Closes#33224 and part of #34064.
The buf_read_from_tls() function was designed to read up to a certain number
of bytes a TLS socket using read_to_chunk_tls() which boils down to SSL_read()
(with OpenSSL, common case).
However, at the end of the loop, the returned number of bytes from
read_to_chunk_tls() was treated like the syscall read() for which if less
bytes than the total asked are returned, it signals EOF.
But, with SSL_read(), it returns up to a TLS record which can be less than
what was asked. The assumption that it was EOF was wrong which made the while
loop exiting before it was able to consume all requested bytes (at_most
parameter).
The general use case that Tor sees is that it will ask the network layer to
give it at most 16KB (that is roughly 32 cells) but because of KIST scheduler,
the highest possible TLS record we currently observe is 4096 bytes (4KB or 8
cells). Thus the loop would at best always return 8 cells even though much
more could be on the TLS socket. See ticket #40006 for more details.
Fixes#40006
Signed-off-by: David Goulet <dgoulet@torproject.org>
I've managed to keep this change mainly contained to our
self-testing module. The changes here are:
* There are two different variables for tracking "is our orport
reachable".
* We have a new function that says whether we can skip a single
family's orport reachability test; the old function for this now
tells whether we can skip _all_ orport reachability testing.
(The name, router_should_skip_orport_reachability_test, is not
so good. I will rename it later if I can think of a good
replacement.)
* The function that launches orport reachability tests now only
launches the ones that haven't completed.
* The function that notes that we're reachable on an ORPort now
takes a family.
* Various log messages are cleaned up.
Since Tor 0.2.9 has been unsupported since January, 0.3.5 is the
oldest supported version, and its features constitute the oldest
recommended feature-set.
This patch updates these recommendations:
DirCache=2
Support for consensus diffs.
New in 0.3.1.1-alpha.
HSDir=2
Support for v3 onion service descriptors.
New in 0.3.0.4-alpha.
HSIntro=4
Support for Ed25519 intropoint authentication keys.
New in 0.3.0-4-alpha.
HSRend=2
Support for rendezvous cells longer than 20 bytes.
New in 0.2.9.4-alpha.
Link=5
Link padding and link padding negotiation.
New in 0.3.3.2-alpha.
LinkAuth=3
Ed25519 link authentication.
New in 0.3.0.1-alpha.
M_SYSCALL is used to report information about a sandbox violation,
but when we don't have a definition for it, it still makes sense to
compile.
Closes ticket 34382.
Resume being willing to use preemptively-built circuits when
UseEntryGuards is set to 0. We accidentally disabled this feature with
that config setting (in our fix for #24469), leading to slower load times.
Fixes bug 34303; bugfix on 0.3.3.2-alpha.
Make Rust protocol version support checks consistent with the
undocumented error behaviour of the corresponding C code.
Fixes bug 34251; bugfix on 0.3.3.5-rc.
Declare support for the onion service introduction point denial of
service extensions, when building tor with Rust.
Fixes bug 34248; bugfix on 0.4.2.1-alpha.
... and DISABLE_ASSERTS_IN_UNIT_TESTS.
Make all of tor's assertion macros support the ALL_BUGS_ARE_FATAL and
DISABLE_ASSERTS_IN_UNIT_TESTS debugging modes.
Implements these modes for IF_BUG_ONCE(). (It used to log a non-fatal
warning, regardless of the debugging mode.)
Fixes bug 33917; bugfix on 0.2.9.1-alpha.
Allow clients and relays to send dual-stack and IPv6-only EXTEND2 cells.
Parse dual-stack and IPv6-only EXTEND2 cells on relays.
Relays do not make connections or extend circuits via IPv6: that's the
next step.
Closes ticket 33901.
Log IPv6 addresses on connections where this relay is the responder.
Previously, responding relays would replace the remote IPv6 address with
the IPv4 address from the consensus.
(The port is replaced with the IPv6 ORPort from the consensus, we will
resolve this issue in 33898.)
Fixes bug 33899; bugfix on 0.3.1.1-alpha.
Consider IPv6 addresses when checking if a connection is canonical.
In 17604, relays assumed that a remote relay could consider an IPv6
connection canonical, but did not set the canonical flag on their side
of the connection.
Fixes bug 33899; bugfix on 0.3.1.1-alpha.
The client auth protocol allows attacker-controlled x25519 private keys being
passed around, which allows an attacker to potentially trigger the all-zeroes
assert for client_auth_sk in hs_descriptor.c:decrypt_descriptor_cookie().
We fixed that by making sure that an all-zeroes client auth key will not be
used.
There are no guidelines for validating x25519 private keys, and the assert was
there as a sanity check for code flow issues (we don't want to enter that
function with an unitialized key if client auth is being used). To avoid such
crashes in the future, we also changed the assert to a BUG-and-err.
This change broke torsocks that by default is expecting an IPv4 for hostname
resolution because it can't ask tor for a specific IP version with the SOCKS5
extension.
PreferIPv6 made it that sometimes the IPv6 could be returned to torsocks that
was expecting an IPv4.
Torsocks is probably a very unique case because the runtime flow is that it
hijacks DNS resolution (ex: getaddrinfo()), gets an IP and then sends it back
for the connect() to happen.
The libc has DNS resolution functions that allows the caller to request a
specific INET family but torsocks can't tell tor to resolve the hostname only
to an IPv4 or IPv6 and thus by default fallsback to IPv4.
Reverting this change into 0.4.3.x series but we'll keep it in the 0.4.4.x
series in the hope that we add this SOCKS5 extension to tor for DNS resolution
and then change torsocks to use that.
Fixes#33804
Signed-off-by: David Goulet <dgoulet@torproject.org>
Avoid conflicts between the fake sockets in tor's unit tests, and real
file descriptors. Resolves issues running unit tests with GitHub
Actions, where the process that embeds or launches the tests has
already opened a large number of file descriptors.
Fixes bug 33782; bugfix on 0.2.8.1-alpha.
Found and fixed by Putta Khunchalee.
If the buf_t's length could potentially become greater than INT_MAX - 1,
it sets off an IF_BUG_ONCE in buf_read_from_tls().
All of the rest of the buffers.c code has similar BUG/asserts for this
invariant.
When IPv6 ORPorts are set to "auto", tor relays and bridges would
advertise an incorrect port in their descriptor.
This may be a low-severity memory safety issue, because the published
port number may be derived from uninitialised or out-of-bounds memory
reads.
Fixes bug 32588; bugfix on 0.2.3.9-alpha.
Previously we just ignored this option, which would leave it unset,
and cause an assertion failure later on when running with the User
option.
Fixes bug 33668; bugfix on 0.4.3.1-alpha.
Copy required DLLs to test and app, before running tor's tests.
This ensures that tor.exe and test*.exe use the correct version of each
DLL. This fix is not required, but we hope it will avoid DLL search
issues in future.
Closes bug 33673; bugfix on 0.3.4.2-alpha.
For example, "TOR_SKIP_TESTCASES=crypto/.. ./src/test/test" will run
the tests and suppress all the "crypto/" tests. You could get the
same effect by running "./src/test/test :crypto/..", but that can be
harder to arrange from CI.
Part of a fix/workaround for 33643.
Given that ed25519 public key validity checks are usually not needed
and (so far) they are only necessary for onion addesses in the Tor
protocol, we decided to fix this specific bug instance without
modifying the rest of the codebase (see below for other fix
approaches).
In our minimal fix we check that the pubkey in
hs_service_add_ephemeral() is valid and error out otherwise.
This function does a nonfatal assertion to make sure that a machine
is not registered twice, but Tobias Pulls found a case where it
happens. Instead, make the function exit early so that it doesn't
cause a remotely triggered memory leak.
Fixes bug 33619; bugfix on 0.4.0.1-alpha. This is also tracked as
TROVE-2020-004.
Now port_cfg_new() returns all default flags and
port_parse_config() acts on defaults returned by port_cfg_new()
that is uses the default port_cfg_t object returned by port_cfg_new()
and modifies them later according to the port specifications in
configuration files
Might close tor#32994.
Stop forcing all non-SOCKSPorts to prefer IPv6 exit connections.
Instead, prefer IPv6 connections by default, but allow users to change
their configs using the "NoPreferIPv6" port flag.
Fixes bug 33608; bugfix on 0.4.3.1-alpha.
To run this, say something like
TOR_DEVTOOL_DIR=~/bin scripts/git/git-install-tools.sh all
To see what it would do, give it the -n flag.
To get help, give it the -h flag.
When a service can not upload its descriptor(s), we have no logs on why. This
adds logging for each possible reason for each descriptors.
That logging is emitted every second so it is rate limited for each reason and
per descriptor.
Closes#33400
Signed-off-by: David Goulet <dgoulet@torproject.org>
For a bridge configured with a pluggable transport, the transport name is
used, with the IP address, for the GeoIP client cache entry.
However, the DoS subsystem was not aware of it and always passing NULL when
doing a lookup into the GeoIP cache.
This resulted in bridges with a PT are never able to apply DoS defenses for
newly created connections.
Fixes#33491
Signed-off-by: David Goulet <dgoulet@torproject.org>
Allow the "practracker" coding best practices checking script to read
unicode files, when using Python 2.
We made the script use unicode literals in 0.4.3.1-alpha, but didn't
change the codec for opening files.
Fixes bug 33374; bugfix on 0.4.3.1-alpha.
