| Commit message (Collapse) | Author | Age | Files | Lines |
| | |
|
| |
|
|
| |
This silences a new nightly warning.
|
| |
|
|
|
| |
These are all aimed at figuring out in more detail what's going on
in #2079 and related issues.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This code uses the algorithm of prop368: When a tunnel is
sufficiently isolated, we want to expire it after it has been
disused for a certain amount of time.
The implementation is a little tricky, since we need to call an
async function on ClientTunnel to look up the disused_since() check.
We don't want to call async functions while holding locks, so
we need to drop the TunnelList lock before we do the disused_since()
calls, and the grab it again.
As part of this change, I've had made expiration functions return
the earliest time at which any tunnel might expire. We can use
this in the future to spawn fewer expiration tasks, and scan the
list of tunnels less often.
|
| |
|
|
|
|
|
| |
This is just code motion (I suggest reviewing with `--color-moved`).
This also moves the implementation-agnostic parts from
`tor_proto::client::circuit` to a new `tor_proto::circuit` module.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
| |
First, run
```
git grep -l "^edition =" |
xargs perl -i -pe 's/^edition *=.*/edition = "2024"/;'
```
Second, manually verify that all Cargo.toml files have changed,
and nothing else has changed.
Third, run cargo fmt again.
|
| |
|
|
| |
It's necessary and more verbose (and it's rather uncommon).
|
| |
|
|
|
| |
`AbstractTunnel::extend()` was calling itself endlessly because there
was no `ClientTunnel::extend()` function to call.
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
Until now, we've been using `ClientCirc::path_ref()` to get the *only*
path of a circuit. Now that `ClientCirc` is a handle to a tunnel reactor
(which may or may not be multi-path), we need to decide for each call
site of `path_ref()`, if we actually want *all* paths in the tunnel, or
if we expect the tunnel to be single-path and thus want the *only* path
in the tunnel.
I've added two new APIs to address this: `all_paths()`, for getting all
the paths in the tunnel, and `single_path()` for getting the only path
in the tunnel, or an error if the tunnel is single-path.
|
| |
|
|
|
|
|
|
|
|
| |
This is the first step towards making the circmgr return high level
tunnel types (wrappers around ClientTunnel).
Future commits will then modify each subsystems to use those specific
types. They are split in order to reduce complexity.
Signed-off-by: David Goulet <[email protected]>
|
| |
|
|
|
|
|
|
|
|
|
|
| |
In order to pull this off, the Arc requirement needs to go away because
the Arc<ClientCirc> is now within the ClientTunnel.
This commit also has a rename of the CircuitBuilder to TunnelBuilder in
order to reflect the change that it now builds a ClientTunnel.
There is a slight rename in tor-proto as well just for accuracy.
Signed-off-by: David Goulet <[email protected]>
|
| |
|
|
|
|
|
|
|
|
|
|
| |
The CircMgr will no longer yield circuits but tunnels (src/tunnel.rs).
This is a first step to rename most circuit related objects to use
"tunnel" instead.
Some "circuit" names have been kept for more precise definitions.
No behavior changes.
Signed-off-by: David Goulet <[email protected]>
|
| |
|
|
|
|
|
|
| |
This is messy, because `ClientCirc::{path_ref, n_hops, ..}` become
fallible (we can't unwrap the result, because when a circuit is closed,
its state gets removed from the `TunnelSharedState`, but its
`ClientCirc` handle continues to exist, so any attempt to retrieve the
state will result in an `Err`).
|
| |
|
|
|
| |
We don't want to be thinking about ntor vs ntor3
in circmgr.
|
| |
|
|
|
|
|
|
|
| |
In the future, when we add more circuit handshakes (PQ anyone?)
we'll want to have the logic for choosing which to use be unified.
Almost nobody calling tor-proto should need to care which circuit
handshake is going to be used.
Closes #1970.
|
| |
|
|
|
|
|
|
|
|
|
| |
This avoids cloning the object and instead allows us to have a
CircParameters per hop on the circuit path. This will come handy with
congestion control where each hop might have different congestion
control parameters.
Part of #1817
Signed-off-by: David Goulet <[email protected]>
|
| |
|
|
| |
- `rand::thread_rng()` has been deprecated and renamed to `rand::rng()`
|
| |
|
|
|
|
| |
Instead, return an error and make all call site handle it.
Signed-off-by: David Goulet <[email protected]>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Congestion control parameters have specific values depending on the
circuit type. Instead of using a CircuitType, which is removed in this
commit, specialize the function in this case onion and exit.
This allows us to get rid of CircuitType and solely use TargetCircUsage
instead.
At this commit, we use .expect() on the Builder. Future commit will
remove this to return a Result in case of failure. Worth noting that we
don't expect one.
Signed-off-by: David Goulet <[email protected]>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
| |
The congestion control parameters are created from the consensus
parameters (netparams) and then put into the CircParameters object that
is then passed down the tor-proto crate.
Because different parameters are selected depending on the circuit type
(onion vs exit vs sbws), a CircuitType enum is introduced for the sole
purpose of being used to select the right parameters.
Related #534
Signed-off-by: David Goulet <[email protected]>
|
| | |
|
| |
|
|
|
|
|
|
|
|
| |
This will allow for testing, as the CircuitBuilder can be replaced with
a mocked version.
This did require moving some of what was in the CircuitBuilder impl into
the AbstractCircuitBuilder type, since Drop implementations can't be
specialized, but that's fine, as we'll probably be doing more of that in
the future anyways.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
AbstractSpec and FakeSpec actually make testing more difficult, since
they prevent using FakeBuilder in code that relies on the concrete
TargetCircUsage and SupportedCircUsage types. Removing them means
FakeBuilder can be used in more places, and also means that the test
code is closer to the real code, since TargetCircUsage and
SupportedCircUsage are now exercised directly in more tests.
This did require making one change to a test, which I think was
previously testing behaviour that was true for FakeSpec but not for the
real code:
The mgr::test::isolated test previously asserted that, in the case where
three circuits were requested, two with isolation and one without, the
non-isolated circuit would be shared with one of the isolated circuits.
This was allowed by the FakeSpec::supports function. However, in the
actual code, the path is as follows:
* AbstractCircMgr::get_or_launch
* AbstractCircMgr::prepare_action
* CircList::find_open
* AbstractSpec::find_supported
* abstract_spec_find_supported
* OpenEntry::supports
* SupportedCircUsage::supports
* StreamIsolation::compatible_same_type
StreamIsolation::compatible_same_type checks owner_type, which is
always zero for non-isolated streams and always non-zero for isolated
streams, meaning that a isolated stream will never be compatible with a
non-isolated stream. The seems like desirable behaviour, so I simply
modified the test to make four connections, two isolated and two not,
and checked that the isolated streams never share any circuits, and that
the two non-isolated streams use the same circuit. As far as I can tell,
this is the intended behaviour in the existing code.
|
| |
|
|
|
|
|
| |
Functions that took `Option<&GuardMgr>` now take only `&GuardMgr`.
Three unit tests were removed that covered behaviour when no guard
manager was set.
|
| |
|
|
| |
This was addressed in !2046, but I forgot to remove the TODO.
|
| | |
|
| | |
|
| |
|
|
| |
Part of #1297
|
| |
|
|
|
|
|
|
|
|
|
|
| |
Now ClientCirc is no longer `Clone`, and the things that need it
to be `Clone` instead return and use an Arc<ClientCirc>
We're doing this so that ClientCirc can participate in the RPC
system, and so that its semantics are more obvious.
Closes #846.
Thanks to the type system, this was a much simpler refactoring than
I had feared it would be.
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
Channel padding depends on what the channel is being used for. We
therefore need to let the channel code know this information.
The implementation of the per-channel padding control logic will be in
the new note_usage function, which for now is simply a stub.
A future commit will introduce a `PaddingControlState` which lives in
the channel frontend; consult the doc comment for that type to see why
the plumbing through the channel manager terminates in the channel
frontend.
|
| |
|
|
|
| |
Remove all `use` statements for `TryFrom` and `TryInto`. These are
now redundant in Rust 2021.
|
| |\
| |
| |
| |
| |
| |
| | |
Don't use SystemTime::now()
Closes #306
See merge request tpo/core/arti!365
|
| | | |
|
| | |
| |
| |
| |
| |
| |
| | |
Code motion and the minimal mechanical changes.
As per
https://gitlab.torproject.org/tpo/core/arti/-/merge_requests/375#note_2783078
|
| | | |
|
| | | |
|
| |/
|
|
|
|
|
|
| |
I wanted this while debugging something.
The ad-hoc impl Debug with f.debug_struct is getting repetitive
and I've already perpetrated one copy-paste mistake.
We should consider using something like the `educe` crate's Clone.
|
| | |
|
| |
|
|
|
|
| |
See the new commentary text on `ClientCirc` for the rationale.
Signed-off-by: Ian Jackson <[email protected]>
|
| | |
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
| |
And now the complexity begins: when the user changes the path_rules,
they not only want new circuits to obey those rules: they want
_all new requests_ to be put onto circuits that obey those rules.
That means that when the path rules become more restrictive, we need
to retire all the circuits, and make sure that currently pending
circuits aren't used for any requests.
If it's any comfort, doing this was even more complicated in C tor. ;)
|
| |\ |
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
Instead of racily advancing time forward, this commit attempts to rework
how WaitFor works, such that it makes advances when all sleeper futures
that have been created have been polled (by handing the MockSleepRuntime
a Waker with which to wake up the WaitFor).
The above described mechanics work well enough for the double timeout
test, but fail in the presence of code that spawns asynchronous /
background tasks that must make progress before time is advanced for the
test to work properly. In order to deal with these cases, a set of APIs
are introduced in order to block time from being advanced until some
code has run, and a carveout added in order to permit small advances in
time where required.
(In some cases, code needed to be hacked up a bit in order to be made
properly testable using these APIs; the `MockablePlan` trait included in
here is somewhat unfortunate.)
This should fix arti#149.
|
| |/ |
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
The previous code would report all failures to build a circuit as
failures of the guard. But of course that's not right: If we
fail to extend to the second or third hop, that might or might not
be the guard's fault.
Now we use the "pending status" feature of the GuardMonitor type so
that an early failure is attributed to the guard, but a later
failure is attributed as "Indeterminate". Only a complete circuit
is called a success. We use a new "GuardStatusHandle" type here so
that we can report the status early if there is a timeout.
|
| | |
|
| |
|
|
|
|
| |
(When we're building a path with a guard, we need to tell the guard
manager whether the path succeeded, and we need to wait to hear
whether the guard is usable.)
|
|
|
This will cause some pain for now, but now is really the best time
to do this kind of thing.
|