This is getting rather confusing; simply reformtting it won't do, I
think. Also there would be much rightward drift.
So move the meat out into the new function.
(And introduce a convenience alias for its captures.)
Docs and reformatting will follow in a moment.
Explicit drops don't work. Instead, introduce a scope.
We need two scopes, actually: one where we do the initial table
wrangling, and one for the retries after relock.
So we must put the meat in a closure so we can reuse it.
And we must return the flow control as an enum. Bah, etc.
Avoid reformatting this for the moment. This makes the delta legible...
This is still correct from a lock hierarchy pov. It moves the guard
relock to the end, which is going to be necessary since it is going to
have to move right outside the loop.
Provide a more cookied "secret keys for use to connecting to a
particular HS" type, with a builder.
This wants to use config stuff, so oughtn't to be in tor-*crypto.
The individual types remain there.
*If* we're going to retain any HS knowledge in circmgr, it definitely
doesn't need to know about per-operation client secrets.
(Maybe there might be ambient secrets, used for .onion diversion, but
they don't need to be in this API.)
Otherwise there has to be a state entry in the circmgr *and* a state
entry in the hs connector, for every HS. This division of
responsibilit will be confusing.
The HS code will then be more completely just a layer on top of circmgr.
This makes our implementation behave the same as the C tor
implementation, by validating all of the expiration and signatures
on the certificates in the inner document.
(It is still not semantically necessary to check these certs: the
document in which they appear is already signed by the key with
which they are allegedly signed.)
Closes#744
The old code parsed and encoded a signature and a mac... but there
was no way to actually set them properly. Now EstablishIntro is
built around an EstablishIntroBody, and has the ability to check
signatures and macs.
Because there is no way to handle one of these messages if we can't
check the signature, we no longer accept unrecognized `auth_key` types
in this message.
I've added a test to make sure that we can validate a message from the
C tor implementation, and a test to make sure we can validate our
own cells. I also had to modify the previous tests so that their
keys were well-formed.
These crates didn't have any changes until now, when I bumped
the versions of some other crates they depend on:
tor-consdiff
arti-hyper
arti-bench
arti-testing
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
These are API breaks, but the crates themselves are currently
100% experimental, so there's no need to bump the minor versions
according to our semver rules.
The four values of times taken in a particular test were changed to both
be human readable and have comments explaining their significance (they
are all important moments after the Unix Epoch for freedom)
Unlike linkspec, this doesn't validate the actual contents of the
specifiers. We'll use this so we can handle the linkspec list for an
introduction point in an HsDesc, and just pass it on when
constructing our circuits.
I haven't added any accessor or constructor functions, because I
don't expect to need them.
It said to check whether C enforces an absence of extraneous bytes
at the end of the link specifiers. It does, in
`hs_desc.c:decode_link_specifiers()`, where it says:
```
if (link_specifier_list_parse(&specs, decoded,
(size_t) decoded_len) < decoded_len) {
goto err;
}
```
The comparison with "decoded_len" checks whether all the bytes were
decoded.
Some code in our tests that worked fine with time 0.3.17 no
longer works with 0.3.19, despite the semver.
See https://github.com/time-rs/time/issues/552 for the upstream bug.
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.
There's a blanket impl of Itertor for &mut impl Iterator, so this
isn't necessary, and it prevents us passing iterators by value
producing syntactic vinegar.
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'll use this to router relay messages on a circuit to the
appropriate stream, and hand them to that stream, without parsing
the message until the stream has been determined.
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.
Because the API assumes that many writes are infallible, this writer
takes ownership of the backing object, and will only return it to
you if you didn't run over the end.
I'm going to use this to save some allocations in relay cell bodies
Several HS message types have an extension list type. They all use
the same framing for extensions, but each of them has separate
extension types and separate extension namespaces.
This commit simplifies establish_intro a little, and adds support
for maintaining unrecognized extension types--at the expense of some
new internal code.
Some of the HS message types have a lot of dependent types, like
extensions and options for those extensions, and so on. Except when
those extensions are portable across cell types, it makes sense
to put them in their own modules.
We want to fuzz these parsers, but there's no currently way to get
at the parsers for inner documents without going through a lot of
encryption. (Coverage-guided fuzzers are powerful, but they
can't find SHA3 preimages.)
There will be two call sites to demonstrate it.
Eventually maybe this will want to be in tor-basic-utils, since it
doesn't depend on any of the tor-netdoc types. But it would be
sensible to wait until the situation with PeekableIterator and
Itertools is improved.
For now we make this #[doc(hidden)] to avoid it becoming part of our
stable API.
This one was generated (by dgoulet) using `ClientDescEncKey`
encryption. Its information is:
```
Address: paozpdhgz2okvc6kgbxvh2bnfsmt4xergrtcl4obkhopyvwxkpjzvoad.onion
Time period: 19397
Client:
paozpdhgz2okvc6kgbxvh2bnfsmt4xergrtcl4obkhopyvwxkpjzvoad:descriptor:x25519:SDZNMD4RP4SCH4EYTTUZPFRZINNFWAOPPKZ6BINZAC7LREV24RBQ
Service:
descriptor:x25519:SACGOAEODFGCYY22NYZV45ZESFPFLDGLMBWFACKEO34XGHASSAMQ
```
Fixes an instance of #768.
The problem with the test vectors is that I used a random time
period number (1234) and the default-in-tor period length (1440)
without checking whether 1440 _meant_ minutes or seconds. I'll add
another test to Tor to make sure that the time period matches now.
With this change, I can test Tor-generated hsdescs with encryption,
so I'm fairly confident that the new behavior is correct.
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.
Actually, to avoid making a breaking change, I'm deprecating
BadMessage and creating a new InvalidMessage variant that takes a
Cow. This way I don't need to track every crate that re-exposes
tor_bytes::Error and call this a breaking change in those.
Making this change will allow tor_bytes errors to be much more
helpful.
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.
This change lets us use ChannelCodec to encode and decode any
restricted channel message type we want. (Later on, we'll turn the
related Codec class in tor-proto into a more type-restricted version
of this.)
* Provide an accessor for the HSDIR flag
* Provide a function for testing a relay for hsdir inclusion
* Provide an iterator on NetDir that returns the hsdirs
* Implement Netdir::compute_rings in terms of a new
HsDirRing::compute, that currently does nothing.
* Actually call Netdir::compute_rings (since now it doesn't panic).
* Make Netdir::compute_rings not be pub. We do this unconditionally,
rather than exposing the distinction between a netdir-without-hsdir
and a netdir-with-hsdir.
The file which contains this type is called hsdir_params.rs. We have
a general problem with slight confusion about when to includen "dir"
and when to include "ring".
Resolve this in favour of the rule now added to the module-level doc
comment.
These variables are going to be struct fields, which will sort of
enforce consistent naming. The struct fields are going to appear in a
moment. We'll call the fields "current" and "secondary" after the
naming in the test cases.
And import hsdir_params::HsRingParams, which we're going to make more
references to.
* Remove the return value, which was not used anywhere.
Also remove the code to calculate the return value.
* Take an Arc<NetDir> rather than a reference. We are going to want
this for HS support. This has no overall effect on the lifetime of
the4 Arc, which was owned at the one call site and then imediately
dropped.
* Change the documentation to explain what the function's role is in
the netdir API, rather than the fiddly details of what it actually
does internally. Relegate the latter to a code comment.
(When we have HS, this will do more, or, at least, make further
arrangements.)
Locally, the only functional effect is that now we refuse to handle
non-whole-number-of-minutes lengths - but since the consensus
parameter can't represent those, there's no overall functional change.
We need to make sure any `#[cfg(feature=...)]` attributes are
applied not only to our variant declarations, but also to the
branches in the match statements that deal with them.
Doing this will make it much easier to implement a macro that
generates restricted instances of the Msg types (for #525).
The Body change is a breaking change. I don't think anybody else
implements Body, but in theory they could.
These are generalizations of RelayCell and ChanCell respectively,
that allow using an arbitrary message type in place of the fully
general RelayMsg and ChanMsg types. Doing this is a prerequisite
for usefully implementing arti#525.
These functions consume a checkable wrapper, and return a new
checkable wrapper with mapped contents but the same not-yet-checked
constraints.
As documented, They are "dangerous" because the provided function
gets access to the contents before they are checked; the caller has
to make sure that the provided function doesn't expose their
contents inappropriately.
There are some places where I note certificates which are not
currently validated, because there is no cryptographic point in
doing so. We should either document that this is okay, or validate
the certificates anyway.
This code might benefit from refactoring to make it prettier.
It turns out that C Tor doesn't add a newline at the end of the
middle layer of an onion service descriptor. I've made a spec MR
(torspec!109) to document this: here, it's time to work around the
issue.
I generated this using C tor (latest main) and a Chutney network
about a week ago.
The subcredential is:
78210A0D2C72BB7A0CAF606BCD938B9A3696894FDDDBC3B87D424753A7E3DF37
The HS_blind_id is:
43CC0D62FC6252F578705CA645A46109E265290343B1137E90189744B20B3F2D
Now we require that, for all `SectionRules`, either the caller say
how to handle unrecognized tokens (using `.add(UNRECOGNIZED...)`),
or that they explicitly reject unrecognized tokens (using
`reject_unrecognized`()`.)
This solution uses an assert!() rather than an Error to indicate
failure. I say that's fine, since
1. This is a crate-internal API.
2. We never dynamically construct SectionRules according to
different behavior: they are always prefabricated in a fixed
code block. Thus, if we test a parser at all, we will make
sure that its SectionRules are well-formed.
I considered and explicitly rejected a solution where the builder
had to be finalized with separate methods `build_strict()` or
`build_tolerant()`: It's too easy IMO for the caller to forget what
these call means.
Prevents further recurrences of #752.
Closes#752.
This fixes an instance of bug#752. Previously, we would reject any
AuthCert that contained an unexpected keyword. (Fortunately, this
data format does not change very often.)
Ian Jackson
Nick Mathewson
Ian Jackson
Saksham Mittal
Dimitris Apostolou
eta