Commit Graph

522 Commits

Author SHA1 Message Date
Nick Mathewson dc0a4e3c3d Move responsibility for GuardMgr NetDir updates to GuardMgr.
Previously it was the job of a task in CircMgr to do this; but we're
going to want to give GuardMgr full access to the latest NetDir for
this, and for other code-simplification reasons.

With this change I'm deprecating a couple of functions in
tor-circmgr.  It's no longer necessary for us to have an artificial
external way for you to feed new NetDirs to a circmgr.  (I could
just remove them, but I want practice deprecating.)
2022-06-07 11:44:51 -04:00
Nick Mathewson 957eb929a0 Remove now-redundant Send+Sync constraints alongside NetDirProvider 2022-06-07 11:44:51 -04:00
Nick Mathewson 967ea67b7d Use testing_rng() in tests throughout our crates.
This only affects uses of thread_rng(), and affects them all more or
less indiscriminately.  One test does not work with
ARTI_TEST_PRNG=deterministic; the next commit will fix it.
2022-06-02 14:56:42 -04:00
Ian Jackson 4f42101554 lints: Add let_unit_value allow to all crates
From running add_warning, with manual picking of the right
hunks/lines.
2022-05-31 15:23:52 +01:00
Ian Jackson ba0843da4a lints: Add lint block delimiters to every crate
This was the result of:
  maint/add_warning crates/*/src/{lib,main}.rs
and then manually curating the results.
2022-05-31 13:00:31 +01:00
Orhun Parmaksız bfd41ddb5f
Lexically sort Cargo.toml dependencies
Utilize cargo-sort: https://github.com/DevinR528/cargo-sort

Signed-off-by: Orhun Parmaksız <orhunparmaksiz@gmail.com>
2022-05-28 20:05:51 +03:00
Nick Mathewson 4326aa1de9 Regenerate version bump from previous commit.
This commit was made by reverting the previous commit, then
re-running the script I used to generate it.  In theory there should
be no semantic changes: only changes due to improved formatting from
cargo edit.
2022-05-27 10:18:52 -04:00
Nick Mathewson b232365a75 Semantic version changes for Arti 0.4.0 release
I followed the following procedure to make these changes:

* I used maint/changed_crates to find out which crates had changed
  since 0.3.0.
* I used grep and maint/list_crates to sort those crates in
  topological (dependency) order.
* I looked through semver_status to find which crates were listed as
  having semver-relevant changes (new APIs and breaking changes).
* I scanned through the git logs of the crates with no
  semver-relevant changes listed to confirm that, indeed, they had
  no changes.  For those crates, I incremented their patch-level
  version _without_ changing the version that other crates depend on.
* I scanned through the git logs of the crates with no
  semver-relevant changes listed to confirm that, indeed, they had
  no obvious breaking changes.
* I treated all crates that depend on `arti` and/or `arti-client` as
  having breaking changes.
* I identified crates that depend on crates that have changed, even
  if they have not changed themselves, and identified them as having
  a non-breaking change.
* For all of the crates, I used `cargo set-version -p $CRATE --bump
  $STATUS` (where `STATUS` is `patch` or `minor`) to update the
  versions, and the depended-upon versions.
2022-05-27 09:01:20 -04:00
Nick Mathewson 546ae3000e Resolve the new `derive_partial_eq_without_eq` lint.
It's a little overzealous sometimes, but it's mostly to the good.
2022-05-23 12:55:37 -04:00
Ian Jackson be5bc04c02 impl_standard_builder: Have it generate FooConfig::builder
This deletes many handcoded impls.  It also generates lots of impls
that we previously didn't have.
2022-05-12 18:50:26 +01:00
eta 2255778afa Merge branch 'builder-default-bis' into 'main'
impl_standard_builder followup

See merge request tpo/core/arti!505
2022-05-12 15:57:56 +00:00
Ian Jackson c1c6f2b376 Rename impl_standard_builder from impl_default_via_builder
I have Plans for this macro.  In particular:

 * I have a wip branch which tests that the Builder can be
   deserialised from an empty config (ie, that config reading
   of a config with a blank section for this item works).

 * I think we should autogenerate $Config::builder(),
   and promote that, rather than $ConfigBuilder::default().
   This macro could do that.
2022-05-12 15:59:13 +01:00
Ian Jackson 888d6e0511 config: Replace more handwritten impl Default 2022-05-12 15:59:10 +01:00
Nick Mathewson 2a5ee2c8c9 Merge branch 'ticket_412_467' into 'main'
Teach DirMgr to use slightly untimely directories

Closes #467 and #412

See merge request tpo/core/arti!500
2022-05-12 14:42:51 +00:00
Ian Jackson 04b8729d6b Add correct serde(default) attrs for humantime_serde::option
Discovered by a test case in my local tree.  The test case was
macro-generated by an extension of impl_standard_builder (which
macro istself currently awaiting review, arti!499)

Have also sent an MR to update the upstream docs
  https://github.com/jean-airoldie/humantime-serde/pull/8
2022-05-12 11:34:53 +01:00
Nick Mathewson dd20ac45ab Note a TODO in exitpath construction. 2022-05-11 12:42:38 -04:00
Nick Mathewson 7b93091f57 Bump the version of every* crate to 0.3.0
* Except for safelog and fs-mistrust, which are new.
2022-05-06 10:03:15 -04:00
Nick Mathewson 89b38b16c7 Change safelog version to 0.1.0.
(This is okay because we haven't published it yet, or any crate that
uses it.)
2022-05-06 09:59:13 -04:00
Nick Mathewson 013bb26040 Merge branch 'derive-builder-fork' into 'main'
Switch to derive_builder_arti_fork

Closes #446

See merge request tpo/core/arti!490
2022-05-06 13:07:51 +00:00
Ian Jackson 030289481f Switch to derive_builder_arti_fork
For reference, the git source for this crate (and the others in its
workspace) currently lives in my personal github account (ijackson).
If this fork turns out to be long-lived and gains features and/or
users, it would be good to move it to a gitlab somewhere.

I have granted Nick crate ownership on the crates.io system.
2022-05-06 13:36:40 +01:00
Nick Mathewson 4679023c39 Apply `sensitive` in some info-level log messages.
This specifically applies the `sensitive` wrapper in the places
where we're logging target addresses at level "info" or higher.
2022-05-06 07:36:50 -04:00
Ian Jackson d47e94b459 config derive attrs: Make builders serde, and validated structs not
* Builders additionally derive: Debug, Serialize, Deserialize.

 * Validated structs no longer derive: Serialize, Deserialize
   and all related attributes deleted.

 * As a consequence, all the `#[serde(deny_unknown_fields)]`
   are gone.  That means that right now unknown fields are totally
   ignored.  This is good for compatibility but poor for useability.
   Doing something better here is arti#417, in progress.

 * As a consequence, delete tor_dirmgr::retry::default_parallelism.
   (The default value was already duplicated into a builder attr.)
2022-05-05 10:35:52 +01:00
Ian Jackson a334f17262 Merge branch 'socket-addr-list-builder' into 'main'
FallbackDir: orports: Introduce and use VecBuilder

See merge request tpo/core/arti!474
2022-05-04 18:13:45 +00:00
Ian Jackson 4ad4cae418 FallbackDir: Use VecBuilder for orports
And drop the ad-hoc orport() method.  This brings FallbackDir's
orports field in line with our list builder API.

The general semver note in "configuation" seems to cover most of this.
2022-05-04 17:18:55 +01:00
Ian Jackson 015db3d78d GuardUsage: restrictions: Use list builder
Although these do not appear in the config, it does have a builder.
It seems sensible to get rid of this ad-hoc list manipulation site,
and replace it with our standard list builder API.

define_list_builder_helper requires that the builder element type be
Deserialize.  Currently GuardUsageRestriction is a transparent, public
enum, so we aren't really exposing anything.

We could introduce GuardUsageRestrictionBuilder now, but
since it's not in the config and thereofore only in the public API of
the lower crates, we can definitely put that off.
2022-05-04 16:16:38 +01:00
Ian Jackson 4bca912715 Change builder list API
The new API is (roughly) as discussed in
  https://gitlab.torproject.org/tpo/core/arti/-/issues/451

This is quite a large commit and it is not convenient to split it up.
It contains the following changes:

 * Redo the list builder and accessor macros implemnetation,
   including docs and tests.

 * Change uses of define_list_config_builder.  In each case:
   - Move the docs about the default value to the containing field.
   - Remove the other docs (which were just recapitulations, and
     are now not needed since the ListBuilder is no longer public).
   - Rewmove or replace `pub` in the define_list_builder_helper call,
     so that the builder is no longer public.
   - Change the main macro call site to use define_list_builder_helper.
   - Add a call to define_list_builder_accessors.

 * Make the module `list_builder` pub so that we have somewhere to
   put the overview documentation.

 * Consequential changes:
   - Change `outer.inner().replace(X)` to `outer.set_inner(X)`
   - Consequential changes to imports (`use` statements).
2022-05-04 13:50:10 +01:00
eta 6f787e1e77 Merge branch 'derive-builder-git-fixup' into 'main'
derive_builder: Use git dep everywhere, rather than cargo patch

See merge request tpo/core/arti!477
2022-04-27 14:31:08 +00:00
Ian Jackson a97ad69855 derive_builder: Use git dep everywhere, rather than cargo patch
The `[patch]` approach causes the tree not to build when used as a
dependency, unless the `[patch]` is replicated into the depending
project.

Instead, replace our `derive_builer =` dependencies with a reference
to a specific git commit:

  perl -i~ -pe 'next unless m/^derive_builder/; s#"(0\.11\.2)"#{ version = "$1", git = "https://github.com/ijackson/rust-derive-builder", rev = "ba0c1a5311bd9f93ddf5f5b8ec2a5f6f03b22fbe" }#' crates/*/Cargo.toml

Note that the commitid has changed.  This is because derive_builder is
in fact a workspace of 4 crates.  3 of them are of interest to arti
itself (the 4th exists only for testing).  So the same "add git
revision" treatment had to be done to the `derive_builder` and
`derive_builder_macro` crates.  Each dependency edge involves a new
commit in the derive_builder workspace, since we can't create a git
commit containing its own commitid.  (We want to use commits, rather
than a branch, so that what we are depending on is actually properly
defined, and not subject to the whims of my personal github
namespace.)

There are no actual code changes in derive_builder.
2022-04-27 14:57:59 +01:00
Samanta Navarro c53818d496 Fix grammar and typos 2022-04-27 13:52:13 +01:00
Nick Mathewson c1ea419477 Merge branch 'main' into 'msrv_1_56'
# Conflicts:
#   crates/tor-config/Cargo.toml
#   crates/tor-dirmgr/src/state.rs
#   doc/semver_status.md
2022-04-26 12:45:16 +00:00
Ian Jackson ce877e4421 Document defaults for all the config lists
And add an imprecation in define_list_config_builder's doc comment do
do so in future for other invocations of the macro.

Add add the missing full stops.
2022-04-25 18:22:54 +01:00
Ian Jackson 1f215da1a3 Rename ThingListBuilder::replace (from set)
As per
  https://gitlab.torproject.org/tpo/core/arti/-/merge_requests/471#note_2798024
2022-04-25 18:15:25 +01:00
Nick Mathewson 2f6bc6bdc4 squash! Bump every crate's edition to 2021.
Remove all `use` statements for `TryFrom` and `TryInto`.  These are
now redundant in Rust 2021.
2022-04-25 13:06:26 -04:00
Nick Mathewson b60b0a266a Bump every crate's edition to 2021.
This is an automated change made with a perl one-liner and verified
with grep -L and grep -l.

Some warnings are introduced with this change; they will be removed
in subsequent commits.

See arti#208 for older discussion on this issue.
2022-04-25 13:05:31 -04:00
Nick Mathewson ca05b0904c Add 'rust-version = "1.56"' to every Cargo.toml file.
This change was made automatically with a perl one-liner, and
confirmed with `grep -L`.

The `rust-version` field itself was introduced in 1.56.0.
2022-04-25 13:04:31 -04:00
Nick Mathewson 0069fd2206 Reformat all not-yet-reformatted Cargo.toml files.
There are no semantic changes here; only formatting.  This is in
preparation for other changes (wrt MSRV and edition)
2022-04-25 13:04:31 -04:00
Ian Jackson dca4f3ede1 Use better syntax for doc comment attribute
As per
  https://gitlab.torproject.org/tpo/core/arti/-/merge_requests/471#note_2798020
2022-04-25 17:05:30 +01:00
Ian Jackson 24518675db Introduce PredictedPortsListBuilder
This means that `NetworkConfig::initial_predicted_ports` is now like
the other list-like things, returning `&mut list_builder` with the same
`set()` and `append()` methods.
2022-04-25 17:05:30 +01:00
trinity-1686a b9dd23de91 fix typo in doc 2022-04-25 00:27:29 +02:00
Ian Jackson 6da7a2e3e2 Use git source for derive_builder for now, for sub_builder feature
This commitid is the current head of my MR branch
  https://github.com/colin-kiegel/rust-derive-builder/pull/253
  https://github.com/ijackson/rust-derive-builder/tree/field-builder
Using the commitid prevents surprises if that branch is updated.

We will require this newer version of derive_builder.  The version
will need to be bumped again later, assuming the upstream MR is merged
and upstream do a release containing the needed changes.

We will need the new version of not only `derive_builder_core` (the
main macro implementation) but also`derive_builder` for a new error
type.
2022-04-21 14:07:28 +01:00
eta 41dd682552 Merge branch 'report-skew' into 'main'
Report skew estimates from arti-client

See merge request tpo/core/arti!455
2022-04-13 12:51:23 +00:00
Nick Mathewson 4582dddca8 circmgr: back off on preemptive circuits if they fail consistently
Rather than running preemptive circuit construction every 10
seconds, we change it to back off when it is "failing".  (We define
"failing" as creating no new circuits, and as giving at least one
error.)

This change means that we'll have one less reason to hammer the
network when our connectivity is failed for some reason.

Closes #437.
Part of #329.
2022-04-12 09:19:10 -04:00
Nick Mathewson 482c022e23 circmgr: Remove now-unused scheduled entry points.
Now that we have TaskSchedule, we don't need to expose these any
longer.
2022-04-12 09:04:10 -04:00
Nick Mathewson 3d5276a9cc circmgr: Report CircProvenance from AbstractCircMgr.
This feature is similar to ChanProvenance from ChanMgr, except that
we don't yet need to report it outside the crate.  I'm going to use
it to distinguish newly created circuits from existing circuits in
the preemptive circuit builder.
2022-04-12 08:41:32 -04:00
Nick Mathewson 72f00daf12 circmgr: re-export clock skew estimates. 2022-04-12 08:03:49 -04:00
Nick Mathewson c3c43b088e Create and use API to report guard/fallback skew.
(The information is not yet recorded.)
2022-04-07 10:47:45 -04:00
Nick Mathewson 0050045867 ChanMgr: Return provenance information from get_or_launch
We need this since we want to report certain conditions only when
they happen on a new channel, not if we observe them on a
preexisting channel.
2022-04-07 10:46:06 -04:00
Nick Mathewson 36440a957c Distinguish UsageMismatch cases by whether a race is possible
This lets us say that the UsageMismatch cases in some parts of the
code reflect a programming error (RetryTime::Never), whereas in
other case it reflects another circuit request getting to the
circuit first (RetryTime::Immediate).
2022-04-04 11:41:00 -04:00
Nick Mathewson 6d8a6b42e7 circmgr: Improve retry-and-or-delay logic.
Use the new RetryTime type and its associates to decide how long to
wait (if at all) between attempts to build a circuit.

Closes #421.

Part of #329.
2022-04-04 11:15:18 -04:00
Nick Mathewson c3b2bcc91e circmgr: implement HasRetryTime. 2022-04-04 11:15:18 -04:00
Nick Mathewson f7810d42eb circmgr: Improve reporting of error origins.
Previously we did not distinguish errors that came from pending
circuits from errors that came from the circuits we were
building.  We also reported errors as coming from "Left" or "Right",
instead of a more reasonable description.
2022-04-04 11:15:18 -04:00
Nick Mathewson 86c59dd1f3 circmgr: Avoid a race condition in circuit usage restriction
We were treating restrict_mut() failures as internal errors, and
using internal errors to represent them.  But in fact, these
failures are entirely possible based on timing.  Here's how it
happens:

* Two different circuit requests arrive at the same time, and both
  notice a pending circuit that they could use.
* The pending circuit completes; both pending requests are notified.
* The first request calls restrict_mut(), and restricts the request
  in such a way that the second couldn't use it.
* The second request calls restrict_mut(), and gets a failure.

Because of this issue, we treat these errors as transient failures
and just wait for another circuit.

Closes #427.

(This is not a breaking API change, since `AbstractSpec` is a
crate-private trait.)
2022-04-04 11:14:52 -04:00
Nick Mathewson 5b2fc118df Bump all arti*, tor* crates to 0.2.0
Not all of these strictly need to be bumped to 0.2.0; many could go
to 0.1.1 instead.  But since everything at the tor-rtcompat and
higher layers has had breaking API changes, it seems not so useful
to distinguish.  (It seems unlikely that anybody at this stage is
depending on e.g. tor-protover but not arti-client.)
2022-04-01 09:15:18 -04:00
Nick Mathewson 432bb5ce62 Use a lower default for max_retries.
The older default seems (experimentally) to be ridiculously high.
Generally, if we can't build a circuit within a handful attempts,
that circuit has already timed out... unless there is a fast-failure
condition, in which case we're just hammering the network (or our
view of it.)

Found with `arti-testing` for #329.
2022-03-30 14:22:28 -04:00
Nick Mathewson bd798764a4 circmgr: limit circuit attempts when launch_parallelism > 1.
Previously, if we had launch_parallelism > 1, and we were willing to
retry building a circuit max_retries times, then we'd launch up to
max_retries * launch_parallelism circuits before giving up.  Ouch!

With this patch, we try to keep the total number of circuits
planned and attempted to the actual max_retries limit.

Part of #329; found with arti-testing.
2022-03-30 14:14:52 -04:00
Nick Mathewson 2bb3ba7886 Run cargo fmt one more time for good measure. 2022-03-30 10:41:57 -04:00
Nick Mathewson 1feb7eecac Reformat several Cargo.toml files with 100-char-wide lines. 2022-03-30 10:41:40 -04:00
Nick Mathewson 6282df34fb Refactor FirstHopId into type-differentiated form
The FirstHopId type now records an enum that stores whether the hop
is a guard or a fallback.  This change addresses concerns about
remembering to check the type or source of an Id before passing it
down to the FallbackState or GuardSet.

Making this change required an API change, so that dirmgr can
report success/failure status without actually knowing whether it's
using a fallback or a guard.
2022-03-30 10:41:39 -04:00
Nick Mathewson 13af6134f6 Rename Guard=>FirstHop, GuardId=>FirstHopId
This is preparation for having separate GuardId and FirstHopId types
that distinguish which back-end they index.
2022-03-30 10:40:59 -04:00
Nick Mathewson 951b800988 DirPathBuilder::pick_path: re-order match cases for clarity. 2022-03-30 10:40:14 -04:00
Nick Mathewson bdd129f230 Rename ExternalFailure => ExternalActivity. 2022-03-30 10:40:12 -04:00
Nick Mathewson d88e9d676e Replace the fallback directories when they change in the config.
The code here uses a new iterator type, since I couldn't find one of
these on crates.io.  I tried writing the code without it, but it was
harder to follow and test.
2022-03-30 10:39:09 -04:00
Nick Mathewson bfb2353a8f Add status tracking to FallbackDir.
We do this by creating a new FallbackSet type that includes status
information, and updating the GuardMgr APIs to record success and
failure about it when appropriate.  We can use this to mark
FallbackDirs retriable (or not).

With this change, FallbackDir is now stored internally as a Guard in
the GuardMgr crate.  That's fine: the FallbackDir type really only
matters for configuration.
2022-03-30 10:39:09 -04:00
Nick Mathewson ed6389acc7 circmgr: Use guard-manager's view of the fallbacks when possible.
If we're building a path with the guard manager involved, we now ask
the guard manager to pick our first hop no matter what.  We only
pick from the fallback list ourselves if we're using the API with no
guard manager.

This causes some follow-on changes where we have to remember an
OwnedChanTarget object in a TorPath we've built, and where we gain
the ability to say we're building a path "from nothing extra at
all."  Those are all internal to the crate, though.

Closes #220, by making sure that we use our guards to get a fresh
netdir (if we can) before falling back to any fallbacks, even if our
consensus is old.

Compilation should be fixed in the next commit.
2022-03-30 10:39:09 -04:00
Nick Mathewson 9da43189f3 Turn FallbackList into a real type, and store one in GuardMgr.
The guard manager is responsible for handing out the first hops of
tor circuits, keeping track of their successes and failures, and
remembering their states.  Given that, it makes sense to store this
information here.  It is not yet used; I'll be fixing that in
upcoming commits.

Arguably, this information no longer belongs in the directory
manager: I've added a todo about moving it.

This commit will break compilation on its own in a couple of places;
subsequent commits will fix it up.
2022-03-30 10:39:06 -04:00
Nick Mathewson 6397b56317 Reformat tor-circmgr/Cargo.toml 2022-03-30 10:34:50 -04:00
Nick Mathewson 80b65c3a4d Move fallback.rs into guardmgr.
This is the logical place for it, I think: the GuardMgr's job is to
pick the first hop for a circuit depending on remembered status for
possible first hops.  Making this change will let us streamline the
code that interacts with these objects.
2022-03-30 10:34:48 -04:00
eta 5d27710ef1 Merge branch 'disallowed_lint' into 'main'
Remove allow(clippy::disallowed_methods) lint flag.

See merge request tpo/core/arti!437
2022-03-30 14:02:13 +00:00
eta ac64bdea27 Make daemon tasks self-contained; introduce NetDirProvider
The various background daemon tasks that `arti-client` used to spawn are
now handled inside their respective crates instead, with functions
provided to spawn them that return `TaskHandle`s.

This required introducing a new trait, `NetDirProvider`, which steals
some functionality from the `DirProvider` trait to enable `tor-circmgr`
to depend on it (`tor-circmgr` is a dependency of `tor-dirmgr`, so it
can't depend on `DirProvider` directly).

While we're at it, we also make some of the tasks wait for events from
the `NetDirProvider` instead of sleeping, slightly increasing
efficiency.
2022-03-30 14:26:43 +01:00
Nick Mathewson 700e491813 Remove allow(clippy::disallowed_methods) lint. 2022-03-30 08:55:58 -04:00
eta fd081742fa Merge branch 'no-system-time' into 'main'
Don't use SystemTime::now()

Closes #306

See merge request tpo/core/arti!365
2022-03-30 12:44:25 +00:00
Nick Mathewson 482b2e3256 Remove a now-irrelevant comment 2022-03-28 08:30:44 -04:00
trinity-1686a 219ad39eca remove usage of 'token' where it's no longer a token 2022-03-27 13:49:08 +02:00
trinity-1686a 85fb91deed implement IsolationHelper for StreamIsolation
but don't use it in a dyn Isolation context
2022-03-25 00:03:48 +01:00
trinity-1686a d2edf25733 move StreamIsolation to isolation module 2022-03-24 21:12:46 +01:00
trinity-1686a 5894a43d38 implement IsolationHelper for tuple of IsolationHelper 2022-03-24 20:34:21 +01:00
trinity-1686a 8dc6e958aa move isolation in separate module 2022-03-24 19:43:54 +01:00
trinity-1686a 70f71ac90b seal trait Isolation 2022-03-24 19:05:06 +01:00
trinity-1686a de5f517da6 rename *_isolation_group to *_isolation 2022-03-24 18:56:22 +01:00
eta c35bd79f5c Merge branch 'more_iso_docs' into 'main'
Expand documentation for isolation traits

See merge request tpo/core/arti!420
2022-03-23 11:42:51 +00:00
Nick Mathewson ee204328dd Expand some comments based on review from @diziet. 2022-03-21 15:16:00 -04:00
Nick Mathewson 4a644a9879 circmgr: When planning, only keep one error; log them all. 2022-03-21 15:06:05 -04:00
Nick Mathewson 1ec0ed45c8 dirmgr: Note errors and inform the circmgr about them.
Some error types indicate that the guard has failed as a dircache.
We should treat these errors as signs to close the circuit, and to
mark the guard as having failed.
2022-03-21 10:27:53 -04:00
Nick Mathewson d38aafa054 Expose more peer information from circuit build failures
We already have the ability to get peer information from ChanMgr
errors, and therefore from any RetryErrors that contain ChanMgr
errors.

This commit adds optional peer information to tor-proto errors, and
a function to expose whatever peer information is available.
2022-03-21 09:06:32 -04:00
Nick Mathewson d778a92225 circmgr: Change API for using FallbackDirs
It'll soon more convenient to pass in FallbackDirs as a slice of
references, rather than just a slice of FallbackDirs: I'm going to
be changing how we handle these in tor-dirmgr.
2022-03-21 09:06:32 -04:00
Nick Mathewson 451a53a5bf circmgr: use AllGuardsDown to retry better
If all guards are down and they won't be retriable for a while, try
waiting that long to get whichever guard _is_ retriable.

Additionally, if we are making multiple circuit plans in parallel,
only report our planning as having failed if we failed at making
_all_ the plans.  Previously we treated any failure as fatal for the
other plans, which could lead to trouble in the case when guards
were all down or pending.

Part of #407.
2022-03-21 09:06:32 -04:00
Nick Mathewson cb29921e16 Expand documentation for isolation traits
These aren't complete yet; I'm just making this commit to capture
the notes we took on a pad when we were discussing these APIs.

Part of #414.
2022-03-17 13:49:32 -04:00
Nick Mathewson 85a20ae4ec Alternative API for set_isolation_group().
Instead of requiring a `Box<dyn Isolation>`, it now takes either a
`Box<dyn Isolation>`, or an arbitrary `T` that implements
`Isolation`.

This API still allows the user to pass in a `Box<dyn Isolation>` if
that's what they have, but it doesn't require them to Box the
isolation on their own.

Part of #414.
2022-03-17 09:07:08 -04:00
Nick Mathewson 2818a3cb35 Merge branch 'test-isolation' into 'main'
new api for isolation

See merge request tpo/core/arti!377
2022-03-17 12:46:13 +00:00
Ian Jackson 40bede587c Merge branch 'config-partials-transparent' into 'main'
Absolish builders for CircMgrConfig and DirMgrConfig

See merge request tpo/core/arti!417
2022-03-17 12:30:46 +00:00
trinity-1686a 575792b583 replace TODOs with documentation 2022-03-16 20:33:36 +01:00
Ian Jackson ab352881e3 Add rationale for CircMgrConfig transparency and trait 2022-03-16 19:31:08 +00:00
Ian Jackson 82aa4b902a Provide define_accessor_trait and use it to generate CircMgrConfig 2022-03-16 19:31:05 +00:00
Ian Jackson 8bde40fdd3 Make CircMgrConfig transparent (and make it a trait)
See commentary for the rationale.
2022-03-16 19:30:59 +00:00
trinity-1686a 4eb90b72c0 add trait to help test isolation related code 2022-03-16 19:24:23 +01:00
trinity-1686a 0b7c71a888 add tests on Isolation and fix conditional compilation issues
it seems I added conditional compilation without noticing it??
and there was some errors when choosing a prefered runtime depending on
feature flags
2022-03-16 19:24:23 +01:00
trinity-1686a 698132a762 refactor restrict_mut 2022-03-16 19:24:23 +01:00
trinity-1686a cb00ac677b replace Arc with Box and use dyn-clone
this also removes JoinResult
2022-03-16 19:24:23 +01:00
trinity-1686a ec7737b322 add some documentation for new traits 2022-03-16 19:24:23 +01:00
trinity-1686a 778d6f3380 replace isolated with compatible
the inverted logic was too easy to mess up
2022-03-16 19:24:23 +01:00
trinity-1686a 4826d757ac use downcast-rs instead of our own AsAny 2022-03-16 19:24:19 +01:00
trinity-1686a 234291f666 fix existing tests 2022-03-16 19:23:21 +01:00
trinity-1686a b3b27c9bd3 add a join() on Isolation 2022-03-16 19:23:21 +01:00
Trinity Pointard a1b8b4999f testing new api for isolation 2022-03-16 19:23:21 +01:00
Ian Jackson da787d074a derive_builder: Switch to upstream 0.11
This has the different syntax for builder field attributes than what I
originally proposed in my MR, and which therefore is in the pinned
branch.

My upstream MR for the field attributes feature was morged:
  https://github.com/colin-kiegel/rust-derive-builder/issues/239
2022-03-16 16:34:44 +00:00
Ian Jackson 409c5dd6d2 Use new upstream humantime_serde_option feature
Replace all uses of our copy of this code.
2022-03-14 10:33:59 +00:00
Ian Jackson 8b8c2a426e humantime: Update to humantime-serde 1.1.1
This has the humantime_serde::option module, which we have upstreamed
and are about to switch to.

The remaining dependency with  version = "1"  is going to be removed
in a moment.
2022-03-14 10:33:59 +00:00
Ian Jackson 1b1ce8cc82 Drop remaining conversion from FooConfig to FooConfigBuilder 2022-03-07 15:58:53 +00:00
Ian Jackson 5203311a97 Derive Deserialize for derive-builder-generated config builders
I used
  git-grep -P '\#\[serde\((?!default|deny_unknown)'
to find places where I needed to add additional attributes on the
builder method fields.

This is currently a bit duplicative, but when #371 is completely done,
the validated (non-builder) configs won't need to be Deserialize any
more.

This is part of #371 and #372.
2022-03-07 15:58:53 +00:00
Ian Jackson 416b56d852 Use git source for derive_builder for now, for attrs feature
We are going to want to specify custom attributes on fields of the
builder struct.  This feature was missing from derive_builder.

This commitid is the current head of my MR branch
  https://github.com/colin-kiegel/rust-derive-builder/pull/237
  https://github.com/ijackson/rust-derive-builder/tree/builder-field-attrs
Using the commitid prevents surprises if that branch is updated.

We will require this newer version of derive_builder.  The version
will need to be bumped again later, assuming the upstream MR is merged
and upstream do a release containing the needed changes.
2022-03-07 15:56:57 +00:00
Ian Jackson b095265257 Merge branch 'educe-traits' into 'main'
Replace many manual trait impls with use of educe

See merge request tpo/core/arti!375
2022-03-04 18:00:17 +00:00
Ian Jackson ebfd734956 Move skip_fmt into tor-basic-utils
Code motion and the minimal mechanical changes.

As per
  https://gitlab.torproject.org/tpo/core/arti/-/merge_requests/375#note_2783078
2022-03-04 11:45:24 +00:00
Ian Jackson f474a583f1 Replace manual Debug impl with educe in tor-circmgr 2022-03-02 18:03:00 +00:00
Ian Jackson 90f86b47e4 Replace manual Debug impl with std derive in tor-circmgr
When I wrote this, I arranged to skip dumping the field `pending`.
This must have been because I thought that either

(a) PendingEntry couldn't `#[derive(Debug)]` (but it can)

and/or

(b) Some of the fields of PendingEntry ought not to be dumped because
they might contain (eg) packet data.  But I think they don't: there's
just the spec, and the Result which is (basically) a Circ.

I tried preseving something closer to the original using educe, but
educe gets somehow tangled up with the generics, and the result fails
to compile.  I haven't investigated this further.
2022-03-02 17:04:07 +00:00
Nick Mathewson 83c8b11c2c Merge branch 'clippy-allow-arc-clone' into 'main'
Disable clippy::clone_on_ref_ptr

See merge request tpo/core/arti!352
2022-03-01 20:38:05 +00:00
Nick Mathewson e8e9791a97 Bump all crates to 0.1.0 2022-03-01 08:59:34 -05:00
trinity-1686a dcd32f4d00 typo 2022-02-28 20:31:22 +01:00
trinity-1686a 0fc7f40575 resolve comment 2022-02-28 20:05:17 +01:00
trinity-1686a 2d430ea69e add some error to retry_error instead of dropping it 2022-02-28 19:30:40 +01:00
Nick Mathewson be288399ec Merge branch 'teardown' into 'main'
tor-circmgr: take_action: Handle Cancelled from the oneshot

Closes #365

See merge request tpo/core/arti!363
2022-02-28 15:16:03 +00:00
Nick Mathewson 12a83e9661 Merge branch 'fix/210' into 'main'
don't return already errored pending circuit when searching new circuit matching spec

Closes #210

See merge request tpo/core/arti!366
2022-02-28 13:16:31 +00:00
Ian Jackson 23f8d33d4d Add a debug! log message for source cancellation 2022-02-28 12:52:38 +00:00
Ian Jackson 6d01c60925 Fix rustfmt 2022-02-28 12:36:48 +00:00
Nick Mathewson 76d8338640 Fix two typos 2022-02-28 11:21:52 +00:00
trinity-1686a 5323825964 don't return already errored pending circuit when searching new circuit matching spec 2022-02-27 13:16:03 +01:00
trinity-1686a f06b256010 use wallclock where possible in tests 2022-02-26 00:33:44 +01:00
trinity-1686a 96daba7747 fix tests 2022-02-25 20:45:16 +01:00
trinity-1686a f9a4f23e83 remove most usage of SystemTime::now 2022-02-25 20:34:27 +01:00
Ian Jackson dbf019e426 tor-circmgr: take_action: Handle Cancelled from the oneshot
Fixes #365

Inspection of the code and logs shows that:
 * One of the plan futures' oneshots must be returning Cancelled
 * This means that the corresponding sender must have been dropped
 * The sender is owned by the task spawned by spawn_launch
Presumably that entire task gets dropped as part of executor shutdown,
or something.

The correct response in this situation is to declare that we are
shutting down, and stop trying to do stuff.

Unfortunately, despite trying quite hard by putting sleeps in various
strategic places, I have not been able to reproduce the problem.  So I
can't be 100% sure that the new behaviour is correct.

But I am reasonably confident that this ought not to be able to occur
unless either 1. the task from spawn_launch is dropped, or 2. that
task somehow panics despite its attempts to trap panics and report
them as errors through the oneshot.

So this "burn it all down" action ought only to occur in actually
serious situations.

I observe that
  3ff9b187ea
  Handle panics from circuit construction.
changed the EK for PendingCanceled to EK::ReactorShuttingDown,
and there's From impl.  I think, therefore, that it is right
to reuse this Error variant.

I don't quite understand why when take_action gets an actual error it
doesn't push it, but just logs it.  But I am not changing that for
now.

Arguably the two instances of retry_error.push are a sign of an
inferior flow control pattern - maybe the loop body including the code
I am adding ought to be an IEFE returning
`Result<Option<circ>, crate::Error>`.
2022-02-25 18:10:36 +00:00
Ian Jackson 6b615b4766 impl Debug for various internal types
I wanted this while debugging something.

The ad-hoc impl Debug with f.debug_struct is getting repetitive
and I've already perpetrated one copy-paste mistake.
We should consider using something like the `educe` crate's Clone.
2022-02-25 17:37:10 +00:00
Ian Jackson afb50fe735 Disable clippy::clone_on_ref_ptr
This lint is IMO inherently ill-conceived.

I have looked for the reasons why this might be thought to be a good
idea and there were basically two (and they are sort of contradictory):

I. "Calling ‘.clone()` on an Rc, Arc, or Weak can obscure the fact
    that only the pointer is being cloned, not the underlying data."

This is the wording from
  https://rust-lang.github.io/rust-clippy/v0.0.212/#clone_on_ref_ptr

It is a bit terse; we are left to infer why it is a bad idea to
obscure this fact.  It seems to me that if it is bad to obscure some
fact, that must be because the fact is a hazard.  But why would it be
a hazard to not copy the underlying data ?

In other languages, faliing to copy the underlying data is a serious
correctness hazard.  There is a whose class of bugs where things were
not copied, and then mutated and/or reused in multiple places in ways
that were not what the programmer intended.  In my experience, this is
a very common bug when writing Python and Javascript.  I'm told it's
common in golang too.

But in Rust this bug is much much harder to write.  The data inside an
Arc is immutable.  To have this bug you'd have use interior mutability
- ie mess around with Mutex or RefCell.  That provides a good barrier
to these kind of accidents.

II. "The reason for writing Rc::clone and Arc::clone [is] to make it
     clear that only the pointer is being cloned, as opposed to the
     underlying data. The former is always fast, while the latter can
     be very expensive depending on what is being cloned."

This is the reasoning found here
  https://github.com/rust-lang/rust-clippy/issues/2048

This is saying that *not* using Arc::clone is hazardous.
Specifically, that a deep clone is a performance hazard.

But for this argument, the lint is precisely backwards.  It's linting
the "good" case and asking for it to be written in a more explicit
way; while the supposedly bad case can be written conveniently.

Also, many objects (in our codebase, and in all the libraries we use)
that are Clone are in fact simply handles.  They contain Arc(s) (or
similar) and are cheap to clone.  Indeed, that is the usual case.

It does not make sense to distinguish in the syntax we use to clone
such a handle, whether the handle is a transparent Arc, or an opaque
struct containing one or more other handles.

Forcing Arc::clone to be written as such makes for code churn when a
type is changed from Arc<Something> to Something: Clone, or vice
versa.
2022-02-24 18:15:44 +00:00
Nick Mathewson 92141c6d72 Merge remote-tracking branch 'origin/mr/340' 2022-02-23 09:29:40 -05:00
eta 05257da72d Merge branch 'restore_needless_borrow_check' into 'main'
Remove clippy::needless_borrow exception in CI.

Closes #310

See merge request tpo/core/arti!338
2022-02-23 13:15:15 +00:00
Nick Mathewson 784f1531bb Make NoLock into BadApiUsage.
To implement this, we had to refactor the tor_circmgr api for
flushing state changes to disk, so that it checks if it has the lock,
and only then tries to store.
2022-02-22 16:13:37 -05:00
Nick Mathewson 04ca1f662f Fold EK::Canceled into TransientFailure
Also add some TODO comments in circmgr for future work.
2022-02-22 15:30:12 -05:00
Nick Mathewson dd55f5ce2d Remove clippy::needless_borrow exception in CI.
This exception is no longer necessary now that the underlying CI bug
is fixed.
2022-02-20 09:09:38 -05:00
Nick Mathewson 3ff9b187ea Handle panics from circuit construction.
We handle them by reporting them to task that's waiting for the
circuit, then relaying the panic.

Doing so allows the waiting task to distinguish panics
(EK::Internal) from cases where the reactor dropped the task
entirely (EK::ReactorShuttingDown).  And doing _that_ removes one
case of EK::Canceled, which helps us on our goals towards #348.

Closes #347.
2022-02-18 14:22:38 -05:00
Nick Mathewson e3e3f9934b Move the main body of our circuit-launching task into a new function
This reduces our nesting, and will help us handle panics.
2022-02-18 08:33:50 -05:00
Nick Mathewson 83d001d661 Merge branch 'remaining-errors' 2022-02-17 14:22:47 -05:00
Nick Mathewson de86ac0b0f Rename CircuitTimeout to TorNetworkTimeout. 2022-02-17 12:22:51 -05:00
Nick Mathewson 4db586cf00 tor_circmgr::Error: Sort variants by interesting-ness.
We can't use discriminants here now, but maybe we can in the future.
2022-02-17 12:12:04 -05:00
Ian Jackson 2d4901ccde tor-circmgr: errors: Use autoconversion for Bug 2022-02-17 13:31:03 +00:00
Ian Jackson 094ddd7cd7 Add a comment about "&mut [&mut ]" 2022-02-17 11:07:52 +00:00
Nick Mathewson 543916e812 Clarify and rename PendingCanceled
From its old name, this error had implied that we were giving no
useful information when we were waiting on a pending cirucit request
that failed.  In fact, this error would only happen if we dropped the
`mpsc::Sender` for a circuit attempt without reporting success or
failure.
2022-02-16 14:35:06 -05:00
Nick Mathewson 1d773e748f Provide a better ErrorKind from RetryError.
(Instead of reporting the _last_ error, report the _worst_ error.)
2022-02-16 14:35:06 -05:00
Nick Mathewson c1899f787b circmgr: Add a Kind for speculative guard failure.
These errors should almost never be seen by the user; we should instead
retry the circuit.  But they _can_ be seen by the use if selecting a
guard takes too long, or too many attempts. (Therefore, they aren't true
"internal" errors.)

I suspect that we might not want to keep this TransientFailure kind, but
I'm not sure what else to do here for now.
2022-02-16 14:35:06 -05:00
Nick Mathewson 96d856e264 Add kinds for *most* circmgr errors.
There are a couple of tricky ones I'll do separately.
2022-02-16 14:35:06 -05:00
Nick Mathewson 900007585a circmgr: Port InternalError to use Bug. 2022-02-16 14:35:06 -05:00
Nick Mathewson ed57157d84 Re-enable clippy::ptr_arg where it had been disabled.
In one of the two places, nightly no longer warns.  In the other
place, it's fine for nightly to warn: I just fixed the code to take
a slice instead.

Partial revert of 856aca8791.

Resolves part of #310.
2022-02-16 11:33:12 -05:00
Yuan Lyu 98b1a5a279 Move persistent state flush from client to circmgr 2022-02-15 20:04:45 -05:00
Nick Mathewson 1cecc7e45a Change deny(clippy::all) to warn(clippy::all).
Closes #338.
2022-02-14 09:24:06 -05:00
Nick Mathewson 3c342ae5d7 Add TODOs on uncertain points about time_since_last_traffic
This edge-case was there even before the migration of
595fe1ab88, but now it's more explicit and ought to be
revisited.
2022-02-09 10:06:53 -05:00
Yuan Lyu 595fe1ab88 Remove the use of Mutex in channel unused_since timestamp 2022-02-08 18:28:45 -05:00
Nick Mathewson f08c0268bc Tests for TargetPorts::display() 2022-02-04 16:41:32 -05:00
Nick Mathewson 070e52653d Make SpawnError wrappers contain a 'spawning' string
(By our convention, these errors should say what we were trying to
spawn when the error occurred.)
2022-02-04 16:06:11 -05:00
Ian Jackson f5e874cf91 errors: Drop "Error" and "Failed" from various enum variants 2022-02-04 14:42:37 +00:00
Ian Jackson d21b2cc6f5 tor-circmgr: Introduce TargetPorts with a pretty Display impl 2022-02-04 14:42:37 +00:00
Ian Jackson 09116d7b4d tor-circmgr::Error: impl HasKind 2022-02-04 14:42:37 +00:00
Ian Jackson f4813e249c tor-circmgr: Handle channel creation errors in the new style 2022-02-04 14:42:37 +00:00
Ian Jackson de17c64412 spawn errors: Fix arti-client, tor-chanmgr, tor-circmgr
Provide an enum variant to contain the SpawnError and a From impl.

We use `#[from]` here because it doesn't really make sense to attach
any context, as it's not likely to be very relevant.
2022-02-04 14:42:37 +00:00
Ian Jackson 6e1dc612cc tor-error: Add as a ddpendency to many crates
Doing this here makes it easier when I rebase/reorder things
2022-02-04 14:42:37 +00:00
Ian Jackson a7e6caa731 tor-circmgr: impl Display for TargetPort
This will be used for error handling, and perhaps other things.
2022-02-04 14:33:09 +00:00
Ian Jackson eaa449d373 tor-circmgr: Turn a type annotation comment into code
The type annotation may not be necessary for inference, but as a
comment it risks becoming false.  So it should be uncommented, or
deleted.

Error types round here are not entirely trivial so uncomment it.
2022-02-04 14:33:09 +00:00
Nick Mathewson 03755a5c7b Merge branch 'dirclient-testing' into 'main'
dir-client: bug fix and more tests

See merge request tpo/core/arti!271
2022-02-03 15:09:06 +00:00
eta cc37c8f5b5 Merge branch 'typos' into 'main'
Fix typos

See merge request tpo/core/arti!285
2022-02-03 13:12:38 +00:00
Ian Jackson 7be3bf6339 Temporarily disable some clippy lints on nightly 2022-02-02 21:57:30 +00:00
Ian Jackson 5bfe94eb0e Untangle two needless Ok(r?) into just r
Prompted by clippy::needless_question_mark.  Sometimes Ok(r?) is
needed to do automatic error conversion.  I assume the lint checks for
that.  Anyway, in these cases it's not needed.
2022-02-02 18:35:28 +00:00
Dimitris Apostolou 6526321851
Fix typos 2022-02-02 20:18:22 +02:00
Nick Mathewson c8dd73d55f Upgrade required version of futures crate to 0.3.14
Earlier versions have a bug in UnboundedReceiver that make our new
dirclient tests fail.
2022-02-01 09:54:47 -05:00
Nick Mathewson 329bde58dd Bump tor-netdir and tor-guardmgr versions
tor-netdir needs to bump because tor-netdoc bumped, even though
there were no other changes in tor-netdir.  Whoops.

tor-guardmgr needs to bump because it already published, with the
older tor-netdir.
2022-01-31 11:05:34 -05:00
Nick Mathewson 01d9937308 Bump the patch version of every crate that changed since 0.0.3 2022-01-31 10:30:52 -05:00
Nick Mathewson 30b3818a9e Make the native-tls crate optional.
This commit puts the native-tls crate behind a feature.  The feature
is off-by-default in the tor-rtcompat crate, but can be enabled
either from arti or arti-client.

There is an included script that I used to test that tor-rtcompat
could build and run its tests with all subsets of its features.

Closes #300
2022-01-26 14:06:58 -05:00
Nick Mathewson 2333d0466e Rename FooRuntime to FooNativeTlsRuntime for consistency. 2022-01-26 14:06:58 -05:00
eta 146fbbaaa8 Merge branch 'ticket255' into 'main'
Refactor our Runtime implementations to allow replacement parts

Closes #255

See merge request tpo/core/arti!251
2022-01-24 14:09:51 +00:00
Ian Jackson aa4d8de16e StreamPrefs: rename from ConnectPrefs
The docs even say this is about stream.

As @nickm writes in
  https://gitlab.torproject.org/tpo/core/arti/-/merge_requests/252#note_2771289

  we generally call end-to-end connections that are tunneled over Tor
  "Streams" to distinguish them from everything else in the Tor
  protocols that could possibly be called a "Connection".

That seems to apply here too.
2022-01-21 11:07:15 +00:00
Nick Mathewson a58e4e3688 Test for PathConfig::at_least_as_permissive_as().
This is totally not just an exercise to get combined test coverage
for tor-circmgr over 90% because I needed something to do that
wouldn't distract anybody else. :)
2022-01-20 09:57:19 -05:00
Nick Mathewson a5288aa15f Rename PathConfig::more_permissive_than()
Since it implements a "<=" type relationship, it should be called
"at_least_as_permissive_as()."  Since it's a crate-private function,
the long name isn't too bad.
2022-01-20 09:55:54 -05:00
Nick Mathewson b0ea74aa60 Remove "self" arg from PathConfig::builder()
This was added by mistake.
2022-01-20 09:53:12 -05:00
Nick Mathewson 17920e43f8 Refactor Runtimes to use separate TLS implementations internally.
This will make it easier to implement them using some other TLS
provider as well, without having to duplicate all of our code.
2022-01-19 15:47:26 -05:00
Nick Mathewson 7d3482ca1a Bump all crate versions to 0.0.3. 2022-01-11 09:40:32 -05:00
eta da848a1b9c Merge branch 'ticket_178' into 'main'
Fix ticket 178: Don't use a NetDir until we have microdescriptors for all of our primary guards.

Closes #178

See merge request tpo/core/arti!220
2022-01-10 14:02:24 +00:00
Nick Mathewson 4841b50c9f Minimize the required version for each dependency.
I found these versions empirically, by using the following process:

First, I used `cargo tree --depth 1 --kind all` to get a list of
every immediate dependency we had.

Then, I used `cargo upgrade --workspace package@version` to change
each dependency to the earliest version with which (in theory) the
current version is semver-compatible.  IOW, if the current version
was 3.2.3, I picked "3".  If the current version was 0.12.8, I
picked "0.12".

Then, I used `cargo +nightly upgrade -Z minimal-versions` to
downgrade Cargo.lock to the minimal listed version for each
dependency.  (I had to override a few packages; see .gitlab-ci.yml
for details).

Finally, I repeatedly increased the version of each of our
dependencies until our code compiled and the tests passed.  Here's
what I found that we need:

anyhow >= 1.0.5: Earlier versions break our hyper example.

async-broadcast >= 0.3.2: Earlier versions fail our tests.

async-compression 0.3.5: Earlier versions handled futures and tokio
    differently.

async-trait >= 0.1.2: Earlier versions are too buggy to compile our
    code.

clap 2.33.0: For Arg::default_value_os().

coarsetime >= 0.1.20: exposed as_ticks() function.

curve25519-dalek >= 3.2: For is_identity().

generic-array 0.14.3: Earlier versions don't implement
    From<&[T; 32]>

httparse >= 1.2: Earlier versions didn't implement Error.

itertools at 0.10.1: For at_most_once.

rusqlite >= 0.26.3: for backward compatibility with older rustc.

serde 1.0.103: Older versions break our code.

serde_json >= 1.0.50: Since we need its Value type to implement Eq.

shellexpand >= 2.1: To avoid a broken dirs crate version.

tokio >= 1.4: For Handle::block_on().

tracing >= 0.1.18: Previously, tracing_core and tracing had separate
    LevelFilter types.

typenum >= 1.12: Compatibility with rust-crypto crates

x25519-dalek >= 1.2.0: For was_contributory().

Closes #275.
2022-01-07 19:08:58 -05:00
Nick Mathewson e9a507af67 Merge branch 'circ_self_by_ref' 2022-01-07 14:48:21 -05:00
Nick Mathewson c123138f74 Merge branch 'remove-type-annotation' into 'main'
tor-circmgr: Remove a type annotation in a method call

See merge request tpo/core/arti!225
2022-01-07 19:33:04 +00:00
Nick Mathewson debac8b973 circmgr: Fix a pair of clippy warnings. 2022-01-07 13:53:21 -05:00
Ian Jackson d63a251afc tor-circmgr: Remove Arc around ClientCirc
See the new commentary text on `ClientCirc` for the rationale.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
2022-01-07 18:19:20 +00:00
Ian Jackson 14d7edc5f8 tor-circmgr: tests: Do fake circuit equality by id
We are going to get rid of the Arc.  Happily there is an id which is
always constructed uniquely and preserved by clone.

(auto-deref lets us make the function take &Self instead of &Arc)

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
2022-01-07 18:16:43 +00:00
Ian Jackson 7c55141e3f tor-circmgr: tests: Introduce and use FakeCirc::eq()
This removes a lot of open-coded Arc::ptr_eq() calls

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
2022-01-07 18:16:43 +00:00
Ian Jackson a034ef3526 tor-circmgr: Replace some Arc::clone with .clone()
This will make the code work when it's not an Arc any more.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
2022-01-07 18:16:39 +00:00
Ian Jackson 5469579ca9 tor-circmgr: Remove a type annotation in a method call
This is a method, so the resolution is automatic.  It's not clear to
me why this was written out this way, given that extend_ntor is right
above.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
2022-01-07 18:12:27 +00:00
Ian Jackson 19a104e2cc tor-circmgr: Require that AbstractCirc are Clone
We are going to get rid of a lot of Arc, so we need the underlying
thing to be Clone.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
2022-01-07 18:12:15 +00:00
Nick Mathewson 5ac0fcb7ef Add API to check if primary MDs are missing.
We need this information to know if it's okay to migrate to a new
NetDir, or if we need to download more information first.

Part of #178.
2022-01-06 15:58:23 -05:00
Neel Chauhan 5dfedb4c6f De-Arc-ify Buildable for ClientCirc 2022-01-06 12:22:49 -08:00
eta f08854bc69 Merge branch 'main' into 'remove_unused_rngs'
# Conflicts:
#   crates/tor-circmgr/src/build.rs
2022-01-06 15:48:42 +00:00
Nick Mathewson 5c3300867d Merge remote-tracking branch 'origin/mr/214' 2022-01-06 09:02:09 -05:00
Daniel Eades 592642a9e6 extend lints to include 'clippy::all' 2021-12-28 20:15:40 +00:00
Neel Chauhan d72d37ff4a Remove a bunch of unused RNGs 2021-12-25 18:44:55 -08:00
Neel Chauhan 383843f0d7 tor-circmgr: Don't clone parameters in create_chantarget() 2021-12-25 17:22:38 -08:00
Nick Mathewson efe74e8c9b Only count timeouts when we've seen net activity.
This closes arti#256.  It makes our behavior match Tor's more closely,
though it has a simpler implementation than Tor. I think that the extra
complexity in Tor's logic is because we used to record timeouts in
the histogram as well as in the success/failure log.
2021-12-20 15:34:17 -05:00
Nick Mathewson 09d0c20c94 Merge branch 'eta/instant-checked-add' into 'main'
preemptive.rs: Use Instant::checked_add instead of raw subtraction

See merge request tpo/core/arti!206
2021-12-20 20:07:59 +00:00
eta 091065454e preemptive.rs: Use Instant::checked_add instead of raw subtraction
The implementations of `Add` / `Sub` (et al.) on `std::time::Instant`
can panic if the underlying OS structure can't represent the result
(like arti#266). Use Instant::checked_add and print a warning instead,
to prevent panicking.

Also, we now add instead of subtracting; I suspect it's reasonable that
you might not be able to go backward past the first `Instant` created on
some platforms, but going *forward* should probably work?
2021-12-20 19:27:55 +00:00
Nick Mathewson 26fc8073f2 Remove XXXs from tor-circmgr::mgr
IIUC, these anticipatd a need to store min_exit_circs_per_port in
CircMgr.  But the current design, where it goes into preemptive.rs and
thence to usage, seems to work fine.
2021-12-20 10:46:13 -05:00
Nick Mathewson a0870f8f79 Adjust comment to be accurate wrt #263. 2021-12-16 08:37:50 -05:00
Nick Mathewson d1d541142f Do not treat spawn failure as a fatal error. 2021-12-15 15:35:04 -05:00
Nick Mathewson 70dc6913e0 Expand some comments about circuit expiration.
Emphasize that circuit expiration functions _decide whether to
expire the circuit_, and don't expire it automatically.
2021-12-15 15:27:33 -05:00
Yuan Lyu a346893065 Add spawn_expiration_task function in circuit manager 2021-12-15 00:22:09 -05:00
eta 8040f7afb2 Merge branch 'reconfigure' into 'main'
Make most arti-client fields reconfigurable.

See merge request tpo/core/arti!181
2021-12-13 14:49:23 +00:00
Neel Chauhan 52cae03621 Don't create circuits if the consensus is stale by over 72 hours 2021-12-12 20:29:53 -08:00
Trinity Pointard 9753a7ee06 fix nightly clippy errors 2021-12-09 12:16:23 +01:00
Nick Mathewson 940ab11b80 Use a safer histogram rebuild algorithm.
Our old algorithm could, on some inputs, exhaust RAM.  That's not great,
since we try to be robust againt corruption to the state file.
2021-12-08 10:59:36 -05:00
Nick Mathewson 8f200223f7 Change an XXX in pareto.rs: a ticket is now open. 2021-12-08 10:29:44 -05:00
Nick Mathewson a359b84318 Resolve an XXXX: timeout scaling _is_ documented 2021-12-08 10:19:11 -05:00
Nick Mathewson 9767a1d063 Fix Rustdoc errors. 2021-12-08 10:06:22 -05:00
Nick Mathewson cac4ce759e Minor circuit predictor tweaks and comments.
Most notably, make min_exit_circs_for_port actually get used.

Also add a couple of comments.
2021-12-07 16:48:09 -05:00
Nick Mathewson 0f8d620757 Make preemptive circuits reconfigurable.
This required re-centralizing the configuration object for preemptive
circuits, since previously the settings from it were a bit spread out
over the crate.
2021-12-07 16:42:58 -05:00
Nick Mathewson a406e8e449 Make circuit_timing reconfigurable. 2021-12-07 16:08:52 -05:00
Nick Mathewson 99fb41218d Add new configuration objects to reconfigure.
(These weren't in the codebase when I started the first version of
this branch.)
2021-12-07 15:47:25 -05:00
Nick Mathewson 11210124da Allow on-the-fly changing of path_rules
And now the complexity begins: when the user changes the path_rules,
they not only want new circuits to obey those rules: they want
_all new requests_ to be put onto circuits that obey those rules.

That means that when the path rules become more restrictive, we need
to retire all the circuits, and make sure that currently pending
circuits aren't used for any requests.

If it's any comfort, doing this was even more complicated in C tor. ;)
2021-12-07 15:42:14 -05:00
Nick Mathewson 606d64eac5 Sketch API for reconfiguration.
This patch doesn't actually make anything reconfigurable, but it
does create an API that will tell you "you can't change the value of
that!"  If the API looks reasonable, I can start making it possible
to change the values of individual items.
2021-12-07 15:42:14 -05:00
Nick Mathewson 48f77a93c0 Clarify names and docs for predictive circuits.
Also, use humantime_serde, rather than a number of seconds, to indicate
configuration time.
2021-12-07 12:09:05 -05:00
Nick Mathewson c2e20a242a Rename circuits_preemptive to preemptive_circuits
This obeys a few conventions:
  * adjective before noun
  * config objects end with "config"
2021-12-07 12:06:53 -05:00
eta a3f2e32485 Merge branch 'bug183a_redux' into 'main'
Squash, refactor, and test !139 (Don't use same family as exit when picking a guard)

Closes #183

See merge request tpo/core/arti!173
2021-12-07 16:01:30 +00:00
eta 85bb40a002 Merge branch 'safe_mul_dur_f64' into 'main'
Use a panic-free function to multiply timeouts.

See merge request tpo/core/arti!175
2021-12-07 15:47:18 +00:00
eta 45b96579b8 Merge branch 'preemptive-config' into 'main'
Allow configurability on preemptive circuits

Closes #245

See merge request tpo/core/arti!164
2021-12-07 15:04:42 +00:00
Neel Chauhan 0e9c2d274e Allow configurability on preemptive circuits 2021-12-07 15:04:41 +00:00
Nick Mathewson d6f628c6b7 Use a panic-free function to multiply timeouts.
Previously we used Duration::mul_f64, which panics if its output is
out-of-range.  That shouldn't actually be possible for the values
we're giving it, but probably it's better to just multiply in a safe
way.

This resolves a couple of XXXXs and therefore relates to #231.
2021-12-06 16:00:17 -05:00
Nick Mathewson 31b385c5b2 Resolve roughly half of the XXXXs.
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.
2021-12-06 15:11:03 -05:00
Nick Mathewson 2c2f774bd1 Move the "real families" code into tor-netdir.
Just as `in_same_family` is a member of Relay, so the function for
getting all the real family members of a relay should belong in the
same crate.

This change also removes the `family()` accessor: it gives the _claimed_ family rather
 than the _acknlowedged_ family, and is therefore a bit dangerous.

 There's still a hole in this logic; I've noted it in the Limitations
section.  If we get a microdescriptor for a relay in between creating
and using  the guard restriction, it might be omitted from the family
list.
2021-12-06 10:48:27 -05:00
Nick Mathewson cfc31dadd4 Use hashset _inside_ GuardRestriction.
This approach saves us from a linear search when picking guards.
2021-12-06 09:44:56 -05:00
Nick Mathewson 54971e3c9a Change GuardUsage to have Vec of restrictions.
There's not much reason to use a HashSet here, since we're just
going over the whole list.

This reverts commit 16e8489abb and does a little more
refactoring.
2021-12-06 09:26:32 -05:00
Neel Chauhan b0016682c3 Implement guard family restriction code 2021-12-06 09:05:48 -05:00
Nick Mathewson d3aecd5192 Add a semicolon. 2021-11-30 15:43:21 -05:00
Nick Mathewson c733374693 Merge remote-tracking branch 'origin/mr/154' 2021-11-30 14:12:08 -05:00
eta 9a94f72e42 Add tests & address review commentary 2021-11-30 16:58:08 +00:00
Nick Mathewson eef81d9d57 Bump every crate by one patch version. 2021-11-29 15:21:58 -05:00
Nick Mathewson eb861b7edd Merge branch 'config-updates-and-tests' 2021-11-29 13:59:41 -05:00
eta ca42139944 Actually build preemptive circuits (and minor fixes)
The new CircMgr::build_circuits_preemptively function actually causes
preemptive circuits to be built; it gets called from arti-client, like
the other daemon tasks the CircMgr has.
2021-11-29 14:47:09 +00:00
Nick Mathewson b640bf75d5 Merge remote-tracking branch 'origin/mr/148' 2021-11-29 09:12:32 -05:00
Neel Chauhan 2461bd86b1 tor-circmgr: Write a test for BadExit support. 2021-11-29 09:08:32 -05:00
Neel Chauhan c98cc793a0 In struct PendingEntry, remove circ_spec 2021-11-28 17:19:58 -08:00
Nick Mathewson 17266305e2 Ensure that all config sections have deny_unknown_fields
When we deserialize a configuration, we should reject unknown variables
(except when we have an explicit reason to allow them).
2021-11-25 09:39:11 -05:00
Daniel Eades db16d13df4 add semicolons if nothing returned 2021-11-25 13:20:37 +00:00
Daniel Eades 052f51ff71 deglob some enums, use concise iteration syntax 2021-11-25 12:39:52 +00:00
Nick Mathewson f55950ab8d Fix a few typos.
Also fix some commonwealth spellings that had slipped in.
2021-11-24 18:12:44 -05:00
eta 8a5a9575c6 Introduce PreemptiveCircuitPredictor and TargetCircUsage::Preemptive
In preparation for making Arti build circuits preemptively, this commit
introduces `TargetCircUsage::Preemptive`, a circuit usage that works
somewhat differently from other ones: it requires at least 2 circuits to
exist that can exit the port it contains in order for an existing
circuit to match against it (path-spec.txt § 2.1.1); if that's not the
case, that usage will require building new circuits (in order that we
build enough to have 2 available).

This required refactoring how circuit reuse worked; now,
`CircList::find_open` uses the new `AbstractSpec::find_supported` trait
method, which we customize to implement the above check in the case of
`Preemptive` circuit usages. To make that work, `OpenEntry` now takes
two type parameters (the spec and circuit types), instead of taking a
builder type parameter and using its associated types. (We also got rid
of type constraints on that struct, yay!)

A WIP implementation of a preemptive circuit predictor that implements
path-spec.txt § 2.1.1 is also included, but this will require additional
effort to wire it up with the `CircMgr` properly.
2021-11-23 16:37:52 +00:00
Nick Mathewson 307ca9b4d0 Implement meta-builder pattern for TorClientConfig
This should be ergonomic than having to construct every section of the
configuration separately.
2021-11-22 14:53:52 +00:00
Nick Mathewson 40ff7113d4 Make every Config type implement Eq.
Doing this is necessary for reconfiguration support, and will help a lot
with testing, too.
2021-11-21 12:06:15 -05:00
Nick Mathewson e7fdf05f50 For every* config type, make defaults consistent.
This patch makes sure that for every* config type we have, the defaults
you get from a Builder match those you get from Serde, and that both
match the value that you get from arti_defaults.toml. Later down the
line I'll be adding some tests to keep these in sync.

* StorageConfig still has no defaults of its own, since we aren't so
sure we want other applications to use Arti's directories by default.
2021-11-21 11:52:43 -05:00
Nick Mathewson 97f5a7a357 Give every ConfigBuilder a From<Config> implementation.
This will make it more convenient to reconfigure things.
2021-11-21 10:54:34 -05:00
Nick Mathewson aa83a5e38a Ensure that every section-level config type has a builder() function. 2021-11-21 10:54:34 -05:00