That log statement can be triggered if somebody on the Internet behaves badly
which is possible with buggy implementation for instance.
Fixes#21293
Signed-off-by: David Goulet <dgoulet@torproject.org>
This patch renames the `compress` parameter of the
`tor_zlib_compress_new()` function to `_compress` to avoid shadowing the
`compress()` function in zlib.h.
This patch splits up `tor_compress_memory_level()` into static functions
in the individual compression backends, which allows us to tune the
values per compression backend rather than globally.
See: https://bugs.torproject.org/21662
Use a switch-statement in `tor_compress()` and `tor_uncompress()` for
the given `compress_method_t` parameter. This allows us to have the
compiler detect if we forgot add a handler in these functions for a
newly added enumeration value.
See: https://bugs.torproject.org/21662
This patch ensures that Tor checks if a given compression method is
supported before printing the version string when calling `tor
--library-versions`.
Additionally, we use the `tor_compress_supports_method()` to check if a
given version is supported for Tor's start-up version string, but here
we print "N/A" if a given compression method is unavailable.
See: https://bugs.torproject.org/21662
This patch adds `tor_compress_version_str()` and
`tor_compress_header_version_str()` to get the version strings of the
different compression schema providers. Both functions returns `NULL` in
case a given `compress_method_t` is unknown or unsupported.
See: https://bugs.torproject.org/21662
This patch adds support for checking if a given `compress_method_t` is
supported by the currently running Tor instance using
`tor_compress_supports_method()`.
See: https://bugs.torproject.org/21662
This patch adds the `tor_compress_get_total_allocation()` which returns
an approximate number of bytes currently in use by all the different
compression backends.
See: https://bugs.torproject.org/21662
This patch adds support for enabling support for Zstandard to our configure
script. By default, the --enable-zstd option is set to "auto" which means if
libzstd is available we'll build Tor with Zstandard support.
See: https://bugs.torproject.org/21662
This patch adds support for enabling support for LZMA to our configure
script. By default, the --enable-lzma option is set to "auto" which
means if liblzma is available we'll build Tor with LZMA support.
See: https://bugs.torproject.org/21662
This patch changes the way `tor_compress_new()`,
`tor_compress_process()`, and `tor_compress_free()` handles different
compression methods. This should give us compiler warnings in case an
additional compression method is added, but the developer forgets to add
handlers in the three aforementioned functions.
See https://bugs.torproject.org/21663
This patch refactors the `torgzip` module to allow us to extend a common
compression API to support multiple compression backends.
Additionally we move the gzip/zlib code into its own module under the
name `compress_zlib`.
See https://bugs.torproject.org/21664
Remove duplicate code that validates a service object which is now in
rend_validate_service().
Add some comments on why we nullify a service in the code path of
rend_config_services().
Signed-off-by: David Goulet <dgoulet@torproject.org>
This new function validates a service object and is used everytime a service
is successfully loaded from the configuration file.
It is currently copying the validation that rend_add_service() also does which
means both functions validate. It will be decoupled in the next commit.
Signed-off-by: David Goulet <dgoulet@torproject.org>
The item referred to the cdm_ht_set_status() case where the item was
not already in the hashtable. But that already happens naturally
when we scan the directory on startup... and we already have a test
for that.
This commit adds some helper functions to look up the diff from one
consensus and to make sure that applying it leads to another. Then
we add them throughout the existing test cases. Doing this turned
up a reference-leaking bug in consensus_diff_worker_replyfn.
In several places in the old code, we had problems that only an
in-memory index of diff status could solve, including:
* Remembering which diffs were in-progress, so that we didn't
re-launch them.
* Remembering which diffs had failed, so that we didn't try to
recompute them over and over.
* Having a fast way to look up the diff from a given consensus to
the latest consensus of a given flavor.
This patch adds a hashtable mapping from (flavor, source diff), to
solve the problem. It maps to a cache entry handle, rather than to
a cache entry directly, so that it doesn't affect the reference
counts of the cache entries, and so that we don't otherwise need to
worry about lifetime management.
This conscache flag tells conscache that it should munmap the
document as soon as reasonably possible, since its usage pattern is
expected to not have a lot of time-locality.
Initial tests. These just try adding a few consensuses, looking
them up, and making sure that consensus diffs are generated in a
more or less reasonable-looking way. It's enough for 87% coverage,
but it leaves out a lot of functionality.
This module's job is to remember old consensus documents, to
calculate their diffs on demand, and to .
There are some incomplete points in this code; I've marked them with
"XXXX". I intend to fix them in separate commits, since I believe
doing it in separate commits will make the branch easier to review.
The GETINFO extra-info/digest/<digest> broke in commit 568dc27a19 that
refactored the base16_decode() API to return the decoded length.
Unfortunately, that if() condition should have checked for the correct length
instead of an error which broke the command in tor-0.2.9.1-alpha.
Fixes#22034
Signed-off-by: David Goulet <dgoulet@torproject.org>
This commit mainly moves the responsibility for directory request
construction one level higher. It also allows a directory request
to contain a pointer to a routerstatus, which will get turned into
the correct contact information at the last minute.
We do dump HS stats now at log info everytime the intro circuit creation retry
period limit has been reached. However, the log was upgraded to warning if we
actually were over the elapsed time (plus an extra slop).
It is actually something that will happen in tor in normal case. For instance,
if the network goes down for 10 minutes then back up again making
have_completed_a_circuit() return false which results in never updating that
retry period marker for a service.
Fixes#22032
Signed-off-by: David Goulet <dgoulet@torproject.org>
This patch makes the internal `get_memlevel()` a part of the public
compression API as `tor_compress_memory_level()`.
See https://bugs.torproject.org/21663
This patch refactors our streaming compression code to allow us to
extend it with non-zlib/non-gzip based compression schemas.
See https://bugs.torproject.org/21663
To allow us to use the API name `tor_compress` and `tor_uncompress` as
the main entry-point for all compression/uncompression and not just gzip
and zlib.
See https://bugs.torproject.org/21663
This patch removes the unused `is_gzip_supported()` and changes the
documentation string around the `compress_method_t` enumeration to
explicitly state that both `ZLIB_METHOD` and `GZIP_METHOD` are both
always supported.
Zlib version 1.2.0 was released on the 9'th of March, 2003 according to
their ChangeLog.
See https://bugs.torproject.org/21663
This patch changes some of the tt_assert() usage in test_util_gzip() to
use tt_int_op() to get better error messages upon failure.
Additionally we move to use explicit NULL checks.
See https://bugs.torproject.org/21663
The consdiff generation logic would skip over lines added at the start of the
second file, and generate a diff that it would the immediately refuse because
it couldn't be used to reproduce the second file from the first. Fixes#21996.
The reason for making the temporary list public is to keep it encapsulated in
the rendservice subsystem so the prop224 code does not have direct access to
it and can only affect it through the rendservice pruning function.
It also has been modified to not take list as arguments but rather use the
global lists (main and temporary ones) because prop224 code will call it to
actually prune the rendservice's lists. The function does the needed rotation
of pointers between those lists and then prune if needed.
In order to make the unit test work and not completely horrible, there is a
"impl_" version of the function that doesn't free memory, it simply moves
pointers around. It is directly used in the unit test and two setter functions
for those lists' pointer have been added only for unit test.
Signed-off-by: David Goulet <dgoulet@torproject.org>
Now we have separate getters and setters for service-side and relay-side. I
took this approach over adding arguments to the already existing methods to
have more explicit type-checking, and also because some functions would grow
too large and dirty.
This commit also fixes every callsite to use the new function names which
modifies the legacy HS (v2) and the prop224 (v3) code.
Signed-off-by: David Goulet <dgoulet@torproject.org>
One of the goals of this change is to have trunnel API/ABI being more explicit
so we namespace them with "trn_*". Furthermore, we can now create
hs_cells.[ch] without having to confuse it with trunnel which used to be
"hs_cell_*" before that change.
Here are the perl line that were used for this rename:
perl -i -pe 's/cell_extension/trn_cell_extension/g;' src/*/*.[ch]
perl -i -pe 's/cell_extension/trn_cell_extension/g;' src/trunnel/hs/*.trunnel
perl -i -pe 's/hs_cell_/trn_cell_/g;' src/*/*.[ch]
perl -i -pe 's/hs_cell_/trn_cell_/g;' src/trunnel/hs/*.trunnel
And then "./scripts/codegen/run_trunnel.sh" with trunnel commit id
613fb1b98e58504e2b84ef56b1602b6380629043.
Fixes#21919
Signed-off-by: David Goulet <dgoulet@torproject.org>
Pinning EntryNodes along with hidden services can be possibly harmful (for
instance #14917 and #21155) so at the very least warn the operator if this is
the case.
Fixes#21155
Signed-off-by: David Goulet <dgoulet@torproject.org>
These tests tried to use ridiculously large buffer sizes to check
the sanity-checking in the code; but since the sanity-checking
changed, these need to change too.
Now that base64_decode() checks the destination buffer length against
the actual number of bytes as they're produced, shared_random.c no
longer needs the "SR_COMMIT_LEN+2" workaround.
Remove base64_decode_nopad() because it is redundant now that
base64_decode() correctly handles both padded and unpadded base64
encodings with "right-sized" output buffers.
Test base64_decode() with odd sized decoded lengths, including
unpadded encodings and padded encodings with "right-sized" output
buffers. Convert calls to base64_decode_nopad() to base64_decode()
because base64_decode_nopad() is redundant.
base64_decode() was applying an overly conservative check on the
output buffer length that could incorrectly produce an error if the
input encoding contained padding or newlines. Fix this by checking
the output buffer length against the actual decoded length produced
during decoding.
When we "fixed" #18280 in 4e4a7d2b0c
in 0291 it appears that we introduced a bug: The base32_encode
function can read off the end of the input buffer, if the input
buffer size modulo 5 is not equal to 0 or 3.
This is not completely horrible, for two reasons:
* The extra bits that are read are never actually used: so this
is only a crash when asan is enabled, in the worst case. Not a
data leak.
* The input sizes passed to base32_encode are only ever multiples
of 5. They are all either DIGEST_LEN (20), REND_SERVICE_ID_LEN
(10), sizeof(rand_bytes) in addressmap.c (10), or an input in
crypto.c that is forced to a multiple of 5.
So this bug can't actually trigger in today's Tor.
Closes bug 21894; bugfix on 0.2.9.1-alpha.
It looks like 32_encoded_size/64_encode_size APIs are inconsistent
not only in the number of "d"s they have, but also in whether they
count the terminating NUL. Taylor noted this in 86477f4e3f,
but I think we should note the inconsistently more loudly in order
to avoid trouble.
(I ran into trouble with this when writing 30b13fd82e243713c6a0d.)
Note down in the routerstatus_t of a node if the router supports the HSIntro=4
version for the ed25519 authentication key and HSDir=2 version for the v3
descriptor supports.
Signed-off-by: David Goulet <dgoulet@torproject.org>
Some of those defines will be used by the v3 HS protocol so move them to a
common header out of rendservice.c. This is also ground work for prop224
service implementation.
Signed-off-by: David Goulet <dgoulet@torproject.org>
Another building blocks for prop224 service work. This also makes the function
takes specific argument instead of the or_option_t object.
Signed-off-by: David Goulet <dgoulet@torproject.org>
When a client tried to connect to an invalid port of an hidden service, a
warning was printed:
[warn] connection_edge_process_relay_cell (at origin) failed.
This is because the connection subsystem wants to close the circuit because
the port can't be found and then returns a negative reason to achieve that.
However, that specific situation triggered a warning. This commit prevents it
for the specific case of an invalid hidden service port.
Fixes#16706
Signed-off-by: David Goulet <dgoulet@torproject.org>
In order to avoid src/or/hs_service.o to contain no symbols and thus making
clang throw a warning, the functions are now exposed not just to unit tests.
Signed-off-by: David Goulet <dgoulet@torproject.org>
Check that route_len_for_purpose() (helper for new_route_len())
correctly fails a non-fatal bug assertion if it encounters an
unhandled circuit purpose when it is called with exit node info.
Add a new helper function route_len_for_purpose(), which explicitly
lists all of the known circuit purposes for a circuit with a chosen
exit node (unlike previously, where the default route length for a
chosen exit was DEFAULT_ROUTE_LEN + 1 except for two purposes). Add a
non-fatal assertion for unhandled purposes that conservatively returns
DEFAULT_ROUTE_LEN + 1.
Add copious comments documenting which circuits need an extra hop and
why.
Thanks to nickm and dgoulet for providing background information.
This change makes it so those those APIs will not require prior
inclusion of openssl headers. I've left some APIs alone-- those
will change to be extra-private.
The old implementation had duplicated code in a bunch of places, and
it interspersed spool-management with resource management. The new
implementation should make it easier to add new resource types and
maintain the spooling code.
Closing ticket 21651.
When calculating max sampled size, Tor would only count the number of
bridges in torrc, without considering that our state file might already
have sampled bridges in it. This caused problems when people swap
bridges, since the following error would trigger:
[warn] Not expanding the guard sample any further; just hit the
maximum sample threshold of 1
This patch changes the way we decide when to check for whether it's time
to rotate and/or expiry our onion keys. Due to proposal #274 we can now
have the keys rotate at different frequencies than before and we thus
do the check once an hour when our Tor daemon is running in server mode.
This should allow us to quickly notice if the network consensus
parameter have changed while we are running instead of having to wait
until the current parameters timeout value have passed.
See: See: https://bugs.torproject.org/21641
This patch adds a new timer that is executed when it is time to expire
our current set of old onion keys. Because of proposal #274 this can no
longer be assumed to be at the same time we rotate our onion keys since
they will be updated less frequently.
See: https://bugs.torproject.org/21641
This patch adds an API to get the current grace period, in days, defined
as the consensus parameter "onion-key-grace-period-days".
As per proposal #274 the values for "onion-key-grace-period-days" is a
default value of 7 days, a minimum value of 1 day, and a maximum value
defined by other consensus parameter "onion-key-rotation-days" also
defined in days.
See: https://bugs.torproject.org/21641
This patch turns `MIN_ONION_KEY_LIFETIME` into a new function
`get_onion_key_lifetime()` which gets its value from a network consensus
parameter named "onion-key-rotation-days". This allows us to tune the
value at a later point in time with no code modifications.
We also bump the default onion key lifetime from 7 to 28 days as per
proposal #274.
See: https://bugs.torproject.org/21641
This patch fixes a regression described in bug #21757 that first
appeared after commit 6e78ede73f which was an attempt to fix bug #21654.
When switching from buffered I/O to direct file descriptor I/O our
output strings from get_string_from_pipe() might contain newline
characters (\n). In this patch we modify tor_get_lines_from_handle() to
ensure that the function splits the newly read string at the newline
character and thus might return multiple lines from a single call to
get_string_from_pipe().
Additionally, we add a test case to test_util_string_from_pipe() to
ensure that get_string_from_pipe() correctly returns multiple lines in a
single call.
See: https://bugs.torproject.org/21757
See: https://bugs.torproject.org/21654
We could use one of these for holding "junk" descriptors and
unparseable things -- but we'll _need_ it for having cached
consensuses and diffs between them.
There was a frequent block of code that did "find the next router
line, see if we've hit the end of the list, get the ID hash from the
line, and enforce well-ordering." Per Ahf's review, I'm extracting
it to its own function.
Previously, we operated on smartlists of NUL-terminated strings,
which required us to copy both inputs to produce the NUL-terminated
strings. Then we copied parts of _those_ inputs to produce an
output smartlist of NUL-terminated strings. And finally, we
concatenated everything into a final resulting string.
This implementation, instead, uses a pointer-and-extent pattern to
represent each line as a pointer into the original inputs and a
length. These line objects are then added by reference into the
output. No actual bytes are copied from the original strings until
we finally concatenate the final result together.
Bookkeeping structures and newly allocated strings (like ed
commands) are allocated inside a memarea, to avoid needless mallocs
or complicated should-I-free-this-or-not bookkeeping.
In my measurements, this improves CPU performance by something like
18%. The memory savings should be much, much higher.
This takes two fuzzers: one which generates a diff and makes sure it
works, and one which applies a diff.
So far, they won't crash, but there's a bug in my
string-manipulation code someplace that I'm having to work around,
related to the case where you have a blank line at the end of a
file, or where you diff a file with itself.
Also, add very strict split/join functions, and totally forbid
nonempty files that end with somethig besides a newline. This
change is necessary to ensure that diff/apply are actually reliable
inverse operations.
The 2-line diff changs is needed to make the unit tests actually
test the cases that they thought they were testing.
The bogus free was found while testing those cases
(This commit was extracted by nickm based on the final outcome of
the project, taking only the changes in the files touched by this
commit from the consdiff_rebased branch. The directory-system
changes are going to get worked on separately.)
Windows doesn't let you check the socket error for a socket with
WSAGetLastError() and getsockopt(SO_ERROR). But
getsockopt(SO_ERROR) clears the error on the socket, so you can't
call it more than once per error.
When we introduced recv_ni to help drain alert sockets, back in
0.2.6.3-alpha, we had the failure path for recv_ni call getsockopt()
twice, though: once to check for EINTR and one to check for EAGAIN.
Of course, we never got the eagain, so we treated it as an error,
and warned about: "No error".
The fix here is to have these functions return -errno on failure.
Fixes bug 21540; bugfix on 0.2.6.3-alpha.
The 64-bit load and store code was generating pretty bad output with
my compiler, so I extracted the code from csiphash and used that instead.
Close ticket 21737
So we require that SMARTLIST_FOREACH_END() have the name of the loop
variable in it. But right now the only enforcement for that is to
clear the variable at the end of the loop, which is really not
sufficient: I spent 45 minutes earlier today debugging an issue
where I had said:
SMARTLIST_FOREACH_BEGIN(spool, spooled_resource_t *, spooled) {
...
} SMARTLIST_FOREACH_END(spool);
This patch makes it so that ONLY loop variables can be used, by
referring to the _sl_idx variable.
Also, relaxed the checks of encrypted_data_length_is_valid() since now
only one encrypted section has padding requirements and we don't
actually care to check that all the padding is there.
Consider starting code review from function encode_superencrypted_data().
- Refactor our HS desc crypto funcs to be able to differentiate between
the superencrypted layer and the encrypted layer so that different
crypto constants and padding is used in each layer.
- Introduce some string constants.
- Add some comments.
As part of the work for proposal #274 we are going to remove the need
for MIN_ONION_KEY_LIFETIME and turn it into a dynamic value defined by a
consensus parameter.
See: https://bugs.torproject.org/21641
The bridges+ipv6-min integration test has a client with bridges:
Bridge 127.0.0.1:5003
Bridge [::1]:5003
which got stuck in guard_selection_have_enough_dir_info_to_build_circuits()
because it couldn't find the descriptor of both bridges.
Specifically, the guard_has_descriptor() function could not find the
node_t of the [::1] bridge, because the [::1] bridge had no identity
digest assigned to it.
After further examination, it seems that during fetching the descriptor
for our bridges, we used the CERTS cell to fill the identity digest of
127.0.0.1:5003 properly. However, when we received a CERTS cell from
[::1]:5003 we actually ignored its identity digest because the
learned_router_identity() function was using
get_configured_bridge_by_addr_port_digest() which was returning the
127.0.0.1 bridge instead of the [::1] bridge (because it prioritizes
digest matching over addrport matching).
The fix replaces get_configured_bridge_by_addr_port_digest() with the
recent get_configured_bridge_by_exact_addr_port_digest() function. It
also relaxes the constraints of the
get_configured_bridge_by_exact_addr_port_digest() function by making it
return bridges whose identity digest is not yet known.
By using the _exact_() function, learned_router_identity() actually
fills in the identity digest of the [::1] bridge, which then allows
guard_has_descriptor() to find the right node_t and verify that the
descriptor is there.
FWIW, in the bridges+ipv6-min test both 127.0.0.1 and [::1] bridges
correspond to the same node_t, which I guess makes sense given that it's
actually the same underlying bridge.
This patch removes the `tor_fgets()` wrapper around `fgets(3)` since it
is no longer needed. The function was created due to inconsistency
between the returned values of `fgets(3)` on different versions of Unix
when using `fgets(3)` on non-blocking file descriptors, but with the
recent changes in bug #21654 we switch from unbuffered to direct I/O on
non-blocking file descriptors in our utility module.
We continue to use `fgets(3)` directly in the geoip and dirserv module
since this usage is considered safe.
This patch also removes the test-case that was created to detect
differences in the implementation of `fgets(3)` as well as the changes
file since these changes was not included in any releases yet.
See: https://bugs.torproject.org/21654
This patch changes a number of read loops in the util module to use
less-than comparison instead of not-equal-to comparison. We do this in
the case that we have a bug elsewhere that might cause `numread` to
become larger than `count` and thus become an infinite loop.
This patch removes the buffered I/O stream usage in process_handle_t and
its related utility functions. This simplifies the code and avoids racy
code where we used buffered I/O on non-blocking file descriptors.
See: https://bugs.torproject.org/21654
This patch modifies `tor_read_all_handle()` to use read(2) instead of
fgets(3) when reading the stdout from the child process. This should
eliminate the race condition that can be triggered in the 'slow/util/*'
tests on slower machines running OpenBSD, FreeBSD and HardenedBSD.
See: https://bugs.torproject.org/21654
Make hidden services with 8 to 10 introduction points check for failed
circuits immediately after startup. Previously, they would wait for 5
minutes before performing their first checks.
Fixes bug 21594; bugfix on commit 190aac0eab in Tor 0.2.3.9-alpha.
Reported by alecmuffett.
This change is the only one necessary to allow future versions of
the microdescriptor consensus to replace every 'published' date with
e.g. 2038-01-01 00:00:00; this will save 50-75% in compressed
microdescriptor diff size, which is quite significant.
This commit is a minimal change for 0.2.9; future series will
reduce the use of the 'published' date even more.
Implements part of ticket 21642; implements part of proposal 275.
Previously, they would stop checking when they exceeded their intro point
creation limit.
Fixes bug 21596; bugfix on commit d67bf8b2f2 in Tor 0.2.7.2-alpha.
Reported by alecmuffett.
Previously, they would stop checking when they exceeded their intro point
creation limit.
Fixes bug 21596; bugfix on commit d67bf8b2f2 in Tor 0.2.7.2-alpha.
Reported by alecmuffett.
This patch resets `buf` in test_util_fgets_eagain() after each succesful
ivocation to avoid stray artifacts left in the buffer by erroneous
tor_fgets() calls.
This patch adds the `tor_fgets()` function to our compatibility layer.
`tor_fgets()` adds an additional check for whether the error-bit have
been enabled for the given file stream, if that is the case and `errno`
is set to `EAGAIN` we make sure that we always return NULL.
Unfortunately `fgets(3)` behaves differently on different versions of
the C library.
See: https://bugs.torproject.org/21416
See: https://bugs.torproject.org/20988
In that chutney test, the bridge client is configured to connect to
the same bridge at 127.0.0.1:5003 _and_ at [::1]:5003, with no
change in transports.
That meant, I think, that the descriptor is only assigned to the
first bridge when it arrives, and never the second.
- Make sure we check at least two guards for descriptor before making
circuits. We typically use the first primary guard for circuits, but
it can also happen that we use the second primary guard (e.g. if we
pick our first primary guard as an exit), so we should make sure we
have descriptors for both of them.
- Remove BUG() from the guard_has_descriptor() check since we now know
that this can happen in rare but legitimate situations as well, and we
should just move to the next guard in that case.
(But use bash if it's available.)
This is a workaround until we remove bash-specific code in 19699.
Fixes bug 21581; bugfix on 21562, not in any released version of tor.
Previously I'd made a bad assumption in the implementation of
prop271 in 0.3.0.1-alpha: I'd assumed that there couldn't be two
guards with the same identity. That's true for non-bridges, but in
the bridge case, we allow two bridges to have the same ID if they
have different addr:port combinations -- in order to have the same
bridge ID running multiple PTs.
Fortunately, this assumption wasn't deeply ingrained: we stop
enforcing the "one guard per ID" rule in the bridge case, and
instead enforce "one guard per <id,addr,port>".
We also needed to tweak our implementation of
get_bridge_info_for_guard, since it made the same incorrect
assumption.
Fixes bug 21027; bugfix on 0.3.0.1-alpha.
This feature makes it possible to turn off memory sentinels (like
those used for safety in buffers.c and memarea.c) when fuzzing, so
that we can catch bugs that they would otherwise prevent.
Since 0.2.4.11-alpha (in 0196647970) we've tried to randomize
the start time to up to some time in the past. But unfortunately we
allowed the start time to be in the future as well, which isn't
really legit.
The new behavior lets the start time be be up to
MAX(cert_lifetime-2days, 0) in the past, but never in the future.
Fixes bug 21420; bugfix on 0.2.4.11-alpha.
Teor thinks that this connection_dirserv_add_dir_bytes_to_outbuf()
might be the problem, if the "remaining" calculation underflows. So
I'm adding a couple of checks there, and improving the casts.
When encoding a legacy ESTABLISH_INTRO cell, we were using the sizeof() on a
pointer instead of using the real size of the destination buffer leading to an
overflow passing an enormous value to the signing digest function.
Fortunately, that value was only used to make sure the destination buffer
length was big enough for the key size and in this case it always was because
of the overflow.
Fixes#21553
Signed-off-by: David Goulet <dgoulet@torproject.org>
strto* and _atoi64 accept +, -, and various whitespace before numeric
characters. And permitted whitespace is different between POSIX and Windows.
Fixes bug 21507 and part of 21508; bugfix on 0.0.8pre1.
This patch makes us store the number of sent and received RELAY_DATA
cells used for directory connections. We log the numbers after we have
received an EOF in connection_dir_client_reached_eof() from the
directory server.
Instead of returning 404 error code, this led to a NULL pointer being used and
thus a crash of tor.
Fixes#21471
Signed-off-by: David Goulet <dgoulet@torproject.org>
Fixes bug 20894; bugfix on 0.2.0.16-alpha.
We already applied a workaround for this as 20834, so no need to
freak out (unless you didn't apply 20384 yet).
This should be "impossible" without making a SHA1 collision, but
let's not keep the assumption that SHA1 collisions are super-hard.
This prevents another case related to 21278. There should be no
behavioral change unless -ftrapv is on.
I think this one probably can't underflow, since the input ranges
are small. But let's not tempt fate.
This patch also replaces the "cmp" functions here with just "eq"
functions, since nothing actually checked for anything besides 0 and
nonzero.
Related to 21278.
Fix for TROVE-2017-001 and bug 21278.
(Note: Instead of handling signed ints "correctly", we keep the old
behavior, except for the part where we would crash with -ftrapv.)
This is a purely cosmetic patch that changes RELAY_BEGINDIR in various
comments to RELAY_BEGIN_DIR, which should make it easier to grep for the
symbols.
According to 21116, it seems to be needed for Wheezy Raspbian build. Also,
manpage of socket(2) does confirm that this errno value should be catched as
well in case of no support from the OS of IPv4 or/and IPv6.
Fixes#21116
Signed-off-by: David Goulet <dgoulet@torproject.org>
This patch ensures that we log the size of the inbuf when a directory
client have reached EOF on the connection.
See: https://bugs.torproject.org/21206
This patch makes the log-statements in `connection_dir_client_reached_eof`
more explicit by writing "body size" instead of just "size" which could
be confused as being the size of the entire response, which would
include HTTP status-line and headers.
See: https://bugs.torproject.org/21206
This reverts commit 5446cb8d3d.
The underlying revert was done in 0.2.6, since we aren't backporting
seccomp2 loosening fixes to 0.2.6. But the fix (for 17354) already
went out in 0.2.7.4-rc, so we shouldn't revert it in 0.2.7.
This patch adds a debug log statement when sending a request to a
directory server. The information logged includes: the payload size (if
available), the total size of the request, the address and port of the
directory server, and the purpose of the directory connection.
See: https://bugs.torproject.org/21206
maint-0.2.7-redux is an attempt to try to re-create a plausible
maint-0.2.7 branch. I've started from the tor-0.2.7.6, and then I
merged maint-0.2.6 into the branch.
This has produced 2 conflicts: one related to the
rendcommon->rendcache move, and one to the authority refactoring.
If a hostname is supplied to tor-resolve which is too long, it will be
silently truncated, resulting in a different hostname lookup:
$ tor-resolve $(python -c 'print("google.com" + "m" * 256)')
If tor-resolve uses SOCKS5, the length is stored in an unsigned char,
which overflows in this case and leads to the hostname "google.com".
As this one is a valid hostname, it returns an address instead of giving
an error due to the invalid supplied hostname.
Check size argument to memwipe() for underflow.
Closes bug #18089. Reported by "gk", patch by "teor".
Bugfix on 0.2.3.25 and 0.2.4.6-alpha (#7352),
commit 49dd5ef3 on 7 Nov 2012.
The length of auth_data from an INTRODUCE2 cell is checked when the
auth_type is recognized (1 or 2), but not for any other non-zero
auth_type. Later, auth_data is assumed to have at least
REND_DESC_COOKIE_LEN bytes, leading to a client-triggered out of bounds
read.
Fixed by checking auth_len before comparing the descriptor cookie
against known clients.
Fixes#15823; bugfix on 0.2.1.6-alpha.
Bug 21242 occurred because we asserted that extend_info_from_node()
had succeeded...even though we already had the code to handle such a
failure. We fixed that in 93b39c5162.
But there were four other cases in our code where we called
extend_info_from_node() and either tor_assert()ed that it returned
non-NULL, or [in one case] silently assumed that it returned
non-NULL. That's not such a great idea. This patch makes those
cases check for a bug of this kind instead.
Fixes bug 21372; bugfix on 0.2.3.1-alpha when
extend_info_from_node() was introduced.
Once a second, we go over all services and consider the validity of the intro
points. Now, also try to remove expiring nodes that have no more circuit
associated to them. This is possible if we moved an intro point object
previously to that list and the circuit actually timed out or was closed by
the introduction point itself.
Signed-off-by: David Goulet <dgoulet@torproject.org>
In rend_service_intro_has_opened(), this is subject to a possible underflow
because of how the if() casts the results. In the case where the expiring
nodes list length is bigger than the number of IP circuits, we end up in the
following situation where the result will be cast to an unsigned int. For
instance, "5 - 6" is actually a BIG number.
Ultimately leading to closing IP circuits in a non stop loop.
Partially fixes#21302.
Signed-off-by: David Goulet <dgoulet@torproject.org>
Previously the dirserv_orconn_tls_done() function would skip routers
when they advertised an ed25519 key but didn't present it during the
link handshake. But that covers all versions between 0.2.7.2-alpha
and 0.2.9.x inclusive!
Fixes bug 21107; bugfix on 0.3.0.1-alpha.
Because we don't allow client functionalities in non anonymous mode,
recommending Tor2web is a bad idea.
If a user wants to use Tor2web as a client (losing all anonymity), it should
run a second tor, not use it with a single onion service tor.
Fixes#21294.
Signed-off-by: David Goulet <dgoulet@torproject.org>
This patch adds checks for expected log messages for failure cases of
different ill-formed ESTABLISH_INTRO cell's.
See: https://bugs.torproject.org/21266
In rend_consider_services_intro_points(), we had a possible interger underflow
which could lead to creating a very large number of intro points. We had a
safe guard against that *except* if the expiring_nodes list was not empty
which is realistic thing.
This commit removes the check on the expiring nodes length being zero. It's
not because we have an empty list of expiring nodes that we don't want to open
new IPs. Prior to this check, we remove invalid IP nodes from the main list of
a service so it should be the only thing to look at when deciding if we need
to create new IP(s) or not.
Partially fixes#21302.
Signed-off-by: David Goulet <dgoulet@torproject.org>
Determining if OpenSSL structures are opaque now uses an autoconf check
instead of comparing the version number. Some definitions have been
moved to their own check as assumptions which were true for OpenSSL
with opaque structures did not hold for LibreSSL. Closes ticket 21359.
This disregards anything smaller than an IPv6 /64, and rejects ports that
are rejected on an IPv6 /16 or larger.
Adjust existing unit tests, and add more to cover exceptional cases.
No IPv4 behaviour changes.
Fixes bug 21357
This interim fix results in too many IPv6 rejections.
No behaviour change for IPv4 counts, except for overflow fixes that
would require 4 billion redundant 0.0.0.0/0 policy entries to trigger.
Part of 21357
Stop modifying the value of our torrc option HiddenServiceStatistics just
because we're not a bridge or relay. This bug was causing Tor Browser users to
write "HiddenServiceStatistics 0" in their torrc files as if they had chosen
to change the config.
Fixes#21150
Signed-off-by: David Goulet <dgoulet@torproject.org>
Since we can call this function more than once before we update all
the confirmed_idx fields, we can't rely on all the relays having an
accurate confirmed_idx.
Fixes bug 21129; bugfix on 0.3.0.1-alpha
We need to call it before nt_service_parse_options(), since
nt_service_parse_options() can call back into nt_service_main(),
which calls do_main_loop().
Fixes bug 21356; bugfix on 0.2.9.1-alpha.
In addition to not wanting to build circuits until we can see most
of the paths in the network, and in addition to not wanting to build
circuits until we have a consensus ... we shouldn't build circuits
till all of our (in-use) primary guards have descriptors that we can
use for them.
This is another bug 21242 fix.
Actually, it's _fine_ to use a descriptorless guard for fetching
directory info -- we just shouldn't use it when building circuits.
Fortunately, we already have a "usage" flag that we can use here.
Partial fix for bug 21242.
This relates to the 21242 fix -- entry_guard_pick_for_circuit()
should never yield nodes without descriptors when the node is going
to be used for traffic, since we won't be able to extend through
them.
This assertion triggered in the (error) case where we got a result
from guards_choose_guard() without a descriptor. That's not
supposed to be possible, but it's not worth crashing over.
I broke "GETCONF *Port" in 20956, when I made SocksPort a
subordinate option of the virtual option SocksPortLines, so that I
could make SocksPort and __SocksPort provide qthe same
functionality. The problem was that you can't pass a subordinate
option to GETCONF.
So, this patch fixes that by letting you fetch subordinate options.
It won't always be meaningful to consider these options
out-of-context, but that can be the controller-user's
responsibility to check.
Closes ticket 21300.
If there are no ephemeral or detached onion services, then
"GETINFO onions/current" or "GETINFO onions/detached" should
return an empty list instead of an error
If tor_mmap_file is called with a file which is larger than SIZE_MAX,
only a small part of the file will be memory-mapped due to integer
truncation.
This can only realistically happen on 32 bit architectures with large
file support.
If a hostname is supplied to tor-resolve which is too long, it will be
silently truncated, resulting in a different hostname lookup:
$ tor-resolve $(python -c 'print("google.com" + "m" * 256)')
If tor-resolve uses SOCKS5, the length is stored in an unsigned char,
which overflows in this case and leads to the hostname "google.com".
As this one is a valid hostname, it returns an address instead of giving
an error due to the invalid supplied hostname.
When marking for close a circuit, the reason value, a integer, was assigned to
a uint16_t converting any negative reasons (internal) to the wrong value. On
the HS side, this was causing the client to flag introduction points to be
unreachable as the internal reason was wrongfully converted to a positive
16bit value leading to flag 2 out of 3 intro points to be unreachable.
Fixes#20307 and partially fixes#21056
Signed-off-by: David Goulet <dgoulet@torproject.org>
- Also remove LCOV marks from blocks of code that can be reachable by tests
if we mock relay_send_command_from_edge().
Signed-off-by: David Goulet <dgoulet@torproject.org>
With the previous commit, we validate the circuit _before_ calling
rend_mid_introduce() which handles the INTRODUCE1 payload.
Signed-off-by: David Goulet <dgoulet@torproject.org>
Adds a better semantic and it also follows the same interface for the
INTRODUCE1 API which is circuit_is_suitable_for_introduce1().
Signed-off-by: David Goulet <dgoulet@torproject.org>
That way, when we are parsing the options and LearnCircuitBuildTimeout is set
to 0, we don't assert trying to get the options list with get_options().
Fixes#21062
Signed-off-by: David Goulet <dgoulet@torproject.org>
This patch refactors duplicated code, to check if a given router
supports fetching the extra-info document, into a common macro called
SKIP_MISSING_TRUSTED_EXTRAINFO.
This patch generalizes the two functions
router_is_already_dir_fetching_rs and router_is_already_dir_fetching_ds
into a single function, router_is_already_dir_fetching_, by lifting the
passing of the IPv4 & IPv6 addresses and the directory port number to
the caller.
So far, the TTLs for both A and AAAA records were not initialised,
resulting in exit relays sending back the value 60 to Tor clients. This
also impacts exit relays' DNS cache -- the expiry time for all domains
is set to 60.
This fixes <https://bugs.torproject.org/19025>.
The server-side clipping now clamps to one of two values, both
for what to report, and how long to cache.
Additionally, we move some defines to dns.h, and give them better
names.
An operator couldn't set the number of introduction point below the default
value which is 3. With this commit, from 0 to the hardcoded maximum is now
allowed.
Closes#21033
Signed-off-by: David Goulet <dgoulet@torproject.org>
Our config code is checking correctly at DataDirectoryGroupReadable but then
when we initialize the keys, we ignored that option ending up at setting back
the DataDirectory to 0700 instead of 0750. Patch by "redfish".
Fixes#19953
Signed-off-by: David Goulet <dgoulet@torproject.org>
In the past, when we exhausted all guards in our sampled set, we just
waited there till we mark a guard for retry again (usually takes 10 mins
for a primary guard, 1 hour for a non-primary guard). This patch marks
all guards as maybe-reachable when we exhaust all guards (this can
happen when network is down for some time).
Let A = UseBridges
Let B = ClientUseIPv4
Then firewall_is_fascist_impl expands and simplifies to:
B || (!(A || ...) && A)
B || (!A && ... && A)
B || 0
B
The microdesc consensus does not contain any IPv6 addresses.
When a client has a microdesc consensus but no microdescriptor, make it
use the hard-coded IPv6 address for the node (if available).
(Hard-coded addresses can come from authorities, fallback directories,
or configured bridges.)
If there is no hard-coded address, log a BUG message, and fail the
connection attempt. (All existing code checks for a hard-coded address
before choosing a node address.)
Fixes 20996, fix on b167e82 from 19608 in 0.2.8.5-alpha.
It is no longer possible for the IPv6 preference options to differ from the
IPv6 usage: preferring IPv6 implies possibly using IPv6.
Also remove the corresponding unit test warning message checks.
(But keep the unit tests themselves - they now run without warnings.)
In order to help an HS operator knowing if the application configured behind
it is not working properly, add a log at warning level for the connection
refused or timeout case. This log will only be printed if a client connection
fails and is rate limited.
Closes#21019
Signed-off-by: David Goulet <dgoulet@torproject.org>
In 8a0ea3ee43 we added a
temp_service_list local variable to rend_config_services, but we
didn't add a corresponding "free" for it to all of the exit paths.
Fixes bug 20987; bugfix on 0.3.0.1-alpha.
This commit adds 3 unit tests which validates a wrong signature length, a
wrong authentication key length and a wrong MAC in the cell.
Closes#20992
Signed-off-by: David Goulet <dgoulet@torproject.org>
Add the "sr/current" and "sr/previous" keys for the GETINFO command in order
to get through the control port the shared random values from the consensus.
Closes#19925
Signed-off-by: David Goulet <dgoulet@torproject.org>
Some DNS NXDOMAIN hijackers hijack truly ridiculous domains, like
"invalid-stuff!!" or "1.2.3.4.5". This would provoke unit test
failures where we used addresses like that to force
tor_addr_lookup() to fail. The fix, for testing, is to mock
tor_addr_lookup() with a variant that always fails when it gets
a name with a !.
Fixes bugs 20862 and 20863.
These relays need to be contacted over their ORPorts using a begindir
connection, and relays try not to use begindir connections.
Fixes bug 20711; bugfix on 0.2.8.2-alpha.
Because <unset> makes more sense than AAAAAAAAAAAAAAAAAAA...
(I have indeed verified that ed25519_fmt() is only used for
logging. This patch also clarifies the intention that ed25519_fmt()
is only for logging.
Closes ticket 21037.
We switched these to be "if (1) " a while back, so we could keep
the indentation and avoid merge conflicts. But it's nice to clean
up from time to time.
Previously we were marking directory guards up in
..._process_inbuf(), but that's wrong: we call that function on
close as well as on success. Instead, we're marking the dirguard up
only after we parse the HTTP headers. Closes 20974.
The abort handler masks the exit status of the backtrace generator by
capturing the abort signal from the backtrace handler and exiting with
zero. Because the output of the backtrace generator is meant to be piped
to `bt_test.py`, its exit status is unimportant and is currently
ignored.
The abort handler calls `exit(3)` which is not asynchronous-signal-safe
and calling it in this context is undefined behavior [0].
Closes ticket 21026.
[0] https://www.securecoding.cert.org/confluence/x/34At
When marking for close a circuit, the reason value, a integer, was assigned to
a uint16_t converting any negative reasons (internal) to the wrong value. On
the HS side, this was causing the client to flag introduction points to be
unreachable as the internal reason was wrongfully converted to a positive
16bit value leading to flag 2 out of 3 intro points to be unreachable.
Fixes#20307 and partially fixes#21056
Signed-off-by: David Goulet <dgoulet@torproject.org>
First, this commit moves the code used to prune the service list when
reloading Tor (HUP signal for instance) to a function from
rend_config_services().
Second, fix bug #21054, improve the code by using the newly added
circuit_get_next_service_intro_circ() function instead of poking at the global
list directly and add _many_ more comments.
Fixes#21054.
Signed-off-by: David Goulet <dgoulet@torproject.org>
This came up on #21035, where somebody tried to build on a linux
system with kernel headers including CLOCK_MONOTONIC_COARSE, then
run on a kernel that didn't support it.
I've adopted a belt-and-suspenders approach here: we detect failures
at initialization time, and we also detect (loudly) failures later on.
Fixes bug 21035; bugfix on 0.2.9.1-alpha when we started using
monotonic time.
This helps protect against bugs where any part of a buf_t's memory
is passed to a function that expects a NUL-terminated input.
It also closes TROVE-2016-10-001 (aka bug 20384).
Replace the 81 remaining fallbacks of the 100 originally introduced
in Tor 0.2.8.3-alpha in March 2016, with a list of 177 fallbacks
(123 new, 54 existing, 27 removed) generated in December 2016.
Resolves ticket 20170.
In get_token(), we could read one byte past the end of the
region. This is only a big problem in the case where the region
itself is (a) potentially hostile, and (b) not explicitly
nul-terminated.
This patch fixes the underlying bug, and also makes sure that the
one remaining case of not-NUL-terminated potentially hostile data
gets NUL-terminated.
Fix for bug 21018, TROVE-2016-12-002, and CVE-2016-1254
They broke stem, and breaking application compatibility is usually a
bad idea.
This reverts commit 6e10130e18,
commit 78a13df158, and
commit 62f52a888a.
We might re-apply this later, if all the downstream tools can handle
it, and it turns out to be useful for some reason.
I got confused when I saw my Tor saying it was opening a file
that doesn't exist. It turns out it isn't opening it, it's just
calling open() on it and then moving on when it's not there.
Since both the client and service will use that data structure to store the
descriptor decoded data, only the public keys are common to both.
Fixes#20572.
Signed-off-by: David Goulet <dgoulet@torproject.org>
The "sig_len" fields was moved below the "end_sig_fields" in the trunnel
specification so when signing the cell content, the function generating such a
cell needed to be adjust.
Closes#20991
Signed-off-by: David Goulet <dgoulet@torproject.org>
Previously, we had NumEntryGuards kind of hardwired to 1. Now we
have the code (but not the configuarability) to choose randomly from
among the first N primary guards that would work, where N defaults
to 1.
Part of 20831 support for making NumEntryGuards work again.
Since we already had a separate function for getting the universe of
possible guards, all we had to do was tweak it to handle very the
GS_TYPE_RESTRICTED case.
asn found while testing that this function can be reached with
GUARD_STATE_COMPLETE circuits; I believe this happens when
cannibalization occurs.
The added complexity of handling one more state made it reasonable
to turn the main logic here into a switch statement.
Letting the maximum sample size grow proportionally to the number of
guards defeats its purpose to a certain extent. Noted by asn during
code review.
Fixes bug 20920; bug not in any released (or merged) version of Tor.
- Correctly maintain the previous guard selection in choose_guard_selection().
- Print bridge identifier instead of nothing in entry_guard_describe()._
If a complete circuit C2 doesn't obey the restrictions of C1, then
C2 cannot block C1.
The patch here is a little big-ish, since we can no longer look
through all the complete circuits and all the waiting circuits on a
single pass: we have to find the best waiting circuit first.
This is an important thing I hadn't considered when writing prop271:
sometimes you have to restrict what guard you use for a particular
circuit. Most frequently, that would be because you plan to use a
certain node as your exit, and so you can't choose that for your
guard.
This change means that the upgrade-waiting-circuits algorithm needs
a slight tweak too: circuit A cannot block circuit B from upgrading
if circuit B needs to follow a restriction that circuit A does not
follow.
I had been asking myself, "hey, doesn't the new code need to look at
this "info" parameter? The old code did!" But it turns out that the
old code hasn't, since 05f7336624.
So instead of "support this!" the comment now says "we can remove
this!"
George pointed out that (-1,0,1) for (never usable, maybe usable
later, usable right now) was a pretty rotten convention that made
the code harder to read.
Here we handle most (all?) of the remaining tasks, and fix some
bugs, in the prop271 bridge implementation.
* We record bridge identities as we learn them.
* We only call deprecated functions from bridges.c when the
deprecated guard algorithm is in use.
* We update any_bridge_descriptors_known() and
num_bridges_usable() to work correctly with the new backend
code. (Previously, they called into the guard selection logic.
* We update bridge directory fetches to work with the new
guard code.
* We remove some erroneous assertions where we assumed that we'd
never load a guard that wasn't for the current selection.
Also, we fix a couple of typos.
Still missing is functionality for picking bridges when we don't
know a descriptor for them yet, and functionality for learning a
bridge ID.
Everything else remains (basically) the same. Neat!
This includes:
* making bridge_info_t exposed but opaque
* allowing guards where we don't know an identity
* making it possible to learn the identity of a guard
* creating a guard that lacks a node_t
* remembering a guard's address and port.
* Looking up a guard by address and port.
* Only enforcing the rule that we need a live consensus to update
the "listed" status for guards when we are not using bridges.
This is safe, because no entry_guard_t ever outlives its
guard_selection_t.
I want this because now that multiple guard selections can be active
during one tor session, we should make sure that any information we
register about guards is with respect to the selection that they came
from.
Currently, this code doesn't actually have the contexts behave
differently, (except for the legacy context), but it does switch
back and forth between them nicely.
If a guard becomes primary as a result of confirming it, consider
the circuit through that guard as a primary circuit.
Also, note open questions on behavior when confirming nonprimary guards
Some of these will get torrc options to override them too; this
is just the mechanical conversion.
Also, add documentation for a couple of undocumented (but now used)
parameters.
Act I.
" But that I am forbid
To tell the secrets of my prison-house,
I could a tale unfold..."
Here's the bug: sometimes, rend_cache/store_v2_desc_as_client would
say:
"Dec 15 08:31:26.147 [warn] rend_cache_store_v2_desc_as_client():
Bug: Couldn't decode base32 [scrubbed] for descriptor id. (on Tor
0.3.0.0-alpha-dev 4098bfa260)"
When we merged ade5005853 back in 0.2.8.1-alpha, we added that
test: it mangles the hidden service ID for a hidden service, and
ensures that when the descriptor ID doesn't match the descriptor's
key, we don't store the descriptor.
How did it mangle the descriptor ID? By doing
desc_id_base32[0]++;
So, if the hidden service ID started with z or 7, we'd wind up with an
invalid base32 string, and get the warning. And if it started with
any other character, we wouldn't.
That there is part 1 of the bug: in 2/32 cases, we'd get a BUG
warning. But we wouldn't display it, since warnings weren't shown
from the unit tests.
Act II.
"Our indiscretion sometime serves us well,
When our deep plots do pall"
Part two: in 0.2.9.3-alpha, for part of #19999, we turned on BUG
warnings in the unit tests, so that we'd actually start seeing them.
At this point we also began to consider each BUG warning that made
it through the unit tests to be an actual bug. So before this
point, we wouldn't actually notice anything happening in those 2/32
cases.
So, at this point it was a nice random _visible_ bug.
Act III.
"Our thoughts are ours, their ends none of our own"
In acbb60cd63, which was part of my prop220 work, I
changed how RSA key generation worked in the unit tests. While
previously we'd use pre-made RSA keys in some cases, this change
made us use a set of pregenerated RSA keys for _all_ 1024 or 2048
keys, and to return them in a rotation when Tor tried to generate a
key.
And now we had the heisenbug: anything that affected the number of
pregenerated keys that we had yielded before reaching
rend_cache/store_v2_desc_as_client would make us return a different
key, which would give us a different base32 ID, which would make the
bug occur, or not. So as we added or removed test cases, the bug
might or might not happen.
So yeah. Don't mangle a base32 ID like that. Do it this way instead.
The new HS circuitmap API replaces old public functions as follows:
circuit_clear_rend_token -> hs_circuitmap_remove_circuit
circuit_get_rendezvous -> hs_circuitmap_get_rend_circ
circuit_get_intro_point -> hs_circuitmap_get_intro_circ_v2
circuit_set_rendezvous_cookie -> hs_circuitmap_register_rend_circ
circuit_set_intro_point_digest -> hs_circuitmap_register_intro_circ_v2
This commit also removes the old rendinfo code that is now unused.
It also fixes the broken rendinfo unittests.
The HS circuitmap is a hash table that maps introduction and rendezvous
tokens to specific circuits such that given a token it's easy to find
the corresponding circuit. It supports rend circuits and v2/v3 intro
circuits.
It will be used by the prop224 ESTABLISH_INTRO code to register and
lookup v3 introduction circuits.
The next commit after this removes the old code and fixes the unittests.
Please consult both commits while reviewing functionality differences
between the old and new code. Let me know if you want this rebased
differently :)
WRT architectural differences, this commit removes the rendinfo pointer
from or_circuit_t. It then adds an hs_token_t pointer and a hashtable
node for the HS circuitmap. IIUC, this adds another pointer to the
weight of or_circuit_t. Let me know if you don't like this, or if you
have suggestions on improving it.
Back when Roger had do do most of our testing on the moria host, we
needed a higher limit for the number of relays running on a single
IP address when that limit was shared with an authority. Nowadays,
the idea is pretty obsolete.
Also remove the router_addr_is_trusted_dir() function, which served
no other purpose.
Closes ticket 20960.
In c35fad2bde, merged in
0.2.4.7-alpha, we removed the code to parse v1 directory
objects. When we did so, we removed everything that could set the
CST_CHECK_AUTHORITY flag for check_signature_token().
So in this code, we remove the flag itself, the code to handle the
flag, and a function that only existed to handle the flag.
Circuit object wasn't freed correctly. Also, the cpath build state object
needed to be zeroed else we were freeing garbage pointers.
Closes#20936
Signed-off-by: David Goulet <dgoulet@torproject.org>
The signed_descriptor_move() was not releasing memory inside the destination
object before overwriting it with the source object. This commit adds a reset
function that free that memory inside a signed descriptor object and zero it.
Closes#20715.
Signed-off-by: David Goulet <dgoulet@torproject.org>
This was breaking the build on debian precise, since it thought that
using a 'const int' to dimension an array made that array
variable-size, and made us not get protection.
Bug not in any released version of Tor.
I will insist that this one wasn't my fault.
"Variables won't. Constants aren't." -- Osborn's Law
If a node can prove its Ed25519 identity, don't consider connections
to it canonical unless they match both identities.
Includes link handshake changes needed to avoid crashing with bug
warnings, since the tests now reach more parts of the code.
Closes ticket 20355
(Only run the connection_or_group_set_badness_() function on groups
of channels that have the same RSA and Ed25519 identities.)
There's a possible opportunity here where we might want to set a
channel to "bad" if it has no ed25519 identity and some other
channel has some. Also there's an opportunity to add a warning if
we ever have an Ed mismatch on open connections with the same RSA
ID.
This function has never gotten testing for the case where an
identity had been set, and then got set to something else. Rather
than make it handle those cases, we forbid them.
If there is some horrible bug in our ed25519 link authentication
code that causes us to label every single ed25519-having node as
non-running, we'll be glad we had this. Otherwise we can remove it
later.
This patch makes two absolutely critical changes:
- If an ed25519 identity is not as expected when creating a channel,
we call that channel unsuccessful and close it.
- When a client creating a channel or an extend cell for a circuit, we
only include the ed25519 identity if we believe that the node on
the other side supports ed25519 link authentication (from
#15055). Otherwise we will insist on nodes without the right
link protocol authenticating themselves.
- When deciding to extend to another relay, we only upgrade the
extend to extend by ed25519 ID when we know the ed25519 ID _and_
we know that the other side can authenticate.
This patch also tells directory servers, when probing nodes, to
try to check their ed25519 identities too (if they can authenticate
by ed25519 identity).
Also, handle the case where we connect by RSA Id, and learn the
ED25519 ID for the node in doing so.
I need to be able to turn on Ed25519 support in client generation
of extend cells so I can test it, but leave it off-by-default until
enough clients support it for us to turn it on for a bunch at once.
This is part of #15056 / prop#220.
- forbid extending to the previous hop by Ed25519 ID.
- If we know the Ed25519 ID for the next hop and the client doesn't,
insist on the one from the consensus.
Right now, there's only a mechanism to look for a channel where the
RSA ID matches *and* the ED ID matches. We can add a separate map
later if we want.
They added clock_gettime(), but with tv_nsec as a long, whereas
tv_usec is a __darwin_suseconds_t (a.k.a. 'int'). Now, why would
they do that? Are they preparing for a world where there are more
than 2 billion nanoseconds per second? Are they planning for having
int be less than 32 bits again? Or are they just not paying
attention to the Darwin API?
Also, they forgot to mark clock_gettime() as Sierra-only, so even
if we fixed the issue here, we'd still be stick with portability
breakage like we were for 0.2.9.
So, just disable clock_gettime() on apple.
Tor 0.2.9 has a broader range of fixes and workarounds here, but for
0.2.8, we're just going to maintain the existing behavior.
(The alternative would be to backport both
1eba088054 and
16fcbd21c9 , but the latter is kind of
a subtle kludge in the configure.ac script, and I'm not a fan of
backporting that kind of thing.)
(OpenSSL 1.1 makes EVP_CIPHER_CTX opaque, _and_ adds acceleration
for counter mode on more architectures. So it won't work if we try
the older approach, and it might help if we try the newer one.)
Fixes bug 20588.
This resolves two issues:
* the checks in rend_add_services were only being performed when adding
the service, and not when the service was validated,
(this meant that duplicate checks were not being performed, and some SETCONF
commands appeared to succeed when they actually failed), and
* if one service failed while services were being added, then the service
list would be left in an inconsistent state (tor dies when this happens,
but the code is cleaner now).
Fixes#20860.
When computing old Tor protocol line version in protover, we were looking at
0.2.7.5 twice instead of the specific case for 0.2.9.1-alpha.
Fixes#20810
Signed-off-by: David Goulet <dgoulet@torproject.org>
