| 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 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
|
| |
|
|
| |
Signed-off-by: David Goulet <[email protected]>
|
| |
|
|
|
|
|
|
|
| |
We have decided that instead of "uniq_id" in logging, we'll use the
"<domaine>-[<type>]-id" syntax to indicate who is that unique ID.
This commit only renames circuit's unique ID to "circ_uniq_id".
Signed-off-by: David Goulet <[email protected]>
|
| |
|
|
|
|
|
|
|
|
| |
This commit only affects logging. Whenever possible, log both the UniqId
and CircId.
This also changes the log lines which is now "uniq_id=" and "circ_id="
as the UniqId is internal and circ_id (CircId) is protocol level.
Signed-off-by: David Goulet <[email protected]>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
A CircId is now a circ_id and a UniqId is a unique_id so we stop
confusing them in the code.
Furthermore, channel_id that are CircId are now circ_id. Channel IDs are
different and encoded internally into a UniqId.
This is the first step to clarify semantic before we change the logging
to log both unique ID and circ ID.
No behavior change.
Signed-off-by: David Goulet <[email protected]>
|
| | |
|
| |\
| |
| |
| |
| | |
tor-protover,tor-proto: Add and use a new `subprotocol_restricted_set` macro
See merge request tpo/core/arti!4241
|
| | | |
|
| |/
|
|
|
|
|
|
|
|
|
|
| |
```text
$ cargo test -p tor-proto --features relay
[...]
error[E0405]: cannot find trait `IncomingStreamRequestFilter` in this scope
--> crates/tor-proto/src/circuit/reactor.rs:631:10
|
631 | impl IncomingStreamRequestFilter for AllowAllStreamsFilter {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ not found in this scope
```
|
| | |
|
| |
|
|
|
|
| |
The advantage of this over using a `SubprotocolRequest` is that we can
use the type system to ensure it only has subprotocols that are allowed
during a handshake.
|
| |
|
|
|
| |
We never used this, it was just needed because we used to convert to a
`CircParameters` which required this.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
... and also remove `CircNetParameters::as_circ_parameters()`.
We used to call `HopSettings::from_params_and_caps()` when handling
incoming circuit requests, but this didn't really make sense because we
already know exactly what settings we want.
The new `HopSettings::from_handshake_params()` takes the exact settings
we want, which means we can also skip constructing a `CircParameters`
and use the raw consensus `CircNetParameters`.
Most of the code in `CircNetParameters::as_circ_parameters()` has been
migrated in some form to `HopSettings::from_handshake_params()`.
|
| |
|
|
|
|
|
| |
This can happen if we get an unexpected BEGIN_DIR/RESOLVE too, so we
can't hard-code "BEGIN" in the error message.
Context: https://gitlab.torproject.org/tpo/core/arti/-/merge_requests/4230#note_3439258,
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This feature-gating has been a source of confusion, and it unnecessarily
complicates the stream message handling flow.
I've previously argued in favour of keeping it, in the spirit of a belt
and braces approach to message validation, but I've been convinced that
in this particular case, the feature-gate is more trouble than it's
worth.
What makes things worse is that the `CircHop::handle_msg()`
function was designed poorly (by yours truly). I plan on refactoring it
at some point, hopefully soon. There is a TODO about this below
its doc comment.
|
| |
|
|
|
| |
This applies to relays too, not just onion services (we need to to
handle BEGIN/BEGIN_DIR/RESOLVE).
|
| | |
|
| |
|
|
|
|
|
| |
We _already_ do not include FLOWCTRL_CC as a listed protocol among
the capabilities we provide, when we make a virtual hop. This
branch will make that behavior even more certain, by not looking at
required-client-protocols any longer.
|
| |
|
|
|
| |
See
https://gitlab.torproject.org/tpo/core/arti/-/merge_requests/4188#note_3432579
|
| | |
|
| |
|
|
|
|
|
| |
Previously, these would fail to parse, because the (WIP) stream reactor
assumed all streams would be exit (BEGIN) streams.
Closes #2613
|
| |
|
|
|
|
|
|
|
|
|
| |
Now that relays no longer use `CtrlCmd::AwaitStreamRequests`, some of
these fields are unused. I'm leaving them in for now, but we should
remove them if they're still unused after we finish the circ reactor
impl.
I'm not removing `AwaitStreamRequests`, because it will be needed by
onion services, when we replace the old client circuit reactor with the
new one.
|
| |
|
|
|
|
|
|
|
| |
Relay circuits always need a filter, so it's best to set it via the
constructor.
Part of #2582
Closes #2577
|
| |
|
|
|
|
|
|
|
|
| |
Instead of converting the `RelayEarly` message to a `Relay` message, we
add a new `RelayMaybeEarlyChanMsg` restricted message set that can hold
either.
Previously we were passing the wrong channel message command to
`decrypt_outbound()`, which would cause the decryption to fail for relay
crypto algorithms that use the command.
|
| |
|
|
|
| |
See
https://gitlab.torproject.org/tpo/core/arti/-/merge_requests/4139#note_3429586
|
| |
|
|
|
|
|
|
|
|
| |
This reworks `get_or_spawn_reactor()` to return `Error` instead of
`ReactorError`.
The main change here is that we now have a dedicated `Error::Spawn`
variant for `SpawnError`s, instead of mapping these to `ReactorError`
(which actually triggers a clean shutdown, which is not quite what we
want here).
|
| | |
|
| |
|
|
|
| |
This doesn't need to contain the `CloseStreamBehavior` or
`TerminateReason`, because we always use the same ones.
|
| |
|
|
|
|
|
| |
I realized the previous naming was ambiguous. Hopefully this makes it
clear that this `StreamEvent` triggers when the MPSC channel connecting
the reactor to the local application stream (for example the local TCP
connection of the stream, in the case of exit streams) is dropped.
|
| |
|
|
| |
Closes #2590
|
| | |
|
| |
|
|
| |
This will soon be reused for implementing `CtrlMsg::ClosePendingStream`.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This will be sent by the FWD reactor.
Essentially, this going to be the final control message in the chain
`IncomingStream::reject()` ->
`RelayCirc::reject()` ->
`forward::CtrlCmd::ClosePendingStream`->
`HopMgr::close_pending()` ->
`stream::CtrlCmd::ClosePendingStream`
-> stream gets removed from the stream map, END sent
Part of #2590
|
| | |
|
| | |
|
| | |
|
| |
|
|
|
|
|
| |
This replaces the `StreamMsg` `StreamReactor` sender with a new
`CtrlMsg` type. This `CtrlMsg` currently only has a `DeliverStreamMsg`
variant (which is the same as the old `StreamMsg` type), but will soon
grow another variant, for terminating a stream.
|
| |
|
|
| |
The tests don't compile otherwise.
|
| |
|
|
|
|
|
|
|
|
|
|
| |
This commit executes maint/add_warning with the just added change to
deny string slices except in tests.
I recommend auditing this by checking out the previous commit followed
by running the script yourself and then verifying that the diff is
identical to this commit.
This commit makes cargo clippy fail. We will add exceptions in the next
commit.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This adds new a `CircuitRxSender`/`CircuitRxReceiver` queue type.
The corresponding (`Sink`-link) sender and `Stream` implementations
prioritize the delivery of `DESTROY` messages, which get delivered
immediately, even if there are other messages queued in the underlying
MPSC message queue.
We are okay with the resulting data loss, because inbound DESTROY can be
indicative of malicious activity on the circuit. We choose to err on the
safe side, and free up the resources associated with such circuits as
soon as possible. DESTROY messages are also sent by relays when they're
about to hibernate, and by clients once they've decided to stop using a
circuit. In the latter case, the lack of an `RELAY_COMMAND_END_ACK`
does mean that this prioritization can cause data loss in cases where
the client closes the circuit immediately after END-ing a stream.
However, this is a deficiency in the protocol, and not something we want
to fix by implementing custom flushing logic in the reactor. See
torspec#196 and the discussion in #2490.
Part of #2490
|
| | |
|
| |
|
|
| |
This better matches nearby code.
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Previously when initializing a stream, we constructed most of the
stream-related queues outside of the reactor and passed the relevant
halves of the queues (senders or receivers) into the reactor.
A downside of the above approach is that the reactor may be better
informed about what queues to construct, and how to construct them. For
example the reactor knows what type of flow control that the hop is
using, so it knows whether we need queues for passing rate limits and
drain rates for XON/XOFF flow control.
This commit moves the construction of these queues into the reactor and
passes the relevant halves out of the reactor. In the future we can make
better decisions about which queues are needed depending on the flow
control method used instead of always constructing them (see arti#2068).
Reviewing with `--color-moved` might be helpful as a few lines have been
moved.
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
| |
Whether a stream queue should be bounded or unbounded is a decision that
needs to be made at runtime depending on the type of flow control used,
not at build-time. Window-based flow control should be bounded and
XON/XOFF flow control should be unbounded.
If we wanted to support both bounded and unbounded queues, it would take
a bunch of boilerplate code to wrap the senders and receivers in enums.
Instead we make the queue always bounded, but use a large bound for
XON/XOFF flow control.
|
| |
|
|
|
|
|
|
|
|
|
| |
The stream queue length (for messages incoming from the Tor network)
depends on the type of flow control we're using for the hop. Currently
we construct the stream queue outside of the circuit reactor, but we
don't have the flow control information here.
Instead of constructing the stream queue outside of the reactor and
passing the sender into the reactor, we construct the stream queue
inside of the reactor and pass the receiver out of the reactor.
|