aboutsummaryrefslogtreecommitdiff
path: root/crates/tor-guardmgr
Commit message (Collapse)AuthorAgeFilesLines
* Merge branch 'clippy-allow-arc-clone' into 'main'Nick Mathewson2022-03-011-1/+0
|\ | | | | | | | | Disable clippy::clone_on_ref_ptr See merge request tpo/core/arti!352
| * Disable clippy::clone_on_ref_ptrIan Jackson2022-02-241-1/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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.
* | Bump all crates to 0.1.0arti-v0.1.0Nick Mathewson2022-03-011-15/+15
| |
* | Merge branch 'always-coarsetime' into 'main'eta2022-02-281-1/+1
|\ \ | | | | | | | | | | | | Make coarsetime dependency and traffic-timestamping non-optional. See merge request tpo/core/arti!358
| * | Make coarsetime dependency and traffic-timestamping non-optional.Nick Mathewson2022-02-251-1/+1
| |/ | | | | | | | | | | | | | | | | | | | | | | | | | | Previously coarsetime and the traffic-timestamp feature were enabled, since they were only required for a small corner of the guardmgr algorithm. But in 1.0 and beyond we'll be adding a bunch of other features (eg, netflow padding, DoS prevention) that will need coarsetime all over the place. And since we're going to be doing coarsetime all over the place, the previous justification for making traffic-timestamping optional (the tiny performance hit) is no longer relevant.
* / impl Debug for various internal typesIan Jackson2022-02-251-0/+11
|/ | | | | | | | 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.
* Remove clippy::needless_borrow exception in CI.Nick Mathewson2022-02-201-1/+0
| | | | | This exception is no longer necessary now that the underlying CI bug is fixed.
* Change deny(clippy::all) to warn(clippy::all).Nick Mathewson2022-02-141-1/+1
| | | | Closes #338.
* Add TODOs on uncertain points about time_since_last_trafficNick Mathewson2022-02-091-0/+1
| | | | | | This edge-case was there even before the migration of 595fe1ab881b94106649, but now it's more explicit and ought to be revisited.
* Remove the use of Mutex in channel unused_since timestampYuan Lyu2022-02-081-5/+10
|
* Make SpawnError wrappers contain a 'spawning' stringNick Mathewson2022-02-041-11/+25
| | | | | (By our convention, these errors should say what we were trying to spawn when the error occurred.)
* errors: impl HasKind for GuardMgrErrorIan Jackson2022-02-041-0/+12
|
* spawn errors: tor-guardmgr: Use formulaic patternIan Jackson2022-02-041-2/+2
| | | | This makes this like all the others, and is marginally shorter
* spawn errors: impl HasKind for futures::SpawnErrorIan Jackson2022-02-041-0/+1
| | | | | | | | | This needs two kinds. We have decided to treat a non-shutdown SpawnError as "unexplained" rather than as an InternalError. There are many crates whose From<futures::task::SpawnError> for Error erroneously treat it as an internal error. We will fix them in a moment.
* tor_persist::Error: impl HasKind and adjust commentsIan Jackson2022-02-041-1/+2
| | | | | And change the comments to slightly reinterpret these errors, to relate to the circumstances rather than error generation site.
* Merge branch 'dirclient-testing' into 'main'Nick Mathewson2022-02-031-1/+1
|\ | | | | | | | | dir-client: bug fix and more tests See merge request tpo/core/arti!271
| * Upgrade required version of futures crate to 0.3.14Nick Mathewson2022-02-011-1/+1
| | | | | | | | | | Earlier versions have a bug in UnboundedReceiver that make our new dirclient tests fail.
* | Temporarily disable some clippy lints on nightlyIan Jackson2022-02-021-0/+1
|/
* Bump tor-netdir and tor-guardmgr versionsarti-v0.0.4Nick Mathewson2022-01-311-3/+3
| | | | | | | | 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.
* Bump the patch version of every crate that changed since 0.0.3Nick Mathewson2022-01-311-7/+7
|
* Make the native-tls crate optional.Nick Mathewson2022-01-261-1/+1
| | | | | | | | | | | 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
* Merge branch 'ticket_176_v2' into 'main'Nick Mathewson2022-01-112-63/+144
|\ | | | | | | | | | | | | guardmgr: Use a better persistent data format Closes #176 See merge request tpo/core/arti!233
| * Remove now-unused GuardSet::new().Nick Mathewson2022-01-111-14/+8
| |
| * guardmgr: Use a better persistent data formatNick Mathewson2022-01-112-50/+137
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Previously we stored only one guard sample, in a state file called "default_guards". That's not future-proof, since we want to have multiple samples in the future. (`guard-spec.txt` specifies separate samples for highly restrictive filters, and for bridge usage.) This patch changes our behavior so that we can store multiple samples in a new "guards" file. I had thought about automatically migrating from the previous file format and location, but I don't think that's necessary given our current (lack of) stability guarantees. Closes #176.
* | guardmgr::..::sample_test: Fix intermittent failure.Nick Mathewson2022-01-112-3/+38
|/ | | | | | | | | | | | | | This test should only fail very rarely (around 1/2.4e8) when guards are chosen from a list of 20 with uniform probability. But that wasn't what we were doing on the mock test network: we were choosing from a list of 10 viable guards, with nonuniform probability. As a fix, we change the test network probabilities so that the guards _are_ chosen with a uniform probability for this test, and we use a modified version of the test network where there are indeed 20 Guard-flagged relays with the required DirCache=2 protocol. Closes #276.
* Bump all crate versions to 0.0.3.Nick Mathewson2022-01-111-13/+13
|
* Merge branch 'ticket_178' into 'main'eta2022-01-103-6/+147
|\ | | | | | | | | | | | | 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
| * Tests for new guardmgr functionality.Nick Mathewson2022-01-062-0/+83
| |
| * Add API to check if primary MDs are missing.Nick Mathewson2022-01-063-2/+34
| | | | | | | | | | | | | | 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.
| * guardmgr: Don't use no-md guards for data circs.Nick Mathewson2022-01-061-5/+31
| | | | | | | | | | | | | | If we don't know a current microdescriptor for a guard, we can't use it for multihop circuits, since we don't know its onion keys. This is part of a fix for #178.
* | Minimize the required version for each dependency.Nick Mathewson2022-01-071-8/+8
|/ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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.
* extend lints to include 'clippy::all'Daniel Eades2021-12-281-0/+1
|
* Remove unused started_at PendingRequestNeel Chauhan2021-12-142-12/+2
|
* Make TlsConnector wrap TCP connections, not create its owneta2021-12-071-1/+1
| | | | | | | | | | | | | | | | | | | | `tor-rtcompat`'s `TlsConnector` trait previously included a method to create a TLS-over-TCP connection, which implied creating a TCP stream inside that method. This commit changes that, and makes the function wrap a TCP stream, as returned from the runtime's `TcpProvider` trait implementation, instead. This means you can actually override `TcpProvider` and have it apply to *all* connections Arti makes, which is useful for issues like arti#235 and other cases where you want to have a custom TCP stream implementation. This required updating the mock TCP/TLS types in `tor-rtmock` slightly; due to the change in API, we now store whether a `LocalStream` should actually be a TLS stream inside the stream itself, and check this property on reads/writes in order to detect misuse. The fake TLS wrapper checks this property and removes it in order to "wrap" the stream, making reads and writes work again.
* Merge branch 'bug183a_redux' into 'main'eta2021-12-072-15/+49
|\ | | | | | | | | | | | | 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
| * Tests for new family-related functions.Nick Mathewson2021-12-061-0/+23
| |
| * Use hashset _inside_ GuardRestriction.Nick Mathewson2021-12-062-1/+4
| | | | | | | | This approach saves us from a linear search when picking guards.
| * Change GuardUsage to have Vec of restrictions.Nick Mathewson2021-12-062-32/+26
| | | | | | | | | | | | | | | | There's not much reason to use a HashSet here, since we're just going over the whole list. This reverts commit 16e8489abbea1581b8e2 and does a little more refactoring.
| * Implement guard family restriction codeNeel Chauhan2021-12-062-9/+23
| |
* | Resolve roughly half of the XXXXs.Nick Mathewson2021-12-061-1/+3
|/ | | | | | | | 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.
* Merge branch 'readme_fixes'Nick Mathewson2021-11-301-1/+1
|\
| * run ./maint/readmes.shdagon2021-11-291-1/+1
| |
* | Bump every crate by one patch version.Nick Mathewson2021-11-291-13/+13
| |
* | add semicolons if nothing returnedDaniel Eades2021-11-252-4/+5
| |
* | deglob some enums, use concise iteration syntaxDaniel Eades2021-11-251-6/+6
|/
* More typo fixes that I forgot to save :(Nick Mathewson2021-11-241-3/+3
|
* Fix a clippy issue on nightlyNick Mathewson2021-11-241-0/+1
|
* Fix a few typos.Nick Mathewson2021-11-242-5/+5
| | | | Also fix some commonwealth spellings that had slipped in.
* Avoid a warning about retain_mut() in nightly.Nick Mathewson2021-11-231-2/+2
| | | | | | | Rust nightly claims that Vec might get its own retain_mut method, which would potentially conflict with the extension method we've grabbed from the retain_mut crate. To solve this, we're calling the method explicitly.
* Merge remote-tracking branch 'origin/mr/140'Nick Mathewson2021-11-231-1/+13
|\