Arti issueshttps://gitlab.torproject.org/tpo/core/arti/-/issues2024-01-24T17:32:09Zhttps://gitlab.torproject.org/tpo/core/arti/-/issues/1153Facility to retroactively apply changed configuration limits to onion service...2024-01-24T17:32:09ZNick MathewsonFacility to retroactively apply changed configuration limits to onion service circuitsAs implemented today, calling `reconfigure` on an `OnionService` or `OnionServiceReverseProxy` doesn't close any existing streams or circuits.
But if, for example, the service administrator lowers `max_concurrent_streams_per_circuit` (s...As implemented today, calling `reconfigure` on an `OnionService` or `OnionServiceReverseProxy` doesn't close any existing streams or circuits.
But if, for example, the service administrator lowers `max_concurrent_streams_per_circuit` (see #1124) or changes a port configuration so that a given port is no longer proxied, it's possible that we might want to close the streams that would no longer be allowed by the current rules.
(This question has a bit of history with C Tor: IIUC, some service admins are surprised if no-longer-configured streams get closed suddenly, and some are surprised if they don't.)
If we _do_ decide that we need a "retroactively enforce new limits" facility, I think at a lower level it would make sense to build it as a function that you'd call on `OnionService` or `OnionServiceReverseProxy` after having called `reconfigure`. I am not too sure what we would want the higher level interface to look like.
Also, IMO this question is emphatically _not_ a must-answer for releasing onion services, unless we decide that the current behavior is 100% unacceptable always.Arti: Onion service supporthttps://gitlab.torproject.org/tpo/core/arti/-/issues/1146purpose of tor_hsservice::svc module is unclear2024-03-04T16:33:17ZIan Jacksoniwj@torproject.orgpurpose of tor_hsservice::svc module is unclearI find this module confusing. About half of the HS service implementation is in it. The module description says just "Principal types for onion services". In practical terms, the location of a functional module within `tor-hsservice/`...I find this module confusing. About half of the HS service implementation is in it. The module description says just "Principal types for onion services". In practical terms, the location of a functional module within `tor-hsservice/` seems to depend mainly on who created it.
What, if anything, is the distinction being made between things that go in `svc/` and things that go in the crate toplevel? The whole of tor-hsservice is for hidden services, so it can't be "is related to hidden services", which seems to suggest that if this module serves a purpose, "svc" is the wrong name for it. (There's the word "Principal" in the title, but if we're making a distinction between "principal" and "ancillary", it seems that the ancillary things should be the ones to go into a separate module.)
At the moment it seems to me that this module should be abolished and all its contents including sub-modules be moved to the crate toplevel. But I'm open to keeping if it does serve a clearly defined purpose. If we're keeping it, should ipt_set and ipt_mgr go into it too? What about IptLocalId from lib.rs?
I'm filing this as a ticket rather than an MR because the disruptive effect of so much code motion, and because I feel I'm missing something. I'm filing it at all because I keep getting tripped up by half of the code not being where my hindbrain expects.Arti: Onion service supportIan Jacksoniwj@torproject.orgIan Jacksoniwj@torproject.orghttps://gitlab.torproject.org/tpo/core/arti/-/issues/969Revise /hssvc-ipt-algorithms.md based on updated client behavior assumptions2023-11-29T15:17:19ZNick MathewsonRevise /hssvc-ipt-algorithms.md based on updated client behavior assumptionsAlso see #966.
@diziet asked me to open this ticket to track the work of revising `doc/dev/notes/hssvc-ipt-algorithms.md` to account for the fact that clients will sometimes retry downloading onion service descriptors before they expire...Also see #966.
@diziet asked me to open this ticket to track the work of revising `doc/dev/notes/hssvc-ipt-algorithms.md` to account for the fact that clients will sometimes retry downloading onion service descriptors before they expire. This means, for example, that we do not need to remember every non-expired descriptor we've ever uploaded.Arti: Onion service supportIan Jacksoniwj@torproject.orgIan Jacksoniwj@torproject.orghttps://gitlab.torproject.org/tpo/core/arti/-/issues/863Interaction between RPC and SOCKS (or similar) datastreams2024-03-26T17:54:32ZNick MathewsonInteraction between RPC and SOCKS (or similar) datastreamsAs part of our RPC design, we need the ability for applications to open a connection (e.g. via SOCKS) and have them managed by an RPC session.
Here is (roughly) the design that we came up with. (This is a summary of a discussion @dizie...As part of our RPC design, we need the ability for applications to open a connection (e.g. via SOCKS) and have them managed by an RPC session.
Here is (roughly) the design that we came up with. (This is a summary of a discussion @diziet and I had on an etherpad.)
* From a TorClient object in an RPC session, it is possible to get a global (non-RPC-session-specific) identifier string.
* If you include this client identifier properly[^1] in your SOCKS request, your stream opened via that TorClient.
* In your SOCKS request, you may also include a _stream identifier_ that you make up.
* It is possible for the RPC session to look up the stream object (or a `StreamHandle`, see #847) using the TorClient object and the stream identifier.
* If you want to pass any other parameters to the stream creation process, you create a new TorClient or TorClient-like object for them.
[^1]: There is a question of how to properly bundle this information. One possibility is to just shove it in the SOCKS4 authentication or SOCKS5 username/password field, like we do for so many other things. But that has other semantics: it's also used for stream isolation. Will that cause a conflict? Another possibility is to define a new address type, or a new authentication mechanism. A third option is to move everybody to HTTP CONNECT, and use headers to pass this information.Arti: RPC SupportNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/tpo/core/arti/-/issues/840RPC: Define initial MVP suite of methods2024-03-26T17:54:37ZNick MathewsonRPC: Define initial MVP suite of methodsWe should have at least (IMO):
* Accessing the bootstrap status
* Watching the bootstrap status
* Resolving a DNS name
* Opening a stream
More TBD based on discussions in this week's meeting.We should have at least (IMO):
* Accessing the bootstrap status
* Watching the bootstrap status
* Resolving a DNS name
* Opening a stream
More TBD based on discussions in this week's meeting.Arti: RPC SupportNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/tpo/core/arti/-/issues/837RPC: How shall we define RPC methods on TorClient<R>?2024-02-26T17:22:19ZNick MathewsonRPC: How shall we define RPC methods on TorClient<R>?We'd like to be able to have RPC methods that work on `TorClient<R>` for all `R:Runtime`. But for now, our dispatch system only lets us define methods on types one at a time.
So for now, as a temporary measure, we're going to have the ...We'd like to be able to have RPC methods that work on `TorClient<R>` for all `R:Runtime`. But for now, our dispatch system only lets us define methods on types one at a time.
So for now, as a temporary measure, we're going to have the RPC system only work for `TorClient<PreferredRuntime>`. But we'd like a better solution. (Not just for `TorClient<R>` but for all cases where we'd like `FooMgr<R>` to get exposed as an Object in our RPC system.)
We've discussed a bunch of such solutions on IRC; I'll try to summarize here and ask @Diziet to expand or edit where my memory or summary is faulty.
## Even cleverer dispatch!
These solutions involve changes to the RPC subsystem only.
### One entry per concrete type
When defining a method that works on `TorClient<R>`, we could somehow make sure that an entry is added to our dispatch table for _every_ `R`. This means a larger dispatch table, which would not be the end of the world. It could maybe involve having to stick an `inventory::submit!` inside a trait implementation: who knows if that would work? (Maybe with something like the old `const _:() = {...}` trick?)
### Have each Object know its method table.
We could change our dispatch system so that every object can provide its own method-to-function dispatch table, and then define `Object for TorClient<R>` with a list of methods. @Diziet has some clever ideas for this case. (Long-term we would probably want to make these per-object dispatch tables shared; having one per `TorClient` is not a big deal, but one per `Stream` would be a lot.)
If we do this it might also be neat to let simpler objects participate in the old system, if that makes them easier to declare?
Doing this could make it tricky to define a method against an object that we are not defining. That's not a big problem for TorClient, which is itself very high level, but it could be a problem elsewhere.
If an object knows its method table, then it can either register that table itself, or give that table when asked, or just have a "dispatch(Box<dyn Method>)" function.
## Too many traits!
### Make a trait for the parts of TorClient we want.
We could define a `TorClientTrait` that provides access to all the APIs that don't expose anything that depends on `Runtime`. Then we could define all of our RPC functionality against some type wrapping `Arc<dyn TorClientTrait>`.
(There are variations of this where we break existing users and move all Arti APIs to the `TorClientTrait`.)
### Change the TorClient API to only use runtimes internally.
In addition to making a `TorClientTrait for TorClient<R>`, we make a `NewTorClient` that wraps `Arc<TorClientTrait>`. Then we only expose `NewTorClient`, and no longer expose generic `TorClient<R>`.
This would be a breaking change, but it's one we've thought about making previously.
## Sledgehammer solutions.
### Make Runtime dyn-safe
We could make Runtime a dyn-safe trait (or make a dyn-safe wrapper for it) and then use that in `TorClient` (or more places!), replacing `TorClient<R>` with just `TorClient` in our APIs.
This could make "get time" annoying, since we want to do that a LOT. But maybe it only needs to be efficient in `tor-proto`, and we don't mind if it's slow in `arti-client`?
### Abolish Runtimes
We could make Arti tokio-only, and declare that we only support building with one runtime at a time. This would make our testing story a bit nastier, since we do use custom Runtimes right now for testing against simulated broken networks, for no-network mocking, time simulation, etc. Also we'd need a way to support user-provided Runtime kludges, since IIRC VPN tools use Runtimes to set magic "don't " flags on our outbound TCP stream.Arti: RPC SupportNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/tpo/core/arti/-/issues/836RPC: dynamic registration of object types and methods2024-02-15T14:36:48ZIan Jacksoniwj@torproject.orgRPC: dynamic registration of object types and methodsThe current approach does not allow object types, or methods, which are not statically linked into the arti binary.
There are two related impediments:
* Use of `typetag` involves a compile-time table. We would need an escape hatch bas...The current approach does not allow object types, or methods, which are not statically linked into the arti binary.
There are two related impediments:
* Use of `typetag` involves a compile-time table. We would need an escape hatch based on `erased-serde` (or to reimplement typetag, which doesn't sound great)
* `tor-rpcbase` exposes methods like `is_method_name` that don't take an RPC registry parameter and just look at the static data. We would need to pass an explicit registry parameter, or decide to have a per-process runtime registry.
CC @nickm, prompted by !1148 148Arti: RPC Supporthttps://gitlab.torproject.org/tpo/core/arti/-/issues/830Error response `data` field doesn't comply with the spec2024-02-24T18:31:10ZIan Jacksoniwj@torproject.orgError response `data` field doesn't comply with the specThe `data` field in `RpcError` is wrong. This approach doesn't provide the necessary tag. (What the spec calls the "error data type name".) I think this needs to be a typetag trait.
And that trait is the same trait as the `HasJsonErr...The `data` field in `RpcError` is wrong. This approach doesn't provide the necessary tag. (What the spec calls the "error data type name".) I think this needs to be a typetag trait.
And that trait is the same trait as the `HasJsonErrorCode` I was proposing in https://gitlab.torproject.org/tpo/core/arti/-/merge_requests/1136#note_2896302 . So err, something like
```
#[typetag]
trait RpcErrorData: Serialize + Deserialize + HasKind + Send + 'static {
fn json_error_code(&self) -> i32 { 2 } // probably use some actual types here
}
#[typetag(tag = "arti:error_detail")] // do we have a - vs _ convention ?
impl RpcErrorData for arti_client::ErrorDetail {}
```Arti: RPC SupportIan Jacksoniwj@torproject.orgIan Jacksoniwj@torproject.orghttps://gitlab.torproject.org/tpo/core/arti/-/issues/1341Sync GeoIP Databases in an as automated way as possible2024-03-20T18:07:33ZAlexander Færøyahf@torproject.orgSync GeoIP Databases in an as automated way as possibleWith C Tor, @dgoulet made this nice toolchain where the CI generates the text files for us for GeoIP usage. It looks like we need something like this for Arti too as the GeoIP DB's are stale right now (not updated since d5632eacb2).With C Tor, @dgoulet made this nice toolchain where the CI generates the text files for us for GeoIP usage. It looks like we need something like this for Arti too as the GeoIP DB's are stale right now (not updated since d5632eacb2).Alexander Færøyahf@torproject.orgAlexander Færøyahf@torproject.orghttps://gitlab.torproject.org/tpo/core/arti/-/issues/1008Path config items should be able to refer to some other path config items2023-12-13T20:19:47ZIan Jacksoniwj@torproject.orgPath config items should be able to refer to some other path config itemsSee proposal here https://gitlab.torproject.org/tpo/core/arti/-/issues/988#note_2932084See proposal here https://gitlab.torproject.org/tpo/core/arti/-/issues/988#note_2932084Ian Jacksoniwj@torproject.orgIan Jacksoniwj@torproject.orghttps://gitlab.torproject.org/tpo/core/arti/-/issues/1005Should we change our `semver.md` process to manage _downstream_ effects of br...2024-03-26T18:52:31ZNick MathewsonShould we change our `semver.md` process to manage _downstream_ effects of breaking changes?Per discussion in !1129, `Release.md`, and elsewhere, and explanations at https://github.com/dtolnay/semver-trick, sometimes a breaking change in a lower-level crate can cause a breaking change in *any* higher level crate that exposes *e...Per discussion in !1129, `Release.md`, and elsewhere, and explanations at https://github.com/dtolnay/semver-trick, sometimes a breaking change in a lower-level crate can cause a breaking change in *any* higher level crate that exposes *even one* type from the lower-level crate... even if the re-exposed type is not the one that broke.
As such, maybe we should:
* reconsider the frequency with which we make breaking changes that could instead be "new API, deprecate old API."
* consider refactoring crates that define important re-exported types so those types are instead defined in a different crate that changes only rarely.
* adjust our semver.md practices so that, if you make a breaking change in an API in crate X, you also have to note a `BREAKING,RE-EXPORTED` change in the every downstream crate that will break. (This process would be annoying, but it is equivalent to the process that we have to do when putting out a release. It might encourage people to look for a process other than API breaks.)
* Declare that more things are not covered by semver.
* Try to re-export less, when we can.
Alternatively, we could:
* say that, for now, when we make a breaking change in _any_ crate at a lower level than `arti-clent`, we simply _assume_ that every higher-level crate other than `arti-client` will have broken too, and we don't bother to investigate.
I do acknowledge that we're still early in Arti's development, and breaking changes will happen and aren't too bad. Still, I think we should aim for a process that we'd be able to use long-term when Arti is more stable, since otherwise we won't notice cases where our design is getting in the way of the process we will eventually need.
Let's discuss!Ian Jacksoniwj@torproject.orgIan Jacksoniwj@torproject.orghttps://gitlab.torproject.org/tpo/core/arti/-/issues/845MPL issues2023-09-20T16:00:29ZIan Jacksoniwj@torproject.orgMPL issuesSee this for analysis of a problem with `license = "MPL-2.0"`:
https://gitlab.torproject.org/tpo/core/arti/-/merge_requests/1160/diffs?commit_id=fb594031f7101045c0368baffdfe807b6e25ab72#note_2899771
We should:
* [x] figure out what is...See this for analysis of a problem with `license = "MPL-2.0"`:
https://gitlab.torproject.org/tpo/core/arti/-/merge_requests/1160/diffs?commit_id=fb594031f7101045c0368baffdfe807b6e25ab72#note_2899771
We should:
* [x] figure out what is a good thing that the Rust community should do
* [x] contact `generational-arena` upstream to ask them to do this (even if we don't end up using that crate)
* [x] contact `option-ext` to ask them to do this (since @nickm says we're going to have that in our deps soon)
* [ ] Probably, write some blog post for TWIR.Ian Jacksoniwj@torproject.orgIan Jacksoniwj@torproject.orghttps://gitlab.torproject.org/tpo/core/arti/-/issues/705Adding a nontrivial new config type is hard2023-10-19T15:12:03ZIan Jacksoniwj@torproject.orgAdding a nontrivial new config type is hardOur configuration arrangements can make it hard to add a new nontrivial kind of configuration. We should review what decisions the programmer must make, and what other work the programmer must necessarily do, in these cases, and try to ...Our configuration arrangements can make it hard to add a new nontrivial kind of configuration. We should review what decisions the programmer must make, and what other work the programmer must necessarily do, in these cases, and try to arrange that the expression in code is as straightforward as possible and that there's as little makework and complication as possible.Ian Jacksoniwj@torproject.orgIan Jacksoniwj@torproject.org