aboutsummaryrefslogtreecommitdiff
path: root/crates/tor-proto
Commit message (Collapse)AuthorAgeFilesLines
* tor-proto: disable flowctrl sidechannel mitigations for relaysSteven Engler7 days4-14/+43
| | | | | | | | 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
* tor-proto: add `StreamHandler::flowctrl_sidechannel_mitigations()`Steven Engler7 days2-0/+10
|
* tor-proto: add `WithSidechannelMitigations` for flow ctrlSteven Engler7 days4-14/+35
|
* tor-proto: make 'testing_exports' available to own unit testsSteven Engler8 days1-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | This fixes: ```text $ cargo test -p tor-proto --features relay error[E0603]: enum import `CtrlMsg` is private --> crates/tor-proto/src/relay/reactor.rs:463:33 | 463 | use crate::channel::CtrlMsg; | ^^^^^^^ private enum import | note: the enum import `CtrlMsg` is defined here... --> crates/tor-proto/src/channel.rs:117:5 | 117 | use testing_exports::*; | ^^^^^^^^^^^^^^^^^^ note: ...and refers to the enum import `CtrlMsg` which is defined here... <snip> ``` There are a few ways we could fix this, but I don't see an advantage of one over another.
* proto: Extend test to check DESTROY is sent to the next hopGabriela Moldovan8 days1-0/+16
| | | | | | | | | | | | | This extends the `destroy_from_client()` test to also check that a DESTROY received from the client (or, more generally speaking, from the "inbound channel") is actually forwarded to the next hop. The reason the `assert_destroy_sent()` assertion is commented out is specified in the TODO that precedes it (tldr: testing the DESTROY behaviour involves both the channel reactor and the circuit reactor, and our test setup is currently quite limited, in that it doesn't actually exercise the right channel reactor code paths for the *inbound* channel). I plan to address this soon.
* proto: Move assert_cell_is_destroy macro out of helper functionGabriela Moldovan8 days1-15/+15
| | | | I am about to need this in a test.
* proto: Add comments explaining why the tests use DestroyDirection::BackwardGabriela Moldovan8 days1-2/+14
|
* proto: Remove misleading comment and rename misnamed variableGabriela Moldovan8 days1-3/+2
| | | | This fixes an old bad copy-paste that I've just noticed.
* proto: Clarify what read_{inbound,outbound} are forGabriela Moldovan8 days1-0/+14
| | | | This adds an example suggested by opara.
* proto: Update the tests to check DESTROY is only sent when neededGabriela Moldovan8 days1-25/+72
|
* proto: Allocate a real circ id in the circ reactor testsGabriela Moldovan8 days1-13/+47
| | | | | | | | | | | | | | | | | | | | | We need this because the channel reactor now only sends DESTROY for circuits that are still in the circ map (and we are about to test this behaviour, so we need the circuit map to actually have an entry for our test circuit). Initially, these tests were meant to test the circuit reactor in isolation, but they've gradually grown more complex, and now require a semi-working channel reactor. In the long run, I think I'd like to: * change the tests from `tor_proto::relay::reactor` to use a proper relay channel reactor as opposed to a `working_dummy_channel()`, and to initialize a circuit through the normal means, namely by sending a CREATE2 through the channel reactor (naturally, this "proper relay channel reactor" still wouldn't be connected to the network). These will test the integration between the channel and the circuit reactor, as well as the circuit reactor behaviour * add new, implementation-agnostic tests for the generic multi-reactor system. These will use a mock channel reactor
* proto: Make Channel::send_control() pub(crate) for testingGabriela Moldovan8 days1-0/+1
| | | | | | The relay circuit reactor tests will soon need the ability to send control messages (for allocating a circuit id for the circuit reactor under test).
* proto: Expect a now-unused test function to be dead codeGabriela Moldovan8 days1-0/+1
|
* proto: Update create handler tests to no longer expect DESTROYGabriela Moldovan8 days1-18/+6
| | | | | Now that we no longer respond to DESTROY by sending a DESTROY ourselves, these tests need to be updated.
* proto: s/client/next hop in test commentGabriela Moldovan8 days1-1/+1
| | | | This test simulates the *next hop* sending us a DESTROY.
* proto: Avoid sending DESTROY if we have received DESTROYGabriela Moldovan8 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
* proto: Remove redundant headingGabriela Moldovan9 days1-2/+0
| | | | | | There is no `BackwardReactor` heading, because there isn't that much to say about it (it moves `RELAY` cells in the opposite direction, and rejects RELAY_EARLY and PADDING_NEGOTIATE).
* proto: Update table to mention all the other commands we handleGabriela Moldovan9 days1-2/+8
|
* proto: Be more specific about where the meta messages are handledGabriela Moldovan9 days1-14/+15
|
* proto: Say how forward DESTROY are handledGabriela Moldovan9 days1-1/+2
|
* proto: Clarify that "it" refers to the forward reactorGabriela Moldovan9 days1-1/+2
|
* proto: Update docs to clarify they apply to RELAY_EARLY tooGabriela Moldovan10 days1-3/+2
|
* proto: Update docs to say EXTEND2 is supportedGabriela Moldovan10 days1-5/+2
|
* proto: Update reactor docs to say TRUNCATE is unsupportedGabriela Moldovan10 days1-2/+3
|
* Merge branch 'unk-circid' into 'main'gabi-25013 days2-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
* | Merge branch 'clippy-fixes' into 'main'gabi-25013 days4-5/+8
|\ \ | | | | | | | | | | | | tor-proto: Some misc rust/clippy warning fixes See merge request tpo/core/arti!4304
| * | tor-proto: 'expect' -> 'allow' in `CircReactorHandle`Steven Engler2026-08-111-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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
| * | tor-proto: fix a `clippy::unnecessary_filter_map`Steven Engler2026-08-111-0/+2
| | |
| * | tor-proto: box `CircParameters` in test-only `CtrlCmd::AddFakeHop`Steven Engler2026-08-112-2/+3
| | | | | | | | | | | | This fixes a `clippy::large_enum_variant`.
| * | tor-proto: fix a 'clippy::useless_conversion'Steven Engler2026-08-111-1/+1
| |/
* | tor-proto: make `CreateRequestHandler` methods asyncSteven Engler2026-08-122-28/+39
| |
* | tor-proto: use `CgoRelayCrypto`/`Tor1RelayCrypto` aliasesSteven Engler2026-08-121-8/+7
| |
* | tor-proto: change `TryFrom<_> for HandshakeSubprotocols` to a dedicated ↵Steven Engler2026-08-122-13/+16
| | | | | | | | | | | | constructor Replaces `TryFrom<SubprotocolRequest> for HandshakeSubprotocols`.
* | tor-proto: check the handshake type in the ntor testSteven Engler2026-08-121-8/+12
| |
* | tor-proto: implement the ntor-v3 handshakeSteven Engler2026-08-121-9/+199
| |
* | tor-proto: add `TryFrom<SubprotocolRequest> for HandshakeSubprotocols`Steven Engler2026-08-121-1/+74
| |
* | tor-proto: change some instances of `sendme_inc` to u8Steven Engler2026-08-123-9/+7
|/
* tor-proto: remove all default features from statrsSteven Engler2026-08-051-1/+1
| | | | | This doesn't appear to be needed, and removes an nalgebra dev-dependency which should help build times.
* tor-proto: upgrade statrsSteven Engler2026-08-041-1/+1
| | | | | | ```bash cargo upgrade --incompatible allow -p statrs ```
* Merge branch 'cc-event-counters' into 'main'opara2026-08-041-6/+7
|\ | | | | | | | | | | | | tor-proto: Fix cc vegas counter updates on clock stall Closes #2651 See merge request tpo/core/arti!4256
| * tor-proto: fix cc vegas counter updates on clock stallSteven Engler2026-07-301-6/+7
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The pseudocode says: ```text if next_cc_event: next_cc_event-- if next_cwnd_event: next_cwnd_event-- if clock_stalled_or_jumped: inflight -= 'cc_sendme_inc' return ``` but we were previously decrementing these after the return rather than before.
* | 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.