=========================
Notes from nickm:
(This differs from pinkforest's original MR: It removes the
Cargo.lock changes and the version bump on tor-llcrypto.)
Minimal Cargo.lock changes from downgrade.
(These are exactly those changes generated by running "build" and
"test".)
There are several reasons to do this:
* It's best to bump all of our dalek dependencies at once to rc.3
or later, rather than the piecemeal approach we've been stuck
with so far.
* We don't want to do this bump right now, since there are some
tricky questions about clamping we need to figure out (see
#808), and we need to make sure we get them right, and we're in
a distracted this week.
* We _do_ need to move away from 2.0.0-rc.2 right now, since
it was causing a failure in `cargo install arti`, and then it
got yanked.
Thanks to pinkforest for helping us out here and explaining all of
this!
Fixes#926.
Commit-edited-by: Nick Mathewson <nickm@torproject.org>
The new path_ref() method returns an Arc<Path>, which gives a much
better API for reasons discussed in the new documentation of path().
(We could just replace path() if we'd prefer, but IMO having
path_ref() here isn't so bad.)
The new PathEntry struct wraps the old PathEntry enum, which has
been renamed to HopDetail. It's an opaque struct because we want to
be able to put new information in the enum as we think best.
This is mostly code movement; you may want to review it with
`--color-moved`.
I'm doing this so we can also use the function in netdoc for
looking up hsdesc authentication.
Onion service hops (pointlessly) use SHA3-256 for their
authentication, but they truncate it to 20 bytes (assuming I'm
reading the C right.)
See torspec#204 for clarification here.
We want the ability to send the same handshake request in parallel
on multiple introduce circuits. This implies encoding the client
handshake more than once.
(Sadly we can't _actually_ do this in the protocol as it stands,
since the onion service can use a separate KP_hss_ntor for each
introduction point; I'll add a comment to that effect later.)
When we break the 1:1 relationship of message and cell, we'll want
this API to take messages, not cells.
This API is experimental, so we don't need to call it a semver
break.
Closes#881.
Formerly we said that it would not return until the handler
was uninstalled. This is incorrect: it returns as soon as the
message is sent and the handler installed.
Closes#885.
I am hoping we can merge this as a "TODO (Diziet)", even though I
think it may be controversial. Ie merging this doesn't represent a
decision to do as I suggest.
Also, no longer talk about handlers being "installed". That's not
something that's exposed by this API.
And, say that `send_control_message` can be called again only
after *`send_control_message`* returns, not when `handle_msg` has
returned `UinstallHandler`. IMO this makes more sense.
Explain that we can't maintain a continuous watch while holding a
conversation with the peer. (This is surely an API bug.)
The idea here is that we want to make DataStream visible to the
RPC system without requiring that the RPC session hold the
DataStream itself (or the Reader, or the Writer). We could solve
this problem by making _all_ the state in the DataStream shared,
but that would introduce unnecessary extra locking in our critical
path.
Instead we're creating the notion of a "control handle" that lets
you manage and observe a stream without actually owning the stream.
Right now the only supported functionality is asking for the
stream's circuit.
Part of #847
- We make the tor-guardmgr "We have found that {} is usable" line
include the word "guard", otherwise it doesn't appear very useful to a
user in safe logging mode, since the guard gets replaced with
[scrubbed].
- The "Actually got an end cell..." message is downgraded to DEBUG.
There were two bugs here that made the behavior unlike that of C
tor: we had swapped the MAC inputs, and we had forgotten to include
the public key X in the input.
I think that these Input structs had been defined so that we could
use hs_ntor interchangeably with other handshakes. The trouble is,
though, that it doesn't really work like any other handshakes we
have.
Note that some of the invocations for this function seem to put the
key and the message in a questionable order. But that's a thing to
figure out later, while debugging.
Now ClientCirc is no longer `Clone`, and the things that need it
to be `Clone` instead return and use an Arc<ClientCirc>
We're doing this so that ClientCirc can participate in the RPC
system, and so that its semantics are more obvious.
Closes#846.
Thanks to the type system, this was a much simpler refactoring than
I had feared it would be.
This does the following:
- Gives every crate a `full`.
- Cause every `full` to depend on `full` from the lower-level
crates.
- Makes every feature listed _directly_ in `experimental` depend
on `__is_experimental`.
If we didn't do this, we would need to transfrom
`EncodedLinkSpec`s into a `LinkSpec::Unrecognized`, which is not
semantically right. What's more, every user of this API wants to
consume encoded link specifiers, so encoding them early saves a
little effort.
This is a workaround for an issue that I'm about to encounter
somewhere in our pile of dependencies as I add arti-rpcserver, and
somehow make serde_json visible in this test code thereby, making
the PartialEq method resolution ambiguous.
These crates had no changes until just a moment ago. But since
we updated the versions on some of their dependents, they have now
changed themselves. Thus they get patchlevel bumps.
```
tor-rtmock
tor-protover
tor-socksproto
tor-consdiff
tor-chanmgr
tor-dirclient
tor-hsservice
```
For these crates, the changes are nontrivial, so we
_do_ bump the versions on which their dependent crates depend.
Fortunately, since they are all pre-1.0, we don't need to
distinguish semver-additions from other changes. (Except for arti,
which _is_ post-1.0, but gets a patchlevel bump anyway.)
These are unstable crates with breaking changes:
```
tor-hscrypto
tor-hsclient
```
These have new or extended APIs:
```
safelog
tor-bytes
tor-cell
tor-linkspec
tor-llcrypto
tor-proto
tor-cert
arti-client
```
These have new unstable APIs or features:
```
tor-netdoc
tor-circmgr (also broke some unstable APIs)
arti (is post-1.0)
```
These have bugfixes only:
```
caret
tor-dirmgr
```
(I found "user request" in one place, and fixed that. I am not
currently going to try to unify "control message" and "meta message"
since both terms are misleading and we already have TODOs to try to
merge them into a third better term.)
This new function combines "sending a message" and "accepting
replies in a stream" into a single call, so that there is no gap
between when the message is sent and the replies are available.
There are a number of compromises here, in order to avoid API
proliferation. I've tried to contain them as best I can.
See comments for additional design discussion.
Now, the MetaCellHandler is responsible for consuming the messages
it gets, and reporting status to whatever task is waiting for a
status message.
Additionally, the MetaCellHandler can decide to remain installed or
shut down the circuit after a successful message. (Previously, it
could only uninstall itself on success and kill the circuit on
failure.)
These changes will enable MetaCellHandlers to be used as the basis
for handling more kinds of message.
(There is some moved and reformatted code here; you may want to
review it with `git {diff or show} --color-moved -b`.)
These crates have had small code changes, but no API additions:
tor-config
tor-socksproto
tor-cert
tor-chanmgr
tor-ptmgr
tor-guardmgr
tor-circmgr
tor-dirclient
tor-dirmgr
arti
tor-hsservice
tor-congestion
These crates have had API extensions:
fs-mistrust
tor-llcrypto
tor-bytes
tor-checkable
tor-linkspec
tor-netdoc
tor-persist
arti-client
This change makes sure that open streams and half-closed streams
have the same stream-type-dependent state machines with respect to
which cells are acceptable.
Fixes#774.
Fixes#769.
The role of CmdChecker is to verify that messages are arriving at
the appropriate sequence on a stream, with respect to the other
messages that have been received. Once the stream becomes
half-closed, the CmdChecker is also in charge of consuming incoming
messages on the stream and making sure that they are well-formed.
This includes a partial solution for #769, but also turned up
another bug (#774) while I was working on it. I'll close them both
once I have a real solution.
We now manipulate raw relay cell bodies as (an alias for)
`Box<[u8;509]>` rather than as (an alias for) `[u8;509]`. This
enables us to do much less copying. It will become more important
soon, as we defer parsing relay cell bodies even longer.
Related to #7.
We also use SliceWriter to avoid allocating a Vec<> for every relay
message we want to encode, and instead encode directly into the
cell.
Unlike C tor, we treat unrecognized commands as reason to kill off
the connection entirely. That's fine; if we need to add an
unrecognized command in the future, we can use VERSIONS to negotiate
it.
Also, if someday we want this code to support relay channels as
well, we can use some type trickery to have that work too.
Every FooMsg type now implements Into<AnyFooMsg>, and
TryFrom<FooMsg>.
Additionally, it now implements From<X> for every distinct type that
it supports. This last part lets us discard a bunch of code.
Unfortunately, I needed some downright hackish trickery in order to
get these macros to avoid generating `From<AnyFooMsg> for AnyFooMsg`
and conflicting with the blanket implementation.
The trickery to deal with RelayEarly and Relay being the same type
was not necessarily worth it; I will be separating them and removing
said trickery in the next commit.
In !948 we renamed a couple of accessor functions, which is a
breaking change in `tor-cell`'s API.
In retrospect, perhaps we should have deprecated the old names and
added the new ones, so we wouldn't have to break the API. (This is
the only API break AFAICT since 1.1.0.)
These changes influence behavior, but not effect compatibility.
(If I messed up, and any crate except for `arti` has non-breaking
API changes, that's still fine, since they are all version
0.x.)
This warning kind of snuck up on us! (See #748) For now, let's
disable it. (I've cleaned it up in a couple of examples, since
those are meant to be more idiomatic and user-facing.)
Closes#748.
This is a little tricky, but I think that we're not actually
exposing too much here. I expect we'll need to tweak this stuff
between now and our final version.
Nick Mathewson
pinkforest
Ian Jackson
Ian Jackson
eta
gabi-250
Dimitris Apostolou