aboutsummaryrefslogtreecommitdiff
path: root/crates/tor-proto/src/channel
Commit message (Collapse)AuthorAgeFilesLines
* proto: Expect a now-unused test function to be dead codeGabriela Moldovan9 days1-0/+1
|
* proto: Avoid sending DESTROY if we have received DESTROYGabriela Moldovan9 days2-0/+25
| | | | | | | | | | | | | | | 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
* Merge branch 'unk-circid' into 'main'gabi-2502026-08-132-40/+49
|\ | | | | | | | | | | | | proto: Silently drop DESTROY/RELAY/CREATED cells on unknown circuits Closes #2655 See merge request tpo/core/arti!4301
| * proto: Explicitly drop the cells with unrecognized CircIdsGabriela Moldovan2026-08-121-0/+20
| | | | | | | | And say why it's okay to do so.
| * proto: Update tests now that unrecognized CREATED are droppedGabriela Moldovan2026-08-111-2/+2
| | | | | | | | | | These are no longer causing the channel reactor to shut down, so we need to update this test accordingly.
| * proto: Update tests now that we tolerate unrecognized CircIdsGabriela Moldovan2026-08-111-24/+8
| |
| * proto: s/channel/circuit in test commentGabriela Moldovan2026-08-111-1/+1
| |
| * proto: Ignore CREATED* with unrecognized CircIdsGabriela Moldovan2026-08-112-9/+14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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
| * proto: Silently drop DESTROY/RELAY cells on unknown circuits (fmt)Gabriela Moldovan2026-08-111-5/+3
| |
| * proto: Silently drop DESTROY/RELAY cells on unknown circuitsGabriela Moldovan2026-08-111-4/+6
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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: make `CreateRequestHandler` methods asyncSteven Engler2026-08-121-10/+13
|/
* proto: Remove a now-done TODOGabriela Moldovan2026-08-041-3/+0
| | | | We now have `add_relay_ent()` for this, so we can remove the TODO.
* proto: Rename "hs" to "hc"Gabriela Moldovan2026-08-042-3/+3
| | | | | I think this might have been c&p from the half-stream code ("hs" = "half-stream"). "hc" ("half-circuit") seems like a better name.
* tor-proto: extend circ handshake tests to close the circuitSteven Engler2026-07-301-0/+10
|
* tor-proto: rename some methods on test `ConnInspector`Steven Engler2026-07-301-4/+4
|
* tor-proto: don't drop stream rx in testsSteven Engler2026-07-301-7/+12
| | | | Otherwise the new circuit gets closed immediately by the relay.
* tor-proto: add 'test_utils' mod for channel testsSteven Engler2026-07-291-0/+305
| | | | These help to establish connected channel objects to be used for tests.
* proto: Use PeerInfo to log responder channel addrDavid Goulet2026-07-291-2/+2
| | | | | | | | | | | | | Inbound channel (as responder) don't have a ChannelMethod as they are not initiating the type of transport to use (PT vs Direct). It would result in a log line when receiving a channel request: DEBUG tor_proto::channel::handshake: Completed handshake without authentication to [? ] stream_id=Chan 2 This commit uses the `PeerInfo` which is wrapped in a `MaybeSensitive` and thus safe to log. Signed-off-by: David Goulet <[email protected]>
* align DestroyReason with torspec!490ramdoys2026-07-282-3/+3
|
* tor-checkable: Rename `TimeBound::is_valid_at` to `check_valid_at`Ian Jackson2026-07-231-1/+1
| | | | | I find this names confusing. To my mind "is" implies a function returning `bool`.
* tor-checkable: TimeBound: Make wrapped type an associated typeIan Jackson2026-07-231-3/+3
| | | | | It wouldn't make much sense for one concrete type to be unwrappable variously as different inner types.
* tor-checkable: TimeBound: remove Error associated typeIan Jackson2026-07-231-1/+1
| | | | | This was always TimeValidityError. And we want to rely on that so we can do the validity checking more centrally.
* proto: Return the incoming streams from the create handlerGabriela Moldovan2026-07-221-3/+4
| | | | | | | | | | | We need to return the "futures::Stream of Tor streams" from the CREATE handler, because these need to be handled from `arti-relay`, as per `doc/dev/notes/relay-streams.md` This commit is intentionally (slightly) misformatted to make reviewing a bit easier (the next commit will rustfmt everything). Part of #2612
* Apply deferred rustfmt churnIan Jackson2026-07-161-1/+1
|
* Use new TimeBound name throughout the treeIan Jackson2026-07-161-2/+2
|
* proto: Pass the allowed incoming commands to CreateRequestHandlerGabriela Moldovan2026-06-301-0/+3
| | | | | | | | | | | This enables us to make these configurable: any relays that are not configured to be an exit will exclude BEGIN and RESOLVE from their list of allowed commands, causing exit and DNS streams to be rejected as soon as the BEGIN/RESOLVE cell is received in the circuit reactor. Context: https://gitlab.torproject.org/tpo/core/arti/-/merge_requests/4145#note_3430345 Part of #2606
* proto: Pass an IncomingStreamRequestFilter factory to the create handlerGabriela Moldovan2026-06-291-0/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This implements what we discussed in `doc/dev/notes/relay-streams.md` (lines 218-234): > Currently, to allow incoming stream requests on a circuit, > you first need to call `RelayCirc::allow_stream_requests()` > to install a `CmdChecker` and `IncomingStreamRequestFilter`. > This is not ideal, because `allow_stream_requests()` will need to be > called unconditionally, on each `RelayCirc`, > right after it's created in the `CreateHandler` impl > (which in turn, would mean making `handle_create()` async too, > because `allow_stream_requests()` is async, which wouldn't be great). > > So, the first step here is to rework the `RelayCirc` API to make relay circuits > be constructable with a list of allowed `RelayCmd`s and `IncomingStreamRequestFilter` > from the get-go ([#2582]), and to get rid of `allow_stream_requests()`, > which will enable the `CREATE*` handler to remain non-`async`. > > In any case, the `CREATE*` handler will still require some changes, > because it needs to be initialized with an `IncomingStreamRequestFilter`, I am not sure using an `IncomingStreamRequestFilter` "factory" is necessarily the right approach here, but the circuit `Reactor`'s constructor needs to take an `IncomingStreamRequestFilter`, and `IncomingStreamRequestFilter` is not `Clone` (and FWIW, I think it's better if we don't make it `Clone`). One obvious limitation is that the `IncomingStreamRequestFilter` of the circuit reactor is fixed for the entire lifetime of the circuit. In practice, I don't think this is going to be a problem, because the arti-relay `IncomingStreamRequestFilter` is only going be used for * preventing single-hop exit streams * per-circuit rate-limiting. Both of these checks will require the filter to have access to a recent `NetDir`, which is straightforward if the filter has an Arc<dyn NetDirProvider> (as mentioned in doc/dev/notes/relay-streams.md, `NetDirProvider` has a handy non-async `timely_netdir()` function we can use). And since these checks are based on consensus params, we don't really need to ever update an already-built circuit with a new `IncomingStreamRequestFilter` (because all `IncomingStreamRequestFilter` will have the ability to obtain a fresh `NetDir` as needed). Nevertheless, I left a TODO about this, because I expect this type to change once we figure out all the other pieces needed for #1448.
* tor-proto: Remove a now-redundant .iter() in a call to iter_joinIan Jackson2026-06-111-1/+1
|
* maint: Run maint/add_warning to deny string slicesClara Engler2026-06-093-0/+3
| | | | | | | | | | | | 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.
* proto: Update the tests to use the new CircuitRx{Receiver,Sender}sGabriela Moldovan2026-06-082-2/+3
|
* proto: add regression tests for verify_link_auth_certmoumenalaoui2026-06-021-0/+79
| | | | Signed-off-by: David Goulet <[email protected]>
* proto: Fix inverted cert sig and AUTHENTICATE compareDavid Goulet2026-06-021-1/+1
| | | | | | | | | | | | Missed at review and no unit tests at the time catched those. The next commit has a unit tests to make sure these checks are now valid. Fixes #2501 Fixes #2502 Special thanks to Moumen Alaoui for reporting this early! Signed-off-by: David Goulet <[email protected]>
* tor-proto: remove `use asynchronous_codec as futures_codec` in ↵Neel Chauhan2026-05-271-13/+14
| | | | | | `/channel/handler.rs` Closes #1690.
* Upgrade rand crates to 0.10.Wesley Aptekar-Cassels2026-05-121-2/+2
| | | | | | | | | | | When the circ-padding feature is enabled, we use maybenot, which does not yet support rand 0.10. In the meantime, enabling this feature pulls in rand 0.9. This is not ideal, but should be okay as a temporary situation. This also replaces the use of ReseedingRng (which was removed in 0.10) with the reseeding_rng crate. This is somewhat less performant, but it should be okay.
* proto: Apply deferred rustfmtGabriela Moldovan2026-04-231-5/+2
|
* proto: Build the PeerInfo from the dummy target in the testsGabriela Moldovan2026-04-231-5/+14
| | | | | The new relay circuit reactor test expect the `PeerInfo` to be populated with the identity keys of the peer, and won't work without this change.
* relay: Modify RelayNtorKeys to use a constructorDavid Goulet2026-04-211-7/+5
| | | | | | | | | | | | Because of the sorting requirement and the fact that a `KeyMgr` can have multiple Ntor keys, the caller now explicitly extract the two Ntor keys it wants from the `KeyMgr` and then creates a `RelayNtorKeys` object with them. Future changes (#2495) will move this into a view and warn if there are more than 2 keys. Signed-off-by: David Goulet <[email protected]>
* proto: Make the CreateRequestHandler::new() take the ntor keysDavid Goulet2026-04-211-0/+16
| | | | Signed-off-by: David Goulet <[email protected]>
* tor-proto: give our rsa ident to the channel reactorSteven Engler2026-04-161-1/+5
| | | | This will be needed for ntor handshakes.
* proto: Remove unused imports in unit testsDavid Goulet2026-04-141-1/+1
| | | | Signed-off-by: David Goulet <[email protected]>
* proto: Fix relay unit tests with new CreateRequestHandlerDavid Goulet2026-04-141-1/+13
| | | | | | | The CreateRequestHandler was introduced prior to the relay handshake unit tests so fix them now that we have a way to build one. Signed-off-by: David Goulet <[email protected]>
* proto: Add channel relay handshake unit testsDavid Goulet2026-04-141-4/+268
| | | | | | | | | | | | | Instead of copying the client unit tests into the channel module, just make both current unit tests run on a client and relay handshake. This required a bit of trickery with type HandshakeConnectFn but works out in the end. It also adds the RelayMsgBuf that wraps a MsgBuf in order to implement CertifiedConn which is very relay only. Signed-off-by: David Goulet <[email protected]>
* proto: Move test certs module at the topDavid Goulet2026-04-141-31/+31
| | | | | | | | | Just for mental sanity to have all const at the same place and easily readable. No behavior change. Signed-off-by: David Goulet <[email protected]>
* proto: Move channel client handshake tests into client moduleDavid Goulet2026-04-141-218/+10
| | | | | | | | | No behavior change. Make the const cells public as they will be useful for more upcoming unit tests especially on the relay side. Signed-off-by: David Goulet <[email protected]>
* tor-proto: small comment improvementSteven Engler2026-04-091-1/+2
|
* tor-proto: replace a tuple with a dedicated structSteven Engler2026-04-091-13/+23
|
* tor-proto: give our ed ident to the channel reactorSteven Engler2026-04-091-2/+7
| | | | This will be needed for ntor handshakes.
* Merge branch 'create-fast' into 'main'gabi-2502026-04-091-1/+1
|\ | | | | | | | | tor-proto: Move CREATE_FAST handling to a helper See merge request tpo/core/arti!3869
| * tor-proto: take `CreateRequest` message by referenceSteven Engler2026-04-081-1/+1
| | | | | | | | | | Clippy has started warning about this since we moved the CREATE_FAST handling to a helper, so this resolves that.
* | proto: Bring back AuthLogDigest and explicitly convert to SLOG/CLOGDavid Goulet2026-04-082-24/+21
| | | | | | | | | | | | | | | | | | | | | | | | From opara's comment: https://gitlab.torproject.org/tpo/core/arti/-/merge_requests/3844#note_3388789 Keep the low level AuthLogDigest type alias and return it. The callsite is the one deciding if the returned digest is a Clog or a Slog. Related to #2441 Signed-off-by: David Goulet <[email protected]>