This patch removes a call to tor_assert_unreached() after execve()
failed. This assertion leads to the child process emitting a stack trace
on its standard output, which makes the error harder for the user to
demystify, since they think it is an internal error in Tor instead of
"just" being a "no such file or directory" error.
The process will now instead output "Error from child process: X" where
X is the stringified version of the errno value.
See: https://bugs.torproject.org/31810
This patch removes a call to tor_assert_unreached() after execve()
failed. This assertion leads to the child process emitting a stack trace
on its standard output, which makes the error harder for the user to
demystify, since they think it is an internal error in Tor instead of
"just" being a "no such file or directory" error.
The process will now instead output "Error from child process: X" where
X is the stringified version of the errno value.
See: https://bugs.torproject.org/31810
When tearing down all periodic events during shutdown, disable them first so
their enable flag is updated.
This allows the tor_api.h to relaunch tor properly after a clean shutdown.
Fixes#32058
Signed-off-by: David Goulet <dgoulet@torproject.org>
Running doxygen with latex gave us all manner of unicode issues,
slowed down the "make doxygen" target by a lot, and added several
latex dependencies... all to produce a 4000-page reference manual
which is probably not what anybody wanted.
Closes ticket 32099.
Found by coverity CID 1454769.
There were a second possible leak that is also fixed in this commit.
Fixes#32063
Signed-off-by: David Goulet <dgoulet@torproject.org>
Code adapted from Rob's proposed patch in #30344.
Also add a comment in connection_mark_for_close_internal_() on why we should
not be adding extra code there without a very good reason.
When considering introduction point of a service's descriptor, do not remove
an intro point that has an established or pending circuit.
Fixes#31652
Signed-off-by: David Goulet <dgoulet@torproject.org>
When encoding introduction points, we were not checking if that intro points
had an established circuit.
When botting up, the service will pick, by default, 3 + 2 intro points and the
first 3 that establish, we use them and upload the descriptor.
However, the intro point is removed from the service descriptor list only when
the circuit has opened and we see that we have already enough intro points, it
is then removed.
But it is possible that the service establishes 3 intro points successfully
before the other(s) have even opened yet.
This lead to the service encoding extra intro points in the descriptor even
though the circuit is not opened or might never establish (#31561).
Fixes#31548
Signed-off-by: David Goulet <dgoulet@torproject.org>
These tests all invoke the hostname resolver in one way or another,
and therefore potentially block if our DNS server is missing,
absent, or extremely slow. Closes ticket 31841.
Our minimum version is now 0.2.9.5-alpha. Series 0.3.0, 0.3.1,
0.3.2, 0.3.3, and 0.3.4 are now rejected.
Also, extract this version-checking code into a new function, so we
can test it.
Closes ticket 31549.
Also reject 0.3.5.0 through 0.3.5.6-rc as unstable.
When pkg-config is not installed, or a library that depends on
pkg-config is not found, tell the user what to do to fix the
problem.
Fixes bug 31922; bugfix on 0.3.1.1-alpha.
The documentation for this function says that the smartlist can
contain NULLs, but the code only handled NULLs if they were at the
start of the list.
We didn't notice this for a long time, because when Tor is run
normally, the sequence of msg_id_t is densely packed, and so this
list (mapping msg_id_t to channel_id_t) contains no NULL elements.
We could only run into this bug:
* when Tor was running in embedded mode, and starting more than once.
* when Tor ran first with more pubsub messages enabled, and then
later with fewer.
* When the second run (the one with fewer enabled pubsub messages)
had at least some messages enabled, and those messages were not
the ones with numerically highest msg_id_t values.
Fixes bug 31898; bugfix on 47de9c7b0a
in 0.4.1.1-alpha.
Merge master's RUST_VERSION="nightly" into the macOS Rust job
from bug31859_041.
Drop master's RUST_VERSION="beta", because we only have two
Rust jobs now.
Since Travis macOS has IPv6 support (and Travis Linux does not), chutney
will now run its IPv6 networks as part of Travis CI.
But since chutney is slow, don't wait for the macOS chutney to finish.
(Travis have fixed the duplicate notification bug in fast_finish. So we
can use fast_finish and allow_failure to finish early. Unfortunately,
allow_failure also means we ignore failures in macOS chutney.)
Also make sure that we have:
* a compile on each platform, with each compiler,
* a check on each platform, and
* a check on each compiler.
Finally, sort builds: allow fail last, macOS first, slowest first.
Closes ticket 30860.
Closes ticket 31859 for 0.2.9.
This test failure happened due to a signed/unsigned integer
comparison.
This bug occurred on SunOS, it may also occur on other systems that
use signed char as the default. (And cast 1-byte integer constants
to an unsigned integer.)
Fixes bug 31897; bugfix on 0.4.1.1-alpha.
Frequently, when a patch fails, it has failures in several files.
Using the "-k" flag will let us learn all the compilation errors,
not just the first one that the compiler hits.
Based on a patch by rl1987.
Closes ticket 31372.
Frequently, when a patch fails, it has failures in several files.
Using the "-k" flag will let us learn all the compilation errors,
not just the first one that the compiler hits.
Based on a patch by rl1987.
- The function `decrypt_desc_layer` has a cleaner interface.
- `is_superencrypted_layer` changed from `int` -> `bool`
[ticket details](https://trac.torproject.org/projects/tor/ticket/31589)
add(changes/*): changes file
fix(src/features/hs): is_superencrypted changed from `int` -> `bool`
fix(changes/ticket31589): header
add(changes/ticket31589): subsystem(onion services) to change
When processing a %included folder, a bug caused the pointer to
the last element of the options list to be set to NULL when
processing a file with only comments or whitepace. This could
cause options from other files on the same folder to be
discarded depending on the lines after the affected %include.
This warning would previously be given every time we tried to open a
connection to a foo.exit address, which could potentially be used to
flood the logs. Now, we don't allow this warning to appear more
than once every 15 minutes.
Fixes bug 31466; bugfix on 0.2.2.1-alpha, when .exit was first
deprecated.
We want to forbid this pattern since, unlike the other log_*()
macros, log_debug() conditionally evaluates its arguments only if
debug-level logging is enabled. Thus, a call to
log_debug("%d", x++);
will only increment x if debugging logs are enabled, which is
probably not what the programmer intended.
One bug caused by this pattern was #30628.
This script detects log_debug( ) calls with any of E++, E--, ++E,
or --E in their arguments, where E is an arbitrary expression.
Closes ticket 30743.
Our dimap code asserts if you try to add the same key twice; this
can't happen if everything is running smoothly, but it's possible if
you try to start a relay where secret_onion_key_ntor is the same as
secret_onion_key_ntor.old.
Fixes bug 30916; bugfix on 0.2.4.8-alpha when ntor keys were
introduced.
We previously used tor_fragile_assert() to declare that this case
could not happen: VERSIONS cells are always supposed to be
variable-sized, right?
This is incorrect, though. On a v1 link protocol connection, all
cells are fixed-sized. There aren't supposed to be any VERSIONS
cells with this version of the protocol, but apparently, somebody
was messing up. (The v1 link protocol is obsolete, so probably the
implementer responsible didn't mean to be using it.)
Fixes bug 31107. Bugfix on 0.2.4.4-alpha, when we introduced a
tor_fragile_assert() for this case.
These errors can occur if we are built on a system with support for
madvise(MADV_NOFORK) but then we are run on a system whose kernel
does not support that flag.
If the error is something that we don't tolerate at all, we now log
it before crashing.
Fixes bug 31696. I am calling this a bugfix on 0.4.1.1-alpha, where
we actually started using the map_anon code.
This is similar to, but not the same as, the fix for #31570.
Our code assumes that when we're configured to get IPv6 addresses
out of a TRANS_PF transparent proxy connection, we actually will.
But we didn't check that, and so FreeBSD started warning us about a
potential NULL pointer dereference.
Fixes part of bug 31687; bugfix on 0.2.3.4-alpha when this code was
added.
We used to do this on Windows only, but it appears to affect
multiple platforms when building with certain versions of GCC, and a
common pattern for defining the floating-point classifier functions.
Fixes part of 31687. I'm calling this a bugfux on 31687, when we
started suppressing these warnings on Windows.
Fix levels for subsystems that depend on log/err
* winprocess (security) doesn't use err:
* call windows process security APIs as early as possible
* init err after winprocess
* move wallclock so it's still after err
* network and time depend on log:
* make sure that network and time can use logging.
* init network and time after log
Add comments explaining the module init order.
Fixes bug 31615; bugfix on 0.4.0.1-alpha.
When tor is missing descriptors for some primary entry guards, make the
log message less alarming. It's normal for descriptors to expire, as long
as tor fetches new ones soon after.
Fixes bug 31657; bugfix on 0.3.3.1-alpha.
tor-github/pr/1283, with the following changes:
* cherry-pick the merge commit in 1283 on top of...
* tor-github/pr/1174, but with the last commit re-worded to remove
the fixup, because fixups break our push rules.
This is an "ours" merge, except for the bugfix version change in
changes/bug30649.
When we parse a CLEAR line (e.g., "/OrPort" or /OrPort blah blah"),
we always suppress the value, even if one exists. That means that
the block of code was meant to handle CLEAR lines didn't actually do
anything, since we previously handled them the same way as with
other empty values.
Closes ticket 31529.
These errors can occur if we are built on a system with support for
madvise(MADV_NOFORK) but then we are run on a system whose kernel
does not support that flag.
If the error is something that we don't tolerate at all, we now log
it before crashing.
Fixes bug 31570. I am calling this a bugfix on 0.4.1.1-alpha, where
we actually started using the map_anon code.
Some platforms (macOS, maybe others?) can swallow the last write before an
abort. This issue is probably caused by a race condition between write
buffer cache flushing, and process termination. So we write an extra
newline, to make sure that the message always gets through.
Fixes bug 31571; bugfix on 0.3.5.1-alpha.
This fixes LTO compilation for Android and -O0 compilation in
general, when --disable-module-dirauth is provided.
Fixes bug 31552; bugfix on 0.4.1.1-alpha.
Skip test branches that are the same as remote maint/release/master
branches.
Add a TOR_PUSH_SAME and -s argument to git-push-all.sh to change this
default.
Part of 31314.
TOR_GIT_PUSH provides the git push command and default arguments.
Also fix handling of git-push-all.sh script arguments and arguments that
are passed through to $TOR_GIT_PUSH, using a "--" argument as a separator.
Fix on 29879.
Add a -t <test-branch-prefix> argument to git-merge-forward.sh and
git-push-all.sh, which makes these scripts create, merge forward, and
push test branches.
Add a -r <remote-name> argument to git-push-all.sh, so the script can
push test branches to a personal remote.
Closes ticket 31314.
Rewrite format_node_description() and router_get_verbose_nickname() to
use strlcpy() and strlcat(). The previous implementation used memcpy()
and pointer arithmetic, which was error-prone.
Closes ticket 31545. This is CID 1452819.
The #[global_allocator] attribute is not available in versions prior to
1.28.0, the default-linker-libraries feature requires rust 1.31.0.
Adapt the CI to prevent accidental increases in Rust version by testing
against 1.31.0, beta and nightly.
The pre-push hook already calls the pre-commit hook, which calls
practracker.
Also update the script comments to avoid similar issues in future.
Fixes bug 31462; bugfix on 0.4.1.1-alpha.
If unsigned int is 32-bits long, then our old code would give a
wrong result with any log domain whose mask was >= (1<<32).
Fortunately, there are no such log domains right now: the domain
mask is only 64 bits long to accommodate some flags.
Found by coverity as CID 1452041.
Fixes bug 31451; bugfix on 0.4.1.4-rc.
New IP address from 194.109.206.212 to 45.66.33.45.
Signed request from Alex de Joode, operator of dizum:
https://trac.torproject.org/projects/tor/ticket/31406
Published descriptor by dizum on August 12th, 2019:
--
r dizum fqbq1v2DCDxTj0QDi7+gd1h911U GZmZtCLaPDQNxkhIFj8UcgTRAuA 2019-08-12 15:28:40 45.66.33.45 443 80
s Authority Fast Running Stable V2Dir Valid
v Tor 0.4.0.5
pr Cons=1-2 Desc=1-2 DirCache=1-2 HSDir=1-2 HSIntro=3-4 HSRend=1-2 Link=1-5 LinkAuth=1,3 Microdesc=1-2 Relay=1-2 Padding=1
w Bandwidth=20 Unmeasured=1
p reject 1-65535
--
Finally, confirmed by DNS:
$ dig +short tor.dizum.com
45.66.33.45
Closes#31406
Signed-off-by: David Goulet <dgoulet@torproject.org>
Add a TOR_PUSH_DELAY variable to git-push-all.sh, which makes the script
push master and maint branches with a delay between each branch. These
delays trigger the CI jobs in a set order, which should show the most
likely failures first.
Also:
* make pushes atomic by default, and
* make the script pass any command-line arguments to git push.
Closes ticket 29879.
On some windows builds, time_t is 64 bits but long is not. This is
causing appveyor builds to fail.
Also, one of our uses of labs() on time_t was logically incorrect:
it was telling us to accept NETINFO cells up to three minutes
_before_ the message they were responding to, which doesn't make
sense.
This patch adds a time_abs() function that we should eventually move
to intmath.h or something. For now, though, it will make merges
easier to have it file-local in channeltls.c.
Fixes bug 31343; bugfix on 0.2.4.4-alpha.
I'm using 500 as a file size limit, and 15 as an include limit.
This affects comparatively few files, but I think they are the worst
ones.
Closes ticket 31175.
Padding circuits were regular cells that got closed before their padding
machine could finish. This means that they can still receive regular cells from
their past life, but they have no way or reason to answer them anymore. Hence
let's ignore them before they even get to the proper subsystems.
There is other code that uses this value, and some of it is
apparently reachable from inside router_dir_info_changed(), which
routerlist_free() apparently calls. (ouch!) This is a minimal fix
to try to resolve the issue without causing other problems.
Fixes bug 31003. I'm calling this a bugfix on 0.1.2.2-alpha, where
the call to router_dir_info_changed() was added to routerlist_free().
When we added LD_MESG, we created a conflict with the LD_NO_MOCK
flag. We now need 64 bits for log domains in order to fix this
issue.
Fixes bug 31080; bugfix on 0.4.1.1-alpha.
Coverity has had trouble figuring out our csiphash implementation,
and has given spurious warnings about its behavior.
This patch changes the csiphash implementation when coverity is in
use, so that coverity can figure out that we are not about to read
beyond the provided input.
Closes ticket 31025.
Coverity doesn't understand that if begin_cell_parse() returns 0 and
sets is_begindir to 0, its address field will always be set.
Fixes bug 30126; bugfix on 0.2.4.7-alpha; Fixes CID 1447296.
Replace the 157 fallbacks originally introduced in Tor 0.3.5.6-rc
in December 2018 (of which ~122 were still functional), with a
list of 148 fallbacks (70 new, 78 existing, 79 removed) generated
in June 2019.
Closes ticket 28795.
Note that we created extra lists from DE and US, but they didn't add
any more fallbacks when we tried to merge them.
When parsing addreses via Tor's internal address:port parsing and
DNS lookup APIs, require IPv6 addresses with ports to have square
brackets.
But allow IPv6 addresses without ports, whether or not they have
square brackets.
Fixes bug 30721; bugfix on 0.2.1.5-alpha.
When parsing addreses via Tor's internal DNS lookup API:
* reject IPv4 addresses in square brackets (with or without a port),
* accept IPv6 addresses in square brackets (with or without a port), and
* accept IPv6 addresses without square brackets, as long as they have no port.
This change completes the work started in 23082, making address parsing
consistent between tor's internal DNS lookup and address parsing APIs.
Fixes bug 30721; bugfix on 0.2.1.5-alpha.
Right now, this has been done at a high level by confparse.c, but it
makes more sense to lower it.
This API is radically un-typesafe as it stands; we'll be wrapping it
in a safer API as we do #30914 and lower the struct manipulation
code as well.
Closes ticket 30864.
If the signature data was removed, but the keyword was kept, this could
result in an unparseable extra info file.
Fixes bug 30958; bugfix on 0.2.7.2-alpha.
Always publish bridge pluggable transport information in the extra info
descriptor, even if ExtraInfoStatistics is 0. This information is
needed by BridgeDB.
Fixes bug 30956; bugfix on 0.4.1.1-alpha.
This will effectively also deny any bridge to be used as a single hop to the
introduction point since bridge do not authenticate like clients.
Fixes#24963
Signed-off-by: David Goulet <dgoulet@torproject.org>
When we consider all circuits in "waiting for guard" state to be promoted to
an "open" state, we were considering all circuits, even the one marked for
close.
This ultiamtely triggers a "circuit_has_opened()" called on the circuit that
is marked for close which then leads to possible undesirable behaviors within
a subsystem.
For instance, the HS subsystem would be unable to find the authentication key
of the introduction point circuit leading to a BUG() warning and a duplicate
mark for close on the circuit.
This commit also adds a unit test to make sure we never select marked for
close circuits when upgrading its guard state from waiting for guard to open.
Fixes#30871
Signed-off-by: David Goulet <dgoulet@torproject.org>
When we consider all circuits in "waiting for guard" state to be promoted to
an "open" state, we were considering all circuits, even the one marked for
close.
This ultiamtely triggers a "circuit_has_opened()" called on the circuit that
is marked for close which then leads to possible undesirable behaviors within
a subsystem.
For instance, the HS subsystem would be unable to find the authentication key
of the introduction point circuit leading to a BUG() warning and a duplicate
mark for close on the circuit.
This commit also adds a unit test to make sure we never select marked for
close circuits when upgrading its guard state from waiting for guard to open.
Fixes#30871
Signed-off-by: David Goulet <dgoulet@torproject.org>
When we consider all circuits in "waiting for guard" state to be promoted to
an "open" state, we were considering all circuits, even the one marked for
close.
This ultiamtely triggers a "circuit_has_opened()" called on the circuit that
is marked for close which then leads to possible undesirable behaviors within
a subsystem.
For instance, the HS subsystem would be unable to find the authentication key
of the introduction point circuit leading to a BUG() warning and a duplicate
mark for close on the circuit.
This commit also adds a unit test to make sure we never select marked for
close circuits when upgrading its guard state from waiting for guard to open.
Fixes#30871
Signed-off-by: David Goulet <dgoulet@torproject.org>
Note that the event base object is _not_ created from the initialize
function, since it is configuration-dependent. This will wait until
configuration is integrated into subsystems.
Closes ticket 30806.
"make test-network-all" shows the warnings from each test-network.sh
run on the console, so developers see new warnings early.
Improve the documentation for this feature, and rename a Makefile
variable so the code is self-documenting.
Fixes bug 30455; bugfix on 0.3.0.4-rc.
This is to avoid having two sendme.{c|h} in the repository since the subsystem
is implemented in src/core/or/sendme.{c|h}.
Fixes#30769
Signed-off-by: David Goulet <dgoulet@torproject.org>
Set the env vars:
* TOR_MASTER_NAME to override the tor master branch name, and
* TOR_WKT_NAME to override the worktree path
Fixes bug 30841; bugfix on 0.4.0.1-alpha.
Skip test_rebind when the TOR_SKIP_TEST_REBIND environmental variable
is set.
Skip test_rebind on macOS in Travis builds, because it is unreliable
on macOS on Travis.
Fixes bug 30713; bugfix on 0.3.5.1-alpha.
Adds ROUTER_AUTHDIR_BUG_ANNOTATIONS to was_router_added_t.
The out-of-order numbering is deliberate: it will be fixed by later commits
for 16564.
Fixes bug 30780; bugfix on 0.2.0.8-alpha.
Leave the other rend and hs_ident data around until circuit free, since code
may still try to inspect it after marking the circuit for close. The
circuitmap is the important thing to clean up, since repurposed
intropoints must be removed from this map to ensure validity.
Leave the other rend and hs_ident data around until circuit free, since code
may still try to inspect it after marking the circuit for close. The
circuitmap is the important thing to clean up, since repurposed
intropoints must be removed from this map to ensure validity.
If "Log debug ..." is not set, the decrement never happens. This lead to the
package/deliver window to be out of sync at the stream level and thus breaking
the connection after 50+ cells.
Fixes#30628
Signed-off-by: David Goulet <dgoulet@torproject.org>
Previously, we were looking at our global settings to see what kind
of proxy we had. But doing this would sometimes give us the wrong
results when we had ClientTransportPlugin configured but we weren't
using it for a particular connection. In several places in the
code, we had added checks to see if we were _really_ using a PT or
whether we were using a socks proxy, but we had forgotten to do so
in at least once case. Instead, since every time we call this
function we are asking about a single connection, it is probably
best just to make this function connection-specific.
Fixes bug 29670; bugfix on 0.2.6.2-alpha.
When we repurpose a hidden service circuit, we need to clean up from the HS
circuit map and any HS related data structured contained in the circuit.
This commit adds an helper function that does it when repurposing a hidden
service circuit.
Fixes#29034
Signed-off-by: David Goulet <dgoulet@torproject.org>
Proposal 289 prevents SENDME-flooding by requiring the other side to
authenticate the data it has received. But this data won't actually
be random if they are downloading a known resource. "No problem",
we said, "let's fell the empty parts of our cells with some
randomness!" and we did that in #26871.
Unfortunately, if the relay data payloads are all completely full,
there won't be any empty parts for us to randomize.
Therefore, we now pick random "randomness windows" between
CIRCWINDOW_INCREMENT/2 and CIRCWINDOW_INCREMENT. We remember whether we have
sent a cell containing at least 16 bytes of randomness in that window. If we
haven't, then when the window is exhausted, we send one. (This window approach
is designed to lower the number of rng checks we have to do. The number 16 is
pulled out of a hat to change the attacker's guessing difficulty to
"impossible".)
Implements 28646.
Previously we purged it in 1-hour increments -- but one-hour is the
maximum TTL for the cache! Now we do it in 25%-TTL increments.
Fixes bug 29617; bugfix on 0.3.5.1-alpha.
This is the first half of implementing proposal 301. The
RecommendedPackages torrc option is marked as obsolete and
the test cases for the option removed. Additionally, the code relating
to generating and formatting package lines in votes is removed.
These lines may still appear in votes from other directory authorities
running earlier versions of the code and so consensuses may still
contain package lines. A new consensus method will be needed to stop
including package lines in consensuses.
Fixes: #28465
