This check makes it so we can reach "done" without setting "conn",
and so the "if (conn)" check will not be redundant, and so coverity
won't complain. Fixes CID 1422205. Not actually a bug.
This function -- a mock replacement used only for fuzzing -- would
have a buffer overflow if it got an RSA key whose modulus was under
20 bytes long.
Fortunately, Tor itself does not appear to have a bug here.
Fixes bug 24247; bugfix on 0.3.0.3-alpha when fuzzing was
introduced. Found by OSS-Fuzz; this is OSS-Fuzz issue 4177.
If it decrypts something that turns out to start with a NUL byte,
then decrypt_desc_layer() will return 0 to indicate the length of
its result. But 0 also indicates an error, which causes the result
not to be freed by decrypt_desc_layer()'s callers.
Since we're trying to stabilize 0.3.2.x, I've opted for the simpler
possible fix here and made it so that an empty decrypted string will
also count as an error.
Fixes bug 24150 and OSS-Fuzz issue 3994.
The original bug was present but unreachable in 0.3.1.1-alpha. I'm
calling this a bugfix on 0.3.2.1-alpha since that's the first version
where you could actually try to decrypt these descriptors.
When running "make test-network-all", test that IPv6-only clients can use
microdescriptors. IPv6-only microdescriptor client support was fixed in
tor 0.3.0.1-alpha.
Requires chutney master 61c28b9 or later.
Closes ticket 24109.
When the directory information changes, callback to the HS client subsystem so
it can check if any pending SOCKS connections are waiting for a descriptor. If
yes, attempt a refetch for those.
Fixes#23762
Signed-off-by: David Goulet <dgoulet@torproject.org>
The new decryption function performs no decryption, skips the salt,
and doesn't check the mac. This allows us to fuzz the
hs_descriptor.c code using unencrypted descriptor test, and exercise
more of the code.
Related to 21509.
The exposed get_voting_schedule() allocates and return a new object everytime
it is called leading to an awful lot of memory allocation when getting the
start time of the current round which is done for each node in the consensus.
Closes#23623
Signed-off-by: David Goulet <dgoulet@torproject.org>
At first, we put the tor_git_revision constant in tor_main.c, so
that we wouldn't have to recompile config.o every time the git
revision changed. But putting it there had unintended side effect
of forcing every program that wanted to link libor.a (including
test, test-slow, the fuzzers, the benchmarks, etc) to declare their
own tor_git_revision instance.
That's not very nice, especially since we want to start supporting
others who want to link against Tor (see 23846).
So, create a new git_revision.c file that only contains this
constant, and remove the duplicated boilerplate from everywhere
else.
Part of implementing ticket 23845.
Skip test_config_include_no_permission() when running as root, because
it will get an unexpected success from config_get_lines_include().
This affects some continuous integration setups. Fixes bug 23758.
Also demote a log message that can occur under natural causes
(if the circuit subsystem is missing descriptors/consensus etc.).
The HS subsystem will naturally retry to connect to intro points,
so no need to make that log user-facing.
Each type of scheduler implements its own static scheduler_t object and
returns a reference to it.
This commit also makes it a const pointer that is it can only change inside
the scheduler type subsystem but not outside for extra protection.
Signed-off-by: David Goulet <dgoulet@torproject.org>
This option is a list of possible scheduler type tor can use ordered by
priority. Its default value is "KIST,KISTLite,Vanilla" which means that KIST
will be used first and if unavailable will fallback to KISTLite and so on.
Signed-off-by: David Goulet <dgoulet@torproject.org>
- massive change to src/tgest/test_options.c since the sched options
were added all over the place in it
- removing the sched options caused some tests to pass/fail in new ways
so I assumed current behavior is correct and made them pass again
- ex: "ConnLimit must be greater" lines
- ex: "Authoritative directory servers must" line
- remove test_options_validate__scheduler in prep for new sched tests
Signed-off-by: David Goulet <dgoulet@torproject.org>
This commit adds a pretty advanced test for the client-side making sure that
picking intro is done properly.
This unittest also reveals a memleak on the client_pick_intro() function which
is fixed by the subsequent commit.
Signed-off-by: David Goulet <dgoulet@torproject.org>
Using a test vector in python, test both hs_build_hsdir_index() and
hs_build_hs_index().
This commit also adds the hs_build_address.py to EXTRA_DIST which was missing.
Signed-off-by: David Goulet <dgoulet@torproject.org>
Do two major improvements:
a) Make the client pick 6 HSDirs instead of just 1 and make sure they
all match the service's HSDirs.
b) Test two additional missing scenarios borrowed from the
test_reachability() test.
Change the contract of control_event_bootstrap_problem() to be more
general and to take a connection_t. New function
control_event_bootstrap_prob_or() has the specific or_connection_t
funcionality previously used.
Fix the test_build_address() test and its test vectors python script.
They were both using a bogus pubkey for building an HS address which
does not validate anymore.
Also fix a few more unittests that were using bogus onion addresses
and were failing the validation. I replaced the bogus address with
the one generated from the test vector script.
We enrich the test_client_cache() test in two ways:
a) We check that transitioning time periods also cleans up expired
descriptors in client memory.
b) We test hs_cache_lookup_as_client() instead of
lookup_v3_desc_as_client(). The former is a higher level function
which calls the latter and allows us to test deeper into the
subsystem.
OpenBSD doesn't like tricks where you use a too-wide sscanf argument
for a too-narrow array, even when you know the input string
statically. The fix here is just to use bigger buffers.
Fixes 15582; bugfix on a3dafd3f58 in 0.2.6.2-alpha.
But when clients are just starting, make them try each bridge a few times
before giving up on it.
These changes make the bridge download schedules more explicit: before
17750, they relied on undocumented behaviour and specific schedule
entries. (And between 17750 and this fix, they were broken.)
Fixes 23347, not in any released version of tor.
This test is important because it tests that upload_descriptor_to_all()
is in synch with pick_hsdir_v3(). That's not the case for the
reachability test which just compares the responsible hsdir sets.
Because of the latest changes on when we rotate, longer lifetime of
descriptors and no more overlap period, the tests needed to be improved to
test more functionnalities.
Signed-off-by: David Goulet <dgoulet@torproject.org>
First, this fixes#23372.
Second, the consensus timings for the build descriptor have been changed to
the current test can pass. More extensive tests of descriptor rotation are
coming in a commit near you because the rotation and time period logic has
been changed.
Signed-off-by: David Goulet <dgoulet@torproject.org>
This is a large and important unit test for the hidden service version
3! It tests the service reachability for a client using different
consensus timings and makes sure that the computed hashring is the same
on both side so it is actually reachable.
Signed-off-by: David Goulet <dgoulet@torproject.org>
With the latest change on how we use the HSDir index, the client and service
need to pick their responsible HSDir differently that is depending on if they
are before or after a new time period.
The overlap mode is active function has been renamed for this and test added.
Signed-off-by: David Goulet <dgoulet@torproject.org>
Because of #23387, we've realized that there is one scenario that makes
the client unable to reach the service because of a desynch in the time
period used. The scenario is as follows:
+------------------------------------------------------------------+
| |
| 00:00 12:00 00:00 12:00 00:00 12:00 |
| SRV#1 TP#1 SRV#2 TP#2 SRV#3 TP#3 |
| |
| $==========|-----------$===========|-----------$===========| |
| ^ ^ |
| C S |
+------------------------------------------------------------------+
In this scenario the HS has a newer consensus than the client, and the
HS just moved to the next TP but the client is still stuck on the old
one. However, the service is not in any sort of overlap mode so it
doesn't cover the old TP anymore, so the client is unable to fetch a
descriptor.
We've decided to solve this by extending the concept of overlap period
to be permanent so that the service always publishes two descriptors and
aims to cover clients with both older and newer consensuses. See the
spec patch in #23387 for more details.
Based on our #23387 findings, it seems like to maintain 24/7
reachability we need to employ different logic when computing hsdir
indices for fetching vs storing. That's to guarantee that the client
will always fetch the current descriptor, while the service will always
publish two descriptors aiming to cover all possible edge cases.
For more details see the next commit and the spec branch.
Signed-off-by: David Goulet <dgoulet@torproject.org>
Use the valid_after time from the consensus to get the time period number else
we might get out of sync with the overlap period that uses valid_after.
Make it an optional feature since some functions require passing a
specific time (like hs_get_start_time_of_next_time_period()).
Signed-off-by: David Goulet <dgoulet@torproject.org>
Undeprecate it;
rename it to TestingClientDNSRejectInternalAddresses;
add the old name as an alias;
reject configurations where it is set but TestingTorNetwork is not;
change the documentation accordingly.
Closes tickets 21031 and 21522.
By convention, a function that frobs a foo_t should be called
foo_frob, and it should have a foo_t * as its first argument. But
for many of the buf_t functions, the buf_t was the final argument,
which is silly.
Our convention is that functions which manipulate a type T should be
named T_foo. But the buffer functions were super old, and followed
all kinds of conventions. Now they're uniform.
Here's the perl I used to do this:
\#!/usr/bin/perl -w -i -p
s/read_to_buf\(/buf_read_from_socket\(/;
s/flush_buf\(/buf_flush_to_socket\(/;
s/read_to_buf_tls\(/buf_read_from_tls\(/;
s/flush_buf_tls\(/buf_flush_to_tls\(/;
s/write_to_buf\(/buf_add\(/;
s/write_to_buf_compress\(/buf_add_compress\(/;
s/move_buf_to_buf\(/buf_move_to_buf\(/;
s/peek_from_buf\(/buf_peek\(/;
s/fetch_from_buf\(/buf_get_bytes\(/;
s/fetch_from_buf_line\(/buf_get_line\(/;
s/fetch_from_buf_line\(/buf_get_line\(/;
s/buf_remove_from_front\(/buf_drain\(/;
s/peek_buf_startswith\(/buf_peek_startswith\(/;
s/assert_buf_ok\(/buf_assert_ok\(/;
This lets us drop the testing-only function buf_get_first_chunk_data(),
and lets us implement proto_http and proto_socks without looking at
buf_t internals.
The function was never returning an error code on failure to parse the
OutboundAddress* options.
In the process, it was making our test_options_validate__outbound_addresses()
not test the right thing.
Fixes#23366
Signed-off-by: David Goulet <dgoulet@torproject.org>
This fixes a serious bug in our hsdir set change logic:
We used to add nodes in the list of previous hsdirs everytime we
uploaded to a new hsdir and we only cleared the list when we built a new
descriptor. This means that our prev_hsdirs list could end up with 7
hsdirs, if for some reason we ended up uploading our desc to 7 hsdirs
before rebuilding our descriptor (e.g. this can happen if the set of
hsdirs changed).
After our previous hdsir set had 7 nodes, then our old algorithm would
always think that the set has changed since it was comparing a smartlist
with 7 elements against a smartlist with 6 elements.
This commit fixes this bug, by clearning the prev_hsdirs list before we
upload to all hsdirs. This makes sure that our prev_hsdirs list always
contains the latest hsdirs!
Since ssize_t is signed and might be 64 bits, we should use
tt_i64_op to make sure it's positive. Otherwise, if it is negative,
and we use tt_u64_op, we'll be treating it as a uint64_t, and we
won't detect negative values.
This fixes CID 1416338 and 1416339. Bug not in any released Tor.
We refactor the descriptor reupload logic to be similar to the v2 logic
where we update a global 'consider_republishing_rend_descriptors' flag
and then we use that to check for hash ring changes during the global
hidden service callbacks.
This fixes bugs where we would inspect the hash ring immediately as we
receive new dirinfo (e.g. consensus) but before running the hidden
service housekeeping events. That was leaving us in an inconsistent
state wrt hsdir indices and causing bugs all around.
The `test-operator-cleanup` patch, and related coccinelle patches,
don't do any checks for line length. This patch fixes the line
length issues caused by the previous commits.
This patch fixes the operator usage in src/test/*.c to use the symbolic
operators instead of the normal C comparison operators.
This patch was generated using:
./scripts/coccinelle/test-operator-cleanup src/test/*.[ch]
Only register the RP circuit when it opens and not when we send the INTRODUCE1
cell else, when re-extending to a new IP, we would register the same RP
circuit with the same cookie twice leading to the circuit being closed.
Signed-off-by: David Goulet <dgoulet@torproject.org>
We can't trigger a valid upload because it would require us to MOCK a long
list of functions ultimately not really testing the upload because we aren't
on a running network.
Signed-off-by: David Goulet <dgoulet@torproject.org>
Conflicts:
src/test/test_hs_service.c
- Add tests that ensure that SOCKS requests for v2/v3 addresses get
intercepted and handled.
- Add test that stores and lookups an HS descriptor in the client-side cache.
Signed-off-by: David Goulet <dgoulet@torproject.org>
- Also add tests for the hidserv_req subsystem.
- Introduce purge_v2_hidserv_req() wrapper to simplify v2 code.
Signed-off-by: David Goulet <dgoulet@torproject.org>
Right now there's a single warn_if_unnamed flag for
router_get_consensus_status_by_nickname() and
node_get_by_nickname(), that is nearly always 1. I've turned it
into an 'unsigned' bitfield, and inverted its sense. I've added the
flags argument to node_get_by_hex_id() too, though it does nothing
there right now.
I've removed the router_get_consensus_status_by_nickname() function,
since it was only used in once place.
This patch changes the warning behavior of GETINFO ns/name/<name>,
since all other name lookups from the controller currently warn.
Later I'm going to add more flags, for ed25519 support.
We need this func so that we recognize SOCKS conns to v3 addresses.
- Also rename rend_valid_service_id() to rend_valid_v2_service_id()
- Also move parse_extended_hostname() tests to their own unittest, and
add a v3 address to the test as well.
Signed-off-by: David Goulet <dgoulet@torproject.org>
We want to reupload our descriptor if its set of responsible HSDirs
changed to minimize reachability issues.
This patch adds a callback everytime we get new dirinfo which checks if
the hash ring changed and reuploads descriptor if needed.
Use the PATH_SEPARATOR for a path comparaison so it works with Windows as
well.
Partially fix#23223
Signed-off-by: David Goulet <dgoulet@torproject.org>
- Fix various ssize_t/size_t confusions in the tests.
- Fix a weird memset argument:
"bad_memset: Argument -16 in memset loses precision in
memset(&desc_two->blinded_kp.pubkey.pubkey, -16, 32UL)."
- Fix check_after_deref instance in check_state_line_for_service_rev_counter():
"check_after_deref: Null-checking items suggests that it may be null,
but it has already been dereferenced on all paths leading to the
check."
We used to do:
h = H(BLIND_STRING | H(A | s | B | N )
when we should be doing:
h = H(BLIND_STRING | A | s | B | N)
Change the logic so that hs_common.c does the hashing, and our ed25519
libraries just receive the hashed parameter ready-made. That's easier
than doing the hashing on the ed25519 libraries, since that means we
would have to pass them a variable-length param (depending on whether
's' is set or not).
Also fix the ed25519 test vectors since they were also double hashing.
- Fix log message format string.
- Do extra circuit purpose check.
- wipe memory in a clear function
- Make sure we don't double add intro points in our list
- Make sure we don't double close intro circuits.
- s/tt_u64_op/tt_i64_op/
To upload the descriptor we needed a state file to write the rev counters in,
but that test did not have a state file initialized.
Also fix the typo in its func name.
Signed-off-by: David Goulet <dgoulet@torproject.org>
We consider to be in overlap mode when we are in the period of time between a
fresh SRV and the beginning of the new time period (in the normal network this
is between 00:00 and 12:00 UTC). This commit edits that function to use the
above semantic logic instead of absolute times.
Signed-off-by: David Goulet <dgoulet@torproject.org>
We used to use NULL subcredential which is a terrible terrible idea. Refactor
HS unittests to use subcredentials.
Also add some non-fatal asserts to make sure that we always use subcredentials
when decoding/encoding descs.
Signed-off-by: David Goulet <dgoulet@torproject.org>
Add the entry point from the circuit subsystem of "circuit has opened" which
is for all type of hidden service circuits. For the introduction point, this
commit actually adds the support for handling those circuits when opened and
sending ESTABLISH_INTRO on a circuit.
Rendevzou point circuit aren't supported yet at this commit.
Signed-off-by: David Goulet <dgoulet@torproject.org>
Add a function for both the client and service side that is building a blinded
key from a keypair (service) and from a public key (client). Those two
functions uses the current time period information to build the key.
Signed-off-by: David Goulet <dgoulet@torproject.org>
Add a new and free function for hs_desc_intro_point_t so the service can use
them to setup those objects properly.
Signed-off-by: David Goulet <dgoulet@torproject.org>
Each piece of queued work now has an associated priority value; each
priority goes on a separate queue.
With probability (N-1)/N, the workers will take work from the highest
priority nonempty queue. Otherwise, they'll look for work in a
queue of lower priority. This behavior is meant to prevent
starvation for lower-priority tasks.
We need to keep these around for TAP and old-style hidden services,
but they're obsolete, and we shouldn't encourage anyone to use them.
So I've added "obsolete" to their names, and a comment explaining
what the problem is.
Closes ticket 23026.
There isn't much of a point of this buggy test afterall to add twice the same
service object but with a different key which ultinately can end up failing
the test because 1/N_BUCKETS of probability that we end up to put the service
in the same bucket.
Fixes#23023
Signed-off-by: David Goulet <dgoulet@torproject.org>
Fix for 22924. Bugfix on 0.2.9.1-alpha when the test was introducd
-- though it couldn't actually overflow until we fixed 17750.
Additionally, this only seems to overflow on 32-bit, and only when
the compiler doesn't re-order the (possibly dead) assignment out of
the way. We ran into it on a 32-bit ubuntu trusty builder.
Clang didn't like that we were passing uint64_t values to an API
that wanted uint32_t. GCC has either not cared, or has figured out
that the values in question were safe to cast to uint32_t.
Fixes bug22916; bugfix on 0.2.7.2-alpha.
