| Commit message (Collapse) | Author | Age | Files | Lines |
| ... | |
| |
|
|
|
|
|
| |
The new `RelayIdFor` type alias I'm about to add will need to call
`RelayIdFor::{for_lookup, for_store}` with a type that isn't
`&OwnedChanTarget`, so I will need these functions to take a generic
`HasRelayIds`.
|
| |
|
|
|
|
|
|
| |
This refactors `RelayIdForExperience` into a more generic `RelayIdFor`
type.
`RelayIdFor` will serve the basis for another key type that will be used
for storing `HsDir` information.
|
| | |
|
| | |
|
| |
|
|
|
|
| |
I find having too many of these nested `else { None }` branches makes
the overall logic harder to follow, so I changed this to use `Option<>`
combinators instead.
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
This avoids us replacing our cached hsdesc with one that has a lower
revision counter.
This was not a problem before, because we'd only ever fetch a new
descriptor when our cahced one expired, but now that we refetch the
descriptor on introduction NACK, we need to make sure the new descriptor
is actually more recent than the one we have.
The implementation is a bit convoluted because I had to avoid retaining
a reference to the known-timely cached `desc` so as not to anger borrowck.
|
| |
|
|
| |
This will soon need to be called from two places, unfortunately.
|
| | |
|
| |
|
|
|
| |
As suggested in
https://gitlab.torproject.org/tpo/core/arti/-/merge_requests/3925?commit_id=22ae30205a5f18fee43f424f8c0f9768b95a2ae6#note_3402779
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Prior to this change, arti clients would retain their cached HS
descriptors until expiry. This is causing us some problems in
[arti#2458], where arti frequently fails to connect to C Tor services
in a chutney test net. One of the problems in #2458 is that all
introduction points are NACK-ing the client's introduction requests,
ultimately causing the connection attempt to fail:
```
2026-04-23T16:25:23Z DEBUG tor_hsclient::state: HS connection failure for ijalr3vpf67zradtlaxze6pex5ypadu5qfsltn42cmhioqory6363tad.onion error=error: Unable to connect to hidden service using any Rendezvous Point / Introduction Point: Tried to make circuit to hidden service 6 times, but all attempts failed
Attempt 1: Introduction point #3 reported error in its INTRODUCE_ACK: NOT_RECOGNIZED
Attempt 2: Introduction point #2 reported error in its INTRODUCE_ACK: NOT_RECOGNIZED
Attempt 3: Introduction point #1 reported error in its INTRODUCE_ACK: NOT_RECOGNIZED
Attempt 4: Introduction point #3 reported error in its INTRODUCE_ACK: NOT_RECOGNIZED
Attempt 5: Introduction point #2 reported error in its INTRODUCE_ACK: NOT_RECOGNIZED
Attempt 6: Introduction point #1 reported error in its INTRODUCE_ACK: NOT_RECOGNIZED
```
I think this is happening because the C Tor service has rotated intro
points and republished its descriptor, before the descriptor's planned
expiry. I believe is something that can (and does) happen, and so arti
should be able to handle it gracefully.
I added some more logs to arti and reran the test, and noticed the
descriptor's lifetime works out to be just over 2 days (54h), which
seems excessive (especially in our test net, where the voting interval
is 20s and the hsdir interval is 8min). In any case, holding on to a
service's descriptor for too long, and not refetching a new one, will
cause the client's introduction requests to be rejected (because the
service might switch intro points).
I think there are at least 2 things we need to do to improve arti's
handling of HS connections:
* In the case of an introduce NACK (with status = `NOT_RECOGNIZED`),
the client should refetch the descriptor and then retry the
introduction. This behavior is not codified in the spec yet (see
torspec#245), but I am told this is what C Tor does
* Rethink the cached descriptor expiry calculation
This commit addresses the first point. The second one seems trickier, so
I have not looked into it yet.
Part of #966
[arti#913]: https://gitlab.torproject.org/tpo/core/arti/-/work_items/913#note_2914448
[arti#2458]: https://gitlab.torproject.org/tpo/core/arti/-/work_items/2458#note_3400768
|
| | |
|
| | |
|
| |
|
|
|
|
|
|
| |
This will soon be used to force a refetch in the case of an
`INTRODUCE_NACK`.
(This commit is intentionally left misindented to make reviewing a bit
easier. A future commit will rustfmt the file).
|
| |
|
|
|
|
|
| |
Gabi correctly points out that since the client uses guarded
circuits for introduction points, whereas the service uses naive
circuits, we shouldn't expect the peer's circuits to be any longer
than ours.
|
| |
|
|
|
|
|
|
|
| |
Both C tor and Arti will retry building a circuit if the first
attempt fails. This means that it can be worthwhile waiting longer
than we might otherwise for the HS to build its rendezvous circuit.
We don't need to make this change for _our_ circuits, since
the CircMgr code takes care of those timeouts for us.
|
| | |
|
| |
|
|
|
|
|
|
| |
We don't actually need to use this method on any circuits that have
a virtual hop, so instead of "fixing" this method to ignore virtual
hops, the simpler approach is to change its name and its documented
behavior, and to explain how the documented behavior is appropriate
for our needs.
|
| | |
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
| |
This time we _do_ need to use the BuildCircuit estimator, since we
have to consider the peer's circuit building.
The peer may be using full vanguards, so we need to use 5 as their
maximum hop estimate.
Additionally, their circuit may be longer than ours, so we ought to
possibly wait a bit longer for them to get our INTRODUCE2.
There are XXXXs here about OneWay timeout estimators, for immediate
followup.
|
| |
|
|
|
|
| |
The circmgr handles timeouts on its own, so we can let it do that.
Use the actual circuit length for calculating round-trip timeouts.
|
| |
|
|
|
|
|
|
| |
The circmgr code handles circuit timeouts, so we don't need to
include that redundantly.
Also, we look at the circuit to find out its number of hops,
so that we estimate the timeout more accurately.
|
| |
|
|
|
|
|
|
|
| |
The hspool operations already include their own timeouts, so we
don't need to recalculate them.
For the directory related operations, we now calculate the timeouts
based on actual circuit lengths, and use those timeouts on the
operations themselves.
|
| |
|
|
| |
We'll use these for timeout estimations.
|
| | |
|
| | |
|
| | |
|
| | |
|
| |
|
|
|
|
| |
Without this fix, it isn't possible to run clippy on higher-level
crates without the hs-pow feature, since this failure stops checking
earlier.
|
| |
|
|
|
|
|
|
|
| |
`clippy::collapsible_if` started triggering after bumping the MSRV to
1.88.
Since this triggers from a lot of places, and since there even are a
couple of instances where we explicitly allow `clippy::collapsible_ifs`,
I've opened #2342 for deciding what to do about it.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This requires the `TlsKeyAndCert` so be passed on the TLS acceptor
settings. We assume that `RelayIdentities` has this information.
The ChanBuilder::new() was getting a bit too convoluted and feature
gated to instead we introduce new_client() and new_relay() and remove
the need for `with_identities()`.
Because of this, the ChanMgr::new() now returns a `Result<>`.
Related to #1597
Signed-off-by: David Goulet <[email protected]>
|
| |
|
|
| |
This adds the lint to all our crates.
|
| |
|
|
|
|
|
|
|
|
|
|
| |
This struct is used to pass configuration parameters to the ChanMgr when
building it.
At the moment, it holds the ChannelConfig and RelayIdentities (feature
gated) which will be used in subsequent commits.
Note that relays do require RelayIdentities to build channels.
Signed-off-by: David Goulet <[email protected]>
|
| |
|
|
|
|
|
| |
We'll rely on a RelayIdentities to pass in the right keys to the ChanMgr
instead of the entire KeyMgr.
Signed-off-by: David Goulet <[email protected]>
|
| | |
|
| | |
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
This will make it easier to see the correspondence between CTorPaths
and the HS client/service key specifiers.
Initially, I was hoping this would make it easier to write a d-d macro
that automatically derives a `CTorPath` variant (e.g.
`HsClientDescEncKeypair`) from the KeySpecifier type name
(`HsClientDescEncKeypairSpecifier`), but alas, I don't think d-d can
"chop off" name suffixes ("Specifier", in this case).
`from_ctor_path()`/`ctor_path()` implementations for converting
`CTorPath`s to and from key specifiers.
|
| | |
|
| |
|
|
|
|
|
| |
This is similar to `#[serde(with = "...")]`, and feels a bit nicer than
having to specify two separate functions for the conversions (because
with two separate functions, you *can* technically only specify one of
them, which shouldn't be allowed).
|
| |
|
|
|
| |
This enables us to implement `KeyMgr::describe()`, which relies on the
ability to extract the key specifier of the key from its `KeyPath`.
|
| | |
|
| | |
|
| | |
|
| |
|
|
|
|
|
|
|
| |
This code was duplicated across hsclient and hsservice.
Logically, it belongs in netdir, since that's where we have the
latest required-protocol information, and the ability to look up
relays by IDs.
Closes #1223
|
| |
|
|
| |
This would have made #2268 clearer to me.
|
| | |
|
| |
|
|
|
| |
These are all aimed at figuring out in more detail what's going on
in #2079 and related issues.
|
| |
|
|
| |
Run maint/add_warning
|