| Commit message (Collapse) | Author | Age | Files | Lines |
|---|
| ... | |
| |
|
|
|
| |
Previously the code would let us try to install a meta-cell handler
before the old one was done, leading to possible confusion.
|
| |\
| |
| |
| |
| | |
tor-proto: use const-time eq on sendme tags.
See merge request tpo/core/arti!201
|
| | |
| |
| |
| |
| |
| |
| | |
There's no known attack here, but it's best practice to always compare
digests using a constant-time comparison operator.
This resolves an XXXX comment.
|
| | |
| |
| |
| |
| |
| |
| |
| | |
Previously we'd always set it to true, allowing one CONNECTED per
half-closed stream even if the stream had already received a
CONNECTED cell.
This resolves an XXXX.
|
| |/ |
|
| |
|
|
|
| |
This was an XXXX before. Now it explains why the behavior is safe for
now, but maybe not forever.
|
| |
|
|
|
|
|
|
|
|
|
|
| |
It makes sense to put the method for human-readable strings onto the
type itself, so that we can format these whenever they occur.
I'm choosing the "human_str" method name here, since caret-generated
types already have a to_str. I was thinking about using Display,
but caret types already implement that.
I've also moved the message from "warn!" to "debug!", since these
aren't necessarily a problem condition.
|
| |\ |
|
| | | |
|
| | | |
|
| | | |
|
| | | |
|
| |\ \
| | |
| | |
| | |
| | |
| | |
| | | |
Always check whether stream-level SENDMEs are expected.
Closes #261
See merge request tpo/core/arti!192
|
| | |/
| |
| |
| |
| |
| |
| |
| |
| |
| | |
(It's a protocol violation to get a SENDME when our send window is
already full.)
This patch makes SendWindow::put return a Result, so that it's
easier to do the right thing with it.
Closes #261.
|
| |/
|
|
|
|
|
|
|
|
|
|
|
| |
arti!126 overhauled the `tor-proto` circuit reactor, but left out one
very important thing: actually decrementing the SENDME window for
streams (not circuits) when we send cells along them.
Since the circuit-level SENDME window would often prevent us from
running into a problem, this wasn't caught until my benchmarking efforts
noticed it (in the form of Tor nodes aborting the circuit for a protocol
violation).
fixes arti#260
|
| | |
|
| |
|
|
|
|
|
| |
[T;N] supports TryFrom<Vec<T>>, and has since Rust 1.48: we can just
use that.
This resolves an XXXX comment.
|
| |\ |
|
| | | |
|
| |/
|
|
|
| |
We generally try to track the latest rust-crypto traits when we can:
fortunately, this upgrade didn't break much, considering.
|
| |
|
|
| |
There is now a ticket about this issue in general, at arti#254.
|
| |
|
|
|
|
|
|
| |
We want to only use TODO in the codebase for non-blockers, and open
tickets for anything that is a bigger blocker than a TODO. These
XXXXs seem like definite non-blockers to me.
Part of arti#231.
|
| | |
|
| | |
|
| |
|
|
|
|
| |
This test seems unreliable on CI: we've got to disable them for now
so that we have a working CI system. The CI failure is #238; the
ticket to repair them is #244.
|
| | |
|
| | |
|
| | |
|
| |
|
|
|
|
|
|
|
|
| |
I traced the problem here to the fact that sometimes "rx" in this
test would be dropped before the test was done. When "rx" is
dropped, the channel reactor shuts down, which in turn kills off the
circuit reactor.
This bug may exist in other cases in these tests. This patch may
fix one case of #238.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The `bad_extend_*` failures were caused by bad test code in
`bad_extend_test_impl` that used `futures::join!`; this meant that the
reactor could receive the `Extended2` cell before it actually got the
`ExtendNtor` request, which caused it to get (quite rightly) confused
and close the circuit. Spawning a background thread which has a short
delay before sending the `Extended2` cell seems to have alleviated this
problem.
`new_circ_create_failure` is similar; I think the reactor was getting
dropped before it had a chance to flush out its `CreateFast` cell
properly, because it had already gotten the result back (since the test
code sends it indiscriminately). This was "fixed" in much the same
manner as the other test: making it wait a bit before sending the result
cell back.
There seem to be other tests that use `futures::join!` (like
`begindir`?), and use similarly erroneous patterns; I haven't gotten any
to fail reliably enough to be able to debug them, though.
|
| |
|
|
| |
Closes #226.
|
| |
|
|
| |
Closes #222.
|
| |
|
|
| |
Thanks to eta's refactoring, these tests no longer need to be async.
|
| | |
|
| |
|
|
|
| |
The clippy code for warning about these on nightly CI can't tell the
difference between cfg(test) and no cfg(test).
|
| |
|
|
| |
The `circid` field in `ClientCirc` is now testing-only.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Previously, the reactor would use an `UnboundedSender` to send things to
the `RawCellStream`, in order that the reactor wouldn't block if you
failed to read from the latter. This is bad, though, since it means
people can just run us out of memory by sending lots of things.
To fix this, we make the new `StreamReader` type (which does the reading
parts from `RawCellStream`) keep track of the stream's receive window
and issue SENDMEs once *it* has consumed enough data to require it, thus
meaning that we shouldn't get sent enough data to fill the channel
between reactor and `StreamReader` (and, if we do, that's someone trying
to flood us, and we abort the circuit).
As hinted to above, the `RawCellStream` was removed and its reading
functionalities replaced by `StreamReader`; its writing functionalities
are handled by `StreamTarget` anyway, so we just give out one of those
for the write end. This now means we don't need any mutexes!
note: this commit introduces a known issue, arti#230
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Rather like e8e9699c3c239d6c30f9ad414f15d3bad6ec03fd ("Get rid of
tor-proto's ChannelImpl, and use the reactor more instead"), this
admittedly rather large commit refactors the way circuits in `tor-proto`
work, centralising all of the logic in one large nonblocking reactor
which other things send messages into and out of, instead of having a
bunch of `-Impl` types that are protected by mutexes.
Congestion control becomes a lot simpler with this refactor, since the
reactor can manage both stream- and circuit-level congestion control
unilaterally without having to share this information with consumers,
meaning we can get rid of some locks.
The way streams work also changes, in order to facilitate better
handling of backpressure / fairness between streams: each stream now has
a set of channels to send and receive messages over, instead of sending
relay cells directly onto the channel (now, the reactor pulls messages
off each stream in each map, and tries to avoid doing so if it won't be
able to forward them yet).
Additionally, a lot of "close this circuit / stream" messages aren't
required any more, since that state is simply indicated by one end of a
channel going away. This should make cleanup a lot less brittle.
Getting all of this to work involved writing a fair deal of intricate
nonblocking code in Reactor::run_once that tries very hard to be mindful
of making backpressure work correctly (and congestion control); the old
code could get away with having tasks .await on things, but the new
reactor can't really do this (as it'd lock the reactor up), so has to do
everything in a nonblocking manner.
|
| |
|
|
|
| |
It seems like a good time to do this, before we add a zillion other
arguments to begin_stream.
|
| |
|
|
|
|
|
| |
* Make it crate-visible only.
* Make it idempotent
* Have it be an internal error if it's called at the wrong time.
* Simplify the return logic.
|
| | |
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
@nickm pointed out that refactoring tor_proto::channel's Reactor to do
sending as well meant that it could only send or receive, but not both,
simultaneously, which was bad!
To fix this, rewrite Reactor::run_once to use a handcrafted future (with
futures::future::poll_fn) that can handle the logic required to push
items onto the sink asynchronously (i.e. checking that it can be written
to before trying to do that, and then flushing it).
This also means we don't use select_biased! any more, and just handroll
that logic ourselves; as a small bonus, we can now process all 3 kinds
of message in one run_once() call, instead of having to do only one of
them.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Instead of awkwardly sharing the internals of a `tor-proto` `Channel`
between the reactor task and any other tasks, move most of the internals
into the reactor and have other tasks communicate with the reactor via
message-passing to allocate circuits and send cells.
This makes a lot of things simple, and has convenient properties like
not needing to wrap the `Channel` in an `Arc` (though some places in the
code still do this for now).
A lot of test code required tweaking in order to deal with the refactor;
in fact, fixing the tests probably took longer than writing the mainline
code (!). Importantly, we now use `tokio`'s `tokio::test` annotation
instead of `async_test`, so that we can run things in the background
(which is required to have reactors running for the circuit tests).
This is an instance of #205, and also kind of #217.
|
| |\ |
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
We need this for the circuit timeout estimator (#57). It needs to
know "how recently have we got some incoming traffic", so that it
can tell whether a circuit has truly timed out, or whether the
entire network is down.
I'm implementing this with coarsetime, since we need to update these
in response to every single incoming cell, and we need the timestamp
operation to be _fast_.
(This reinstates an earlier commit, f30b2280, which I reverted
because we didn't need it at the time.)
Closes #179.
|
| | |
| |
| |
| |
| |
| |
| |
| | |
Basically the same thing as 371437d3384ed73520e7141e66874be3d85f1df0
("Refactor tor_proto::channel::Reactor to use an UnboundedSender"), but
for tor_proto::circuit's Reactor instead.
(part of arti#217)
|
| |\ \ |
|
| | |/
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
There wasn't any good reason for tor-proto's channel reactor to use a
shedload of oneshot channels instead of just an mpsc UnboundedSender,
and the whole `CtrlResult` thing made even less sense.
Straighten this code out by replacing all of that machinery with a
simple UnboundedSender, instead.
(part of arti#218)
|
| |/ |
|
