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.
The clean_consdiffmgr() callback is only for relays acting as a directory
server, not all relays.
This commit adds a role for only directory server and sets the
clean_consdiffmgr() callback to use it.
Signed-off-by: David Goulet <dgoulet@torproject.org>
We can't end up in the removed else {} condition since we first validate the
flavor we get and then we validate the flavor we parse from the given
consensus which means we can only handle the two flavors of the if/elseif
conditions.
Fixes#25914
Signed-off-by: David Goulet <dgoulet@torproject.org>
Previously, we were ignoring values _over_ EPSILON. This bug was
also causing a warning at startup because the default value is set
to -1.0.
Fixes bug 25577; bugfix on 6b1dba214d. Bug not in any released tor.
Two helper functions to enable an event and disable an event which wraps the
launch and destroy of an event but takes care of the enabled flag.
They are also idempotent that is can be called multiple time on the same event
without effect if the event was already enabled or disabled.
Signed-off-by: David Goulet <dgoulet@torproject.org>
In case we transitionned to a new role in Tor, we need to launch and/or
destroy some periodic events.
Signed-off-by: David Goulet <dgoulet@torproject.org>
In tor, we have a series of possible "roles" that the tor daemon can be
enabled for. They are:
Client, Bridge, Relay, Authority (directory or bridge) and Onion service.
They can be combined sometimes. For instance, a Directory Authority is also a
Relay. This adds a "roles" field to a periodic event item object which is used
to know for which roles the event is for.
The next step is to enable the event only if the roles apply. No behavior
change at this commit.
Pars of #25762
Signed-off-by: David Goulet <dgoulet@torproject.org>
No behavior change, just to make it easier to find callbacks and for the sake
of our human brain to parse the list properly.
Signed-off-by: David Goulet <dgoulet@torproject.org>
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.
In order to fix 25691 and 25692, we need to pass the "direct_conn"
flag to more places -- particularly when choosing single-hop
tunnels. The right way to do this involves having a couple more
functions accept router_crn_flags_t, rather than a big list of
boolean arguments.
This commit also makes sure that choose_good_exit_server_general()
honors the direct_conn flag, to fix 25691 and 25692.
In router_add_running_nodes_to_smartlist(), we had an inline
implementation of the logic from node_has_descriptor(), which should
be changed to node_has_preferred_descriptor().
This patch adds a new node_has_preferred_descriptor() function, and
replaces most users of node_has_descriptor() with it. That's an
important change, since as of d1874b4339 (our fix for #25213),
we are willing to say that a node has _some_ descriptor, but not the
_right_ descriptor for a particular use case.
Part of a fix for 25691 and 25692.
Now that we allow cpuworkers for dirport-only hosts (to fix 23693),
we need to allow dup_onion_keys() to succeed for them.
The change to construct_ntor_key_map() is for correctness,
but is not strictly necessary.
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/;
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.
Previously this was done as part of the refill callback, but there's
no real reason to do it like that. Since we're trying to remove the
refill callback completely, we can do this work as part of
record_num_bytes_transferred_impl(), which already does quite a lot
of this.
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.
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>
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>
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.
A linked connection_t is one that gets its I/O, not from the
network, but from another connection_t. When such a connection has
something to write, we want the corresponding connection to run its
read callback ... but not immediately, to avoid infinite recursion
and/or event loop starvation.
Previously we handled this case by activating the read events
outside the event loop. Now we use the "postloop event" logic.
This lets us simplify do_main_loop_once() a little.
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 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
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
* 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 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.
We are going to stop recommending 0.2.5 so there is no reason to keep the
undef statement anymore.
Fixes#20522.
Signed-off-by: Fernando Fernandez Mancera <ffmancera@riseup.net>
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>
Next commit is addressing the circuit queue cell limit so cleanup before doing
anything else.
Part of #25226
Signed-off-by: David Goulet <dgoulet@torproject.org>
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".
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".
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.
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
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 function is used upon receiving a cell, and only handles the
decrypting part. The encryption part is currently handled inside
circuit_package_relay_cell.
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.
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 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.
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>
The reason to do so is because these functions haven't been used in years so
since 0.2.4, every callsite is NOP.
In future commits, we'll remove the round robin circuit policy which is mostly
validated within those function.
This simplifies the code greatly and remove dead code for which we never had a
configure option in the first place nor an easy way to use them in production.
Part of #25268
Signed-off-by: David Goulet <dgoulet@torproject.org>
It is rename to something more meaningful that explains what it does exactly
which is sets the EWMA options (currently only one exists). The new name is
cmux_ewma_set_options().
Also, remove a public function from circuitmux_ewma.h that is only used in the
C file. Make it static inline as well.
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>
We had this safeguard around dos_init() but not when the consensus changes
which can modify consensus parameters and possibly enable the DoS mitigation
even if tor wasn't a public relay.
Fixes#25223
Signed-off-by: David Goulet <dgoulet@torproject.org>
Explicitly inform the operator of the rejected relay to set a valid email
address in the ContactInfo field and contact bad-relays@ mailing list.
Fixes#25170
Signed-off-by: David Goulet <dgoulet@torproject.org>
This is to avoid positively identifying Exit relays if tor client connection
comes from them that is reentering the network.
One thing to note is that this is done only in the DoS subsystem but we'll
still add it to the geoip cache as a "client" seen. This is done that way so
to avoid as much as possible changing the current behavior of the geoip client
cache since this is being backported.
Closes#25193
Signed-off-by: David Goulet <dgoulet@torproject.org>
