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.
This patch refactors our compression tests such that deflate, gzip,
lzma, and zstd are all tested using the same code.
Additionally we use run-time checks to see if the given compression
method is supported instead of using HAVE_LZMA and HAVE_ZSTD.
See: https://bugs.torproject.org/22085
This patch adds support for measuring the approximated memory usage by
the individual `tor_zstd_compress_state_t` object instances.
See: https://bugs.torproject.org/22066
This patch fixes the documentation string for `tor_uncompress()` to
ensure that it does not explicitly mention zlib or gzip since we now
support multiple compression backends.
This patch adds support for measuring the approximated memory usage by
the individual `tor_lzma_compress_state_t` object instances.
The LZMA library provides the functions `lzma_easy_encoder_memusage()`
and `lzma_easy_decoder_memusage()` which is used to find the estimated
usage in bytes.
See: https://bugs.torproject.org/22066
We hadn't needed this before, because most getpid() callers on Linux
were looking at the vDSO version of getpid(). I don't know why at
least one version of OpenSSL seems to be ignoring the vDSO, but this
change should fix it.
Fixes bug 21943; bugfix on 0.2.5.1-alpha when the sandbox was
introduced.
The `tor_compress_state_t` data-type is used as a wrapper around the
more specialized state-types used by the various compression backends.
This patch ensures that the overhead of this "thin" wrapper type is
included in the value returned by `tor_compress_get_total_allocation()`.
See: https://bugs.torproject.org/22066
OS X's ar(1) doesn't allow us to create an archive with no object files.
This patch adds a stub file with a stub function in it to make OS X
happy again.
Since we have a streaming API for each compression backend, we don't
need a non-streaming API for each: we can build a common
non-streaming API at the front-end.
This commit adds the src/trace directory containing the basics for our tracing
subsystem. It is not used in the code base. The "src/trace/debug.h" file
contains an example on how we can map our tor trace events to log_debug().
The tracing subsystem can only be enabled by tracing framework at compile
time. This commit introduces the "--enable-tracing-debug" option that will
make all "tor_trace()" function be maped to "log_debug()".
Closes#13802
Signed-off-by: David Goulet <dgoulet@torproject.org>
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.
