Previously, there was a bug in the way that our code used our SOCKS
implementations. If the buffer used for a SOCKS handshake became full
without completing the handshake, then rather than expanding the buffer
or closing the connection, our code would keep trying to read into the
zero-byte slice available in the full buffer forever, in a tight loop.
We're classifying this as a LOW-severity issue, since it is only
exploitable by pluggable transports (which are trusted) and by
local applications with access to the SOCKS port.
Closes#861.
Fixes TROVE-2023-001.
Reported-By: Jakob Lell <jakob AT srlabs DOT de>
Method dispatch rules mean that if the receiver type of the actual
function changes, `self.call()` can turn into a purely-recursive call
which overflows the stack.
Async Rust doesn't have the usual warning for this situation :-(.
UFCS is clumsier but doesn't have that problem because it involves
much less magical dispatch. Instead of generating a recursive call
which overflows the stack, it fails to compile.
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
Fixes warning from
cargo -o doc --document-private-items --all-features --workspace
This was evidentlhy overlooked during recent replacement of unescorted
private keys in the code.
When building our list of acknowledgments, previously we would only
include author and committer names.
Now we also include anybody listed in the "Reported-by",
"Co-authored-by", and "Thanks" trailers.
I could also have stopped using `::default()` to construct this
(testing-only) object, but I think it makes more sense to turn it
into a non-unit object.
- 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.
Per #798, we want to make sure that we never pass around an
`ed25519::SecretKey`; only an `ed25519::Keypair` (or
`ExpandedKeypair`). This is because, when you're computing an
ed25519 signature, you have to use the public key as one of your
inputs, and if you ever use a mismatched public key you are
vulnerable to a nonce reuse attack.
(For more info see
https://moderncrypto.org/mail-archive/curves/2020/001012.html )
This is like an `ed25519::Keypair`, except that instead of a
`SecretKey` it contains an `ExpandedSecretKey`.
We'll be using this to implement #798, where we impose a rule that
there must be no "unescorted" ed25519 secret keys.
We never want to create one of these from its parts except when we
are testing it; we only want to forward an Introduce1 message with a
new command on it.