Arti issueshttps://gitlab.torproject.org/tpo/core/arti/-/issues2024-02-21T13:42:07Zhttps://gitlab.torproject.org/tpo/core/arti/-/issues/1290Channel build task disappeared internal error2024-02-21T13:42:07Zgabi-250Channel build task disappeared internal errorSpotted in the arti svc logs from https://gitlab.torproject.org/tpo/core/arti/-/issues/1280#note_2997720:
```
2024-02-19T11:25:19Z DEBUG tor_circmgr::mgr: While waiting on circuit: Ok(Err(Channel { peer: OwnedChanTarget { addrs: [23.88.7...Spotted in the arti svc logs from https://gitlab.torproject.org/tpo/core/arti/-/issues/1280#note_2997720:
```
2024-02-19T11:25:19Z DEBUG tor_circmgr::mgr: While waiting on circuit: Ok(Err(Channel { peer: OwnedChanTarget { addrs: [23.88.75.121:9001], method: Direct([23.88.75.121:9001]), ids: RelayIds { ed_identity: Some(Ed25519Identity { eJ/RzeShStQOHZR2IKO8DY39SiYsAujwWG3XoT5fcWc }), rsa_identity: Some(RsaIdentity { $6ea127366d2dbd8b05bc150fb1b93382a2e29b1f }) } }, cause: Internal(Bug(BugRepr { message: "channel build task disappeared", location: Location { file: "/home/stefani/2024/02/07/arti/crates/tor-chanmgr/src/mgr.rs", line: 229, col: 54 }, backtrace: Captured( 0: tor_error::internal::Bug::new_inner
1: tor_chanmgr::mgr::AbstractChanMgr<CF>::get_or_launch::{{closure}}
2: <tor_proto::circuit::ClientCirc as tor_circmgr::build::Buildable>::create_chantarget::{{closure}}
3: tor_circmgr::build::double_timeout::{{closure}}::{{closure}}
4: tokio::runtime::task::raw::poll
5: tokio::runtime::scheduler::multi_thread::worker::Context::run_task
6: tokio::runtime::task::raw::poll
7: tokio::runtime::task::UnownedTask<S>::run
8: std::sys_common::backtrace::__rust_begin_short_backtrace
9: core::ops::function::FnOnce::call_once{{vtable.shim}}
10: <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once
at /rustc/82e1608dfa6e0b5569232559e3d385fea5a93112/library/alloc/src/boxed.rs:2007:9
<alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once
at /rustc/82e1608dfa6e0b5569232559e3d385fea5a93112/library/alloc/src/boxed.rs:2007:9
std::sys::unix::thread::Thread::new::thread_start
at /rustc/82e1608dfa6e0b5569232559e3d385fea5a93112/library/std/src/sys/unix/thread.rs:108:17
11: <unknown>
12: <unknown>
), source: None, kind: Internal })) })) from circuit we're building
2024-02-19T11:25:19Z DEBUG tor_circmgr::mgr: circuit we're building sent error Channel { peer: OwnedChanTarget { addrs: [23.88.75.121:9001], method: Direct([23.88.75.121:9001]), ids: RelayIds { ed_identity: Some(Ed25519Identity { eJ/RzeShStQOHZR2IKO8DY39SiYsAujwWG3XoT5fcWc }), rsa_identity: Some(RsaIdentity { $6ea127366d2dbd8b05bc150fb1b93382a2e29b1f }) } }, cause: Internal(Bug(BugRepr { message: "channel build task disappeared", location: Location { file: "/home/stefani/2024/02/07/arti/crates/tor-chanmgr/src/mgr.rs", line: 229, col: 54 }, backtrace: Captured( 0: tor_error::internal::Bug::new_inner
1: tor_chanmgr::mgr::AbstractChanMgr<CF>::get_or_launch::{{closure}}
2: <tor_proto::circuit::ClientCirc as tor_circmgr::build::Buildable>::create_chantarget::{{closure}}
3: tor_circmgr::build::double_timeout::{{closure}}::{{closure}}
4: tokio::runtime::task::raw::poll
5: tokio::runtime::scheduler::multi_thread::worker::Context::run_task
6: tokio::runtime::task::raw::poll
7: tokio::runtime::task::UnownedTask<S>::run
8: std::sys_common::backtrace::__rust_begin_short_backtrace
9: core::ops::function::FnOnce::call_once{{vtable.shim}}
10: <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once
at /rustc/82e1608dfa6e0b5569232559e3d385fea5a93112/library/alloc/src/boxed.rs:2007:9
<alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once
at /rustc/82e1608dfa6e0b5569232559e3d385fea5a93112/library/alloc/src/boxed.rs:2007:9
std::sys::unix::thread::Thread::new::thread_start
at /rustc/82e1608dfa6e0b5569232559e3d385fea5a93112/library/std/src/sys/unix/thread.rs:108:17
11: <unknown>
12: <unknown>
), source: None, kind: Internal })) }
```
They're logged at `DEBUG` level, so I assume these aren't fatal (NB: I haven't looked at the relevant code yet).
The error type (`Internal`) suggests this is either a) a bug that needs to be fixed, or b) that we need a separate, non-`Bug`, error type for this sort of failure.https://gitlab.torproject.org/tpo/core/arti/-/issues/1289RPC: Finish "TODO RPC" issues that implicate protocol changes2024-02-24T18:29:35ZNick MathewsonRPC: Finish "TODO RPC" issues that implicate protocol changesFortunately, only a few of our `TODO RPC` comments require protocol changes. But we should get to them as part of our next round of API work.Fortunately, only a few of our `TODO RPC` comments require protocol changes. But we should get to them as part of our next round of API work.https://gitlab.torproject.org/tpo/core/arti/-/issues/1288RPC: Safe connection method on windows2024-02-24T18:29:44ZNick MathewsonRPC: Safe connection method on windowsOn Unix, we can use AF_UNIX sockets to make sure that we've got a request from an authorized user. But on Windows, we don't have the equivalent. We shoul build some other authentication mechanism (SSL? Disk Cookie? Windows named pipes)...On Unix, we can use AF_UNIX sockets to make sure that we've got a request from an authorized user. But on Windows, we don't have the equivalent. We shoul build some other authentication mechanism (SSL? Disk Cookie? Windows named pipes) before we ship RPC.https://gitlab.torproject.org/tpo/core/arti/-/issues/1286Double-check implementation for SRV disaster fallback values2024-02-20T17:15:11ZNick MathewsonDouble-check implementation for SRV disaster fallback valuesWe do have a check for our disaster_srv calculation (in `tor_netdir::hsdir_params::test::disaster)`, and its value does seem to match up with the C tor implementation's test value in (`test_hs_common.c:test_disaster_srv()`).
But we shou...We do have a check for our disaster_srv calculation (in `tor_netdir::hsdir_params::test::disaster)`, and its value does seem to match up with the C tor implementation's test value in (`test_hs_common.c:test_disaster_srv()`).
But we should create higher level disaster SRV tests, to make sure that we actually build the same ring that C tor does.
We suspect that this could be related to a bug encountered on a day with the authorities failed to negotiate an SRV (‽).Nick MathewsonNick Mathewsonhttps://gitlab.torproject.org/tpo/core/arti/-/issues/1285Implement channel-based rate-limiting design2024-03-13T13:08:11ZNick MathewsonImplement channel-based rate-limiting designThis is a subset of #102, for the part needed for secure onion services. The design is in doc/dev/notes/bw-rate-limit.md
For the current estimate, I'm planning to do a minimal version of this, hiding the implementation details so that ...This is a subset of #102, for the part needed for secure onion services. The design is in doc/dev/notes/bw-rate-limit.md
For the current estimate, I'm planning to do a minimal version of this, hiding the implementation details so that we can make it better down the line.Nick MathewsonNick Mathewsonhttps://gitlab.torproject.org/tpo/core/arti/-/issues/1284Clients should identify services by nickname, not hsid2024-02-21T15:36:38Zgabi-250Clients should identify services by nickname, not hsid * [ ] Add a client-side `ClientSideFooHsNickname` type. Pick a name for it
(it should be distinguishable from the service-side `HsNickname`), define
its charset and decide what limit to impose on its length
* [ ] Add a `Clie... * [ ] Add a client-side `ClientSideFooHsNickname` type. Pick a name for it
(it should be distinguishable from the service-side `HsNickname`), define
its charset and decide what limit to impose on its length
* [ ] Add a `ClientSideFooHsNickname` to the client key specifiers, and make
clients error if there is more than one nickname for any given HsIdArti: Feature parity with the C implementationgabi-250gabi-250https://gitlab.torproject.org/tpo/core/arti/-/issues/1282generational-arena is unmaintained2024-02-24T18:29:59ZIan Jacksoniwj@torproject.orggenerational-arena is unmaintained[RUSTSEC-2024-0014](https://rustsec.org/advisories/RUSTSEC-2024-0014) See also https://github.com/fitzgen/generational-arena/issues/55
See !1990. See also #851 for why this is not so straightforward.[RUSTSEC-2024-0014](https://rustsec.org/advisories/RUSTSEC-2024-0014) See also https://github.com/fitzgen/generational-arena/issues/55
See !1990. See also #851 for why this is not so straightforward.Arti: RPC Supporthttps://gitlab.torproject.org/tpo/core/arti/-/issues/1281Implement the arti hsc prepare-stealth-mode-key subcommand2024-02-21T15:36:37Zgabi-250Implement the arti hsc prepare-stealth-mode-key subcommandSee the proposal in https://gitlab.torproject.org/tpo/core/arti/-/merge_requests/1987#note_2996998
* [ ] Pick a name for the subcommand (`prepare-restricted-mode-key`? `prepare-shielded-mode-key`?)
* [ ] Implement the subcommand:
``...See the proposal in https://gitlab.torproject.org/tpo/core/arti/-/merge_requests/1987#note_2996998
* [ ] Pick a name for the subcommand (`prepare-restricted-mode-key`? `prepare-shielded-mode-key`?)
* [ ] Implement the subcommand:
```
arti hsc prepare-stealth-mode-key
--hs[-]nick[name] ... # no default, option has shorter convenience aliases
[ --config arti.toml ] # default is default arti.toml
[ --keystore ... ] # default is `default`; no client nicknames yet
[ --output FOO.auth ] # default is <hs-nickname>.auth, use `-` for stdout
[ --overwrite ] # overwrites any existing output file; default is to refuse
[ --generate=no|yes|if-needed ] # if-needed is the default; otherwise, can error
```
This depends on #1283, #1284 and #1291Arti: Feature parity with the C implementationgabi-250gabi-250https://gitlab.torproject.org/tpo/core/arti/-/issues/1275Design the `VanguardMgr` and/or `VanguardPool`2024-03-19T15:07:31Zgabi-250Design the `VanguardMgr` and/or `VanguardPool`TODO: split into multiple issuesTODO: split into multiple issuesArti: Guard discovery researchgabi-250gabi-250https://gitlab.torproject.org/tpo/core/arti/-/issues/1273Implement vanguard pool persistence2024-02-21T17:48:29Zgabi-250Implement vanguard pool persistenceThe pools will likely be stored in the state dir.
This is a prerequisite to implementing vanguards-fullThe pools will likely be stored in the state dir.
This is a prerequisite to implementing vanguards-fullArti: Guard discovery researchgabi-250gabi-250https://gitlab.torproject.org/tpo/core/arti/-/issues/1272Add vanguard configuration options2024-03-26T14:59:04Zgabi-250Add vanguard configuration optionsWe have a few options here:
* have a single, global configuration option the says whether to use
vanguards-lite, or vanguards-full. This would apply to all HS
circuits (to both client and service circuits)
* have separate con...We have a few options here:
* have a single, global configuration option the says whether to use
vanguards-lite, or vanguards-full. This would apply to all HS
circuits (to both client and service circuits)
* have separate configuration options for clients and services (the
service vanguard config would be per-service). This would enable us
to configure clients and services, as well as different services
running in the same arti instance, independently from each other
(I'm not sure whether this is useful though).
We might also want to add config options for overriding the
`guard-hs-l2-number`, `guard-hs-l2-lifetime-min`, `guard-hs-l2-lifetime-max`
consensus parameters, so perhaps we need a proper `VanguardsConfig` (rather than a
simple `vanguards: auto|lite|full` option)
```toml
[onion_services."allium-cepa".vanguards]
kind = "lite"
l2_guard_num = 5
# TODO: there are no consensus params for the l3 guards
l3_guard_num = 15
...
```Arti: Guard discovery researchgabi-250gabi-250https://gitlab.torproject.org/tpo/core/arti/-/issues/1270Split service Recovering state into RecoveringReachable and RecoveringUnreach...2024-02-12T12:36:58Zgabi-250Split service Recovering state into RecoveringReachable and RecoveringUnreachableThe following discussion from !1966 should be addressed:
- [ ] @Diziet started a [discussion](https://gitlab.torproject.org/tpo/core/arti/-/merge_requests/1966#note_2994338): (+1 comment)
> I have two comments about this.
>
...The following discussion from !1966 should be addressed:
- [ ] @Diziet started a [discussion](https://gitlab.torproject.org/tpo/core/arti/-/merge_requests/1966#note_2994338): (+1 comment)
> I have two comments about this.
>
> Firstly, "The service may or may not be reachable" means this status cannot be properly handled by a caller that wants to (for example) wait during startup for the service to be readhable. So I think this status ought to be split, or something.
>
> Secondly, the doc comments form part of the public API but all talk about information that is only available inside the crate, and that a caller can't determine. By writing this here, you're inviting the author of calling code to attempt to figure out how to obtain this other information, which they can't.
>
> I think probably the best thing to do for this MR is to make the ifs and buts be `//` comments rather than doc comments, and make a ticket about the first problem.Arti: Onion service supporthttps://gitlab.torproject.org/tpo/core/arti/-/issues/1268Consider migrating from config-rs to figment2024-03-26T17:53:51ZNick MathewsonConsider migrating from config-rs to figmentThe [`figment`] crate seems pretty well designed, and in many ways less ad-hoc than the [`config`] crate. We may want to consider migrating.
Found while investigating #1267; I have done no more investigation here than to make verify th...The [`figment`] crate seems pretty well designed, and in many ways less ad-hoc than the [`config`] crate. We may want to consider migrating.
Found while investigating #1267; I have done no more investigation here than to make verify that `figment`'s error reporting is indeed much nicer in the case in question.
[`figment`]: https://crates.io/crates/figment
[`config`]: https://crates.io/crates/configNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/tpo/core/arti/-/issues/1267Unhelpful error messages when deserializing configuration fails2024-03-13T14:01:57ZNick MathewsonUnhelpful error messages when deserializing configuration failsWhen you try to start Arti with this configuration file:
```
[proxy]
socks_listen = "fred"
```
You get an not-so-helpful error message:
```
./target/quicktest/arti: error: read configuration: Config contents not as expected: Invalid lis...When you try to start Arti with this configuration file:
```
[proxy]
socks_listen = "fred"
```
You get an not-so-helpful error message:
```
./target/quicktest/arti: error: read configuration: Config contents not as expected: Invalid listen specification: failed to parse string: invalid socket address syntax
```
Note that it doesn't mention `proxy.socks_listen`, or `"fred"`!
Under the hood, this is:
```
anyhow::Error {
context: "read configuration",
source: tor_config::ConfigResolveError::Deserialize(
config::ConfigError::Message(
"Invalid listen specification: failed to parse string: invalid socket address syntax",
),
),
}
```
The inner "invalid listen specification:…" string was apparently `Display`ed from a `tor_config::misc::InvalidListen::InvalidString`. That latter error was constructed when we tried to deserialize a `tor_config::misc::Listen`.
I hope that this doesn't require major serde shenanigans.Nick MathewsonNick Mathewsonhttps://gitlab.torproject.org/tpo/core/arti/-/issues/1265Add a (nightly?) CI for musl libc?2024-02-06T23:25:41ZNick MathewsonAdd a (nightly?) CI for musl libc?See #1264; do we want a CI task to try building against musl on linux?
(Our reproducible-build code already tests windows and osx IIUC)See #1264; do we want a CI task to try building against musl on linux?
(Our reproducible-build code already tests windows and osx IIUC)https://gitlab.torproject.org/tpo/core/arti/-/issues/1262Rethink descriptor publisher rate-limiting2024-02-01T18:10:55Zgabi-250Rethink descriptor publisher rate-limitingThe following discussion from !1951 should be addressed:
- [ ] @Diziet started a [discussion](https://gitlab.torproject.org/tpo/core/arti/-/merge_requests/1951#note_2991914): (+1 comment)
> So, suppose it's 50s since we last uploa...The following discussion from !1951 should be addressed:
- [ ] @Diziet started a [discussion](https://gitlab.torproject.org/tpo/core/arti/-/merge_requests/1951#note_2991914): (+1 comment)
> So, suppose it's 50s since we last uploaded. We reach this point and see that `duration_since_upload` is 50s, which is less than `UPLOAD_RATE_LIM_THRESHOLD` (60s).
>
> Then we call `start_rate_limit(60s)`. `start_rate_limit` calls `runtime.now()` and adds its argument, so scheduling a wakeup 60s from now.
>
> We will upload again 110s after the last upload. I think though, that we should do it 60s after.
>
> I think the root cause of this bug is the *storage* of a separate "we are rate limited" state in the reactor state, and using it to control the upload logic. Whether "we are rate limited" is really just "is the last upload more than `UPLOAD_RATE_LIM_THRESHOLD` ago" - ie, we could recalculate that on each loop iteration.
>
> In terms of `PublishStatus` (the status reporting output) I'm not sure "we are rate limited" is a particularly useful status to advertise. I think it's an entirely normal condition.
>
> Also we should perhaps randomise this?
>
> OTOH I don't think either of these questions are a blockers for this MR. The code here is a lot nicer, so thanks :-).Arti: Onion service supportgabi-250gabi-250https://gitlab.torproject.org/tpo/core/arti/-/issues/1257Upgrade to educe 0.52024-02-28T14:33:54ZNick MathewsonUpgrade to educe 0.5Educe 0.5 is out; we should upgrade eventually. There are compatibility issues.Educe 0.5 is out; we should upgrade eventually. There are compatibility issues.Ian Jacksoniwj@torproject.orgIan Jacksoniwj@torproject.orghttps://gitlab.torproject.org/tpo/core/arti/-/issues/1256Should we split up the MissingIdKeypair error case?2024-01-24T13:31:02ZNick MathewsonShould we split up the MissingIdKeypair error case?I think that the `MissingIdKeypair` error might be covering too many cases. Right now, IIUC, it can mean:
* "We forgot to generate an identity key for this onion service somehow!"
* "This service is set up to run in offline-keygen ...I think that the `MissingIdKeypair` error might be covering too many cases. Right now, IIUC, it can mean:
* "We forgot to generate an identity key for this onion service somehow!"
* "This service is set up to run in offline-keygen mode, and we aren't prepared to deal with that."
* "This service is set up to run in offline-keygen mode but we are missing keys that we should have been given, like the public hs_id key."
I think it makes sense to leave it as-is for now, until we build offline-keygen mode. That is, to have a single MissingIdKeypair error type, and to give it `ErrorKind::Internal`.
(I don't think this is MUST.)
cc @gabi-250 for your opinion herehttps://gitlab.torproject.org/tpo/core/arti/-/issues/1252OnionServicePrefs2024-01-23T16:54:08Zgabi-250OnionServicePrefsThe following discussion from !1887 should be addressed:
- [ ] @nickm started a [discussion](https://gitlab.torproject.org/tpo/core/arti/-/merge_requests/1887#note_2984621): (+5 comments)
> Here's a thought. What would you think ...The following discussion from !1887 should be addressed:
- [ ] @nickm started a [discussion](https://gitlab.torproject.org/tpo/core/arti/-/merge_requests/1887#note_2984621): (+5 comments)
> Here's a thought. What would you think about adding a new `prefs` argument to OnionService here? It could, for example, include information about whether we should construct the keys if they are not already found.Arti: Onion service supporthttps://gitlab.torproject.org/tpo/core/arti/-/issues/1251Refactor IptStatus reporting so that we don't need a read+write task?2024-01-22T18:44:56ZNick MathewsonRefactor IptStatus reporting so that we don't need a read+write task?Right now, `Ipt::start_establisher` launches a task that reads from `postage::watch::Receiver` and writes into an mspc::Sender, with a minor transformation. We should investigate whether we can just wire up the sender and receiver direc...Right now, `Ipt::start_establisher` launches a task that reads from `postage::watch::Receiver` and writes into an mspc::Sender, with a minor transformation. We should investigate whether we can just wire up the sender and receiver directly (possibly with a transformation of types) and avoid the need for this task entirely.
If not, maybe we can just use `StreamExt::map` and `StreamExt::forward` to simplify the creation of the task.
cc @DizietIan Jacksoniwj@torproject.orgIan Jacksoniwj@torproject.org