Any lookup now will be certain and not probabilistic as the bloomfilter.

Closes #40269

Signed-off-by: David Goulet <dgoulet@torproject.org>

This is to minimize false positive and thus deny reentry to Exit connections
that were in reality not re-entering. Helps with overall UX.

Signed-off-by: David Goulet <dgoulet@torproject.org>

George Kadianakis authored 4 years ago and David Goulet committed 4 years ago

Signed-off-by: David Goulet <dgoulet@torproject.org>

In order to deny re-entry in the network, we now keep a bloomfilter of relay
ORPort + address and authorities ORPort + address and DirPort + address
combinations.

So when an Exit stream is handled, we deny anything connecting back into the
network on the ORPorts for relays and on the ORPort+DirPort for the
authorities.

Related to #2667

Signed-off-by: David Goulet <dgoulet@torproject.org>

This one should work on GCC _and_ on Clang.  The previous version
made Clang happier by not having unreachable "fallthrough"
statements, but made GCC sad because GCC didn't think that the
unconditional failures were really unconditional, and therefore
_wanted_ a FALLTHROUGH.

This patch adds a FALLTHROUGH_UNLESS_ALL_BUGS_ARE_FATAL macro that
seems to please both GCC and Clang in this case: ordinarily it is a
FALLTHROUGH, but when ALL_BUGS_ARE_FATAL is defined, it's an
abort().

Fixes bug 40241 again.  Bugfix on earlier fix for 40241, which was
merged into maint-0.3.5 and forward, and released in 0.4.5.3-rc.

Some days before this commit, the network experienced a DDoS on the directory
authorities that prevented them to generate a consensus for more than 5 hours
straight.

That in turn entirely disabled onion service v3, client and service side, due
to the subsystem requiring a live consensus to function properly.

We know require a reasonably live consensus which means that the HSv3
subsystem will to its job for using the best consensus tor can find. If the
entire network is using an old consensus, than this should be alright.

If the service happens to use a live consensus while a client is not, it
should still work because the client will use the current SRV it sees which
might be the previous SRV for the service for which it still publish
descriptors for.

If the service is using an old one and somehow can't get a new one while
clients are on a new one, then reachability issues might arise. However, this
is a situation we already have at the moment since the service will simply not
work if it doesn't have a live consensus while a client has one.

Fixes #40237

Signed-off-by: David Goulet <dgoulet@torproject.org>

We're getting "fallback annotation annotation in unreachable code"
warnings when we build with ALL_BUGS_ARE_FATAL. This patch fixes
that.

Fixes bug 40241.  Bugfix on 0.3.5.4-alpha.

David Goulet authored 4 years ago and Nick Mathewson committed 4 years ago

If at least one service is configured as a version 2, a log warning is emitted
once and only once.

Closes #40003

Signed-off-by: David Goulet <dgoulet@torproject.org>

Fixes #40135

Signed-off-by: David Goulet <dgoulet@torproject.org>

tl;dr We were not counting cells flying from the client to the service, but we
were counting cells flying from the service to the client.

When a rendezvous cell arrives from the client to the RP, the RP forwards it to
the service.

For this to happen, the cell first passes through command_process_relay_cell()
which normally does the statistics counting. However because the `rend_circ`
circuit was not flagged with `circuit_carries_hs_traffic_stats` in
rend_mid_rendezvous(), the cell is not counted there.

Then the cell goes to circuit_receive_relay_cell() which has a special code
block based on `rend_splice` specifically for rendezvous cells, and the cell
gets directly passed to `rend_circ` via a direct call to
circuit_receive_relay_cell(). The cell never passes through
command_process_relay_cell() ever again and hence is never counted by our
rephist module.

The fix here is to flag the `rend_circ` circuit with
`circuit_carries_hs_traffic_stats` so that the cell is counted as soon as it
hits command_process_relay_cell().

Furthermore we avoid double-counting cells since the special code block of
circuit_receive_relay_cell() makes us count rendezvous cells only as they enter
the RP and not as they exit it.

Fixes #40117.

Without this fix, if an PT forgets to send a USERADDR command, that
results in a connection getting treated as local for the purposes of
rate-limiting.

If the PT _does_ use USERADDR, we still believe it.

Closes ticket 33747.

This is not actually a bug!  It can happen for a bunch of reasons,
which all boil down to "trying to add an extrainfo for which we no
longer have the corresponding routerinfo".

Fixes #16016; bugfix on 0.2.6.3-alpha.

Resume being willing to use preemptively-built circuits when
UseEntryGuards is set to 0. We accidentally disabled this feature with
that config setting (in our fix for #24469), leading to slower load times.

Fixes bug 34303; bugfix on 0.3.3.2-alpha.

GCC added an implicit-fallthrough warning a while back, where it
would complain if you had a nontrivial "case:" block that didn't end
with break, return, or something like that.  Clang recently added
the same thing.

GCC, however, would let you annotate a fall-through as intended by
any of various magic "/* fall through */" comments.  Clang, however,
only seems to like "__attribute__((fallthrough))".  Fortunately, GCC
accepts that too.

A previous commit in this branch defined a FALLTHROUGH macro to do
the right thing if GNUC is defined; here we replace all of our "fall
through" comments with uses of that macro.

This is an automated commit, made with the following perl one-liner:

  #!/usr/bin/perl -i -p
  s#/\* *falls? ?thr.*?\*/#FALLTHROUGH;#i;

Return early when there is no suitable IPv6 ORPort.
Show the address and port on error, using a convenience function.

Code simplification and refactoring.

Cleanup after 32588.

When IPv6 ORPorts are set to "auto", tor relays and bridges would
advertise an incorrect port in their descriptor.

This may be a low-severity memory safety issue, because the published
port number may be derived from uninitialised or out-of-bounds memory
reads.

Fixes bug 32588; bugfix on 0.2.3.9-alpha.

George Kadianakis authored 5 years ago and Nick Mathewson committed 5 years ago

Given that ed25519 public key validity checks are usually not needed
and (so far) they are only necessary for onion addesses in the Tor
protocol, we decided to fix this specific bug instance without
modifying the rest of the codebase (see below for other fix
approaches).

In our minimal fix we check that the pubkey in
hs_service_add_ephemeral() is valid and error out otherwise.

David Goulet authored 5 years ago and teor committed 5 years ago

Fixes #28992

Signed-off-by: David Goulet <dgoulet@torproject.org>

Alexander Hansen Færøy authored 5 years ago and teor committed 5 years ago

This patch ensures that we always lowercase the BridgeDistribution from
torrc in descriptors before submitting it.

See: https://bugs.torproject.org/32753

Private-key validation is fairly expensive for long keys in openssl,
so we need to avoid it sooner.

We hit this assertion with bug 32868, but I'm stymied figuring out
how we wound up with a routerstatus like this.  This patch is a
diagnostic to attempt to figure out what is going on, and to avoid a
crash in the meantime.

hs_client_purge_state() and hs_cache_clean_as_client() can remove a descriptor
from the client cache with a NEWNYM or simply when the descriptor expires.

Which means that for an INTRO circuit being established during that time, once
it opens, we lookup the descriptor to get the IP object but hey surprised, no
more descriptor.

The approach here is minimalist that is accept the race and close the circuit
since we can not continue. Before that, the circuit would stay opened and the
client wait the SockTimeout.

Fixers #28970.

Signed-off-by: David Goulet <dgoulet@torproject.org>

David Goulet authored 5 years ago and Nick Mathewson committed 5 years ago

When encoding introduction points, we were not checking if that intro points
had an established circuit.

When botting up, the service will pick, by default, 3 + 2 intro points and the
first 3 that establish, we use them and upload the descriptor.

However, the intro point is removed from the service descriptor list only when
the circuit has opened and we see that we have already enough intro points, it
is then removed.

But it is possible that the service establishes 3 intro points successfully
before the other(s) have even opened yet.

This lead to the service encoding extra intro points in the descriptor even
though the circuit is not opened or might never establish (#31561).

Fixes #31548

Signed-off-by: David Goulet <dgoulet@torproject.org>

Our dimap code asserts if you try to add the same key twice; this
can't happen if everything is running smoothly, but it's possible if
you try to start a relay where secret_onion_key_ntor is the same as
secret_onion_key_ntor.old.

Fixes bug 30916; bugfix on 0.2.4.8-alpha when ntor keys were
introduced.

No behavioral change here: this is just refactoring.

When tor is missing descriptors for some primary entry guards, make the
log message less alarming. It's normal for descriptors to expire, as long
as tor fetches new ones soon after.

Fixes bug 31657; bugfix on 0.3.3.1-alpha.

Previously, v3 single onion services failed when all intro nodes were
unreachable via a 1-hop path. Now, we select intros that are only available
via a 3-hop path, and use a 3-hop path to connect to them.

Fixes bug 23507; bugfix on 0.3.2.1-alpha.

Previously, we always used a 1-hop path, no matter how many times a v3
single onion intro failed.

Fixes bug 23818; bugfix on 0.3.2.1-alpha.

Previously, we used a 1-hop path when a single onion rend failed
immediately, and a 3-hop path when it failed after trying to build
a circuit.

Fixes bug 23818; bugfix on 0.3.2.1-alpha.

There is other code that uses this value, and some of it is
apparently reachable from inside router_dir_info_changed(), which
routerlist_free() apparently calls.  (ouch!)  This is a minimal fix
to try to resolve the issue without causing other problems.

Fixes bug 31003. I'm calling this a bugfix on 0.1.2.2-alpha, where
the call to router_dir_info_changed() was added to routerlist_free().

When we consider all circuits in "waiting for guard" state to be promoted to
an "open" state, we were considering all circuits, even the one marked for
close.

This ultiamtely triggers a "circuit_has_opened()" called on the circuit that
is marked for close which then leads to possible undesirable behaviors within
a subsystem.

For instance, the HS subsystem would be unable to find the authentication key
of the introduction point circuit leading to a BUG() warning and a duplicate
mark for close on the circuit.

This commit also adds a unit test to make sure we never select marked for
close circuits when upgrading its guard state from waiting for guard to open.

Fixes #30871

Signed-off-by: David Goulet <dgoulet@torproject.org>

When this function was implemented, it counted all the entry guards
in the bridge set.  But this included previously configured bridges,
as well as currently configured ones!  Instead, only count the
_filtered_ bridges (ones that are configured and possibly reachable)
as maybe usable.

Fixes bug 29875; bugfix on 0.3.0.1-alpha.

Previously we purged it in 1-hour increments -- but one-hour is the
maximum TTL for the cache!  Now we do it in 25%-TTL increments.

Fixes bug 29617; bugfix on 0.3.5.1-alpha.

Closes ticket 30452.