Because the test is adding entries to the "rend_cache" directly, the
rend_cache_increment_allocation() was never called which made the
rend_cache_clean() call trigger that underflow warning:
rend_cache/clean: [forking] Nov 29 09:55:04.024 [warn] rend_cache_decrement_allocation(): Bug: Underflow in rend_cache_decrement_allocation (on Tor 0.4.0.0-alpha-dev 2240fe63feb9a8cf)
The test is still good and valid.
Fixes#28660
Signed-off-by: David Goulet <dgoulet@torproject.org>
The code checked for sysctl being available and HW_PHYSMEM being
defined, but HW_USERMEM was actually being used with sysctl instead
of HW_PHYSMEM.
The case for OpenBSD, etc. use HW_PHYSMEM64 (which is obviously a
64-bit variant of HW_PHYSMEM) and the case for OSX uses HW_MEMSIZE
(which appears to be a 64-bit variant of HW_PHYSMEM).
Signed-off-by: Kris Katterjohn <katterjohn@gmail.com>
We log these messages at INFO level, except when we are reading a
private key from a file, in which case we log at WARN.
This fixes a regression from when we re-wrote our PEM code to be
generic between nss and openssl.
Fixes bug 29042, bugfix on 0.3.5.1-alpha.
Stop logging "Tried to establish rendezvous on non-OR circuit..." as
a warning. Instead, log it as a protocol warning, because there is
nothing that relay operators can do to fix it.
Fixes bug 29029; bugfix on 0.2.5.7-rc.
Prior to this commit, the testsuite was failing on OpenBSD. After
this commit the testsuite runs fine on OpenBSD.
It was previously decided to test for the OpenBSD macro (rather than
__OpenBSD__, etc.) because OpenBSD forks seem to have the former
macro defined. sys/param.h must be included for the OpenBSD macro
definition; however, many files tested for the OpenBSD macro without
having this header included.
This commit includes sys/param.h in the files where the OpenBSD macro
is used (and sys/param.h is not already included), and it also
changes some instances of the __OpenBSD__ macro to OpenBSD.
See commit 27df23abb6 which changed
everything to use OpenBSD instead of __OpenBSD__ or OPENBSD. See
also tickets #6982 and #20980 (the latter ticket is where it was
decided to use the OpenBSD macro).
Signed-off-by: Kris Katterjohn <katterjohn@gmail.com>
In get_local_listener used by tor_ersatz_socketpair, the address
family used when binding the IPv6 socket was AF_INET instead of
AF_INET6.
Fixes bug 28995; bugfix on 0.3.5.1-alpha.
Signed-off-by: Kris Katterjohn <katterjohn@gmail.com>
In theory it would be better to detect this bug in advance, but this
approach is much simpler, and therefore safer to backport.
This closes tor issue 28973.
Linked connections aren't woken up by libevent due to I/O but rather
artificially so we can, by chunks, empty the spooled object(s).
Commit 5719dfb48f (in 0.3.4.1-alpha) made it
that the schedule_active_linked_connections_event would be only called once at
startup but this is wrong because then we would never go through again the
active linked connections.
Fortunately, everytime a new linked connection is created, the event is
activated and thus we would go through the active list again. On a busy relay,
this issue is mitigated by that but on a slower relays or bridge, a connection
could get stuck for a while until a new directory information request would
show up.
Fixes#28717, #28912
The point of this function is to make sure that the ed25519-based
implementation of curve25519_basepoint() actually works when we
start tor, and use the regular fallback implementation if it
doesn't. But it accounts for 9% of our startup time in the case
when we have directory information, and I think it's safe to make
the test shorter. After all, it has yet to find any actual bugs in
curved25519_scalarmult_basepoint_donna() on any platforms.
Closes ticket 28838.
Add the bootstrap tag name to the log messages, so people
troubleshooting connection problems can look up a symbol instead of a
number. Closes ticket 28731.
Merge Phoul's two lists into teor's list.
Replace the 150 fallbacks originally introduced in Tor 0.3.3.1-alpha in
January 2018 (of which ~115 were still functional), with a list of
157 fallbacks (92 new, 65 existing, 85 removed) generated in
December 2018.
Closes ticket 24803.
Replace the 150 fallbacks originally introduced in Tor 0.3.3.1-alpha in
January 2018 (of which ~115 were still functional), with a list of
148 fallbacks (89 new, 59 existing, 91 removed) generated in
December 2018.
Closes ticket 24803.
When retrying all SOCKS connection because new directory information just
arrived, do not BUG() if a connection in state AP_CONN_STATE_RENDDESC_WAIT is
found to have a usable descriptor.
There is a rare case when this can happen as detailed in #28669 so the right
thing to do is put that connection back in circuit wait state so the
descriptor can be retried.
Fixes#28669
Signed-off-by: David Goulet <dgoulet@torproject.org>
This helper function marks an entry connection as pending for a circuit and
changes its state to AP_CONN_STATE_CIRCUIT_WAIT. The timestamps are set to
now() so it can be considered as new.
No behaviour change, this helper function will be used in next commit.
Part of #28669
Signed-off-by: David Goulet <dgoulet@torproject.org>
Use the helper function connection_ap_mark_as_waiting_for_renddesc()
introduced in previous commit everywhere in the code where an AP connection
state is transitionned to AP_CONN_STATE_RENDDESC_WAIT.
Part of #28669
Signed-off-by: David Goulet <dgoulet@torproject.org>
The transition for a connection to either become or go back in
AP_CONN_STATE_RENDDESC_WAIT state must make sure that the entry connection is
_not_ in the waiting for circuit list.
This commit implements the helper function
connection_ap_mark_as_waiting_for_renddesc() that removes the entry connection
from the pending list and then change its state. This code pattern is used in
many places in the code where next commit will remove this code duplication to
use this new helper function.
Part of #28669
Signed-off-by: David Goulet <dgoulet@torproject.org>
To succesful compile tor-print-ed-signing-cert.exe on Windows we
sometimes need to include the @TOR_LIB_GDI@ library.
See: https://bugs.torproject.org/28485
Correctly identify Windows 8.1, Windows 10, and Windows Server 2008
and later from their NT versions.
On recent Windows versions, the GetVersionEx() function may report
an earlier Windows version than the running OS. To avoid user
confusion, add "[or later]" to Tor's version string on affected
versions of Windows.
Remove Windows versions that were never supported by the
GetVersionEx() function.
Stop duplicating the latest Windows version in get_uname().
Fixes bug 28096; bugfix on 0.2.2.34; reported by Keifer Bly.
In conn_close_if_marked(), we can decide to keep a connection open that still
has data to flush on the wire if it is being rate limited on the write side.
However, in this process, we were also looking at the read() side which can
still have token in its bucket and thus not stop the reading. This lead to a
BUG() introduced in 0.3.4.1-alpha that was expecting the read side to be
closed due to the rate limit but which only applies on the write side.
This commit removes any bandwidth check on the read side and simply stop the
read side on the connection regardless of the bucket state. If we keep the
connection open to flush it out before close, we should not read anything.
Fixes#27750
Signed-off-by: David Goulet <dgoulet@torproject.org>
Apparently some freebsd compilers can't tell that 'c' will never
be used uninitialized.
Fixes bug 28413; bugfix on 0.2.9.3-alpha when we added support for
longer AES keys to this function.
We don't use this syscall, but openssl apparently does.
(This syscall puts a socket into a half-closed state. Don't worry:
It doesn't shut down the system or anything.)
Fixes bug 28183; bugfix on 0.2.5.1-alpha where the sandbox was
introduced.
Apparently, even though the manpage says it returns an int, it
can return a long instead and cause a warning.
Bug not in any released Tor. Part of #28399
Resume refusing to start with relative file paths and RunAsDaemon
set (regression from the fix for bug 22731).
Fixes bug 28298; bugfix on 0.3.3.1-alpha.
If tor terminates due to SIGNAL HALT before test_rebind.py calls
tor_process.terminate(), an OSError 3 (no such process) is thrown.
Fixes part of bug 27968 on 0.3.5.1-alpha.
Remember, you can't check to see if there are N bytes left in a
buffer by doing (buf + N < end), since the buf + N computation might
take you off the end of the buffer and result in undefined behavior.
Fixes 28202; bugfix on 0.2.0.3-alpha.
It is not enough to look at protover for v3 rendezvous support but also we
need to make sure that the curve25519 onion key is present or in other words
that the descriptor has been fetched and does contain it.
Fixes#27797.
Signed-off-by: David Goulet <dgoulet@torproject.org>
With the new refresh_service_descriptor() function we had both
refresh_service_descriptor() and update_service_descriptor() which is basically
the same thing.
This commit renames update_service_descriptor() to
update_service_descriptor_intro_points() to make it clear it's not a generic
refresh and it's only about intro points.
Commit changes no code.
Treat backtrace test failures as expected on NetBSD, OpenBSD, and
macOS/Darwin, until we solve bug 17808.
(FreeBSD failures have been treated as expected since 18204 in 0.2.8.)
Fixes bug 27948; bugfix on 0.2.5.2-alpha.
Before this commit, we would create the descriptor signing key certificate
when first building the descriptor.
In some extreme cases, it lead to the expiry of the certificate which triggers
a BUG() when encoding the descriptor before uploading.
Ticket #27838 details a possible scenario in which this can happen. It is an
edge case where tor losts internet connectivity, notices it and closes all
circuits. When it came back up, the HS subsystem noticed that it had no
introduction circuits, created them and tried to upload the descriptor.
However, in the meantime, if tor did lack a live consensus because it is
currently seeking to download one, we would consider that we don't need to
rotate the descriptors leading to using the expired signing key certificate.
That being said, this commit does a bit more to make this process cleaner.
There are a series of things that we need to "refresh" before uploading a
descriptor: signing key cert, intro points and revision counter.
A refresh function is added to deal with all mutable descriptor fields. It in
turn simplified a bit the code surrounding the creation of the plaintext data.
We keep creating the cert when building the descriptor in order to accomodate
the unit tests. However, it is replaced every single time the descriptor is
uploaded.
Fixes#27838
Signed-off-by: David Goulet <dgoulet@torproject.org>
When storing a descriptor in the client cache, if we are about to replace an
existing descriptor, make sure to close every introduction circuits of the old
descriptor so we don't have leftovers lying around.
Ticket 27471 describes a situation where tor is sending an INTRODUCE1 cell on
an introduction circuit for which it doesn't have a matching intro point
object (taken from the descriptor).
The main theory is that, after a new descriptor showed up, the introduction
points changed which led to selecting an introduction circuit not used by the
service anymore thus for which we are unable to find the corresponding
introduction point within the descriptor we just fetched.
Closes#27471.
Signed-off-by: David Goulet <dgoulet@torproject.org>
It won't be used if there are no authorized client configured. We do that so
we can easily support the addition of a client with a HUP signal which allow
us to avoid more complex code path to generate that cookie if we have at least
one client auth and we had none before.
Fixes#27995
Signed-off-by: David Goulet <dgoulet@torproject.org>
Occasionally, key pinning doesn't catch a relay that shares an ed25519
ID with another relay. Log the identity fingerprints and the shared
ed25519 ID when this happens, instead of making a BUG() warning.
Fixes bug 27800; bugfix on 0.3.2.1-alpha.
Commit 488e2b00bf introduced an issue, most
likely introduced by a bad copy paste, that made us stop reading on the
connection if our write bandwidth limit was reached.
The problem is that because "read_blocked_on_bw" was never set, the connection
was never reenabled for reading.
This is most likely the cause of #27813 where bytes were accumulating in the
kernel TCP bufers because tor was not doing reads. Only relays with
RelayBandwidthRate would suffer from this but affecting all relays connecting
to them. And using that tor option is recommended and best practice so many
many relays have it enabled.
Fixes#28089.
It turns out that if _only_ the ControlPort is set and nothing else, tor would
simply not bootstrap and thus not start properly. Commit 67a41b6306
removed that requirement for tor to be considered a "client".
Unfortunately, this made the mainloop enable basically nothing if only the
ControlPort is set in the torrc.
This commit now makes it that we also consider the ControlPort when deciding
if we are a Client or not. It does not revert 67a41b6306 meaning
options_any_client_port_set() stays the same, not looking at the control port.
Fixes#27849.
Signed-off-by: David Goulet <dgoulet@torproject.org>
Nothing should ever look at them on failure, but in some cases,
the unit tests don't check for failure, and then GCC-LTO freaks out.
Fixes part of 27772.
These confused GCC LTO, which thought they might be used
uninitialized. I'm pretty sure that as long as 'res' indicates
success, they will always be set to something, but let's unconfuse
the compiler in any case.
OpenSolaris apparently doesn't have timeradd(), so we added a
replacement, but we weren't including it here after the big
refactoring in 0.3.5.1-alpha.
Fixes bug 27963; bugfix on 0.3.5.1-alpha.
It looks to be the case that Rust's standard allocator, jemalloc, is
incompatible with sanitizers. The incompatibility, for whatever reason,
seems to cause segfaults at runtime when jemalloc is linked with
sanitizers.
Without actually trying to figure out what's going on here this commit
instead takes the hammer of "let's remove jemalloc when testing". The
`tor_allocate` crate now by default switches to the system allocator
(eventually this will want to be the tor allocator). Most crates then
link to `tor_allocate` ot pick this up, but the `smartlist` crate had to
manually switch to the system allocator in testing and the `external`
crate had to be sure to link to `tor_allocate`.
The final gotcha here is that this patch also switches to
unconditionally passing `--target` to Cargo. For weird and arcane
reasons passing `--target` with the host target of the compiler (which
Cargo otherwise uses as the default) is different than not passing
`--target` at all. This ensure that our custom `RUSTFLAGS` with
sanitizer options doesn't make its way into build scripts, just the
final testing artifacts.
This is no longer necessary with upstream rust-lang/rust changes as well
as some local tweaks. Namely:
* The `-fsanitize=address`-style options are now passed via `-C
link-args` through `RUSTFLAGS`. This obviates the need for the shell
script.
* The `-C default-linker-libraries`, disabling `-nodefaultlibs`, is
passed through `RUSTFLAGS`, which is necessary to ensure that
`-fsanitize=address` links correctly.
* The `-C linker` option is passed to ensure we're using the same C
compiler as normal C code, although it has a bit of hackery to only
get the `gcc` out of `gcc -std=c99`
Allowing this didn't do any actual harm, since there aren't any
shared structures or leakable objects here. Still, it's bad style
and might cause trouble in the future.
Closes ticket 27856.
Various places in our code try to activate these events or check
their status, so we should make sure they're initialized as early as
possible. Fixes bug 27861; bugfix on 0.3.5.1-alpha.
When freeing a configuration object from confparse.c in
dump_config(), we need to call the appropriate higher-level free
function (like or_options_free()) and not just config_free().
This only happens with options (since they're the one where
options_validate allocates extra stuff) and only when running
--dump-config with something other than minimal (since
OPTIONS_DUMP_MINIMAL doesn't hit this code).
Fixes bug 27893; bugfix on 0.3.2.1-alpha.
It differs from the rest of the rephist code in that it's actually
necessary for Tor to operate, so it should probably go somewhere
else. I'm not sure where yet, so I'll leave it in the same
directory, but give it its own file.
Since this is completely core functionality, I'm putting it in
core/mainloop, even though it depends on feature/hibernate. We'll
have to sort that out in the future.
If a tor client gets a descriptor that it can't decrypt, chances are that the
onion requires client authorization.
If a tor client is configured with client authorization for an onion but
decryption fails, it means that the configured keys aren't working anymore.
In both cases, we'll log notice the former and log warn the latter and the
rest of the decryption errors are now at info level.
Two logs statement have been removed because it was redundant and printing the
fetched descriptor in the logs when 80% of it is encrypted wat not helping.
Fixes#27550
Signed-off-by: David Goulet <dgoulet@torproject.org>
The main.c code is responsible for initialization and shutdown;
the mainloop.c code is responsible for running the main loop of Tor.
Splitting the "generic event loop" part of mainloop.c from the
event-loop-specific part is not done as part of this patch.
The parts for handling cell formats should be in src/core/or.
The parts for handling onionskin queues should be in src/core/or.
Only the crypto wrapper belongs in src/core/crypto.
When sending the INTRODUCE1 cell, we acquire the needed data for the cell but
if the RP node_t has invalid data, we'll fail the send and completely kill the
SOCKS connection.
Instead, close the rendezvous circuit and return a transient error meaning
that Tor can recover by selecting a new rendezvous point. We'll also do the
same when we are unable to encode the INTRODUCE1 cell for which at that point,
we'll simply take another shot at a new rendezvous point.
Fixes#27774
Signed-off-by: David Goulet <dgoulet@torproject.org>
The result of CString::into_raw() is not safe to free
with free() except under finicky and fragile circumstances
that we definitely don't meet right now.
This was missed in be583a34a3.
