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>
Our main function, though accurate on all platforms, can be very
slow on 32-bit hosts. This one is faster on all 32-bit hosts, and
accurate everywhere except apple, where it will typically be off by
1%. But since 32-bit apple is a relic anyway, I think we should be
fine.
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.
Consensus method 25 is the oldest one supported by any stable
version of 0.2.9, which is our current most-recent LTS. Thus, by
proposal 290, they should be removed.
This commit does not actually remove the code to implement these
methods: it only makes it so authorities will no longer support
them. I'll remove the backend code for them in later commits.
It tried to pick nodes for which only routerinfo_t items are set,
but without setting UseMicroDescriptors to 0. This won't work any
more, now that we're strict about using the right descriptor types
due to 25691/25692/25213.
This is done as follows:
* Only one function (find_dl_schedule()) actually returned a
smartlist. Now it returns an int.
* The CSV_INTERVAL type has been altered to ignore everything
after the first comma, and to store the value before the first
comma in an int.
This commit won't compile. It was made with the following perl
scripts:
s/smartlist_t \*(.*)DownloadSchedule;/int $1DownloadInitialDelay;/;
s/\b(\w*)DownloadSchedule\b/$1DownloadInitialDelay/;
sizeof(ret) is the size of the pointer, not the size of what it
points to. Fortunately, we already have a function to compare
tor_addr_port_t values for equality.
Bugfix on c2c5b13e5d8a77e; bug not in any released Tor. Found by
clang's scan-build.
We recently merged a circuit cell queue size safeguard. This commit adds the
number of killed circuits that have reached the limit to the DoS heartbeat. It
now looks like this:
[notice] DoS mitigation since startup: 0 circuits killed with too many
cells. 0 circuits rejected, 0 marked addresses. 0 connections closed. 0
single hop clients refused.
Second thing that this patch does. It makes tor always print the DoS
mitigation heartbeat line (for a relay) even though no DoS mitigation have
been enabled. The reason is because we now kill circuits that have too many
cells regardless on if it is enabled or not but also it will give the operator
a chance to learn what is enabled with the heartbeat instead of suddenly
appearing when it is enabled by let say the consensus.
Fixes#25824
Signed-off-by: David Goulet <dgoulet@torproject.org>
Unfortunately, the units passed to
monotime_coarse_stamp_units_to_approx_msec() was always 0 due to a type
conversion.
Signed-off-by: David Goulet <dgoulet@torproject.org>
Really, the uint32_t is only an optimization; any kind of unit
should work fine. Some users might want to use time_t or
monotime_coarse_t or something like that.
Begin by creating a lowest-level triple of the types needed to
implement a token bucket: a configuration, a timestamp, and the raw
bucket itself.
Note that for low-level buckets, the units of the timestamp and the
bucket itself are unspecified: each user can use a different type.
(This patch breaks check-spaces; a later patch will fix it)
This is a simple search-and-replace to rename the token bucket type
to indicate that it contains both a read and a write bucket, bundled
with their configuration. It's preliminary to refactoring the
bucket type.
This test works by having two post-loop events activate one another
in a tight loop. If the "post-loop" mechanism didn't work, this
would be enough to starve all other events.
This differs from our previous token bucket abstraction in a few
ways:
1) It is an abstraction, and not a collection of fields.
2) It is meant to be used with monotonic timestamps, which should
produce better results than calling gettimeofday over and over.
When size_t is 32 bits, the unit tests can't fit anything more than
4GB-1 into a size_t.
Additionally, tt_int_op() uses "long" -- we need tt_u64_op() to
safely test uint64_t values for equality.
Bug caused by tests for #24782 fix; not in any released Tor.
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 roughly doubles our test coverage of the bridges.c module.
* ADD new testing module, .../src/test/test_bridges.c.
* CHANGE a few function declarations from `static` to `STATIC`.
* CHANGE one function in transports.c, transport_get_by_name(), to be
mockable.
* CLOSES#25425: https://bugs.torproject.org/25425
* ADD new /src/common/crypto_rand.[ch] module.
* ADD new /src/common/crypto_util.[ch] module (contains the memwipe()
function, since all crypto_* modules need this).
* FIXES part of #24658: https://bugs.torproject.org/24658
This module doesn't actually need to mock the libevent mainloop at
all: it can just use the regular mainloop that the test environment
sets up.
Part of ticket 23750.
This change makes cpuworker and test_workqueue no longer need to
include event2/event.h. Now workqueue.c needs to include it, but
that is at least somewhat logical here.
There's now no difference in these tests w.r.t. the C or Rust: both
fail miserably (well, Rust fails with nice descriptive errors, and C
gives you a traceback, because, well, C).
The DoS potential is slightly higher in C now due to some differences to the
Rust code, see the C_RUST_DIFFERS tags in src/rust/protover/tests/protover.rs.
Also, the comment about "failing at the splitting stage" in Rust wasn't true,
since when we split, we ignore empty chunks (e.g. "1--1" parses into
"(1,None),(None,1)" and "None" can't be parsed into an integer).
Finally, the comment about "Rust seems to experience an internal error" is only
true in debug mode, where u32s are bounds-checked at runtime. In release mode,
code expressing the equivalent of this test will error with
`Err(ProtoverError::Unparseable)` because 4294967295 is too large.
Previously, if "Link=1-5" was supported, and you asked protover_all_supported()
(or protover::all_supported() in Rust) if it supported "Link=3-999", the C
version would return "Link=3-999" and the Rust would return "Link=6-999". These
both behave the same now, i.e. both return "Link=6-999".
There's now no difference in these tests w.r.t. the C or Rust: both
fail miserably (well, Rust fails with nice descriptive errors, and C
gives you a traceback, because, well, C).
The DoS potential is slightly higher in C now due to some differences to the
Rust code, see the C_RUST_DIFFERS tags in src/rust/protover/tests/protover.rs.
Also, the comment about "failing at the splitting stage" in Rust wasn't true,
since when we split, we ignore empty chunks (e.g. "1--1" parses into
"(1,None),(None,1)" and "None" can't be parsed into an integer).
Finally, the comment about "Rust seems to experience an internal error" is only
true in debug mode, where u32s are bounds-checked at runtime. In release mode,
code expressing the equivalent of this test will error with
`Err(ProtoverError::Unparseable)` because 4294967295 is too large.
Previously, if "Link=1-5" was supported, and you asked protover_all_supported()
(or protover::all_supported() in Rust) if it supported "Link=3-999", the C
version would return "Link=3-999" and the Rust would return "Link=6-999". These
both behave the same now, i.e. both return "Link=6-999".
These tests handle incoming and outgoing cells on a three-hop
circuit, and make sure that the crypto works end-to-end. They don't
yet test spec conformance, leaky-pipe, or various error cases.
Additionally, this change extracts the functions that created and
freed these elements.
These structures had common "forward&reverse stream&digest"
elements, but they were initialized and freed through cpath objects,
and different parts of the code depended on them. Now all that code
is extacted, and kept in relay_crypto.c
This should help us improve modularity, and should also make it
easier for people to experiment with other relay crypto strategies
down the road.
This commit is pure function movement.
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
These were meant to demonstrate old behavior, or old rust behavior.
One of them _should_ work in Rust, but won't because of
implementation details. We'll fix that up later.
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.
I've refactored these to be a separate function, to avoid tricky
merge conflicts.
Some of these are disabled with "XXXX" comments; they should get
fixed moving forward.
* ADD includes for "torint.h" and "container.h" to crypto_digest.h.
* ADD includes for "crypto_digest.h" to a couple places in which
crypto_digest_t was then missing.
* FIXES part of #24658: https://bugs.torproject.org/24658#comment:30
Folks have found two in the past week or so; we may as well fix the
others.
Found with:
\#!/usr/bin/python3
import re
def findMulti(fname):
includes = set()
with open(fname) as f:
for line in f:
m = re.match(r'^\s*#\s*include\s+["<](\S+)[>"]', line)
if m:
inc = m.group(1)
if inc in includes:
print("{}: {}".format(fname, inc))
includes.add(m.group(1))
import sys
for fname in sys.argv[1:]:
findMulti(fname)
Since 0.2.4, tor uses EWMA circuit policy to prioritize. The previous
algorithm, round-robin, hasn't been used since then but was still used as a
fallback.
Now that EWMA is mandatory, remove that code entirely and enforce a cmux
policy to be set.
This is part of a circuitmux cleanup to improve performance and reduce
complexity in the code. We'll be able to address future optimization with this
work.
Closes#25268
Signed-off-by: David Goulet <dgoulet@torproject.org>
To achieve this, a default value for the CircuitPriorityHalflife option was
needed. We still look in the options and then the consensus but in case no
value can be found, the default CircuitPriorityHalflifeMsec=30000 is used. It
it the value we've been using since 0.2.4.4-alpha.
This means that EWMA, our only policy, can not be disabled anymore fallbacking
to the round robin algorithm. Unneeded code to control that is removed in this
commit.
Part of #25268
Signed-off-by: David Goulet <dgoulet@torproject.org>
On slow system, 1 msec between one read and the other was too tight. For
instance, it failed on armel with a 4msec gap:
https://buildd.debian.org/status/package.php?p=tor&suite=experimental
Increase to 10 msec for now to address slow system. It is important that we
keep this OP_LE test in so we make sure the msec/usec/nsec read aren't
desynchronized by huge gaps. We'll adjust again if we ever encounter a system
that goes slower than 10 msec between calls.
Fixes#25113
Signed-off-by: David Goulet <dgoulet@torproject.org>
Since we're making it so that unstable zstd apis can be disabled,
we need to test them. I do this by adding a variant setup/cleanup
function for the tests, and teaching it about a fake compression
method called "x-zstd:nostatic".
If the cache is using 20% of our maximum allowed memory, clean 10% of it. Same
behavior as the HS descriptor cache.
Closes#25122
Signed-off-by: David Goulet <dgoulet@torproject.org>
The current code flow makes it that we can release a channel in a PENDING
state but not in the pending list. This happens while the channel is being
processed in the scheduler loop.
Fixes#25125
Signed-off-by: David Goulet <dgoulet@torproject.org>
This tests many cases of the KIST scheduler with the pending list state by
calling entry point in the scheduler while channels are scheduled or not.
Also, it adds a test for the bug #24700.
Signed-off-by: David Goulet <dgoulet@torproject.org>
In 0.3.2.1-alpha, we've added notify_networkstatus_changed() in order to have
a way to notify other subsystems that the consensus just changed. The old and
new consensus are passed to it.
Before this patch, this was done _before_ the new consensus was set globally
(thus NOT accessible by getting the latest consensus). The scheduler
notification was assuming that it was set and select_scheduler() is looking at
the latest consensus to get the parameters it might needs. This was very wrong
because at that point it is still the old consensus set globally.
This commit changes the notify_networkstatus_changed() to be the "before"
function and adds an "after" notification from which the scheduler subsystem
is notified.
Fixes#24975
When we stopped looking at the "protocols" variable directly, we
broke the hs_service/build_update_descriptors test, since it didn't
actually update any of the flags.
The fix here is to call summarize_protover_flags() from that test,
and to expose summarize_protover_flags() as "STATIC" from
routerparse.c.
Since helper_create_introduce1_cell() checks "cell" for nullness,
scan-build is concerned that test_introduce1_validation()
dereferences it without checking it. So, add a check.
Not backporting, since this is spurious, _and_ tests-only.
These are all about local variables shadowing global
functions. That isn't normally a problem, but at least one
compiler we care about seems to treat this as a case of -Wshadow
violation, so let's fix it.
Fixes bug 24634; bugfix on 0.3.2.1-alpha.
Using tt_assert in these helpers was implying to scan-build that our
'new' functions might be returning NULL, which in turn would make it
warn about null-pointer use.
We've been seeing problems with destroy cells queues taking up a
huge amount of RAM. We can mitigate this, since while a full packed
destroy cell takes 514 bytes, we only need 5 bytes to remember a
circuit ID and a reason.
Fixes bug 24666. Bugfix on 0.2.5.1-alpha, when destroy cell queues
were introduced.
Using absolute_msec requires a 64-bit division operation every time
we calculate it, which gets expensive on 32-bit architectures.
Instead, just use the lazy "monotime_coarse_get()" operation, and
don't convert to milliseconds until we absolutely must.
In this case, it seemed fine to use a full monotime_coarse_t rather
than a truncated "stamp" as we did to solve this problem for the
timerstamps in buf_t and packed_cell_t: There are vastly more cells
and buffer chunks than there are channels, and using 16 bytes per
channel in the worst case is not a big deal.
There are still more millisecond operations here than strictly
necessary; let's see any divisions show up in profiles.
Retry directory downloads when we get our first bridge descriptor
during bootstrap or while reconnecting to the network. Keep retrying
every time we get a bridge descriptor, until we have a reachable bridge.
Stop delaying bridge descriptor fetches when we have cached bridge
descriptors. Instead, only delay bridge descriptor fetches when we
have at least one reachable bridge.
Fixes bug 24367; bugfix on 0.2.0.3-alpha.
networkstatus_consensus_has_ipv6() tells us whether the consensus method of
our current consensus supports IPv6 ORPorts in the consensus.
Part of #23827.
This commit was made mechanically by this perl script:
\#!/usr/bin/perl -w -i -p
next if /^#define FREE_AND_NULL/;
s/\bFREE_AND_NULL\((\w+),/FREE_AND_NULL\(${1}_t, ${1}_free_,/;
s/\bFREE_AND_NULL_UNMATCHED\(/FREE_AND_NULL\(/;
Couple things happen in this commit. First, we do not re-queue a cell back in
the circuit queue if the write packed cell failed. Currently, it is close to
impossible to have it failed but just in case, the channel is mark as closed
and we move on.
The second thing is that the channel_write_packed_cell() always took ownership
of the cell whatever the outcome. This means, on success or failure, it needs
to free it.
It turns out that that we were using the wrong free function in one case and
not freeing it in an other possible code path. So, this commit makes sure we
only free it in one place that is at the very end of
channel_write_packed_cell() which is the top layer of the channel abstraction.
This makes also channel_tls_write_packed_cell_method() return a negative value
on error.
Two unit tests had to be fixed (quite trivial) due to a double free of the
packed cell in the test since now we do free it in all cases correctly.
Part of #23709
Signed-off-by: David Goulet <dgoulet@torproject.org>
This makes sure that a non opened channel is never put back in the channel
pending list and that its state is consistent with what we expect that is
IDLE.
Test the fixes in #24502.
Signed-off-by: David Goulet <dgoulet@torproject.org>
