Add two new files (crypto_digest.c, crypto_digest.h) as new module of
crypto.[ch]. This new module includes all functions and dependencies related
to digest and xof operations. Those have been removed from crypto.[ch].
Follows #24658.
Signed-off-by: Fernando Fernandez Mancera <ffmancera@riseup.net>
Becasue the circuit creation burst and rate can change at runtime it is
possible that between two refill of a bucket, we end up setting the bucket
value to less than there currently is.
Fixes#25128
Signed-off-by: David Goulet <dgoulet@torproject.org>
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>
This patch makes the wording around error cases for selecting an entropy
source in Tor slightly more verbose. We also let the user know when
something goes wrong that we are trying out a fallback method instead.
See: https://bugs.torproject.org/25120
Included crypto_rsa.[ch] into include.am in order to resolve a compiling issue.
Follows #24658.
Signed-off-by: Fernando Fernandez Mancera <ffmancera@riseup.net>
crypto_get_rsa_padding_overhead() and crypto_get_rsa_padding() are
not static inline anymore in order to split the crypto_rsa module
from crypto.[ch].
Also included necessary modules in order to solve dependency issues.
Also made two functions in crypto.c use crypto_pk_asn1_encdoe()
instead of reaching into the crypto_pk_t struct.
This reverts commit 9a06282546.
It appears that I misunderstood how the seccomp2 filter rules
interact. It appears that `SCMP_ACT_ERRNO()` always takes
precedence over `SCMP_ACT_ALLOW()` -- I had thought instead that
earlier rules would override later ones. But this change caused bug
25115 (not in any released Tor).
The accurate address of a connection is real_addr, not the addr member.
channel_tls_get_remote_addr_method() now returns real_addr instead.
Fixes#24952; bugfix on 707c1e2 in 0.2.4.11-alpha.
Signed-off-by: Fernando Fernandez Mancera <ffmancera@riseup.net>
Tor preemptiely builds circuits and they can be cannibalized later in their
lifetime. A Guard node can become unusable (from our guard state) but we can
still have circuits using that node opened. It is important to not pick those
circuits for any usage through the cannibalization process.
Fixes#24469
Signed-off-by: David Goulet <dgoulet@torproject.org>
These are no longer meaningful, since there's no longer an upper
limit to how many times (in the exponential-backoff world) one can
retry a download. download_status_is_ready() didn't check these any
more, and neither do we.
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.
This is the quick fix that is keeping the channel in PENDING state so if we
ever try to reschedule the same channel, it won't happened.
Fixes#24700
Signed-off-by: David Goulet <dgoulet@torproject.org>
It is possible in normal circumstances that a client fetches a descriptor
that has a lower revision counter than the one in its cache. This can happen
due to HSDir desync.
Fixes#24976
Signed-off-by: David Goulet <dgoulet@torproject.org>
Setting the default for this at 10 and the learning timeout to 3 minutes means
we will complete our cbt learning in 30 minutes, which is under the reduced
padding connection timeout window.
In 0.3.2.1-alpha, we've added this function in order to have a way to notify
other subsystems that the consensus just changed. The old consensus and the
new one 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.
With this commit, notify_networkstatus_changed() has been moved _after_ the
new consensus is set globally. The main obvious reasons is to fix the bug
described above and in #24975. The other reason is that this notify function
doesn't return anything which could be allowing the possibility of refusing to
set the new consensus on error. In other words, the new consensus is set right
after the notification whatever happens.
It does no harm or change in behavior to set the new consensus first and then
notify the subsystems. The two functions currently used are for the control
port using the old and new consensus and sending the diff. The second is the
scheduler that needs the new consensus to be set globally before being called.
Of course, the function has been documented accordinly to clearly state it is
done _after_ the new consensus is set.
Fixes#24975
Signed-off-by: David Goulet <dgoulet@torproject.org>
Stop adding unneeded channel padding right after we finish flushing
to a connection that has been trying to flush for many seconds.
Instead, treat all partial or complete flushes as activity on the
channel, which will defer the time until we need to add padding.
This fix should resolve confusing and scary log messages like
"Channel padding timeout scheduled 221453ms in the past."
Fixes bug 22212; bugfix on 0.3.1.1-alpha.
I think technically we could resolve bug 22212 by adding a call to
channel_timestamp_active() only in the finished_flushing case. But I added
a call in the flushed_some case too since that seems to more accurately
reflect the notion of "active".
Using get_uptime() and reset_uptime() instead of
accessing stats_n_seconds_working directly.
stats_n_seconds_working is not extern anymore.
Ticket #25081
Because this touches too many commits at once, it is made into one single
commit.
Remove the use of "tenths" for the circuit rate to simplify things. We can
only refill the buckets at best once every second because of the use of
approx_time() and our token system is set to be 1 token = 1 circuit so make
the rate a flat integer of circuit per second.
Signed-off-by: David Goulet <dgoulet@torproject.org>
Imagine this scenario. We had 10 connections over the 24h lifetime of a geoip
cache entry. The lifetime of the entry has been reached so it is about to get
freed but 2 connections remain for it. After the free, a third connection
comes in thus making us create a new geoip entry for that address matching the
2 previous ones that are still alive. If they end up being closed, we'll have
a concurrent count desynch from what the reality is.
To mitigate this probably very rare scenario in practice, when we free a geoip
entry and it has a concurrent count above 0, we'll go over all connections
matching the address and clear out the tracked flag. So once they are closed,
we don't try to decrement the count.
Signed-off-by: David Goulet <dgoulet@torproject.org>
This option refuses any ESTABLISH_RENDEZVOUS cell arriving from a client
connection. Its default value is "auto" for which we can turn it on or off
with a consensus parameter. Default value is 0.
Signed-off-by: David Goulet <dgoulet@torproject.org>
If the client address was detected as malicious, apply a defense which is at
this commit to return a DESTROY cell.
Signed-off-by: David Goulet <dgoulet@torproject.org>
Add a function that notifies the DoS subsystem that a new CREATE cell has
arrived. The statistics are updated accordingly and the IP address can also be
marked as malicious if it is above threshold.
At this commit, no defense is applied, just detection with a circuit creation
token bucket system.
Signed-off-by: David Goulet <dgoulet@torproject.org>
Implement a basic connection tracking that counts the number of concurrent
connections when they open and close.
This commit also adds the circuit creation mitigation data structure that will
be needed at later commit to keep track of the circuit rate.
Signed-off-by: David Goulet <dgoulet@torproject.org>
This commit introduces the src/or/dos.{c|h} files that contains the code for
the Denial of Service mitigation subsystem. It currently contains basic
functions to initialize and free the subsystem. They are used at this commit.
The torrc options and consensus parameters are defined at this commit and
getters are implemented.
Signed-off-by: David Goulet <dgoulet@torproject.org>
And fix the unsupported protover example so it uses a Link protover much
higher than 5.
Part of #25070, bugfix on 0.3.3.1-alpha, which introduced the protover crate.
I'm leaving the getsockname code in transproxy alone, since it is
comparatively isolated, rather platform-specific, and hard to test.
Implements 18105.
Add two new files (crypto_rsa.c, crypto_rsa.h) as new module of crypto.[ch].
This new module includes all functions and dependencies related to RSA
operations. Those have been removed from crypto.[ch].
All new changes related to RSA operations must be done in these files.
Follows #24658
Signed-off-by: Fernando Fernandez Mancera <ffmancera@riseup.net>
The upcoming DoS mitigation subsytem needs to keep information on a per-IP
basis which is also what the geoip clientmap does.
For another subsystem to access that clientmap, this commit adds a lookup
function that returns the entry. For this, the clientmap_entry_t had to be
moved to the header file.
Signed-off-by: David Goulet <dgoulet@torproject.org>
We'd been using crypto_digest_dup() and crypto_digest_assign() here,
but they aren't necessary. Instead we can just use the stack to
store the previous state of the SHA_CTX and avoid a malloc/free pair.
Closes ticket 24914.
In order to make the OR and dir checking functions in router.c less confusing
we renamed some functions and splitted consider_testing_reachability() into
router_should_check_reachability() and router_do_reachability_checks(). Also we
improved the documentation.
Fixes#18918.
Signed-off-by: Fernando Fernandez Mancera <ffmancera@riseup.net>
Previously, we wouldn't do this when running with a routerinfo_t in
some cases, leading to many needless calls to the protover module.
This change also cleans up the code in nodelist.c a bit.
Fixes bug 25008; bugfix on 0.2.9.4-alpha.
Without this patch, not only will TLS1.3 not work with Tor, but
OpenSSL 1.1.1 with TLS1.3 enabled won't build any connections at
all: It requires that either TLS1.3 be disabled, or some TLS1.3
ciphersuites be listed.
Closes ticket 24978.
As we're trying not to have all the other modules in Tor, we moved the openssl
namespace includes back into crypto.c and crypto_openssl_mgt.c files.
Follows #24658.
Signed-off-by: Fernando Fernandez Mancera <ffmancera@riseup.net>
Included crypto_openssl_mgt.[ch] into the appropiate files in order to resolve
compiling and dependencies issues.
Follows #24658.
Signed-off-by: Fernando Fernandez Mancera <ffernandezmancera@gmail.com>
Renamed free_openssl() to crypto_openssl_free_all(). Also we made variables and
functions static again.
Follows #24658.
Signed-off-by: Fernando Fernandez Mancera <ffernandezmancera@gmail.com>
For 23847, we want Tor to be able to shut down and then restart in
the same process. Here's a patch to make the Tor binary do that.
To test it, you need to build with --enable-restart-debugging, and
then you need to set the environment variable TOR_DEBUG_RESTART.
With this option, Tor will then run for 5 seconds, then restart
itself in-process without exiting. This only happens once.
You can change the 5-second interval using
TOR_DEBUG_RESTART_AFTER_SECONDS.
Implements ticket 24583.
Fix an "off by 2" error in counting rendezvous failures on the onion
service side.
While we thought we would stop the rendezvous attempt after one failed
circuit, we were actually making three circuit attempts before giving up.
Fixes bug 24895; bugfix on 0.0.6.
Rename crypto_openssl.[ch] to crypto_openssl_mgt.[ch] because it is possible we
need crypto_openssl.[ch] in the future.
Follows #24658.
Signed-off-by: Fernando Fernandez Mancera <ffernandezmancera@gmail.com>
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.
Fix a set of false positives where relays would consider connections
to other relays as being client-only connections (and thus e.g.
deserving different link padding schemes) if those relays fell out
of the consensus briefly.
Now we look only at the initial handshake and whether the connection
authenticated as a relay.
Fixes bug 24898; bugfix on 0.3.1.1-alpha.
New-style (v3) onion services now obey the "max rendezvous circuit
attempts" logic.
Previously they would make as many rendezvous circuit attempts as they
could fit in the MAX_REND_TIMEOUT second window before giving up.
Fixes bug 24894; bugfix on 0.3.2.1-alpha.
Define TOR_PRIuSZ as minGW compiler doesn't support zu format specifier for
size_t type.
Fixes#24861 on ac9eebd.
Signed-off-by: Fernando Fernandez Mancera <ffernandezmancera@gmail.com>
... in get_interface_addresses_ioctl().
This pointer alignment issue exists on x86_64 macOS, but is unlikely to exist
elsewhere. (i386 macOS only requires 4-byte alignment, and other OSs have
8-byte ints.)
Fixes bug 24733; not in any released version of tor.
Add free_openssl() function to free the memory allocated for OpenSSL version
management variables. It is required since OpenSSL management has been isolated
from the crypto module.
Follows #24658.
Signed-off-by: Fernando Fernandez Mancera <ffernandezmancera@gmail.com>
Add two new files (crypto_openssl.c, crypto_openssl.h) as new module of
crypto.[ch]. This new module includes all functions and dependencies related
to OpenSSL management. Those have been removed from crypto.[ch].
All new changes related to OpenSSL management must be done in these files.
Follows #24658
Signed-off-by: Fernando Fernandez Mancera <ffernandezmancera@gmail.com>
If we tried to move a descriptor from routerlist->old_routers
back into the current routerlist, we were preparing a buffer with
format_iso_time() on ri->cert_expiration_time, and doing it preemptively
since router_add_to_routerlist() might free ri so we wouldn't be able
to get at it later in the function.
But if the descriptor we're moving doesn't have any ed signature, then
its cert will be marked to expire at TIME_MAX, and handing TIME_MAX
to format_iso_time() generates this log warning:
correct_tm(): Bug: gmtime(9223372036854775807) failed with error Value too large for defined data type: Rounding down to 2037
The fix is to preemptively remember the expiry time, but only prepare
the buffer if we know we're going to need it.
Bugfix on commit a1b0a0b9, which came about as part of a fix for bug
20020, and which is not yet in any released version of Tor (hence no
changes file).
Using this script:
sed -i.bak $'s|^,$|/* ===== */\\\n,|' src/or/fallback_dirs.inc
(Due to embedded newlines, this script only works in bash.)
And manually add a delimiter to the end of the header, and the start of
the fallback list.
This allows us to check that the code compiles, and the unit tests pass.
And it allows downstream users stem and atlas to adapt to the new format.
The upcoming fallback rebuild will automatically generate this new format.
Follow-up to 24725.
Using this script:
sed -i.bak $'s|^,$|/* extrainfo=0 */\\\n,|' src/or/fallback_dirs.inc
(Due to embedded newlines, this script only works in bash.)
This allows us to check that the code compiles, and the unit tests pass.
And it allows downstream users stem and atlas to adapt to the new format.
The upcoming fallback rebuild will automatically generate this new format,
with actual relay extrainfo cache flags.
Follow-up to 22759.
Using this script:
sed -i.bak $'s|^,$|/* nickname= */\\\n,|' src/or/fallback_dirs.inc
(Due to embedded newlines, this script only works in bash.)
This allows us to check that the code compiles, and the unit tests pass.
And it allows downstream users stem and atlas to adapt to the new format.
The upcoming fallback rebuild will automatically generate this new format,
with actual relay nicknames.
Follow-up to 24600.
Using this script:
sed -i.bak 's/" weight=10",/,/' src/or/fallback_dirs.inc
This allows us to check that the code compiles, and the unit tests pass.
And it allows downstream users stem and atlas to adapt to the new format.
The upcoming fallback rebuild will automatically generate this new format.
Follow-up to 24679.
The code had been using c_char and i8 interchangeably, but it turns
out that c_char is only i8 on platforms where "char" is signed. On
other platforms, c_char is u8.
Fixes bug 24794; bug not on any released Tor.
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.
Tor now sets IPv6 preferences on rewrite_node_address_for_bridge() even if
there is only ri or rs. It always warns about them.
Also Tor now sets the IPv6 address in rs as well as it sets the one in ri.
Fixes#24572 on 9e9edf7 in 0.2.4.5-alpha.
Fixes#24573 on c213f27 in 0.2.8.2-alpha.
Signed-off-by: Fernando Fernandez Mancera <ffernandezmancera@gmail.com>
When the fascist_firewall_choose_address_ functions don't find a
reachable address, set the returned address to the null address and port.
This is a precautionary measure, because some callers do not check the
return value.
Fixes bug 24736; bugfix on 0.2.8.2-alpha.
This makes clients on the public tor network prefer to bootstrap off fallback
directory mirrors.
This is a follow-up to 24679, which removed weights from the default fallbacks.
Implements ticket 24681.
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.
With extra_space negative, it means that the "notsent" queue is quite large so
we must consider that value with the current computed tcp_space. If we end up
to have negative space, we should not add more data to the kernel since the
notsent queue is just too filled up.
Fixes#24665
Signed-off-by: David Goulet <dgoulet@torproject.org>
Instead of using INT_MAX as a write limit for KISTLite, use the lower layer
limit which is using the specialized num_cells_writeable() of the channel that
will down the line check the connection's outbuf and limit it to 32KB
(OR_CONN_HIGHWATER).
That way we don't take the chance of bloating the connection's outbuf and we
keep the cells in the circuit queue which our OOM handler can take care of,
not the outbuf.
Finally, this commit adds a log_debug() in the update socket information
function of KIST so we can get the socket information in debug.
Fixes#24671
Signed-off-by: David Goulet <dgoulet@torproject.org>
Exposing cell_queues_get_total_allocation(), buf_get_total_allocation(),
tor_compress_get_total_allocation(), tor_compress_get_total_allocation() when
hit MaxMemInQueues threshold.
Fixes#24501
Signed-off-by: Fernando Fernandez Mancera <ffernandezmancera@gmail.com>
This patch adds support for MainloopStats that allow developers to get
main event loop statistics via Tor's heartbeat status messages. The new
status log message will show how many succesful, erroneous, and idle
event loop iterations we have had.
See: https://bugs.torproject.org/24605
Adding tor_remove_file(filename) and refactoring tor_cleanup().
Removing CookieAuthFile and ExtORPortCookieAuthFile when tor_cleanup() is
called.
Fixes#23271.
Signed-off-by: Fernando Fernandez Mancera <ffernandezmancera@gmail.com>
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.
When entry_list_is_constrained() is true, guards_retry_optimistic()
always returns true.
When entry_list_is_constrained() is false,
options->UseBridges is always false,
therefore !options->UseBridges is always true,
therefore (!options->UseBridges || ...) is always true.
Cleanup after #24367.
Commit e80893e51b made tor call
hs_service_intro_circ_has_closed() when we mark for close a circuit.
When we cleanup intro points, we iterate over the descriptor's map of intro
points and we can possibly mark for close a circuit. This was problematic
because we would MAP_DEL_CURRENT() the intro point then free it and finally
mark for close the circuit which would lookup the intro point that we just
free in the map we are iterating over.
This can't be done and leads to a use-after-free because the intro point will
be returned successfully due to the fact that we are still in the loop
iterating. In other words, MAP_DEL_CURRENT() followed by a digest256map_get()
of the same object should never be done in the same loop.
Fixes#24595
Signed-off-by: David Goulet <dgoulet@torproject.org>
This patch lifts the check for whether a given log file (`logfile_t`) is
an "external logfile" (handled by an external logging system such as
syslog, android's logging subsystem, or as an external C callback
function) into a function on its own.
See: https://bugs.torproject.org/24362
In KIST, we could have a small congestion window value than the unacked
packets leading to a integer overflow which leaves the tcp_space value to be
humongous.
This has no security implications but it results in KIST scheduler allowing to
send cells on a potentially saturated connection.
Found by #24423. Fixes#24590.
Signed-off-by: David Goulet <dgoulet@torproject.org>
This function leaks memory when the event_base is freed before the
event itself fires. That's not harmful, but it's annoying when
trying to debug other memory leaks.
Fixes bug 24584; bugfix on 0.2.8.1-alpha.
When we didn't do this before, we'd have some still-reachable memory
warnings, and we'd find ourselves crashing when we tried to
reinitialize libevent.
Part of 24581 (don't crash when restarting Tor in-process)
This patch adds support for Android's logging subsystem in Tor. When
debugging Android applications it is useful to be able to collect
information about the application running on the platform via the
various system services that is available on the platform.
This patch allows you to add "Log notice android" to your torrc and have
Tor send everything above and including the notice severity to Android's
ring buffer which can be inspected using the 'adb logcat' program.
See: https://bugs.torproject.org/24362
This patch ensures that we more easily can extend our log backends that
does not take any additional argument other than a single keyword. This
patch is mostly reindentation of some code which is why it is split out
into its own patch.
See: https://bugs.torproject.org/24362
Also make IPv6-only clients wait for microdescs for relays, even if we were
previously using descriptors (or were using them as a bridge) and have
a cached descriptor for them.
But if node_is_a_configured_bridge(), stop waiting for its IPv6 address in
a microdescriptor, because we'll never use it.
Implements #23827.
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\(/;
This commit removes the old FREE_AND_NULL, and renames the old
FREE_AND_NULL_UNMATCHED so that it is now called FREE_AND_NULL.
This will break all the FREE_AND_NULL_* users; the next commit will
fix them.
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>
Split hs_circuitmap_get_rend_circ_client_side(). One returns only established
circuits (hs_circuitmap_get_established_rend_circ_client_side()) and the other
returns all kinds of circuits.
Fixes#23459
Signed-off-by: Fernando Fernandez Mancera <ffernandezmancera@gmail.com>
Previously, circuit_stream_is_being_handled incorrectly reported
that (1) an exit port was "handled" by a circuit regardless of
whether the circuit was already isolated in some way, and
(2) that a stream could be "handled" by a circuit even if their
isolation settings were incompatible.
As a result of (1), in Tor Browser, circuit_get_unhandled_ports was
reporting that all ports were handled even though all non-internal
circuits had already been isolated by a SOCKS username+password.
Therefore, circuit_predict_and_launch_new was declining to launch
new exit circuits. Then, when the user visited a new site in Tor
Browser, a stream with new SOCKS credentials would be initiated,
and the stream would have to wait while a new circuit with those
credentials could be built. That wait was making the
time-to-first-byte longer than it needed to be.
Now, clean, not-yet-isolated circuit(s) will be automatically
launched ahead of time and be ready for use whenever a new stream
with new SOCKS credentials (or other isolation criteria) is
initiated.
Fixes bug 18859. Thanks to Nick Mathewson for improvements.
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>
This patch adds a check for the return value of `hs_parse_address()` in
`hs_control_hspost_command()`. Since it should not be possible for
`hs_parse_address()` to fail in this context we wrap the error check
with the `BUG()` macro.
See: https://bugs.torproject.org/24543
This patch is a result of auditing all of our uses of
get_datadir_fname() and its kin, and dividing them into cache vs
keys vs other data.
The new get_keydir_fname() and get_cachedir_fname() functions don't
actually do anything new yet.
This introduces the test_hs_control.c file which at this commit contains basic
unit test for the HS_DESC event.
Signed-off-by: David Goulet <dgoulet@torproject.org>
This is removed for two reasons. First, HSDir accepts descriptor even though
they don't think they are in fact an HSDir. This is to avoid consensus desync
between client/service and directories.
Second, our malicious HSDir scanner uses the HSPOST command to post on all
relays in order to test them before they could become HSDir. We had to remove
that check from the tor code that the scanner uses.
Thus, this check should not be enforced by the control port for the above use
cases. It is also a bit more complex with v3 support for which not all HSDir
support it so basically irrelevant check.
Signed-off-by: David Goulet <dgoulet@torproject.org>
This is groundwork for the HSPOST control port command that needs a way in the
HS subsystem to upload a service descriptor to a specific HSDir.
To do so, we add a public function that takes a series of parameters including
a fully encoded descriptor and initiate a directory request to a specific
routerstatut_t object.
It is for now not used but should be, in future commit, by the HSPOST command.
This commit has no behavior change, only refactoring.
Signed-off-by: David Goulet <dgoulet@torproject.org>
This makes the REPLICA= field optional for the control port event. A v2
service will always pass it and v3 is ignored.
Signed-off-by: David Goulet <dgoulet@torproject.org>
A new v3 specific function has been added named
control_event_hsv3_descriptor_failed().
The HS v3 subsystem now uses it.
Signed-off-by: David Goulet <dgoulet@torproject.org>
This changes the control_event_hs_descriptor_requested() call to add the hsdir
index optional value. v2 passes NULL all the time.
This commit creates hs_control.{c|h} that contains wrappers for the HS
subsystem to interact with the control port subsystem.
The descriptor REQUESTED event is implemented following proposal 284 extension
for v3.
Signed-off-by: David Goulet <dgoulet@torproject.org>
Make control_event_hs_descriptor_received() and
control_event_hs_descriptor_failed() v2 specific because they take a
rend_data_t object and v3 will need to pass a different object.
No behavior change.
Signed-off-by: David Goulet <dgoulet@torproject.org>
First, rename and make that function static because it is internal to
control.c and called by two HS_DESC events.
Second, make it take more basic parameters and thus not a rend_data_t object
so we can still use the function for v3 HS that doesn't use that object.
Third, move the descriptor ID lookup to the two specific events (yes little
code duplication there) because they get a rend_data_t object which won't be
the case for v3.
Finally, through this refactoring, change the pointer check to BUG() and
change some parameter names to reflect what they really are.
No behavior change at this commit.
Signed-off-by: David Goulet <dgoulet@torproject.org>
This is a naming refactor mostly _except_ for a the events' function that take
a rend_data_t which will require much more refactoring.
No behavior change at this commit, cleanup and renaming stuff to not be only
v2 specific.
Signed-off-by: David Goulet <dgoulet@torproject.org>
When an intro circuit has closed, do not warn anymore when we can't find the
service. It is possible to hit that condition if the service is removed before
the circuits were fully closed. This happens in the case of deleting an
ephemeral service.
Signed-off-by: David Goulet <dgoulet@torproject.org>
The functions are now used by the ADD_ONION/DEL_ONION control port command as
well. This commits makes them fully functionnal with hidden service v3.
Part of #20699
Signed-off-by: David Goulet <dgoulet@torproject.org>
Instead of using the cwd to specify the location of Cargo.toml, we
use the --manifest-path option to specify its location explicitly.
This works around the bug that isis diagnosed on our jenkins builds.
Making errno error log more useful for getrandom() call. Adding if statement to
make difference between ENOSYS and other errors.
Fixes#24500
Signed-off-by: Fernando Fernandez Mancera <ffernandezmancera@gmail.com>
First, hs_service_intro_circ_has_closed() is now called in circuit_mark_for
close() because the HS subsystem needs to learn when an intro point is
actually not established anymore as soon as possible. There is a time window
between a close and a free.
Second, when we mark for close, we also remove it from the circuitmap because
between the close and the free, a service can launch an new circuit to that
same intro point and thus register it which only succeeds if the intro point
authentication key is not already in the map.
However, we still do a remove from the circuitmap in circuit_free() in order
to also cleanup the circuit if it wasn't marked for close prior to the free.
Fixes#23603
Signed-off-by: David Goulet <dgoulet@torproject.org>
The hs_service_intro_circ_has_closed() was removing intro point objects if too
many retries.
We shouldn't cleanup those objects in that function at all but rather let
cleanup_intro_points() do its job and clean it properly.
This was causing an issue in #23603.
Furthermore, this moves the logic of remembering failing intro points in the
cleanup_intro_points() function which should really be the only function to
know when to cleanup and thus when an introduction point should be remembered
as a failed one.
Fixes#23603
Signed-off-by: David Goulet <dgoulet@torproject.org>
In the KIST main loop, if the channel happens to be not opened, set its state
to IDLE so we can release it properly later on. Prior to this fix, the channel
was in PENDING state, removed from the channel pending list and then kept in
that state because it is not opened.
This bug was introduced in commit dcabf801e5 for
which we made the scheduler loop not consider unopened channel.
This has no consequences on tor except for an annoying but harmless BUG()
warning.
Fixes#24502
Signed-off-by: David Goulet <dgoulet@torproject.org>
Some platforms don't have good monotonic time support so don't warn when the
diff between the last run of the scheduler time and now is negative. The
scheduler recovers properly from this so no need to be noisy.
Fixes#23696
Signed-off-by: David Goulet <dgoulet@torproject.org>
When creating a routerstatus (vote) from a routerinfo (descriptor),
set the IPv6 address to the unspecified IPv6 address, and explicitly
initialise the port to zero.
Also clarify the documentation for the function.
Fixes bug 24488; bugfix on 0.2.4.1-alpha.
Fortunately, use_cached_ipv4_answers was already 0, so we wouldn't
actually use this info, but it's best not to have it.
Fixes bug 24050; bugfix on 0.2.6.3-alpha
TROVE-2017-12. Severity: Medium
When choosing a random node for a circuit, directly use our router
descriptor to exclude ourself instead of the one in the global
descriptor list. That list could be empty because tor could be
downloading them which could lead to not excluding ourself.
Closes#21534
TROVE-2017-12. Severity: Medium
Thankfully, tor will close any circuits that we try to extend to
ourselves so this is not problematic but annoying.
Part of #21534.
TROVE-2017-13. Severity: High.
In the unlikely case that a hidden service could be missing intro circuit(s),
that it didn't have enough directory information to open new circuits and that
an intro point was about to expire, a use-after-free is possible because of
the intro point object being both in the retry list and expiring list at the
same time.
The intro object would get freed after the circuit failed to open and then
access a second time when cleaned up from the expiring list.
Fixes#24313
Going from 4 hours to 24 hours in order to try reduce the efficiency of guard
discovery attacks.
Closes#23856
Signed-off-by: David Goulet <dgoulet@torproject.org>
The goal here is to replace our use of msec-based timestamps with
something less precise, but easier to calculate. We're doing this
because calculating lots of msec-based timestamps requires lots of
64/32 division operations, which can be inefficient on 32-bit
platforms.
We make sure that these stamps can be calculated using only the
coarse monotonic timer and 32-bit bitwise operations.
First, that test was broken from the previous commit because the
channel_queue_cell() has been removed. This now tests the
channel_process_cell() directly.
Second, it wasn't testing much except if the channel subsystem actually went
through the cell handler. This commit adds more checks on the state of a
channel going from open, receiving a cell and closing.
Third, this and the id_map unit test are working, not the others so they've
been marked as not working and future commit will improve and fix those.
Signed-off-by: David Goulet <dgoulet@torproject.org>
This removed code that was either never reached or irrelevant after the
incoming/outgoing queue removal such as the "timestamp_drained".
Lots of things are also removed from channel.h that do not exists anymore or
not used.
Signed-off-by: David Goulet <dgoulet@torproject.org>
If the channel layer failed to write a cell from the circuit queue, requeue it
so it can be retried on the same channel later.
Signed-off-by: David Goulet <dgoulet@torproject.org>
The channel_write_cell() and channel_write_var_cell() can't be possibly called
nor are used by tor. We only write on the connection outbuf packed cell coming
from the scheduler that takes them from the circuit queue.
This makes channel_write_packed_cell() the only usable function. It is
simplify and now returns a code value. The reason for this is that in the next
commit(s), we'll re-queue the cell onto the circuit queue if the write fails.
Finally, channel unit tests are being removed with this commit because they do
not match the new semantic. They will be re-written in future commits.
Signed-off-by: David Goulet <dgoulet@torproject.org>
The channel subsystem was doing a whole lot to track and try to predict the
channel queue size but they are gone due to previous commit.
Signed-off-by: David Goulet <dgoulet@torproject.org>
For the rationale, see ticket #23709.
This is a pretty massive commit. Those queues were everywhere in channel.c and
it turns out that it was used by lots of dead code.
The channel subsystem *never* handles variable size cell (var_cell_t) or
unpacked cells (cell_t). The variable ones are only handled in channeltls and
outbound cells are always packed from the circuit queue so this commit removes
code related to variable and unpacked cells.
However, inbound cells are unpacked (cell_t), that is untouched and is handled
via channel_process_cell() function.
In order to make the commit compile, test have been modified but not passing
at this commit. Also, many tests have been removed but better improved ones
get added in future commits.
This commit also adds a XXX: which indicates that the handling process of
outbound cells isn't fully working. This as well is fixed in a future commit.
Finally, at this commit, more dead code remains, it will be cleanup in future
commits.
Fixes#23709
Signed-off-by: David Goulet <dgoulet@torproject.org>
This function is part of the tor fast path so this commit adds more
documentation to it as it is critical.
Signed-off-by: David Goulet <dgoulet@torproject.org>
append_cell_to_circuit_queue() had code disabled from commit
2a95f31716
This code is 4+ years old related to bug #9072 so if we ever want to revisit
it, lets inspect/revert this commit.
Signed-off-by: David Goulet <dgoulet@torproject.org>
This applies the changes in 23524 to num_usable_bridges(), because it has
replaced any_bridge_descriptors_known().
The original changes file still applies.
Stop checking for bridge descriptors when we actually want to know if
any bridges are usable. This avoids potential bootstrapping issues.
Fixes bug 24367; bugfix on 0.2.0.3-alpha.
Stop stalling when bridges are changed at runtime. Stop stalling when
old bridge descriptors are cached, but they are not in use.
Fixes bug 24367; bugfix on 23347 in 0.3.2.1-alpha.
We used to check whether we have enough filtered guards (guard set when
torrc is applied) but that's not good enough, since that might be bad in
some cases where many guards are not reachable (might cause overblocking
and hence reacahbility issues).
We now check if we have enough reachable filtered guards before applying
md restrictions which should prevent overblocking.
Previously, if store_multiple() reported a partial success, we would
store all the handles it gave us as if they had succeeded. But it's
possible for the diff to be only partially successful -- for
example, if LZMA failed but the other compressors succeeded.
Fixes bug 24086; bugfix on 0.3.1.1-alpha.
Move it to hs_common.h and rename it "hs_service_add_ephemeral_status_t". It
will be shared between v2 and v3 services.
Part of #20699
Signed-off-by: David Goulet <dgoulet@torproject.org>
At this commit, the key handling and generation is supported for a v3 service
(ED25519-V3). However, the service creation is not yet implemented. This only
adds the interface and code to deal with the new ED25519-V3 key type.
Tests have been updated for RSA key type but nothing yet for ED25519-v3.
Part of #20699
Signed-off-by: David Goulet <dgoulet@torproject.org>
This will be used by the control port command "GETINFO
hs/service/desc/id/<ADDR>" which returns the encoded current descriptor for
the given onion address.
Signed-off-by: David Goulet <dgoulet@torproject.org>
This commit adds hs_cache_lookup_encoded_as_client() function that returns the
encoded descriptor for a given service public key. This will be needed by the
"GETINFO hs/client/desc/id/<ADDR>" control port command.
Signed-off-by: David Goulet <dgoulet@torproject.org>
If we can't read a file because of an FS issue, we say "we can't
read that" and move on. But if we can't read it because it's empty,
because it has no labels, or because its labels are misformatted, we
should remove it.
Fixes bug 24099; bugfix on 0.3.1.1-alpha.
A circuit with purpose C_INTRODUCING means that its state is opened but the
INTRODUCE1 cell hasn't been sent yet. We shouldn't consider that circuit when
looking for timing out "building circuit". We have to wait on the rendezvous
circuit to be opened before sending that cell so the intro circuit needs to be
kept alive for at least that period of time.
This patch makes that the purpose C_INTRODUCING is ignored in the
circuit_expire_building() which means that we let the circuit idle timeout
take care of it if we end up never using it.
Fixes#23681
Signed-off-by: David Goulet <dgoulet@torproject.org>
