| Commit message (Collapse) | Author | Age | Files | Lines |
| |
|
|
|
|
|
|
| |
The end result should be:
1. outgoing streams - enable sidechannel mitigations
2. incoming hs streams - enable sidechannel mitigations
3. incoming exit streams - disable sidechannel mitigations
|
| | |
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This fixes:
```text
$ cargo test -p tor-proto --features relay
error[E0603]: enum import `CtrlMsg` is private
--> crates/tor-proto/src/relay/reactor.rs:463:33
|
463 | use crate::channel::CtrlMsg;
| ^^^^^^^ private enum import
|
note: the enum import `CtrlMsg` is defined here...
--> crates/tor-proto/src/channel.rs:117:5
|
117 | use testing_exports::*;
| ^^^^^^^^^^^^^^^^^^
note: ...and refers to the enum import `CtrlMsg` which is defined here...
<snip>
```
There are a few ways we could fix this, but I don't see an advantage of
one over another.
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
This extends the `destroy_from_client()` test to also check that a
DESTROY received from the client (or, more generally speaking, from the
"inbound channel") is actually forwarded to the next hop.
The reason the `assert_destroy_sent()` assertion is commented out is
specified in the TODO that precedes it (tldr: testing the DESTROY
behaviour involves both the channel reactor and the circuit reactor, and
our test setup is currently quite limited, in that it doesn't actually
exercise the right channel reactor code paths for the *inbound*
channel). I plan to address this soon.
|
| |
|
|
| |
I am about to need this in a test.
|
| | |
|
| |
|
|
| |
This fixes an old bad copy-paste that I've just noticed.
|
| |
|
|
| |
This adds an example suggested by opara.
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
We need this because the channel reactor now only sends DESTROY for
circuits that are still in the circ map (and we are about to test this
behaviour, so we need the circuit map to actually have an entry for our
test circuit).
Initially, these tests were meant to test the circuit reactor in
isolation, but they've gradually grown more complex, and now require
a semi-working channel reactor. In the long run, I think I'd like to:
* change the tests from `tor_proto::relay::reactor` to use a proper
relay channel reactor as opposed to a `working_dummy_channel()`, and
to initialize a circuit through the normal means, namely by sending
a CREATE2 through the channel reactor (naturally, this "proper relay
channel reactor" still wouldn't be connected to the network). These
will test the integration between the channel and the circuit
reactor, as well as the circuit reactor behaviour
* add new, implementation-agnostic tests for the generic multi-reactor
system. These will use a mock channel reactor
|
| |
|
|
|
|
| |
The relay circuit reactor tests will soon need the ability to send
control messages (for allocating a circuit id for the circuit reactor
under test).
|
| | |
|
| |
|
|
|
| |
Now that we no longer respond to DESTROY by sending a DESTROY ourselves,
these tests need to be updated.
|
| |
|
|
| |
This test simulates the *next hop* sending us a DESTROY.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This change prevents the channel reactor from sending DESTROY cells on
already-closed (or non-existent) circuits. Upon receiving a DESTROY
cell, the channel reactor removes the corresponding circuit entry, if
any, from its circmap. It then passes the DESTROY to the circuit reactor
for handling. The circuit reactor handles it by shutting down, and
calling `Channel::close_circuit()` on drop. Previously, this would
unconditionally send a DESTROY cell, which caused #2648 and #2646.
This affects both clients and relays, because both circuit reactors call
`Channel::close_circuit()` on drop.
Closes #2648, #2646
|
| |
|
|
|
|
| |
There is no `BackwardReactor` heading, because there isn't that much to
say about it (it moves `RELAY` cells in the opposite direction, and
rejects RELAY_EARLY and PADDING_NEGOTIATE).
|
| | |
|
| | |
|
| | |
|
| | |
|
| | |
|
| | |
|
| | |
|
| |\
| |
| |
| |
| |
| |
| | |
proto: Silently drop DESTROY/RELAY/CREATED cells on unknown circuits
Closes #2655
See merge request tpo/core/arti!4301
|
| | |
| |
| |
| | |
And say why it's okay to do so.
|
| | |
| |
| |
| |
| | |
These are no longer causing the channel reactor to shut down, so we need
to update this test accordingly.
|
| | | |
|
| | | |
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
If we're a relay, we need to tolerate CREATED* with unrecognized
CircIds: for example, if we time out[^1] while trying to extend the circuit
by another hop, we will send a DESTROY to the extending hop, which can
race with the CREATED* response. In other words, a CREATED* cell
arriving on a closed circuit shouldn't be treated as a protocol
violation.
There are, however, a few cases where a CREATED* with an unknown CircId
*is* a protocol violation (and probably *should* cause us to close down
the channel):
* if the CREATED* is moving in the forward direction (towards the
exit), or
* if we have not previously sent a CREATE* with that particular CircId
As before, distinguishing these from the "closed circuit" case above
would involve some tricky logic, and the benefits are unclear, while the
downsides of closing a channel when we shouldn't have are significant.
It seems better to just drop these cells for now.
Closes #2655
[^1]: at the time of writing, we don't have timeouts for the circuit
extension logic, so what I've described here cannot actually happen
today. However, we *do* have a TODO for it, so the time outs I've
described here will be implemented at some point
|
| | | |
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
An unrecognized circuit ID is not always a protocol violation, so we
shouldn't close down the channel if it happens.
This change makes the channel reactor drop DESTROY and RELAY cells with
unknown CircIds without closing down the channel. It affects both
clients and relays.
Instead of dropping these unconditionally, we could have implemented
some more sophisticated checks to distinguish the bogus CircIds from the
CircIds of closed circuits, but it's unclear if it's worth the added
complexity (see discussion in #2655).
This partly addresses #2646 (an unrecognized circuit ID shouldn't cause
us to close down the channel if we're a relay).
This commit partially undoes the changes from
4f567e4a9432b340c2799e600c8ceb3724ad3082,
which was originally intended to mitigate flooding attacks.
Part of #2655
|
| |\ \
| | |
| | |
| | |
| | | |
tor-proto: Some misc rust/clippy warning fixes
See merge request tpo/core/arti!4304
|
| | | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
This fixes an `unfulfilled_lint_expectations` warning.
tor-proto conditionally sets a global `allow(unused)`, and if you have
an `expect(unused)` field within an `allow(unused)` struct, rust seems
to warn with 'unfulfilled_lint_expectations'.
https://github.com/rust-lang/rust/issues/160942
|
| | | | |
|
| | | |
| | |
| | |
| | | |
This fixes a `clippy::large_enum_variant`.
|
| | |/ |
|
| | | |
|
| | | |
|
| | |
| |
| |
| |
| |
| | |
constructor
Replaces `TryFrom<SubprotocolRequest> for HandshakeSubprotocols`.
|
| | | |
|
| | | |
|
| | | |
|
| |/ |
|
| |\
| |
| |
| |
| |
| |
| | |
tor-proto: Fix cc vegas counter updates on clock stall
Closes #2651
See merge request tpo/core/arti!4256
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
The pseudocode says:
```text
if next_cc_event:
next_cc_event--
if next_cwnd_event:
next_cwnd_event--
if clock_stalled_or_jumped:
inflight -= 'cc_sendme_inc'
return
```
but we were previously decrementing these after the return rather than
before.
|
| | |
| |
| |
| | |
We now have `add_relay_ent()` for this, so we can remove the TODO.
|
| | |
| |
| |
| |
| | |
I think this might have been c&p from the half-stream code ("hs" =
"half-stream"). "hc" ("half-circuit") seems like a better name.
|
| | |
| |
| |
| | |
Signed-off-by: David Goulet <[email protected]>
|
| | |
| |
| |
| | |
Signed-off-by: David Goulet <[email protected]>
|