Now that we have a reasonable implementation for overriding the
default options for TestingTorNetwork, we don't need to modify
config_var_t structs any more. And therefore, we can have constant
format options, like reasonable people.
It's important to make sure that we don't change a config_mgr_t
after we start using it to make objects, or we could get into
inconsistent states. This feature is the start of a safety
mechanism to prevent this problem.
Remember that our goal in the present refactoring is to allow each
subsystem to declare its own configuration structure and
variables. To do this, each module will get its own
config_format_t, and so we'll want a different structure that wraps
several config_format_t objects. This is a "config_mgr_t".
The function make_intro_from_plaintext() in test_introduce.c would
leak memory if we ever hit a failure from our underlying crypto
functions. This kind of failure should be impossible, but it's best
to be safe here.
Bugfix on 0.2.4.1-alpha.
Coverity is worried that we check "service" at the end of these test
functions, since it doesn't see any way to reach the cleanup code
without having first dereferenced the variable.
Removing the check would be unwise in this case: instead we add a
tt_assert check before using "service" so that coverity thinks that
the check is doing something useful.
Bugfix on 0.3.2.1-alpha.
Coverity can't see that it is not in fact going to read
uninitialized memory here, so we initialize these values
unconditionally.
Bugfix on 0.4.0.1-alpha.
And that it does something sensible with host and host:port.
Also reorder the tests into valid, invalid, and ambiguous.
And add some missing cases.
Note: tor_addr_port_lookup() handles ip, ip:port, host, and host:port.
Tests for 30721.
Fix add_onion_helper_clientauth() and add_onion_helper_keyarg() to
explicitly call the appropriate control reply abstractions instead of
allocating a string to pass to their callers.
Part of ticket 30889.
Right now, this has been done at a high level by confparse.c, but it
makes more sense to lower it.
This API is radically un-typesafe as it stands; we'll be wrapping it
in a safer API as we do #30914 and lower the struct manipulation
code as well.
Closes ticket 30864.
When we consider all circuits in "waiting for guard" state to be promoted to
an "open" state, we were considering all circuits, even the one marked for
close.
This ultiamtely triggers a "circuit_has_opened()" called on the circuit that
is marked for close which then leads to possible undesirable behaviors within
a subsystem.
For instance, the HS subsystem would be unable to find the authentication key
of the introduction point circuit leading to a BUG() warning and a duplicate
mark for close on the circuit.
This commit also adds a unit test to make sure we never select marked for
close circuits when upgrading its guard state from waiting for guard to open.
Fixes#30871
Signed-off-by: David Goulet <dgoulet@torproject.org>
When we consider all circuits in "waiting for guard" state to be promoted to
an "open" state, we were considering all circuits, even the one marked for
close.
This ultiamtely triggers a "circuit_has_opened()" called on the circuit that
is marked for close which then leads to possible undesirable behaviors within
a subsystem.
For instance, the HS subsystem would be unable to find the authentication key
of the introduction point circuit leading to a BUG() warning and a duplicate
mark for close on the circuit.
This commit also adds a unit test to make sure we never select marked for
close circuits when upgrading its guard state from waiting for guard to open.
Fixes#30871
Signed-off-by: David Goulet <dgoulet@torproject.org>
When we consider all circuits in "waiting for guard" state to be promoted to
an "open" state, we were considering all circuits, even the one marked for
close.
This ultiamtely triggers a "circuit_has_opened()" called on the circuit that
is marked for close which then leads to possible undesirable behaviors within
a subsystem.
For instance, the HS subsystem would be unable to find the authentication key
of the introduction point circuit leading to a BUG() warning and a duplicate
mark for close on the circuit.
This commit also adds a unit test to make sure we never select marked for
close circuits when upgrading its guard state from waiting for guard to open.
Fixes#30871
Signed-off-by: David Goulet <dgoulet@torproject.org>
Note that the event base object is _not_ created from the initialize
function, since it is configuration-dependent. This will wait until
configuration is integrated into subsystems.
Closes ticket 30806.
Skip test_rebind when the TOR_SKIP_TEST_REBIND environmental variable
is set.
Skip test_rebind on macOS in Travis builds, because it is unreliable
on macOS on Travis.
Fixes bug 30713; bugfix on 0.3.5.1-alpha.
Because it invokes the Tor mainloop, it does unpredictable things to
test coverage of a lot of code that it doesn't actually test at
all. (It is more an integration test than anything else.)
We must not accumulate digests on the circuit if the other end point is using
another SENDME version that is not using those digests like v0.
This commit makes it that we always pop the digest regardless of the version.
Part of #30428
Signed-off-by: David Goulet <dgoulet@torproject.org>
There was a missing cell version check against our max supported version. In
other words, we do not fallback to v0 anymore in case we do know the SENDME
version.
We can either handle it or not, never fallback to the unauthenticated version
in order to avoid gaming the authenticated logic.
Add a unit tests making sure we properly test that and also test that we can
always handle the default emit and accepted versions.
Fixes#30428
Signed-off-by: David Goulet <dgoulet@torproject.org>
The validation of the SENDME cell is now done as the very first thing when
receiving it for both client and exit. On failure to validate, the circuit is
closed as detailed in the specification.
Part of #30428
Signed-off-by: David Goulet <dgoulet@torproject.org>
It turns out that only the exit side is validating the authenticated SENDME v1
logic and never the client side. Which means that if a client ever uploaded
data towards an exit, the authenticated SENDME logic wouldn't apply.
For this to work, we have to record the cell digest client side as well which
introduced a new function that supports both type of edges.
This also removes a test that is not valid anymore which was that we didn't
allow cell recording on an origin circuit (client).
Part of #30428
Signed-off-by: David Goulet <dgoulet@torproject.org>
Like the previous commit about the INTRODUCE_ACK status code, change all auth
key type to use the one defined in the trunnel file.
Standardize the use of these auth type to a common ABI.
Part of #30454
Signed-off-by: David Goulet <dgoulet@torproject.org>
The client side had garbage histograms and deadcode here, too. That code has
been removed.
The tests have also been updated to properly test the intro circ by sending
padding from the relay side to the client, and verifying that both shut down
when padding was up. (The tests previously erroneously tested only the client
side of intro circs, which actually were supposed to be doing nothing).
This is the first half of implementing proposal 301. The
RecommendedPackages torrc option is marked as obsolete and
the test cases for the option removed. Additionally, the code relating
to generating and formatting package lines in votes is removed.
These lines may still appear in votes from other directory authorities
running earlier versions of the code and so consensuses may still
contain package lines. A new consensus method will be needed to stop
including package lines in consensuses.
Fixes: #28465
Our other tests tested state lengths against padding packets, and token counts
against non-padding packets. This test checks state lengths against
non-padding packets (and also padding packets too), and checks token counts
against padding packets (and also non-padding packets too).
The next three commits are needed to make this test pass (it found 3 bugs).
Yay?
Since the reproducible RNG dumps its own seed, we don't need to do
it for it. Since tinytest can tell us if the test failed, we don't
need our own test_failed booleans.
Some of these functions are now public and cpath-specific so their name should
signify the fact they are part of the cpath module:
assert_cpath_layer_ok -> cpath_assert_layer_ok
assert_cpath_ok -> cpath_assert_ok
onion_append_hop -> cpath_append_hop
circuit_init_cpath_crypto -> cpath_init_circuit_crypto
circuit_free_cpath_node -> cpath_free
onion_append_to_cpath -> cpath_extend_linked_list
We are using an opaque pointer so the structure needs to be allocated on the
heap. This means we now need a constructor for crypt_path_t.
Also modify all places initializing a crypt_path_t to use the constructor.
For various reasons, this was a nontrivial movement. There are
several places in the code where we do something like "update the
flags on this routerstatus or node if we're an authority", and at
least one where we pretended to be an authority when we weren't.
Split the core reply formatting code out of control_fmt.c into
control_proto.c. The remaining code in control_format.c deals with
specific subsystems and will eventually move to join those subsystems.
When we tell the periodic event manager about an event, we are
"registering" that event. The event sits around without being
usable, however, until we "connect" the event to libevent. In the
end, we "disconnect" the event and remove its libevent parts.
Previously, we called these operations "add", "setup", and
"destroy", which led to confusion.
We need a little refactoring for this to work, since the
initialization code for the periodic events assumes that libevent is
already initialized, which it can't be until it's configured.
This change, combined with the previous ones, lets other subsystems
declare their own periodic events, without mainloop.c having to know
about them. Implements ticket 30293.
We add random padding to every cell if there is room. This commit not only
fixes how we compute that random padding length/offset but also improves its
safety with helper functions and a unit test.
Part of #26288
Signed-off-by: David Goulet <dgoulet@torproject.org>
The digest object is as large as the entire internal digest object's state,
which is often much larger than the actual set of bytes you're transmitting.
This commit makes it that we keep the digest itself which is 20 bytes.
Part of #26288
Signed-off-by: David Goulet <dgoulet@torproject.org>
To achieve such, this commit also changes the trunnel declaration to use a
union instead of a seperate object for the v1 data.
A constant is added for the digest length so we can use it within the SENDME
code giving us a single reference.
Part of #26288
Signed-off-by: David Goulet <dgoulet@torproject.org>
The nodelist_idx for each node_t serves as a unique identifier for
the node, so we can use a bitarray to hold all the excluded
nodes, and then remove them from the smartlist.
Previously use used smartlist_subtract(sl, excluded), which is
O(len(sl)*len(excluded)).
We can use this function in other places too, but this is the one
that showed up on the profiles of 30291.
Closes ticket 30307.
The two options are mutually exclusive, since otherwise an entry
like "Foo" would be ambiguous. We want to have the ability to treat
entries like this as keys, though, since some controller commands
interpret them as flags.
The end goal here is to move the periodic callback to their
respective modules, so that mainloop.c doesn't have to include so
many other things.
This patch doesn't actually move any of the callbacks out of
mainloop.c yet.
In "make test-network-all", test IPv6-only v3 single onion services,
using the chutney network single-onion-v23-ipv6-md. This test will
not pass until 23588 has been merged.
Closes ticket 27251.
The smartlist code takes great care to set all unused pointers inside
the smartlist memory to NULL. Check if this is also the case after
modifying the smartlist multiple times.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
Coverity doesn't like to see a path where we test a pointer for
NULL if we have already ready dereferenced the pointer on that
path. While in this case, the check is not needed, it's best not to
remove checks from the unit tests IMO. Instead, I'm adding an
earlier check, so that coverity, when analyzing this function, will
think that we have always checked the pointer before dereferencing
it.
Closes ticket 30180; CID 1444641.
Fixes bug 29922; bugfix on 0.2.9.3-alpha when we tried to capture
all these warnings. No need to backport any farther than 0.3.5,
though -- these warnings don't cause test failures before then.
This one was tricky to find because apparently it only happened on
_some_ windows builds.
And fix the documentation on the function: it does produce trailing
"="s as padding.
Also remove all checks for the return value, which were redundant anyway,
because the function never failed.
Part of 29660.
... and ed25519_public_to_base64(). Also remove all checks for the return
values, which were redundant anyway, because the functions never failed.
Part of 29960.
This test was disabled in 0.4.0 and later, but the fix in #29298 was only
merged to 0.4.1. So this test will never be re-enabled in 0.4.0.
Part of 29500.
