To ease debugging of miscount issues, attach vanguards with --loglevel DEBUG
and obtain control port logs (or use any other control port CIRC and
CIRC_MINOR event logging mechanism).
If circuit padding wants to keep a circuit open and pathbias used to ignore
it, pathbias should continue to ignore it.
This may catch other purpose-change related miscounts (such as timeout
measurement, cannibalization, onion service circuit transitions, and
vanguards).
When a circuit is marked for close, check to see if any of our padding
machines want to take ownership of it and continue padding until the machine
hits the END state.
For safety, we also ensure that machines that do not terminate are still
closed as follows: Because padding machine timers are UINT32_MAX in size, if
some sort of network event doesn't happen on a padding-only circuit within
that time, we can conclude it is deadlocked and allow
circuit_expire_old_circuits_clientside() to close it.
If too much network activity happens, then per-machine padding limits can be
used to cease padding, which will cause network cell events to cease, on the
circuit, which will cause circpad to abandon the circuit as per the above time
limit.
We need to check here because otherwise we can try to schedule padding with no
tokens left upon the receipt of a padding event when our bins just became
empty.
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.
This commit moves code that updates the state length and padding limit counts
out from the callback to its own function, for clarity.
It does not change functionality.
This commit moves the padding state limit checks and the padding rate limit
checks out of the token removal codepath, and causes all three functions to
get called from a single circpad_machine_count_nonpadding_sent() function.
It does not change functionality.
The code flow in theory can end up with a layer_hint to be NULL but in
practice it should never happen because with an origin circuit, we must have
the layer_hint.
Just in case, BUG() on it if we ever end up in this situation and recover by
closing the circuit.
Fixes#30467.
Signed-off-by: David Goulet <dgoulet@torproject.org>
Fortunately, in 0.3.5.1-alpha we improved logging for various
failure cases involved with onion service client auth.
Unfortunately, for this one, we freed the file right before logging
its name.
Fortunately, tor_free() sets its pointer to NULL, so we didn't have
a use-after-free bug.
Unfortunately, passing NULL to %s is not defined.
Fortunately, GCC 9.1.1 caught the issue!
Unfortunately, nobody has actually tried building Tor with GCC 9.1.1
before. Or if they had, they didn't report the warning.
Fixes bug 30475; bugfix on 0.3.5.1-alpha.
The INTRODUCE1 trunnel definition file doesn't support that value so it can
not be used else it leads to an assert on the intro point side if ever tried.
Fortunately, it was impossible to reach that code path.
Part of #30454
Signed-off-by: David Goulet <dgoulet@torproject.org>
See proposal 289 section 4.3 for more details.
It describes the flow control protocol at the circuit and stream level. If
there is no FlowCtrl protocol version, tor supports the unauthenticated flow
control features from its supported Relay protocols.
At this commit, relay will start advertising FlowCtrl=1 meaning they support
authenticated SENDMEs v1.
Closes#30363
Signed-off-by: David Goulet <dgoulet@torproject.org>
- Move test-only cpath_get_n_hops() to crypt_path.c.
- Move onion_next_hop_in_cpath() and rename to cpath_get_next_non_open_hop().
The latter function was directly accessing cpath->state, and it's a first step
at hiding ->state.
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
Now that we are using a constructor we should be more careful that we are
always using the constructor to initialize crypt_path_t, so make sure that
->private is initialized.
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.
I don't believe any of these represent a real timing vulnerability
(remote timing against memcmp() on a modern CPU is not easy), but
these are the ones where I believe we should be more careful.
For memeq and friends, "tor_" indicates constant-time and "fast_"
indicates optimized. I'm fine with leaving the constant-time
"safe_mem_is_zero" with its current name, but the "tor_" prefix on
the current optimized version is misleading.
Also, make the tor_digest*_is_zero() uniformly constant-time, and
add a fast_digest*_is_zero() version to use as needed.
A later commit in this branch will fix all the users of
tor_mem_is_zero().
Closes ticket 30309.
Manually fix up some reply-generating code that the Coccinelle scripts
won't match. Some more complicated ones remain -- these are mostly
ones that accumulate data to send, and then call connection_buf_add()
or connection_write_str_to_buf() directly.
Create a set of abstractions for controller commands and events to
output replies to the control channel. The control protocol has a
relatively consistent SMTP-like structure, so it's helpful when code
that implements control commands and events doesn't explicitly format
everything on its own.
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.
Because this function is poking within the relay_crypto_t object, move the
function to the module so we can keep it opaque as much as possible.
Part of #26288
Signed-off-by: David Goulet <dgoulet@torproject.org>
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>
We'll use it this in order to know when to hash the cell for the SENDME
instead of doing it at every cell.
Part of #26288
Signed-off-by: David Goulet <dgoulet@torproject.org>
When adding random to a cell, skip the first 4 bytes and leave them zeroed. It
has been very useful in the past for us to keep bytes like this.
Some code trickery was added to make sure we have enough room for this 4 bytes
offset when adding random.
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>
No behavior change but code had to be refactored a bit. Also, the tor_memcmp()
was changed to tor_memneq().
Part of #26288
Signed-off-by: David Goulet <dgoulet@torproject.org>
The circuit and stream level functions that update the package window have
been renamed to have a "_note_" in them to make their purpose more clear.
Part of #26288
Signed-off-by: David Goulet <dgoulet@torproject.org>
No behavior change. Only moving code and fixing part of it in order to use the
parameters passed as pointers.
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>
In order to do so, depending on where the cell is going, we'll keep the last
cell digest that is either received inbound or sent outbound.
Then it can be used for validation.
Part of #26288
Signed-off-by: David Goulet <dgoulet@torproject.org>
Now that we keep the last seen cell digests on the Exit side on the circuit
object, use that to match the SENDME v1 transforming this whole process into a
real authenticated SENDME mechanism.
Part of #26841
Signed-off-by: David Goulet <dgoulet@torproject.org>
This makes tor remember the last seen digest of a cell if that cell is the
last one before a SENDME on the Exit side.
Closes#26839
Signed-off-by: David Goulet <dgoulet@torproject.org>
This commit makes tor able to parse and handle a SENDME version 1. It will
look at the consensus parameter "sendme_accept_min_version" to know what is
the minimum version it should look at.
IMPORTANT: At this commit, the validation of the cell is not fully
implemented. For this, we need #26839 to be completed that is to match the
SENDME digest with the last cell digest.
Closes#26841
Signed-off-by: David Goulet <dgoulet@torproject.org>
This code will obey the consensus parameter "sendme_emit_min_version" to know
which SENDME version it should send. For now, the default is 0 and the
parameter is not yet used in the consensus.
This commit adds the support to send version 1 SENDMEs but aren't sent on the
wire at this commit.
Closes#26840
Signed-off-by: David Goulet <dgoulet@torproject.org>
In order to be able to deploy the authenticated SENDMEs, these two consensus
parameters are needed to control the minimum version that we can emit and
accept.
See section 4 in prop289 for more details.
Note that at this commit, the functions that return the values aren't used so
compilation fails if warnings are set to errors.
Closes#26842
Signed-off-by: David Goulet <dgoulet@torproject.org>
