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.
This change also makes tor_ersatz_socketpair() follow the same
interface as socketpair() rather than tor_socketpair(), so it now
needs to be wrapped in the same code as socketpair() does.
This is comparatively straightforward too, except for a couple of
twists:
* For as long as we're building with two crypto libraries, we
want to seed _both_ their RNGs, and use _both_ their RNGs to
improve the output of crypto_strongest_rand()
* The NSS prng will sometimes refuse to generate huge outputs.
When it does, we stretch the output with SHAKE. We only need
this for the tests.
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.
The following bug was causing many issues for this branch in chutney:
In sr_state_get_start_time_of_current_protocol_run() we were using the
consensus valid-after to calculate beginning_of_current_round, but we were
using time(NULL) to calculate the current_round slot. This was causing time
sync issues when the consensus valid-after and time(NULL) were disagreeing on
what the current round is. Our fix is to use the consensus valid-after in both
places.
This also means that we are not using 'now' (aka time(NULL)) anymore in that
function, and hence we can remove that argument from the function (and its
callers). I'll do this in the next commit so that we keep things separated.
Furthermore, we fix a unittest that broke.
Now that the rev counter depends on the local time, we need to be more careful
in the unittests. Some unittests were breaking because they were using
consensus values from 1985, but they were not updating the local time
appropriately. That was causing the OPE module to complain that it was trying
to encrypt insanely large values.
This is meant for use when encrypting the current time within the
period in order to get a monotonically increasing revision counter
without actually revealing our view of the time.
This scheme is far from the most state-of-the-art: don't use it for
anything else without careful analysis by somebody much smarter than
I am.
See ticket #25552 for some rationale for this logic.
also add tests for bw_file_headers.
Headers are all that is found before a correct relay line or
the terminator.
Tests include:
* a empty bandwidth file
* a bandwidth file with only timestamp
* a bandwidth file with v1.0.0 headers
* a bandwidth file with v1.0.0 headers and relay lines
* a bandwidth file with v1.1.0 headers and v1.0.0 relay lines
* a bandwidth file with v1.0.0 headers, malformed relay lines and
relay lines
* a bandwidth file with v1.0.0 headers, malformed relay lines,
relay lines and malformed relay lines
* a bandwidth file with v1.1.0 headers without terminator
* a bandwidth file with v1.1.0 headers with terminator
* a bandwidth file with v1.1.0 headers without terminator and
relay lines
* a bandwidth file with v1.1.0 headers with terminator and relay
lines
* a bandwidth file with v1.1.0 headers without terminator, bad
relay lines and relay lines
* a bandwidth file with v1.1.0 headers with terminator, bad relay
lines and relay lines
* add bwlist_headers argument to dirserv_read_measured_bandwidth
in order to store all the headers found when parsing the file
* add bwlist_headers to networkstatus_t in order to store the
the headers found by the previous function
* include the bandwidth headers as string in vote documents
* add test to check that dirserv_read_measured_bandwidth generates
the bwlist_headers
We need this in our unit tests, since otherwise NSS will notice
we've forked and start cussing us out.
I suspect we'll need a different hack for daemonizing, but this
should be enough for tinytest to work.
These are now part of crypto_init.c. The openssl-only parts now
live in crypto_openssl_mgt.c.
I recommend reviewing this patch with -b and --color-moved.
Fun fact: these files used to be called log.[ch] until we ran into
conflicts with systems having a log.h file. But now that we always
include "lib/log/log.h", we should be fine.
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.
This is a very gentle commit that just lays the groundwork in the
build system: it puts the include files to build libtor-app.a into
src/core, and to build the tor executable into src/app. The
executable is now "src/app/tor".
This is temporary, until src/or is split.
Putting this in containers would be another logical alternative,
except that addresses depend on containers, and we don't like
cycles.
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.
This code was in compat_threads, since it was _used_ for efficiently
notifying the main libevent thread from another thread. But in
spite of its usage, it's fundamentally a part of the network code.
The "conffile" module knows about includes and filesystem access,
whereas confline doesn't. This will make it possible to put these
functions into libraries without introducing a cycle.
Fixes bug 26480; bug appeared when we re-enabled the geoip tests on
windows. Bug originally introduced by our fix to 25787; bug not in
any released Tor.
This patch fixes a memory leak in new_establish_intro_cell() that could
happen if a test assertion fails and the *cell_out value isn't properly
free'd.
See: Coverity CID 1437445
This patch fixes a memory leak in hs_helper_build_hs_desc_impl() where
if a test assertion would fail we would leak the storage that `desc`
points to.
See: Coverity CID 1437448
This patch fixes a potential memory leak in test_hs_auth_cookies() if a
test-case fails and we goto the done label where no memory clean up is
done.
See: Coverity CID 1437453
This patch fixes a potential memory leak in
hs_helper_build_intro_point() where a `goto done` is called before the
`intro_point` variable have been assigned to the value of the `ip`
variable.
See: Coverity CID 1437460
See: Coverity CID 1437456
This patch:
- introduces an fdio module for low-level fd functions that don't
need to log.
- moves the responsibility for opening files outside of torlog.c,
so it won't need to call tor_open_cloexec.
Out-of-tree builds could fail to run the rust tests if built in
offline mode. cargo expects CARGO_HOME to point to the .cargo
directory, not the directory containing .cargo.
Fixes bug 26455; bug not in any released tor.
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
We'd like to feature gate code that calls C from Rust, as a workaround
to several linker issues when running `cargo test` (#25386), and we
can't feature gate anything out of test code if `cargo test` is called
with `--all-features`.
* FIXES#26400: https://bugs.torproject.org/26400
Previously we had code like this for bad things happening from
signal handlers, but it makes sense to use the same logic to handle
cases when something is happening at a level too low for log.c to be
involved.
My raw_assert*() stuff now uses this code.
We had accumulated a bunch of cruft here. Now let's only include
src and src/ext. (exception: src/trunnel is autogenerated code, and
need to include src/trunnel.)
This commit will break the build hard. The next commit will fix it.
We need this trick because some of our Rust tests depend on our C
code, which in turn depend on other native libraries, which thereby
pulls a whole mess of our build system into "cargo test".
To solve this, we add a build script (build.rs) to set most of the
options that we want based on the contents of config.rust. Some
options can't be set, and need to go to the linker directly: we use
a linker replacement (link_rust.sh) for these. Both config.rust and
link_rust.sh are generated by autoconf for us.
This patch on its own should enough to make the crypto test build,
but not necessarily enough to make it pass.
After the big or.h refactoring, one single unit test file was missing two
headers for node_t and microdesc_t.
Signed-off-by: David Goulet <dgoulet@torproject.org>
Also make sure that we're actually running the test from within the right
cwd, like we do when we're building. This seems necessary to avoid
an error when running offline.
Amusingly, it appears that we had this bug before: we just weren't
noticing it, because of bug 26258.
Some versions of GCC complain that the bfn_mock_node_get_by_id
function might return NULL, but we're assuming that it won't.
(We're assuming it won't return NULL because we know in the tests
that we're passing it valid IDs.)
To make GCC happy, tt_assert() that each node_t is set before using
it.
Fixes a second case of bug26269; bugfix on 0.3.0.1-alpha.
In protover.c, the `expand_protocol_list()` function expands a `smartlist_t` of
`proto_entry_t`s to their protocol name concatenated with each version number.
For example, given a `proto_entry_t` like so:
proto_entry_t *proto = tor_malloc(sizeof(proto_entry_t));
proto_range_t *range = tor_malloc_zero(sizeof(proto_range_t));
proto->name = tor_strdup("DoSaaaaaaaaaaaaaaaaaaaaaa[19KB]aaa");
proto->ranges = smartlist_new();
range->low = 1;
range->high = 65536;
smartlist_add(proto->ranges, range);
(Where `[19KB]` is roughly 19KB of `"a"` bytes.) This would expand in
`expand_protocol_list()` to a `smartlist_t` containing 65536 copies of the
string, e.g.:
"DoSaaaaaaaaaaaaaaaaaaaaaa[19KB]aaa=1"
"DoSaaaaaaaaaaaaaaaaaaaaaa[19KB]aaa=2"
[…]
"DoSaaaaaaaaaaaaaaaaaaaaaa[19KB]aaa=65535"
Thus constituting a potential resource exhaustion attack.
The Rust implementation is not subject to this attack, because it instead
expands the above string into a `HashMap<String, HashSet<u32>` prior to #24031,
and a `HashMap<UnvalidatedProtocol, ProtoSet>` after). Neither Rust version is
subject to this attack, because it only stores the `String` once per protocol.
(Although a related, but apparently of too minor impact to be usable, DoS bug
has been fixed in #24031. [0])
[0]: https://bugs.torproject.org/24031
* ADDS hard limit on protocol name lengths in protover.c and checks in
parse_single_entry() and expand_protocol_list().
* ADDS tests to ensure the bug is caught.
* FIXES#25517: https://bugs.torproject.org/25517
In protover.c, the `expand_protocol_list()` function expands a `smartlist_t` of
`proto_entry_t`s to their protocol name concatenated with each version number.
For example, given a `proto_entry_t` like so:
proto_entry_t *proto = tor_malloc(sizeof(proto_entry_t));
proto_range_t *range = tor_malloc_zero(sizeof(proto_range_t));
proto->name = tor_strdup("DoSaaaaaaaaaaaaaaaaaaaaaa[19KB]aaa");
proto->ranges = smartlist_new();
range->low = 1;
range->high = 65536;
smartlist_add(proto->ranges, range);
(Where `[19KB]` is roughly 19KB of `"a"` bytes.) This would expand in
`expand_protocol_list()` to a `smartlist_t` containing 65536 copies of the
string, e.g.:
"DoSaaaaaaaaaaaaaaaaaaaaaa[19KB]aaa=1"
"DoSaaaaaaaaaaaaaaaaaaaaaa[19KB]aaa=2"
[…]
"DoSaaaaaaaaaaaaaaaaaaaaaa[19KB]aaa=65535"
Thus constituting a potential resource exhaustion attack.
The Rust implementation is not subject to this attack, because it instead
expands the above string into a `HashMap<String, HashSet<u32>` prior to #24031,
and a `HashMap<UnvalidatedProtocol, ProtoSet>` after). Neither Rust version is
subject to this attack, because it only stores the `String` once per protocol.
(Although a related, but apparently of too minor impact to be usable, DoS bug
has been fixed in #24031. [0])
[0]: https://bugs.torproject.org/24031
* ADDS hard limit on protocol name lengths in protover.c and checks in
parse_single_entry() and expand_protocol_list().
* ADDS tests to ensure the bug is caught.
* FIXES#25517: https://bugs.torproject.org/25517
We alloc/free X.509 structures in three ways:
1) X509 structure allocated with X509_new() and X509_free()
2) Fake X509 structure allocated with fake_x509_malloc() and fake_x509_free()
May contain valid pointers inside.
3) Empty X509 structure shell allocated with tor_malloc_zero() and
freed with tor_free()
Since we're going to be disabling the second-elapsed callback, we're
going to sometimes have long periods when no events file, and so the
current second is not updated. Handle that by having a better means
to detect "clock jumps" as opposed to "being idle for a while".
Tolerate far more of the latter.
Part of #26009.
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.
construct_consensus() in test_routerlist.c created votes using a
timestamp from time(). Tests that called construct_consensus() might
have nondeterministic results if they rely on time() not changing too
much on two successive calls.
Neither existing of the two existing tests that calls
construct_consensus is likely to have a failure due to this problem.
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.
This is needed for libressl-2.6.4 compatibility, which we broke when
we merged a15b2c57e1 to fix bug 19981. Fixes bug 26005; bug
not in any released Tor.
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.
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.
By doing so, it is renamed to voting_schedule_recalculate_timing(). This
required a lot of changes to include voting_schedule.h everywhere that this
function was used.
This effectively now makes voting_schedule.{c|h} not include dirauth/dirvote.h
for that symbol and thus no dependency on the dirauth module anymore.
Signed-off-by: David Goulet <dgoulet@torproject.org>
Originally, it was made public outside of the dirauth module but it is no
longer needed. In doing so, we put it back in dirvote.c and reverted its name
to the original one:
dirvote_authority_cert_dup() --> authority_cert_dup()
Signed-off-by: David Goulet <dgoulet@torproject.org>
Adds two unittests:
- First checks the path selection of basic Tor circs.
- Second checks the path selection of vanguard circs.
There is a TODO on the second unittest that we might want to test sooner than
later, but it's not trivial to do it right now.
To do these unittests we needed the following mods:
- Make some functions STATIC.
- Add some more fields to the big fake network nodes of test_entrynodes.c
- Switch fake node nicknames to base32 (because base64 does not produce valid nicknames).
This is a pretty big commit but it only moves these files to src/or/dirauth:
dircollate.c dirvote.c shared_random.c shared_random_state.c
dircollate.h dirvote.h shared_random.h shared_random_state.h
Then many files are modified to change the include line for those header files
that have moved into a new directory.
Without using --disable-module-dirauth, everything builds fine. When using the
flag to disable the module, tor doesn't build due to linking errors. This will
be addressed in the next commit(s).
No code behavior change.
Signed-off-by: David Goulet <dgoulet@torproject.org>
Many functions become static to the C file or exposed to the tests within the
PRIVATE define of dirvote.h.
This commit moves a function to the top. No code behavior change.
Signed-off-by: David Goulet <dgoulet@torproject.org>
Because we rescan the main loop event list if the global map of services has
changed, this makes sure it does work.
Signed-off-by: David Goulet <dgoulet@torproject.org>
Because ADD_ONION/DEL_ONION can modify the global service map (both for v2 and
v3), we need to rescan the event list so we either enable or disable the HS
service main loop event.
Fixees #25939
Signed-off-by: David Goulet <dgoulet@torproject.org>
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>
Needed to run tests from the tarball else the geoip unit test would fail by
not finding that file.
Signed-off-by: David Goulet <dgoulet@torproject.org>
