aboutsummaryrefslogtreecommitdiff
path: root/crates/tor-hsclient/src/connect.rs
Commit message (Collapse)AuthorAgeFilesLines
...
* hsclient: Only replace the hsdesc with a more recent one (fmt)Gabriela Moldovan2026-05-071-11/+9
|
* hsclient: Only replace the hsdesc with a more recent oneGabriela Moldovan2026-05-071-6/+36
| | | | | | | | | | | | | 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.
* hsclient: Move HsDesc extraction hack to a closureGabriela Moldovan2026-05-071-7/+11
| | | | This will soon need to be called from two places, unfortunately.
* hsclient: Replace bool with Option<RefetchDescriptor> (fmt)Gabriela Moldovan2026-05-071-1/+5
|
* hsclient: Replace bool with Option<RefetchDescriptor>Gabriela Moldovan2026-05-071-7/+11
| | | | | As suggested in https://gitlab.torproject.org/tpo/core/arti/-/merge_requests/3925?commit_id=22ae30205a5f18fee43f424f8c0f9768b95a2ae6#note_3402779
* hsclient: Refetch the descriptor if any introduction attempts are NACKedGabriela Moldovan2026-05-071-1/+41
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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
* hsclient: Update the docs to say what the refetch flag doesGabriela Moldovan2026-05-071-0/+3
|
* hsclient: Add a flag for forcing a descriptor refetch (fmt)Gabriela Moldovan2026-05-071-14/+18
|
* hsclient: Add a flag for forcing a descriptor refetchGabriela Moldovan2026-05-071-2/+4
| | | | | | | | 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).
* hsclient: Remove estimate of extra intro distance.Nick Mathewson2026-05-071-14/+8
| | | | | | | 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.
* hsclient: Account for peer circuit retries.Nick Mathewson2026-05-071-1/+5
| | | | | | | | | 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.
* hsclient: Fix documentation about where timeouts are calculated.Nick Mathewson2026-05-071-8/+2
|
* hsclient: Simplify h_num_own_{real_}hopsNick Mathewson2026-05-071-7/+11
| | | | | | | | 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.
* hsclient: remove now-needless "allow(unused)" markers.Nick Mathewson2026-05-071-3/+0
|
* circmgr, hsclient: Introduce and use a OneWay timeout estimator.Nick Mathewson2026-05-071-16/+8
|
* hsclient: Move and correct timeouts for waiting for RENDEZVOUS2Nick Mathewson2026-05-071-66/+89
| | | | | | | | | | | | | | 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.
* hsclient: move and correct timeouts for intro/ack.Nick Mathewson2026-05-071-30/+20
| | | | | | The circmgr handles timeouts on its own, so we can let it do that. Use the actual circuit length for calculating round-trip timeouts.
* hsclient: Move and correct timeouts for establishing rend circuits.Nick Mathewson2026-05-071-33/+19
| | | | | | | | 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.
* hsclient: Move and correct timeouts for hsdescriptor downloads.Nick Mathewson2026-05-071-17/+21
| | | | | | | | | 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.
* circmgr: Add num_hops members to mock tunnel types.Nick Mathewson2026-05-071-0/+38
| | | | We'll use these for timeout estimations.
* tor-hsclient: include period metadata in trace log messageJim Newsome2026-04-021-1/+2
|
* tor-hsclient: port to web-time-compat.Nick Mathewson2026-03-261-3/+1
|
* Fix word duplicate typosTobias Stoeckmann2026-03-151-1/+1
|
* fix: use wallclock timestamps in all push_timed callsNihal2025-12-171-3/+3
|
* refactor: clean codeNihal2025-12-171-2/+3
|
* feat(retry-error): add timestamps to retry errorsNihal2025-12-171-6/+9
|
* tor-hsservice: Fix typo in error message.Wesley Aptekar-Cassels2025-11-241-1/+1
|
* opentelemetry: Instrument a bunch of functions.Wesley Aptekar-Cassels2025-11-241-2/+11
| | | | | These are all aimed at figuring out in more detail what's going on in #2079 and related issues.
* Fix name of clippy lint to unchecked_time_subtraction (2)Ian Jackson2025-11-061-1/+1
| | | | Run maint/add_warning
* Stop using MockSleepProvider in a few cratesNeel Chauhan2025-11-021-4/+2
| | | | Part of #1885.
* proto: Add a circuit module shared between client and relay impls.Gabriela Moldovan2025-08-281-12/+12
| | | | | | | 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.
* proto: Move the `stream` module under `client` (breaking).Gabriela Moldovan2025-08-181-1/+1
| | | | | | | | | | | | The `stream` module is client-specific, for the most part, so I am moving it under `client`. Later on, we will factor out the parts that can be shared with the relay implementation. Note: this is a breaking change as the deleted `stream` module was `pub`. We could've kept the module and reexported from it the public types from `tor_proto::client::stream`, but I think it's better to have this `client` namespacing, because it makes the separation between the client and relay parts clearer.
* Switch Cargo.toml files to edition 2024.Nick Mathewson2025-08-071-11/+11
| | | | | | | | | | | | | | 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.
* tunnel: Implement start_conversation() for all tunnel typesDavid Goulet2025-08-051-3/+4
| | | | | | | | | | The BaseTunnel now has a start_conversation() which takes a TargetHop meaning it can be used with a multi path tunnel. The Conversation object has been moved into the tunnel namespace out of the circuit one. Signed-off-by: David Goulet <[email protected]>
* hs: Use the new Tunnel interface for onion serviceDavid Goulet2025-08-051-97/+204
|
* Use new DisplayRedacted/DebugRedacted code for HsId.Nick Mathewson2025-07-311-3/+3
| | | | Closes #2012.
* hs*: Define some HsDesc errors as _suspicious_.Nick Mathewson2025-07-101-8/+21
| | | | | These errors are suspicious as hsdir inflation attacks, in the context of prop360.
* hs*: Include SourceInfo when making HsDesc requests.Nick Mathewson2025-07-101-1/+18
|
* hs: Remove the use of HopNum and instead use TargetHopDavid Goulet2025-06-261-2/+2
| | | | | | | | | | This is in the spirit of making everything going inbound the tor-proto crate to use a TargetHop. This becomes much easier for the HS subsystem as it only uses the last hop for its conversation and setup. Signed-off-by: David Goulet <[email protected]>
* proto: Move NegotiatedHopSettings to a higher levelNick Mathewson2025-06-101-1/+10
| | | | | | We will construct this object based on the circuit parameters _and_ on the target's supported protocol versions, so we need to do so when we have both pieces of info.
* *: suppress cognitive_complexity warnings from nightlyNick Mathewson2025-05-291-0/+2
| | | | | | | | | | | | | Apparently clippy nightly is better (or worse?) about detecting complex functions than before, so I'm suppressing these warnings where they occur. I have mixed feelings about these warnings: On the plus side, they really do help to detect functions that are twistier than they need to be. On the minus side, they get confused by tracing macros, and the "allows" do pile up. But on the plus side, those "allows" do provide a way to find functions that need to be refactored, and they are never uglier than the functions they decorate.
* squash! Upgrade rand dependency to 0.9.Nick Mathewson2025-03-181-2/+2
| | | | - The Rng::gen() functions have been renamed to Rng::random().
* squash! Upgrade rand dependency to 0.9.Nick Mathewson2025-03-181-1/+1
| | | | - `rand::thread_rng()` has been deprecated and renamed to `rand::rng()`
* tor-rtmock: allow-Decorate every use of MockSleepProviderIan Jackson2025-03-061-0/+2
| | | | | | | MockSleepProvider and MockSleepRuntime have been declared deprecated by the docs for some time. We're about to mark them `#[deprecated]`. This commit has been split out for clarity of review.
* hsclient: Include rsa_id in debugClara Engler2025-03-041-1/+2
| | | | | | | This commit adds the RSA ID of a relay into a debug statement, as found in other places in the code. It mostly serves the purpose that the Ed25519 ID in itself is rather inconvenient, as metrics.torproject.org only allows querying from the RSA ID.
* proto: Remove ConversationInHandlerDavid Goulet2025-02-041-5/+1
| | | | | | | | | | | | | | | | | | | It is unused but most importantly it allows any RELAY cell to be sent from anywhere in the code which is really not desirable because it is skipping congestion control. It also allows us to remove the `control_tx` from the reactor which is one less channel to track/understand/think about. This opens up the door to all sorts of problems especially side channel that can be exploited if we are not careful. We can always bring this back if we need it but for now, it is unused and allows us to remove the `CtrlMsg::SendRelayCell` control message. No code behavior change. Signed-off-by: David Goulet <[email protected]>
* tor-proto: Rewrite circuit reactor run_once() loop to use select!.Gabriela Moldovan2025-01-291-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This rewrites the circuit reactor main loop to use `select_biased!` to poll multiple futures simultaneously. The new `run_once()`, like the old, first waits for an initial `CtrlMsg::Create`. Then, it uses a `select_biased!` to poll the `chan_sender` sink and shutdown channel for readiness. When the channel sink is ready, we poll the `control` and `input` channels like before, as well as the new `ready_streams` `Stream` (`ready_streams` is a `futures::Stream` that replaces the previous `send_outbound()` function). Most of the implementation remains unchanged, except the `handle_input`, `handle_cell` and `handle_control` functions no longer send anything on the `chan_sender` channel. Instead, they may do some (synchronous) processing, and send instructions for the remaining work that needs to be done (for example, for writing the cell to the `chan_sender` channel). These instructions are handled at the end of `run_once()`, and are encoded in the `RunOnceCmdInner` enum. What this change does **not** do: * the control channel *still* bypasses congestion control. We could fix this by making the various reactor functions send the `RunOnceCmdInner` commands to `run_once()` via a channel (instead of returning them). This would enable the reactor to stop reading the commands (except for handle `Sendme`, which would be handled separately) if it's blocked on congestion control.
* circmgr: Remove the CircParameters build .expect()David Goulet2025-01-161-1/+2
| | | | | | Instead, return an error and make all call site handle it. Signed-off-by: David Goulet <[email protected]>
* circ: Specialize the circparams from netparams functionDavid Goulet2025-01-161-3/+2
| | | | | | | | | | | | | | | 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]>
* circmgr: Modify CircParameters for congestion controlDavid Goulet2025-01-161-2/+3
| | | | | | | | | | | | | | 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]>