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>
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.
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.
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>
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.
When we close a socket via tor_tls_free(), we previously had no way
for our socket accounting logic to learn about it. This meant that
the socket accounting code would think we had run out of sockets,
and freak out.
Fixes bug 27795; bugfix on 0.3.5.1-alpha.
Client side, when a descriptor is finally fetched and stored in the cache, we
then go over all pending SOCKS request for that descriptor. If it turns out
that the intro points are unusable, we close the first SOCKS request but not
the others for the same .onion.
This commit makes it that we'll close all SOCKS requests so we don't let
hanging the other ones.
It also fixes another bug which is having a SOCKS connection in RENDDESC_WAIT
state but with a descriptor in the cache. At some point, tor will expire the
intro failure cache which will make that descriptor usable again. When
retrying all SOCKS connection (retry_all_socks_conn_waiting_for_desc()), we
won't end up in the code path where we have already the descriptor for a
pending request causing a BUG().
Bottom line is that we should never have pending requests (waiting for a
descriptor) with that descriptor in the cache (even if unusable).
Fixees #27410.
Signed-off-by: David Goulet <dgoulet@torproject.org>
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>
Instead, have it call a mockable function. We don't want
crypto_strongest_rand() to be mockable, since doing so creates a
type error when we call it from ed25519-donna, which we do not build
in a test mode.
Fixes bug 27728; bugfix on 0.3.5.1-alpha
Use the Travis Homebrew addon to install packages on macOS. The package
list is the same, but the Homebrew addon does not do a `brew update` by
default.
This makes builds faster, at the cost of using slightly older packages.
Implements ticket 27738.
This argument was added to match an older idea for the C api, but we
decided not to do it that way in C.
Fixes bug 27741; bugfix on 0.3.3.6 / TROVE-2018-005 fix.
Since the default cache directory is the same as the default data
directory, we don't want the default CacheDirectoryGroupReadable
value (0) to override an explicitly set "DataDirectoryGroupReadable
1".
To fix this, I'm making CacheDirectoryGroupReadable into an
autobool, and having the default (auto) value mean "Use the value of
DataDirectoryGroupReadable if the directories are the same, and 0
otherwise."
Fixes bug 26913; bugfix on 0.3.3.1-alpha when the CacheDirectory
option was introduced.
In hs_config.c, we do validate the permission of the hidden service directory
but we do not try to create it. So, in the event that the directory doesn't
exists, we end up in the loading key code path which checks for the
permission and possibly creates the directory. On failure, don't BUG() since
there is a perfectly valid use case for that function to fail.
Fixes#27335
Signed-off-by: David Goulet <dgoulet@torproject.org>
On GCC and Clang, there's a feature to warn you about bad
conditionals like "if (a = b)", which should be "if (a == b)".
However, they don't warn you if there are extra parentheses around
"a = b".
Unfortunately, the tor_assert() macro and all of its kin have been
passing their inputs through stuff like PREDICT_UNLIKELY(expr) or
PREDICT_UNLIKELY(!(expr)), both of which expand to stuff with more
parentheses around "expr", thus suppressing these warnings.
To fix this, this patch introduces new macros that do not wrap
expr. They're only used when GCC or Clang is enabled (both define
__GNUC__), since they require GCC's "({statement expression})"
syntax extension. They're only used when we're building the
unit-test variant of the object files, since they suppress the
branch-prediction hints.
I've confirmed that tor_assert(), tor_assert_nonfatal(),
tor_assert_nonfatal_once(), BUG(), and IF_BUG_ONCE() all now give
compiler warnings when their argument is an assignment expression.
Fixes bug 27709.
Bugfix on 0.0.6, where we first introduced the "tor_assert()" macro.
.retain() would allocating a Vec of billions of integers and check them
one at a time to separate the supported versions from the unsupported.
This leads to a memory DoS.
Closes ticket 27206. Bugfix on e6625113c9.
Before 0.3.3.1-alpha, we would exit() in this case immediately. But
now that we leave tor_main() more conventionally, we need to make
sure we restore things so as not to cause a double free.
Fixes bug 27708; bugfix on 0.3.3.1-alpha.
All node_get_all_orports() does is allocate and return a smartlist
with at most two tor_addr_port_t members that match ORPort's of
node configuration. This is harmful for memory efficiency, as it
allocates the same stuff every time it is called. However,
node_is_a_configured_bridge() does not need to call it, as it
already has all the information to check if there is configured
bridge for a given node.
The new code is arranged in a way that hopefully makes each succeeding
linear search through bridge_list less likely.
It's possible for a unit test to report success via its pipe, but to
fail as it tries to clean up and exit. Notably, this happens on a
leak sanitizer failure.
Fixes bug 27658; bugfix on 0.2.2.4-alpha when tinytest was
introduced.
Existing cached directory information can cause misleadingly high
bootstrap percentages. To improve user experience, defer reporting of
directory information progress until at least one connection has
succeeded to a relay or bridge.
Closes ticket 27169.
There are three reasons we use a cached_dir_t to hold a consensus:
1. to serve that consensus to a client
2. to apply a consensus diff to an existing consensus
3. to send the consensus to a controller.
But case 1 is dircache-only. Case 2 and case 3 both fall back to
networkstatus_read_cached_consensus(). So there's no reason for us
to store this as a client. Avoiding this saves about 23% of our RAM
usage, according to our experiments last month.
This is, semantically, a partial revert of e5c608e535.
Fixes bug 27247; bugfix on 0.3.0.1-alpha.
As of August 29, 2018, Appveyor images come with gcc 8.2.0 by
default. 64-bit Windows executables compiled with gcc 8.2.0 and
tor's --enable-gcc-hardening crash.
Fixes bug 27460; bugfix on 0.3.4.1-alpha.
It is not supported, and always fails. Some compilers warn about the
function pointer cast on 64-bit Windows.
Fixes bug 27461; bugfix on 0.2.2.23-alpha.
gcc 8 warns that extend_info_t.nickname might be truncated by strncpy().
But it doesn't know that nickname can either contain a hex id, or a
nicknames. hex ids are only used for general and HSDir circuits.
Fixes bug 27463; bugfix on 0.1.1.2-alpha.
GetProcAddress() returns FARPROC, which is (long long int(*)()) on
64-bit Windows:
https://msdn.microsoft.com/en-us/library/windows/desktop/ms683212(v=vs.85).aspx
But GetAdaptersAddresses() is (long unsigned int(*)()), on both 32-bit
and 64-bit Windows:
https://docs.microsoft.com/en-us/windows/desktop/api/iphlpapi/nf-iphlpapi-getadaptersaddresses
So gcc 8 issues a spurious "incompatible function pointer" warning
about the cast to GetAdaptersAddresses_fn_t.
Silence this warning by casting to a void function pointer, before
the cast to GetAdaptersAddresses_fn_t.
This issue is already fixed by 26481 in 0.3.5 and later, by removing
the lookup and cast.
Fixes bug 27465; bugfix on 0.2.3.11-alpha.
We allow their CONNECTEDs, RESOLVEDs, ENDs, SENDMEs, and DATA cells to not
count as dropped until the windows are empty, or we get an END.
This commit does not change behavior. It only changes CIRC_BW event field
values.
No behaviour change.
A previous fix to chutney removed v3 onion services from the
mixed+hs-v23 network, so seeing "mixed+hs-v23" in tests is
confusing.
Fixes bug 27345; bugfix on 0.3.2.1-alpha.
LinkAuth method 1 is the one where we pull the TLS master secrets
out of the OpenSSL data structures and authenticate them with
RSA. Right now we list method 1 as required for clients and relays.
That's a problem, since we can't reasonably support it with NSS. So
let's remove it as a requirement and a recommendation.
As for method 3: I'd like to recommend it it, but that would make
0.2.9 start warning. Let's not do that till at least some time
after 0.3.5 (the next LTS) is stable.
Closes ticket 27286
Previously, Tor would only check the exit flag. In small networks, Tor
could bootstrap once it received a consensus with exits, without fetching
the new descriptors for those exits.
After bootstrap, Tor delays descriptor fetches, leading to failures in
fast networks like chutney.
Fixes 27236; bugfix on 0.2.6.3-alpha.
Unlike the old test, this test no will no longer mess around with
the forbidden internals of any openssl data structures.
Additionally, it verifies several other behaviors of
tor_tls_cert_matches_key() that we had wanted to verify, such as
the possibility of the certificate's key not matching.
Fixes bug 27226; bugfix on 0.2.5.1-alpha.
Until https://github.com/rust-lang/rust/issues/52652 is fixed,
unwinding on panic is potentially unsound in a mixed C/Rust codebase.
The codebase is supposed to be panic-free already, but just to be safe.
This started mattering at commit d1820c1516.
Fixes#27199; bugfix on tor-0.3.3.1-alpha.
It's impossible for spaces to get here, since spaces are used as
separators between individual protocol entries higher up.
And it shouldn't ignore whitespace that isn't a literal space
character, because that would differ from the C implementation.
These were added in 9925d2e687.
Fixes#27177. Bugfix on 0.3.3.5-rc.
It was parsing "1-2-3" as if it were 1-2, ignoring the 2nd hyphen
and everything after.
Introduced in d1820c1516.
Fixes#27164; bugfix on 0.3.3.1-alpha.
Replace master .travis.yml with 034 .travis.yml.
All the changes in master have been backported to the
034 .travis.yml already.
Replace master src/test/test_rust.sh with 034
src/test/test_rust.sh, which was backported from
master. One 033/034-specific commit needs to be
reverted.
Replace 034 .travis.yml with 033 .travis.yml.
Subsequent commits will restore 034 functionality.
Replace 034 src/test/test_rust.sh with 033
src/test/test_rust.sh, which was backported from
master.
Replace 033 .travis.yml with 032 .travis.yml.
Subsequent commits will restore 033 functionality.
src/rust/tor_util/include.am is deleted in 033.
Subsequent commits will apply 032 changes to
src/rust/tor_rust/include.am.
Replace 033 src/test/test_rust.sh with 032
src/test/test_rust.sh, which was backported from
master.
Creating the configure flags once avoids inconsistent flags
between configure and distcheck configure.
Echoing the flags helps developers work out what configure is
doing.
(Backported to 0.2.9 and later as a precaution.)
Fixes 27088 on 0.3.4.1-alpha, adds logging in previous releases.
I hope that the debian clang maintainers will look at debian bug
903709 soon. But until they do, this should keep our users and our
CI happy on sid with clang.
Closes ticket 26779.
If we're building for Windows, we want to use windows threads no
matter what, and we don't want to link a pthread library even if it
is present. Fixes bug 27081; bugfix on 1790dc6760 in 0.1.0.1-rc.
Based on a patch from Hello71 on ticket 20424.
This patch additionally fixes openbsd-malloc support, switches
our tcmalloc support to use pkgconfig, and tells the compiler to
omit system malloc implementations as appropriate.
squash! Add a --with-malloc option.
Edit changelog file to fix a typo and credit Alex Xu in preferred format.
When we fixed 25939 in f7633c1fca, we
introduced a call to rescan_periodic_events() from inside the onion
service logic. But this meant that we could rescan the event list --
thereby running event callbacks! -- from inside the hidden service code.
This could cause us to run some of our event callbacks from an
inconsistent state, if we were in the middle of changing options.
A related bug (#25761) prevented us from rescanning our periodic
events as appropriate, but when we fixed THAT one, this bug reared
its ugly head.
The fix here is that "enabling" an event should cause us to run it
from the event loop, but not immediately from the point where we
enable it.
Fixes bug 27003; bugfix on 0.3.4.1-alpha.
We would usually call it through tor_cleanup(), but in some code
paths, we wouldn't. These paths would break restart-in-process,
since leaving fields uncleared would cause assertion failures on
restart.
Fixes bug 26948; bugfix on 0.3.3.1-alpha
Conditionalize the pragma that temporarily disables
-Wunused-const-variable. Some versions of gcc don't support it. We
need to do this because of an apparent bug in some libzstd headers.
Fixes bug 26785; bugfix on 0.3.2.11.
Instead, log a protocol warning when single onion services or
Tor2web clients fail to authenticate direct connections to relays.
Fixes bug 26924; bugfix on 0.2.9.1-alpha.
Stop putting ed25519 link specifiers in v3 onion service descriptors,
when the intro point doesn't support ed25519 link authentication.
Fixes bug 26627; bugfix on 0.3.2.4-alpha.
Stop sending ed25519 link specifiers in v3 onion service introduce
cells, when the rendezvous point doesn't support ed25519 link
authentication.
Fixes bug 26627; bugfix on 0.3.2.4-alpha.
Work around two different bugs in the OS X 10.10 and later SDKs that would
prevent us from successfully targeting earlier versions of OS X.
Fixes bug 26876; bugfix on 0.3.3.1-alpha.
The "Bifroest" bridge authority has been retired; the new bridge authority
is "Serge", and it is operated by George from the TorBSD project.
Closes ticket 26771.
(When the ticket isn't a bugfix, we don't say "fixes bug XYZ". When
we do, then the version in "bugfix on XXX" is supposed to be the
first version that had the bug.)
This function has a nasty API, since whether or not it invokes the
resolver depends on whether one of its arguments is NULL. That's a
good way for accidents to happen.
This patch incidentally makes tor-resolve support socks hosts on
IPv6.
Recent Python3 versions seem to require this on Windows.
Fixes bug 26535; bug introduced in f4be34f70d, which
was apparently intended itself as a Python3 workaround.
For HSv3, the HSADDRESS= wasn't properly parsed for the HSPOST command. It now
correctly use it and furthermore sends back a "200 OK" in case the command is
successful for a v3 descriptor.
Fixes#26523
Signed-off-by: David Goulet <dgoulet@torproject.org>
When I wrote the first one of these, it needed the path of the geoip
file. But that doesn't translate well in at least two cases:
- Mingw, where the compile-time path is /c/foo/bar and the
run-time path is c:\foo\bar.
- Various CI weirdnesses, where we cross-compile a test binary,
then copy it into limbo and expect it to work.
Together, these problems precluded these tests running on windows.
So, instead let's just generate some minimal files ourselves, and
test against them.
Fixes bug 25787
Fix a memory leak where directory authorities would leak a chunk of
memory for every router descriptor every time they considered voting.
This bug was taking down directory authorities in the live network due
to out-of-memory issues.
Fixes bug 26435; bugfix on 0.3.3.6.
The doctests for src/rust/crypto don't compile for multiple reasons,
including some missing exports and incorrect identifier paths. Fixes
bug 26415; bugfix on 0.3.4.1-alpha.
There are a few reasons that relays might be uploading desciptors
without saying X-Desc-Gen-Reason:
1. They are running an old version of our software, before 0.3.2.stable.
2. They are not running our software, but they are claiming they
are.
3. They are uploading through a proxy that strips X-Desc-Gen-Reason.
4. They somehow had a bug in their software.
According to the 25686 data, 1 is the most common reason. This
ticket is an attempt to diagnose case 4, or prove that case 4
doesn't actually happen.
With the work on #25500 (reducing CPU client usage), the HS service main loop
callback is enabled as soon as the HS service map changes which happens when
registering a new service.
Unfortunately, for an ephemeral service, we were building the onion address
*after* the registration leading to the "service->onion_address` to be an
empty string.
This broke the "HS_DESC CREATED" event which had no onion address in it. And
also, we were logging an empty onion address for that service.
Fixes#25939
Signed-off-by: David Goulet <dgoulet@torproject.org>
Apparently, even though I had tested on OpenSSL 1.1.1 with
no-deprecated, OpenSSL 1.1.0 is different enough that I should have
tested with that as well.
Fixes bug 26156; bugfix on 0.3.4.1-alpha where we first declared
support for this configuration.
The new gcov sometimes emits *s if there is a line containing
multiple basic blocks, and some are not executed. (The gcov
documentation says something weird about this point, so I'm trying
to interpret it into the compilerese that I'm familiar with.)
That's bug 26101.
Also, when we're looking for unique variations in our coverage, we
would like cov-diff to suppress timestamps on the diffs. That's bug
26102.
Both of these are bugfixes on 0.2.5.1-alpha when the cov-diff script
was introduced.
My apologies for the perl.
The specification describes the signature token to be right after a newline
(\n) then the token "signature" and then a white-space followed by the encoded
signature.
This commit makes sure that when we parse the signature from the descriptor,
we are always looking for that extra white-space at the end of the token.
It will allow us also to support future fields that might start with
"signature".
Fixes#26069
Signed-off-by: David Goulet <dgoulet@torproject.org>
There are three cases where this can happen: changes in our
controller events, changes in our DisableNetwork setting, and
changes in our hibernation state.
Closes ticket 26063.
We still do this time update here, since we do it from all
callbacks, but it is no longer a reason to keep the once-per-second
callback enabled.
Closes ticket 26009.
The any_client_port_set() returns true if the ControlPort is set which is
wrong because we can have that port open but still not behave as a tor client
(like many relays for instance).
Fixes#26062
Signed-off-by: David Goulet <dgoulet@torproject.org>
The options_any_client_port_set() returns true if the ControlPort is set which
is wrong because we can have that port open but still not behave as a tor
client (like many relays for instance).
Fixes#26062
Signed-off-by: David Goulet <dgoulet@torproject.org>
Remove v3 optimization which made Tor not detect disabling services.
This optimization is not so needed because we only call that function after HUP
anyway.
Fixes bug #25761.
Previously the coverage on this function was mostly accidental,
coming as it did from test_entryconn.c. These new tests use mocking
to ensure that we actually hit the different failure and retry cases
of addressmap_get_virtual_address(), and make our test coverage a
bit more deterministic.
Closes ticket 25993.
Previously, an authority with a clock more than 60 seconds ahead could
cause a client with a correct clock to warn that the client's clock
was behind. Now the clocks of a majority of directory authorities
have to be ahead of the client before this warning will occur.
Relax the early-consensus check so that a client's clock must be 60
seconds behind the earliest time that a given sufficiently-signed
consensus could possibly be available.
Add a new unit test that calls warn_early_consensus() directly.
Fixes bug 25756; bugfix on 0.2.2.25-alpha.
Stop saying in the manual that clients cache ipv4 dns answers from exit
relays. We haven't used them since 0.2.6.3-alpha, and in ticket 24050
we stopped even caching them as of 0.3.2.6-alpha, but we forgot to say
so in the man page.
Fixes bug 26052; bugfix on 0.3.2.6-alpha.
Our previous algorithm had a nonzero probability of picking no
events to cancel, which is of course incorrect. The new code uses
Vitter's good old reservoir sampling "algorithm R" from 1985.
Fixes bug 26008; bugfix on 0.2.6.3-alpha.
This functionality was covered only accidentally by our voting-test
code, and as such wasn't actually tested at all. The tests that
called it made its coverage nondeterministic, depending on what time
of day you ran the tests.
Closes ticket 26014.
We cleared this value in second_elapsed_callback. But what were we
using it for? For detecting if Libevent returned EINVAL too often!
We already have a way to detect too-frequent events, and that's with
a ratelim_t. Refactor the code to use that instead. Closes ticket
26016.
This test was using the current time to pick the time period number,
and a randomly generated hs key. Therefore, it sometimes picked an
index that would wrap around the example dht, and sometimes would
not.
The fix here is just to fix the time period and the public key.
Fixes bug 25997; bugfix on 0.3.2.1-alpha.
When directory authorities read a zero-byte bandwidth file, they log
a warning with the contents of an uninitialised buffer. Log a warning
about the empty file instead.
Fixes bug 26007; bugfix on 0.2.2.1-alpha.
LibreSSL, despite not having the OpenSSL 1.1 API, does define
OPENSSL_VERSION in crypto.h. Additionally, it apparently annotates
some functions as returning NULL, so that our unit tests need to be
more careful about checking for NULL so they don't get compilation
warnings.
Closes ticket 26006.
This test, in test_client_pick_intro(), will have different coverage
depending on whether it selects a good intro point the first time or
whether it has to try a few times. Since it produces the shorter
coverage with P=1/4, repeat this test 64 times so that it only
provides reduced coverage with P=1/2^128. The performance cost is
negligible.
Closes ticket 25996. This test was introduced in 0.3.2.1-alpha.
I'd prefer not to do this for randomized tests, but as things stand
with this test, it produces nondeterministic test coverage.
Closes ticket 25995; bugfix on 0.2.2.2-alpha when this test was
introduced.
This change should make it impossible for the monotonic time to roll
over from one EWMA tick to the next during this test, and make it so
that this test never invokes scale_active_circuits() (which it
doesn't test).
(Earlier changes during the 0.3.4 series should make this call even
rarer than it was before, since we fixed#25927 and removed
cached_gettimeofday. Because this test didn't update
cached_gettimeofday, the chance of rolling over a 10-second interval
was much higher.)
Closes ticket 25994; bugfix on 0.3.3.1-alpha when this test was
introduced.
Arguably, the conditions under which these events happen should be a
bit different, but the rules are complex enough here that I've tried
to have this commit be pure refactoring.
Closes ticket 25952.
Finally, before this code goes away, take a moment to look at the
amazing way that we used to try to have an event happen
every N seconds:
get_uptime() / N != (get_uptime()+seconds_elapsed) / N
Truly, it is a thing of wonder. I'm glad we didn't start using this
pattern everywhere else.
Implement the ability to set flags per events which influences the set up of
the event.
This commit only adds one flag which is "need network" meaning that the event
is not enabled if tor has disabled the network or if hibernation mode.
Signed-off-by: David Goulet <dgoulet@torproject.org>
Prior to #23100, we were not counting HS circuit build times in our
calculation of the timeout. This could lead to a condition where our timeout
was set too low, based on non HS circuit build times, and then we would
abandon all HS circuits, storing no valid timeouts in the histogram.
This commit avoids the assert.
Previously were using this value to have a cheap highish-resolution
timer. But we were only using it in one place, and current dogma is
to use monotime_coarse_t for this kind of thing.
This part of the code was the only part that used "cached
getttimeofday" feature, which wasn't monotonic, which we updated at
slight expense, and which I'd rather not maintain.
Now that we update our buckets on demand before reading or writing,
we no longer need to update them all every TokenBucketRefillInterval
msec.
When a connection runs out of bandwidth, we do need a way to
reenable it, however. We do this by scheduling a timer to reenable
all blocked connections for TokenBucketRefillInterval msec after a
connection becomes blocked.
(If we were using PerConnBWRate more, it might make sense to have a
per-connection timer, rather than a single timeout. But since
PerConnBWRate is currently (mostly) unused, I'm going to go for the
simpler approach here, since usually whenever one connection has
become blocked on bandwidth, most connections are blocked on
bandwidth.)
Implements ticket 25373.
We used to do this 10x per second in connection_buckets_refill();
instead, we now do it when the bucket becomes empty. This change is
part of the work of making connection_buckets_refill() obsolete.
Closes ticket 25828; bugfix on 0.2.3.5-alpha.
This commit introduces the consensus parameter "circ_max_cell_queue_size"
which controls the maximum number of cells a circuit queue should have.
The default value is currently 50000 cells which is above what should be
expected but keeps us a margin of error for padding cells.
Related to this is #9072. Back in 0.2.4.14-alpha, we've removed that limit due
to a Guard discovery attack. Ticket #25226 details why we are putting back the
limit due to the memory pressure issue on relays.
Fixes#25226
Signed-off-by: David Goulet <dgoulet@torproject.org>
Add to the Denial of Service section of the man page an explanation about the
three different mitigation Tor has.
Fixes#25248.
Signed-off-by: David Goulet <dgoulet@torproject.org>
In d1874b4339, we adjusted this check so that we insist on
using routerinfos for bridges. That's almost correct... but if we
have a bridge that is also a regular relay, then we should use
insist on its routerinfo when connecting to it as a bridge
(directly), and be willing to use its microdescriptor when
connecting to it elsewhere in our circuits.
This bug is a likely cause of some (all?) of the (exit_ei == NULL)
failures we've been seeing.
Fixes bug 25691; bugfix on 0.3.3.4-alpha
This patch changes the algorithm of compute_real_max_mem_in_queues() to
use 0.4 * RAM iff the system has more than or equal to 8 GB of RAM, but
will continue to use the old value of 0.75 * RAM if the system have less
than * GB of RAM available.
This patch also adds tests for compute_real_max_mem_in_queues().
See: https://bugs.torproject.org/24782
This one happens if for some reason you start with DirPort enabled
but server mode turned off entirely.
Fixes a case of bug 23693; bugfix on 0.3.1.1-alpha.
This patch lifts the list of default directory authorities from config.c
into their own auth_dirs.inc file, which is then included in config.c
using the C preprocessor.
Patch by beastr0.
See: https://bugs.torproject.org/24854
Both in geoip_note_client_seen() and options_need_geoip_info(), switch from
accessing the options directly to using the should_record_bridge_info() helper
function.
Fixes#25290
Signed-off-by: David Goulet <dgoulet@torproject.org>
When a relay is collecting internal statistics about how many
create cell requests it has seen of each type, accurately count the
requests from relays that temporarily fall out of the consensus.
(To be extra conservative, we were already ignoring requests from clients
in our counts, and we continue ignoring them here.)
Fixes bug 24910; bugfix on 0.2.4.17-rc.
Directory authorities no longer vote in favor of the Guard flag
for relays that don't advertise directory support.
Starting in Tor 0.3.0.1-alpha, Tor clients have been avoiding using
such relays in the Guard position, leading to increasingly broken load
balancing for the 5%-or-so of Guards that don't advertise directory
support.
Fixes bug 22310; bugfix on 0.3.0.6.
Add a missing lock acquisition around access to queued_control_events
in control_free_all(). Use the reassign-and-unlock strategy as in
queued_events_flush_all(). Fixes bug 25675. Coverity found this bug,
but only after we recently added an access to
flush_queued_event_pending.
The anchor here was originally introduced to make sure that the
manpage would format correctly, but we had mistaken it for a section
anchor, and introduced a bogus section heading in 3563a2c819.
Fixes bug 25582; bugfix on 0.3.3.1-alpha.
Coverity found a null pointer reference in nodelist_add_microdesc().
This is almost certainly impossible assuming that the routerstatus_t
returned by router_get_consensus_status_by_descriptor_digest() always
corresponds to an entry in the nodelist. Fixes bug 25629.
Coverity found a null pointer reference in nodelist_add_microdesc().
This is almost certainly impossible assuming that the routerstatus_t
returned by router_get_consensus_status_by_descriptor_digest() always
corresponds to an entry in the nodelist. Fixes bug 25629.
Coverity found a null pointer reference in nodelist_add_microdesc().
This is almost certainly impossible assuming that the routerstatus_t
returned by router_get_consensus_status_by_descriptor_digest() always
corresponds to an entry in the nodelist. Fixes bug 25629.
If we failed to connect at the TCP level to a relay, note it down and refuse
to connect again for another 60 seconds.
Fixes#24767
Signed-off-by: David Goulet <dgoulet@torproject.org>
This update is needed to make it consistent with the behavior of
node_awaiting_ipv6(), which doesn't believe in the addresses from
routerinfos unless it actually plans to use those routerinfos.
Fixes bug 25213; bugfix on b66b62fb75 in 0.3.3.1-alpha,
which tightened up the definition of node_awaiting_ipv6().
There was a nonfatal assertion in pathbias_should_count that would
trigger if onehop_tunnel was set, but the desired_path_length was
greater than 1. This patch fixes that. Fixes bug 24903; bugfix on
0.2.5.2-alpha.
Make sure we actually only report client channel to the geoip cache instead of
looking if it is a known relay. Looking if it is a known relay can be
unreliable because they come and go from the consensus.
Fixes#24904
Signed-off-by: David Goulet <dgoulet@torproject.org>
Because of #25306 for which we are unable to reproduce nor understand how it
is possible, this commit removes the asserts() and BUG() on the missing
descriptors instead when rotating them.
This allows us to log more data on error but also to let tor recover
gracefully instead of dying.
Signed-off-by: David Goulet <dgoulet@torproject.org>
This patch adds some additional logging to circuits_handle_oom() to give
us more information about which specific compression backend that is
using a certain amount of memory.
See: https://bugs.torproject.org/25372
This should avoid most intermittent test failures on developer and CI machines,
but there could (and probably should) be a more elegant solution.
Also, this test was testing that the IP was created and its expiration time was
set to a time greater than or equal to `now+INTRO_POINT_LIFETIME_MIN_SECONDS+5`:
/* Time to expire MUST also be in that range. We add 5 seconds because
* there could be a gap between setting now and the time taken in
* service_intro_point_new. On ARM, it can be surprisingly slow... */
tt_u64_op(ip->time_to_expire, OP_GE,
now + INTRO_POINT_LIFETIME_MIN_SECONDS + 5);
However, this appears to be a typo, since, according to the comment above it,
adding five seconds was done because the IP creation can be slow on some
systems. But the five seconds is added to the *minimum* time we're comparing
against, and so it actually functions to make this test *more* likely to fail on
slower systems. (It should either subtract five seconds, or instead add it to
time_to_expire.)
* FIXES#25450: https://bugs.torproject.org/25450
The C code and the rust code had different separate integer overflow
bugs here. That suggests that we're better off just forbidding this
pathological case.
Also, add tests for expected behavior on receiving a bad protocol
list in a consensus.
Fixes another part of 25249.
This one can only be exploited if you can generate a correctly
signed consensus, so it's not as bad as 25074.
Fixes bug 25251; also tracked as TROVE-2018-004.
In some cases we had checked for it, but in others we had not. One
of these cases could have been used to remotely cause
denial-of-service against directory authorities while they attempted
to vote.
Fixes TROVE-2018-001.
since all it does is produce false positives
this commit should get merged into 0.2.9 and 0.3.0 *and* 0.3.1, even
though the code in the previous commit is already present in 0.3.1. sorry
for the mess.
[Cherry-picked]
since all it does is produce false positives
this commit should get merged into 0.2.9 and 0.3.0 *and* 0.3.1, even
though the code in the previous commit is already present in 0.3.1. sorry
for the mess.
This commit takes a piece of commit af8cadf3a9 and a piece of commit
46fe353f25, with the goal of making channel_is_client() be based on what
sort of connection handshake the other side used, rather than seeing
whether the other side ever sent a create_fast cell to us.
