David Goulet [Wed, 22 May 2019 15:05:36 +0000 (11:05 -0400)]
sendme: Add non fatal asserts for extra safety
Two non fatal asserts are added in this commit. First one is to see if the
SENDME digest list kept on the circuit for validation ever grows bigger than
the maximum number of expected SENDME on a circuit (currently 10).
The second one is to know if we ever send more than one SENDME at a time on a
circuit. In theory, we shouldn't but if we ever do, the v1 implementation
wouldn't work because we only keep one single cell digest (the previous cell
to the SENDME) on the circuit/cpath. Thus, sending two SENDME consecutively
will lead to a mismatch on the other side because the same cell digest would
be use and thus the circuit would collapse.
Finally, add an extra debug log in case we emit a v0 which also includes the
consensus emit version in that case.
Part of #30428
Signed-off-by: David Goulet <dgoulet@torproject.org>
David Goulet [Tue, 21 May 2019 19:19:30 +0000 (15:19 -0400)]
sendme: Clarify how sendme_circuit_cell_is_next() works
Commit 4ef8470fa5480d3b was actually reverted before because in the end we
needed to do this minus 1 check on the window.
This commit clarifies that in the code, takes the useful comment changes from 4ef8470fa5480d3b and makes sendme_circuit_cell_is_next() private since it
behaves in a very specific way that one external caller might expect.
Part of #30428.
Signed-off-by: David Goulet <dgoulet@torproject.org>
David Goulet [Wed, 15 May 2019 14:16:05 +0000 (10:16 -0400)]
sendme: Properly record SENDMEs on both edges
Turns out that we were only recording the "b_digest" but to have
bidirectionnal authenticated SENDMEs, we need to use the "f_digest" in the
forward cell situation.
Because of the cpath refactoring, this commit plays with the crypt_path_ and
relay_crypto_t API a little bit in order to respect the abstractions.
Previously, we would record the cell digest as the SENDME digest in the
decrypt cell function but to avoid code duplication (both directions needs to
record), we now do that right after iff the cell is recognized (at the edge).
It is now done in circuit_receive_relay_cell() instead.
We now also record the cell digest as the SENDME digest in both relay cell
encryption functions since they are split depending on the direction.
relay_encrypt_cell_outbound() and relay_encrypt_cell_inbound() need to
consider recording the cell digest depending on their direction (f vs b
digest).
Fixes #30428
Signed-off-by: David Goulet <dgoulet@torproject.org>
David Goulet [Tue, 7 May 2019 13:44:10 +0000 (09:44 -0400)]
sendme: Never fallback to v0 if unknown version
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>
David Goulet [Tue, 7 May 2019 13:19:41 +0000 (09:19 -0400)]
sendme: Validate v1 SENDMEs on both client and exit side
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>
David Goulet [Tue, 7 May 2019 13:16:39 +0000 (09:16 -0400)]
sendme: Record cell digest on both client and exit
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>
Mike Perry [Wed, 8 May 2019 21:28:00 +0000 (21:28 +0000)]
Add control port circuit ID to all pathbias bug messages.
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).
Mike Perry [Wed, 8 May 2019 21:21:03 +0000 (21:21 +0000)]
Pathbias should continue to ignore previously ignored circs.
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).
Mike Perry [Thu, 18 Apr 2019 20:40:55 +0000 (20:40 +0000)]
Bug 28780: Make use of purpose to keep padding circuits open.
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.
Mike Perry [Wed, 15 May 2019 04:58:21 +0000 (04:58 +0000)]
Practracker beatings are even more fun when they get caused mid-PR due to a github auto-rebase of a PR
Because github PRs choose the most recent origin/master at the time of the PR
(and for any fixups pushed to a PR later to send to CI), there are tons of
conflicts and unexpected practracker issues.
This means CI can suddenly fail after fixups to a branch that pass locally.
Then CI fails and we have to close and re-open the PR.
Mike Perry [Wed, 15 May 2019 03:43:57 +0000 (03:43 +0000)]
Check the token supply when we received a padding event, too.
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.
Mike Perry [Wed, 15 May 2019 03:45:03 +0000 (03:45 +0000)]
Add test to explicitly check state lengths and token counts.
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?
Mike Perry [Sat, 11 May 2019 02:51:14 +0000 (02:51 +0000)]
Bug 29085: Refactor non-padding accounting out of token removal.
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.
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>
David Goulet [Fri, 3 May 2019 17:24:06 +0000 (13:24 -0400)]
sendme: Add FlowCtrl protover value
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>
Hiding crypt_path_t: Ensure that ->private is initialized.
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.
Nick Mathewson [Tue, 30 Apr 2019 16:42:33 +0000 (12:42 -0400)]
Move voteflags.[ch] to become dirauth only.
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.