On an hidden service rendezvous circuit, a BEGIN_DIR could be sent
(maliciously) which would trigger a tor_assert() because
connection_edge_process_relay_cell() thought that the circuit is an
or_circuit_t but is an origin circuit in reality.
Fixes#22494
Reported-by: Roger Dingledine <arma@torproject.org>
Signed-off-by: David Goulet <dgoulet@torproject.org>
This fixes an assertion failure in relay_send_end_cell_from_edge_() when an
origin circuit and a cpath_layer = NULL were passed.
A service rendezvous circuit could do such a thing when a malformed BEGIN cell
is received but shouldn't in the first place because the service needs to send
an END cell on the circuit for which it can not do without a cpath_layer.
Fixes#22493
Reported-by: Roger Dingledine <arma@torproject.org>
Signed-off-by: David Goulet <dgoulet@torproject.org>
Apparently, the unit tests relied on being able to make ed->x509
link certs even when they hadn't set any server flags in the
options. So instead of making "client" mean "never generate an
ed->x509 cert", we'll have it mean "it's okay not to generate an
ed->x509 cert".
(Going with a minimal fix here, since this is supposed to be a
stable version.)
The tests previously assumed that the link handshake code would be
calling get_my_certs() -- when I changed it to call get_own_cert()
instead for the (case 2) 22460 fix, the tests failed, since the tls
connection wasn't really there.
This change makes us start mocking out the tor_tls_get_own_cert()
function too.
It also corrects the behavior of the mock_get_peer_cert() function
-- it should have been returning a newly allocated copy.
It's okay to call add_ed25519_cert with a NULL argument: so,
document that. Also, add a tor_assert_nonfatal() to catch any case
where we have failed to set own_link_cert when conn_in_server_mode.
Whenever we rotate our TLS context, we change our Ed25519
Signing->Link certificate. But if we've already started a TLS
connection, then we've already sent the old X509 link certificate,
so the new Ed25519 Signing->Link certificate won't match it.
To fix this, we now store a copy of the Signing->Link certificate
when we initialize the handshake state, and send that certificate
as part of our CERTS cell.
Fixes one case of bug22460; bugfix on 0.3.0.1-alpha.
Previously we could sometimes change our signing key, but not
regenerate the certificates (signing->link and signing->auth) that
were signed with it. Also, we would regularly replace our TLS x.509
link certificate (by rotating our TLS context) but not replace our
signing->link ed25519 certificate. In both cases, the resulting
inconsistency would make other relays reject our link handshakes.
Fixes two cases of bug 22460; bugfix on 0.3.0.1-alpha.
The encrypted_data_length_is_valid() function wasn't validating correctly the
length of the encrypted data of a v3 descriptor. The side effect of this is
that an HSDir was rejecting the descriptor and ultimately not storing it.
Fixes#22447
Signed-off-by: David Goulet <dgoulet@torproject.org>
A fair number of our mock_impl declarations were messed up so that
even our special AM_ETAGSFLAGS couldn't find them.
This should be a whitespace-only patch.
When directory authorities reject a router descriptor due to keypinning,
free the router descriptor rather than leaking the memory.
Fixes bug 22370; bugfix on 0.2.7.2-alpha.
If we add the element itself, we will later free it when we free the
descriptor, and the next time we go to look at MyFamily, things will
go badly.
Fixes the rest of bug 22368; bugfix on 0.3.1.1-alpha.
If we free them here, we will still attempt to access the freed memory
later on, and also we will double-free when we are freeing the config.
Fixes part of bug 22368.
This patch lifts the return value, rv, variable to the beginning of the
function, adds a 'done' label for clean-up and function exit and makes
the rest of the function use the rv value + goto done; instead of
cleaning up in multiple places.
See: https://bugs.torproject.org/22305
We used to not set the guard state in launch_direct_bridge_descriptor_fetch().
So when a bridge descriptor fetch failed, the guard subsystem would never
learn about the fail (and hence the guard's reachability state would not
be updated).
We used to not set the guard state in launch_direct_bridge_descriptor_fetch().
So when a bridge descriptor fetch failed, the guard subsystem would never
learn about the fail (and hence the guard's reachability state would not
be updated).
Directory authorities now reject relays running versions
0.2.9.1-alpha through 0.2.9.4-alpha, because those relays
suffer from bug 20499 and don't keep their consensus cache
up-to-date.
Resolves ticket 20509.
parse_config_line_from_str_verbose() already looks for strings
that are surrounded by quotes, and processes them with
unescape_string(). So things were getting decoded twice, which was
(in turn) playing havoc with backslashes on Windows.
This adds a couple of configure commands to control whether we're
requiring all dependencies to be available locally (default) or not
(--enable-cargo-online-mode). When building from a tarball, we require
the RUST_DEPENDENCIES variable to point to the local repository of
crates. This also adds src/ext/rust as a git submodule that contains
such a local repository for easy setup.
Passing --enable-cargo-online-mode during configure allows cargo to make
network requests while building Tor or running tests. If this flag is
not supplied, the dependencies need to be available in the form of a
local mirror.
This gives an indication in the log that Tor was built with Rust
support, as well as laying some groundwork for further string-returning
APIs to be converted to Rust
config_get_lines is now split into two functions:
- config_get_lines which is the same as before we had %include
- config_get_lines_include which actually processes %include
Before we've set our options, we can neither call get_options() nor
networkstatus_get_latest_consensus().
Fixes bug 22252; bugfix on 4d9d2553ba
in 0.2.9.3-alpha.
Replace the 177 fallbacks originally introduced in Tor 0.2.9.8 in
December 2016 (of which ~126 were still functional), with a list of
151 fallbacks (32 new, 119 existing, 58 removed) generated in May 2017.
Resolves ticket 21564.
This patch makes us use FALLBACK_COMPRESS_METHOD to try to fetch an
object from the consensus diff manager in case no mutually supported
result was found. This object, if found, is then decompressed using the
spooling system to the client.
See: https://bugs.torproject.org/21667
This patch removes the calls to spooled_resource_new() when trying to
download the consensus. All calls should now be going through the
consdiff manager.
See: https://bugs.torproject.org/21667
This patch ensures that we use the current consensus in the case where
no consensus diff was found or a consensus diff wasn't requested.
See: https://bugs.torproject.org/21667
These still won't do anything till I get the values to be filled in.
Also, I changed the API a little (with corresponding changes in
directory.c) to match things that it's easier to store.
This patch changes handle_get_current_consensus() to make it read the
current consensus document from the consensus caching subsystem.
See: https://bugs.torproject.org/21667
Failure to do this caused an assertion failure with #22246 . This
assertion failure can be triggered remotely, so we're tracking it as
medium-severity TROVE-2017-002.
Closes bug 22245; bugfix on 0.0.9rc1, when bandwidth accounting was
first introduced.
Found by Andrey Karpov and reported at https://www.viva64.com/en/b/0507/
This patch refactors connection_dir_client_reached_eof() to use
compression_method_get_human_name() to set description1 and
description2 variables.
See: https://bugs.torproject.org/21667
One (HeapEnableTerminationOnCorruption) is on-by-default since win8;
the other (PROCESS_DEP_DISABLE_ATL_THUNK_EMULATION) supposedly only
affects ATL, which (we think) we don't use. Still, these are good
hygiene. Closes ticket 21953.
Cleanup logic in test_intro_point_registration() invoked tt_assert()
in a way that could cause it to jump backward into the cleanup code if
the assertion failed, causing Coverity to see a double free (CID
1397192). Move the tt_assert() calls into a helper function having
the well-defined task of testing hs_circuitmap_free_all().
Fixes#22231.
A descriptor only contains the curve25519 public key in the enc-key field so
the private key should not be in that data structure. The service data
structures will have access to the full keypair (#20657).
Furthermore, ticket #21871 has highlighted an issue in the proposal 224 about
the encryption key and legacy key being mutually exclusive. This is very wrong
and this commit fixes the code to follow the change to the proposal of that
ticket.
Signed-off-by: David Goulet <dgoulet@torproject.org>
A for-loop in test_channelpadding_timers() would never run because it
was trying to increment a counter up to CHANNELS_TO_TEST/3 after an
earlier block already incremented it to CHANNELS_TO_TEST/2.
Fixes#22221, CID 1405983.
We do this by treating the presence of .z as meaning ZLIB_METHOD,
even if Accept-Encoding does not include deflate.
This fixes bug 22206; bug not in any released tor.
Nothing was setting or inspecting these fields, and they were marked
as OBSOLETE() in config.c -- but somehow we still had them in the
or_options_t structure. Ouch.
There was a bug that got exposed with the removal of ORListenAddress. Within
server_mode(), we now only check ORPort_set which is set in parse_ports().
However, options_validate() is using server_mode() at the start to check if we
need to look at the uname but then the ORPort_set is unset at that point
because the port parsing was done just after. This commit fixes that.
Signed-off-by: David Goulet <dgoulet@torproject.org>
Deprecated in 0.2.9.2-alpha, this commits changes it as OBSOLETE() and cleans
up the code associated with it.
Partially fixes#22060
Signed-off-by: David Goulet <dgoulet@torproject.org>
Deprecated in 0.2.9.2-alpha, this commits changes it as OBSOLETE() and cleans
up the code associated with it.
Partially fixes#22060
Signed-off-by: David Goulet <dgoulet@torproject.org>
Deprecated in 0.2.9.2-alpha, this commits changes it as OBSOLETE() and cleans
up the code associated with it.
Partially fixes#22060
Signed-off-by: David Goulet <dgoulet@torproject.org>
Deprecated in 0.2.9.2-alpha, this commits changes it as OBSOLETE() and cleans
up the code associated with it.
Partially fixes#22060
Signed-off-by: David Goulet <dgoulet@torproject.org>
Deprecated in 0.2.9.2-alpha, this commits changes it as OBSOLETE() and cleans
up the code associated with it.
Partially fixes#22060
Signed-off-by: David Goulet <dgoulet@torproject.org>
Deprecated in 0.2.9.2-alpha, this commits changes it as OBSOLETE() and cleans
up the code associated with it.
Partially fixes#22060
Signed-off-by: David Goulet <dgoulet@torproject.org>
Deprecated in 0.2.9.2-alpha, this commits changes it as OBSOLETE() and cleans
up the code associated with it.
Partially fixes#22060
Signed-off-by: David Goulet <dgoulet@torproject.org>
Deprecated in 0.2.9.2-alpha, this commits changes it as OBSOLETE() and cleans
up the code associated with it.
Partially fixes#22060
Signed-off-by: David Goulet <dgoulet@torproject.org>
Deprecated in 0.2.9.2-alpha, this commits changes it as OBSOLETE() and cleans
up the code associated with it.
Partially fixes#22060
Signed-off-by: David Goulet <dgoulet@torproject.org>
Deprecated in 0.2.9.2-alpha, this commits changes it as OBSOLETE() and cleans
up the code associated with it.
Partially fixes#22060
Signed-off-by: David Goulet <dgoulet@torproject.org>
The descriptor fields can't be validated properly during encoding because they
are signed by a descriptor signing key that we don't have in the unit test.
Removing the test case for now but ultimately we need an independent
implementation that can encode descriptor and test our decoding functions with
that.
Signed-off-by: David Goulet <dgoulet@torproject.org>
Create the hs_test_helpers.{c|h} files that contains helper functions to
create introduction point, descriptor and compare descriptor.
Used by both the hs cache and hs descriptor tests. Unify them to avoid code
duplication.
Also, this commit fixes the usage of the signing key that was wrongly used
when creating a cross signed certificate.
Signed-off-by: David Goulet <dgoulet@torproject.org>
This change prevents a no-longer-supported behavior where we change
options that would later be written back to torrc with a SAVECONF.
Also, use the "Pointer to final pointer" trick to build the
normalized list, to avoid special-casing the first element.
asan was finding an alignment issue with a cast, so set the field in the
trunnel struct and then encode it instead. Also, enable log capture and
verification.
Checking all of these parameter lists for every single connection every second
seems like it could be an expensive waste.
Updating globally cached versions when there is a new consensus will still
allow us to apply consensus parameter updates to all existing connections
immediately.
IMO, these tests should be calling options_init() to properly set everything
to default values, but when that is done, about a dozen tests fail. Setting
the one default value that broke the tests for my branch. Sorry for being
lame.
Accomplished via the following:
1. Use NETINFO cells to determine if both peers will agree on canonical
status. Prefer connections where they agree to those where they do not.
2. Alter channel_is_better() to prefer older orconns in the case of multiple
canonical connections, and use the orconn with more circuits on it in case
of age ties.
Also perform some hourly accounting on how many of these types of connections
there are and log it at info or notice level.
This unifies CircuitIdleTimeout and PredictedCircsRelevanceTime into a single
option, and randomizes it.
It also gives us control over the default value as well as relay-to-relay
connection lifespan through the consensus.
Conflicts:
src/or/circuituse.c
src/or/config.c
src/or/main.c
src/test/testing_common.c
This defense will cause Cisco, Juniper, Fortinet, and other routers operating
in the default configuration to collapse netflow records that would normally
be split due to the 15 second flow idle timeout.
Collapsing these records should greatly reduce the utility of default netflow
data for correlation attacks, since all client-side records should become 30
minute chunks of total bytes sent/received, rather than creating multiple
separate records for every webpage load/ssh command interaction/XMPP chat/whatever
else happens to be inactive for more than 15 seconds.
The defense adds consensus parameters to govern the range of timeout values
for sending padding packets, as well as for keeping connections open.
The defense only sends padding when connections are otherwise inactive, and it
does not pad connections used solely for directory traffic at all. By default
it also doesn't pad inter-relay connections.
Statistics on the total padding in the last 24 hours are exported to the
extra-info descriptors.
Right now it just sets an if-modified-since header, but it's about
to get even bigger.
This patch avoids changing indentation; the next patch will be
whitespace fixes.
We need to index diffs by the digest-as-signed of their source
consensus, so that we can find them even from consensuses whose
signatures are encoded differently.
In this patch I add support for "delete through end of file" in our
ed diff handler, and generate our diffs so that they remove
everything after in the consensus after the signatures begin.
test_options_validate_impl() incorrectly executed subsequent phases of
config parsing and validation after an expected error. This caused
msg to leak when those later phases (which would likely produce errors
as well) overwrote it.
This was introduced 90562fc23a adding a code
path where we pass a NULL pointer for the HSDir fingerprint to the control
event subsystem. The HS desc failed function wasn't handling properly that
pointer for a NULL value.
Two unit tests are also added in this commit to make sure we handle properly
the case of a NULL hsdir fingerprint and a NULL content as well.
Fixes#22138
Signed-off-by: David Goulet <dgoulet@torproject.org>
Code movement in the commit introducings tests for #22103 uncovered a
latent memory management bug.
Refactor the log message checking from test_options_checkmsgs() into a
helper test_options_checklog(). This avoids a memory leak (and
possible double-free) in a test failure condition.
Don't reuse variables (especially pointers to allocated memory!) for
multiple unrelated purposes.
Fixes CID 1405778.
Also factor out the error message comparisions from
test_options_validate_impl() into a separate function so it can check
for error messages in different phases of config parsing.
config_parse_interval() and config_parse_msec_interval() were checking
whether the variable "ok" (a pointer to an int) was null, rather than
derefencing it. Both functions are static, and all existing callers
pass a valid pointer to those static functions. The callers do check
the variables (also confusingly named "ok") whose addresses they pass
as the "ok" arguments, so even if the pointer check were corrected to
be a dereference, it would be redundant.
Fixes#22103.
This was a >630-line function, which doesn't make anybody happy. It
was also mostly composed of a bunch of if-statements that handled
different directory responses differently depending on the original
purpose of the directory connection. The logical refactoring here
is to move the body of each switch statement into a separate handler
function, and to invoke those functions from a separate switch
statement.
This commit leaves whitespace mostly untouched, for ease of review.
I'll reindent in the next commit.
These required some special-casing, since some of the assumption
about real compression algorithms don't actually hold for the
identity transform. Specifically, we had assumed:
- compression functions typically change the lengths of their
inputs.
- decompression functions can detect truncated inputs
- compression functions have detectable headers
None of those is true for the identity transformation.
This will allow us to treat NO_METHOD as a real compression method,
and to simplify code that currently does
if (compressing) {
compress
} else {
copy
}
Inform the control port with an HS_DESC failed event when the client is unable
to pick an HSDir. It's followed by an empty HS_DESC_CONTENT event. In order to
achieve that, some control port code had to be modified to accept a NULL HSDir
identity digest.
This commit also adds a trigger of a failed event when we are unable to
base64-decode the descriptor cookie.
Fixes#22042
Signed-off-by: David Goulet <dgoulet@torproject.org>
Introduce a way to optionally enable Rust integration for our builds. No
actual Rust code is added yet and specifying the flag has no effect
other than failing the build if rustc and cargo are unavailable.
Increase the maximum allowed size passed to mprotect(PROT_WRITE)
from 1MB to 16MB. This was necessary with the glibc allocator
in order to allow worker threads to allocate more memory --
which in turn is necessary because of our new use of worker
threads for compression.
Closes ticket #22096. Found while working on #21648.
This patch changes two things in our LZMA compression backend:
- We lower the preset values for all `compression_level_t` values to
ensure that we can run the LZMA decoder with less than 65 MB of memory
available. This seems to have a small impact on the real world usage
and fits well with our needs.
- We set the upper bound of memory usage for the LZMA decoder to 16 MB.
See: https://bugs.torproject.org/21665
There were two issues here: first, zstd didn't exhibit the right
behavior unless it got a very large input. That's fine.
The second issue was a genuine bug, fixed by 39cfaba9e2.
