diff options
| -rw-r--r-- | Cargo.lock | 1 | ||||
| -rw-r--r-- | crates/tor-hsservice/Cargo.toml | 1 | ||||
| -rw-r--r-- | crates/tor-hsservice/src/lib.rs | 7 | ||||
| -rw-r--r-- | crates/tor-hsservice/src/nickname.rs | 2 | ||||
| -rw-r--r-- | crates/tor-hsservice/src/state.rs | 4 | ||||
| -rw-r--r-- | crates/tor-hsservice/src/state_dir.md | 74 | ||||
| -rw-r--r-- | crates/tor-hsservice/src/state_dir.rs | 579 | ||||
| -rw-r--r-- | crates/tor-netdoc/src/doc/hsdesc/build.rs | 2 | ||||
| -rw-r--r-- | crates/tor-netdoc/src/doc/hsdesc/build/inner.rs | 5 |
9 files changed, 669 insertions, 6 deletions
diff --git a/Cargo.lock b/Cargo.lock index 86e04b0e0..58ea95f88 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5373,6 +5373,7 @@ dependencies = [ name = "tor-hsservice" version = "0.6.0" dependencies = [ + "amplify", "anyhow", "async-trait", "base64ct", diff --git a/crates/tor-hsservice/Cargo.toml b/crates/tor-hsservice/Cargo.toml index dff3c793e..a1e9d15e1 100644 --- a/crates/tor-hsservice/Cargo.toml +++ b/crates/tor-hsservice/Cargo.toml @@ -38,6 +38,7 @@ full = [ ] [dependencies] +amplify = { version = "4", default-features = false, features = ["derive"] } async-trait = "0.1.54" base64ct = "1.5.1" derive-adhoc = "0.8" diff --git a/crates/tor-hsservice/src/lib.rs b/crates/tor-hsservice/src/lib.rs index 11d84c093..c04a23ce5 100644 --- a/crates/tor-hsservice/src/lib.rs +++ b/crates/tor-hsservice/src/lib.rs @@ -73,6 +73,9 @@ pub mod status; mod svc; mod timeout_track; +#[rustfmt::skip] // to let us have inline { todo!() }; TODO HSS remove +mod state_dir; + // rustdoc doctests can't use crate-public APIs, so are broken if provided for private items. // So we export the whole module again under this name. // Supports the Example in timeout_track.rs's module-level docs. @@ -87,6 +90,10 @@ pub mod timeout_track_for_doctests_unstable_no_semver_guarantees { pub use crate::timeout_track::*; } #[doc(hidden)] +pub mod state_dir_for_doctests_unstable_no_semver_guarantees { + pub use crate::state_dir::*; +} +#[doc(hidden)] pub mod time_store_for_doctests_unstable_no_semver_guarantees { pub use crate::time_store::*; } diff --git a/crates/tor-hsservice/src/nickname.rs b/crates/tor-hsservice/src/nickname.rs index 73bc88ca7..9948399a6 100644 --- a/crates/tor-hsservice/src/nickname.rs +++ b/crates/tor-hsservice/src/nickname.rs @@ -110,7 +110,7 @@ mod test { #[test] fn serde() { - // TODO HSS clone-and-hack with tor_keymgr::::key_specifier::test::serde + // TODO: clone-and-hack with tor_keymgr::::key_specifier::test::serde #[derive(Serialize, Deserialize, Debug)] struct T { n: HsNickname, diff --git a/crates/tor-hsservice/src/state.rs b/crates/tor-hsservice/src/state.rs index 876f4bb65..442edb54d 100644 --- a/crates/tor-hsservice/src/state.rs +++ b/crates/tor-hsservice/src/state.rs @@ -11,7 +11,7 @@ use crate::{HsIdPublicKeySpecifier, HsNickname}; /// A helper for managing the persistent state of hidden services. // -// TODO HSS decide what API we want here and implement it +// TODO (#1220) decide what API we want here and implement it // See https://gitlab.torproject.org/tpo/core/arti/-/merge_requests/1837#note_2977513 pub struct StateMgr { /// The key manager @@ -23,7 +23,7 @@ impl StateMgr { pub fn new(keystore_dir: impl AsRef<Path>, permissions: &Mistrust) -> tor_keymgr::Result<Self> { let arti_store = ArtiNativeKeystore::from_path_and_mistrust(&keystore_dir, permissions)?; - // TODO HSS: make the default store configurable + // TODO (#1106): make the default store configurable let default_store = arti_store; let keymgr = KeyMgrBuilder::default() diff --git a/crates/tor-hsservice/src/state_dir.md b/crates/tor-hsservice/src/state_dir.md new file mode 100644 index 000000000..03dde2e9c --- /dev/null +++ b/crates/tor-hsservice/src/state_dir.md @@ -0,0 +1,74 @@ +# LISTING OF AN ARTI STATE DIRECTORY for planning `state_dir.rs` + +TODO HSS delete these random notes at some point +(possibly after making extra docs somewhere about these things?) + +```text +keymgr + + keystore/ + +dirmgr +not in state/ because ??? +storage.rs ad hoc sqlite3 +EXPIRY built-in + + dir_blobs/ + dir_blobs/con:microdesc:sha3-256-3f4d5d6519d51b20d7161d3f12cb7e23114d0f0f4d252a73077dfe9719011962 + dir.sqlite3 + +tor_persist +FsStateMgr +.local/share/arti/state.lock + + state/ + state/state.lock + +guardmgr +via storage handle +EXPIRTY singleton + + state/guards.json + +circmgr? +via storage handle +EXPIRTY singleton + + state/circuit_timeouts.json + +ipt_mgr x2 +via storage handle +*no* lock against multiple instantiation of same HS +EXPIRTY needs to linked to hs (ie to instance) + + state/hs_iptpub_ztest.json + state/hs_ipts_ztest.json + +? +not per instance ? +should it be ? +no locking ? + pt_state/ + +? + 3161169 0 -rw-r--r-- 1 rustcargo rustcargo 0 Apr 28 2022 .local/share/arti/state.lock + state.lock + +HS IPT replay log +ad-hoc via Path +lock against multiple instantiation of same HS +secondary internal lock via mutex +EXPIRY whole dir needs to be linked to hs +EXPIRY needs internal expiry mechanism too + + hss_iptreplay/ + hss_iptreplay/replay_ztest/ + hss_iptreplay/replay_ztest/lock + hss_iptreplay/replay_ztest/9aa9517e6901c280a550911d3a3c679630403db1c622eedefbdf1715297f795f.bin + hss_iptreplay/replay_ztest/92d897263497b7e9f998bc7b14cb60a09bfb1beb418cc8266b7e1cc36709b3bf.bin + hss_iptreplay/replay_ztest/816885a3bf50c90f659304406bde9df0ec926c4b62c556a423b0f6ee7e646c0c.bin + hss_iptreplay/replay_ztest/62355f8672a24cd76e3f6f769fdb66f829aca898824f88cd16f5dca12cab0fd1.bin + hss_iptreplay/replay_ztest/7c3bc3ff6f8737b29bc54fd2dd0addf598dad2b69a3993e9f962c512fa42d6f7.bin + hss_iptreplay/replay_ztest/a35dab4c73c2a2492833c38ea127eb64dfd62b03104b570cd303069e45db9cbb.bin + +``` diff --git a/crates/tor-hsservice/src/state_dir.rs b/crates/tor-hsservice/src/state_dir.rs new file mode 100644 index 000000000..b14474bfb --- /dev/null +++ b/crates/tor-hsservice/src/state_dir.rs @@ -0,0 +1,579 @@ +//! State helper utility +//! +//! All the methods in this module perform appropriate mistrust checks. +//! +//! All the methods arrange to ensure suitably-finegrained exclusive access. +//! "Read-only" or "shared" mode is not supported. +//! +//! ### Differences from `tor_persist::StorageHandle` +//! +//! * Explicit provision is made for multiple instances of a single facility. +//! For example, multiple hidden services, +//! each with their own state, and own lock. +//! +//! * Locking (via filesystem locks) is mandatory, rather than optional - +//! there is no "shared" mode. +//! +//! * Locked state is represented in the Rust type system. +//! +//! * We don't use traits to support multiple implementations. +//! Platform support would be done in the future with `#[cfg]`. +//! Testing is done by temporary directories (as currently with `tor_persist`). +//! +//! * The serde-based `StorageHandle` requires `&mut` for writing. +//! This ensures proper serialisation of 1. read-modify-write cycles +//! and 2. use of the temporary file. +//! Or to put it another way, we model `StorageHandle` +//! as *containing* a `T` without interior mutability. +//! +//! * There's a way to get a raw directory for filesystem operations +//! (currently, will be used for IPT replay logs). +//! +//! ### Implied filesystem structure +//! +//! ```text +//! STATE_DIR/ +//! STATE_DIR/KIND/INSTANCE/ +//! STATE_DIR/KIND/INSTANCE/lock +//! STATE_DIR/KIND/INSTANCE/SLUG.json +//! STATE_DIR/KIND/INSTANCE/SLUG.new +//! STATE_DIR/KIND/INSTANCE/SLUG/ +//! +//! eg +//! +//! STATE_DIR/hss/allium-cepa.lock +//! STATE_DIR/hss/allium-cepa/ipts.json +//! STATE_DIR/hss/allium-cepa/iptpub.json +//! STATE_DIR/hss/allium-cepa/iptreplay/ +//! STATE_DIR/hss/allium-cepa/iptreplay/9aa9517e6901c280a550911d3a3c679630403db1c622eedefbdf1715297f795f.bin +//! ``` +//! +//! (The lockfile is outside the instance directory to facilitate +//! concurrency-correct deletion.) +//! +//! ### Comprehensive example +//! +//! ``` +//! use std::{collections::HashSet, fmt, time::Duration}; +//! use tor_error::{into_internal, Bug}; +//! # use tor_hsservice::state_dir_for_doctests_unstable_no_semver_guarantees as state_dir; +//! # #[cfg(all)] // works like #[cfg(FALSE)]. Instead, we have this workaround ^. +//! use crate::state_dir; +//! use state_dir::{InstanceIdString, InstanceIdentity, InstancePurgeHandler}; +//! use state_dir::{InstancePurgeInfo, InstanceStateHandle, StateDirectory, StorageHandle}; +//! # +//! # // fake up some things; we do this rather than using real ones +//! # // since this example will move, with the module, to a lower level crate. +//! # struct OnionService { } +//! # #[derive(derive_more::Display)] struct HsNickname(String); +//! # type Error = anyhow::Error; +//! # mod ipt_mgr { pub mod persist { +//! # #[derive(serde::Serialize, serde::Deserialize)] pub struct StateRecord {} +//! # } } +//! +//! impl InstanceIdentity for HsNickname { +//! fn kind() -> &'static str { "hss" } +//! fn write_identity(&self, f: &mut fmt::Formatter) -> Result<(), Bug> { +//! write!(f, "{self}").map_err(into_internal!("failed to write HS nickname")) +//! } +//! } +//! +//! impl OnionService { +//! fn new( +//! nick: HsNickname, +//! state_dir: &StateDirectory, +//! ) -> Result<Self, Error> { +//! let instance_state = state_dir.acquire_instance(&nick)?; +//! let replay_log_dir = instance_state.raw_subdir("ipt_replay")?; +//! let ipts_storage: StorageHandle<ipt_mgr::persist::StateRecord> = +//! instance_state.storage_handle("ipts")?; +//! // .. +//! # Ok(OnionService { }) +//! } +//! } +//! +//! struct PurgeHandler<'h>(&'h HashSet<&'h str>, Duration); +//! impl InstancePurgeHandler for PurgeHandler<'_> { +//! fn name_filter(&mut self, id: &InstanceIdString) +//! -> state_dir::Result<state_dir::Liveness> { +//! Ok(if self.0.contains(id.as_str()) { +//! state_dir::Liveness::Live +//! } else { +//! state_dir::Liveness::PossiblyUnused +//! }) +//! } +//! fn retain_unused_for(&mut self, id: &InstanceIdString) -> state_dir::Result<Duration> { +//! Ok(self.1) +//! } +//! fn dispose(&mut self, _info: &InstancePurgeInfo, handle: InstanceStateHandle) +//! -> state_dir::Result<()> { +//! // here might be a good place to delete keys too +//! handle.delete() +//! } +//! } +//! pub fn expire_hidden_services( +//! state_dir: &StateDirectory, +//! currently_configured_nicks: &HashSet<&str>, +//! retain_for: Duration, +//! ) -> Result<(), Error> { +//! state_dir.purge_instances(&mut PurgeHandler(currently_configured_nicks, retain_for))?; +//! Ok(()) +//! } +//! ``` +//! +//! ### Platforms without a filesystem +//! +//! The implementation and (in places) the documentation +//! is in terms of filesystems. +//! But, everything except `InstanceStateHandle::raw_subdir` +//! is abstract enough to implement some other way. +//! +//! If we wish to support such platforms, the approach is: +//! +//! * Decide on an approach for `StorageHandle` +//! and for each caller of `raw_subdir`. +//! +//! * Figure out how the startup code will look. +//! (Currently everything is in terms of `fs_mistrust` and filesystems.) +//! +//! * Provide a version of this module with a compatible API +//! in terms of whatever underlying facilities are available. +//! Use `#[cfg]` to select it. +//! Don't implement `raw_subdir`. +//! +//! * Call sites using `raw_subdir` will no longer compile. +//! Use `#[cfg]` at call sites to replace the `raw_subdir` +//! with whatever is appropriate for the platform. + +#![allow(unused_variables, dead_code)] +#![allow(unreachable_pub)] // TODO this module will hopefully move to tor-persist and be pub + +use std::cell::Cell; +use std::fmt; +use std::iter; +use std::marker::PhantomData; +use std::path::Path; +use std::sync::Arc; +use std::time::{Duration, SystemTime}; + +use derive_more::{AsRef, Deref, Into}; +use serde::{de::DeserializeOwned, Serialize}; +use thiserror::Error; +use void::Void; + +use fs_mistrust::{CheckedDir, Mistrust}; +use tor_error::Bug; + +/// TODO HSS remove +type Todo = Void; + +use std::result::Result as StdResult; + +/// [`Result`](StdResult) throwing a [`state_dir::Error`](enum@Error) +pub type Result<T> = StdResult<T, Error>; + +/// The whole program's state directory +/// +/// Representation of `[storage] state_dir` and `permissions` +/// from the Arti configuration. +/// +/// This type does not embody any subpaths relating to +/// any particular facility within Arti. +/// +/// Constructing a `StateDirectory` may involve filesystem permissions checks, +/// so ideally it would be created once per process for performance reasons. +/// +/// Existence of a `StateDirectory` also does not imply exclusive access. +/// +/// This type is passed to each facility's constructor; +/// the facility implements [`InstanceIdentity`] +/// and calls [`acquire_instance`](StateDirectory::acquire_instance). +/// +/// ### Use for caches +/// +/// In principle this type and the methods and subtypes available +/// would be suitable for cache data as well as state data. +/// +/// However the locking scheme does not tolerate random removal of files. +/// And cache directories are sometimes configured to point to locations +/// with OS-supplied automatic file cleaning. +/// That would not be correct, +/// since the automatic file cleaner might remove an in-use lockfile, +/// effectively unlocking the instance state +/// even while a process exists that thinks it still has the lock. +#[allow(clippy::missing_docs_in_private_items)] // TODO HSS remove +pub struct StateDirectory { + path: Todo, + mistrust: Todo, +} + +/// An instance of a facility that wants to save persistent state (caller-provided impl) +/// +/// Each value of a type implementing `InstanceIdentity` +/// designates a specific instance of a specific facility. +/// +/// For example, `HsNickname` implements `state_dir::InstanceIdentity`. +/// +/// The kind and identity are strings from a restricted character set: +/// Only lowercase ASCII alphanumerics, `_` , and `+`, are permitted, +/// and the first character must be an ASCII alphanumeric. +/// +/// (The output from `write_identity` will be converted to an [`InstanceIdString`].) +pub trait InstanceIdentity { + /// Return the kind. For example `hss` for a Tor Hidden Service. + /// + /// This must return a fixed string, + /// since usually all instances represented the same Rust type + /// are also the same kind. + // + // This precludes dynamically chosen instance kind identifiers. + // If we ever want that, we'd need an InstanceKind trait that is implemented + // not for actual instances, but for values representing a kind. + fn kind() -> &'static str; + + /// Obtain identity + /// + /// The instance identity distinguishes different instances of the same kind. + /// + /// For example, for a Tor Hidden Service the identity is the nickname. + // + // Throws Bug rather than fmt::Error so that in case of problems we can dump a stack trace. + fn write_identity(&self, f: &mut fmt::Formatter) -> StdResult<(), Bug>; +} + +/// For a facility to be expired using [`purge_instances`](StateDirectory::purge_instances) (caller-provided impl) +/// +/// A filter which decides which instances to delete, +/// and deletes them if appropriate. +/// +/// See [`purge_instances`](StateDirectory::purge_instances) for full documentation. +pub trait InstancePurgeHandler { + /// Can we tell by its name that this instance is still live ? + fn name_filter(&mut self, identity: &InstanceIdString) -> Result<Liveness>; + + /// How long should we retain an unused instance for ? + /// + /// Many implementations won't need to use `identity`. + /// To pass every possibly-unused instance + /// through to `dispose`, return `Duration::ZERO`. + fn retain_unused_for(&mut self, identity: &InstanceIdString) -> Result<Duration>; + + /// Decide whether to keep this instance + /// + /// When it has made its decision, `dispose` should + /// either call [`delete`](InstanceStateHandle::delete), + /// or simply drop `handle`. + /// + /// Called only after `name_filter` returned [`Liveness::PossiblyUnused`] + /// and only if the instance has not been acquired or modified recently. + /// + /// `info` includes the instance name and other useful information + /// such as the last modification time. + fn dispose(&mut self, info: &InstancePurgeInfo, handle: InstanceStateHandle) -> Result<()>; +} + +/// Information about an instance, passed to [`InstancePurgeHandler::dispose`] +#[derive(amplify::Getters)] +#[derive(AsRef)] +pub struct InstancePurgeInfo<'i> { + /// The instance's identity string + #[as_ref] + identity: &'i InstanceIdString, + + /// When the instance state was last updated, according to the filesystem timestamps + /// + /// See `[InstanceStateHandle::purge_instances]` + /// for details of what kinds of events count as modifications. + last_modified: SystemTime, +} + +/// String identifying an instance, within its kind +/// +/// Instance identities are from a restricted character set. +/// See [`InstanceIdentity`]. +#[derive(Into, derive_more::Display)] +pub struct InstanceIdString(String); + +impl InstanceIdString { + /// Obtain this `InstanceIdString` as a `&str` + pub fn as_str(&self) -> &str { + self.as_ref() + } +} +impl AsRef<str> for InstanceIdString { + fn as_ref(&self) -> &str { + &self.0 + } +} + +impl TryFrom<String> for InstanceIdString { + // TODO this should probably be a general InvalidSlug from a lower-level Slug type + type Error = Bug; + fn try_from(s: String) -> StdResult<Self, Self::Error> { + todo!() + } +} + +/// Types which can be used as a `slug` +/// +/// "Slugs" are used to distinguish different pieces of state within an instance. +/// Typically, each call site that needs to provide an `impl Slug` +/// will provide a fixed `&'static str`. +/// +/// Slugs have the same character set restrictions as kinds and instance identities; +/// see [`InstanceIdentity`]. +/// (This is checked at runtime by the `state_dir` implementation.) +/// +/// Slugs may not be the same as the reserved device filenames on Windows, +/// (eg, `con`, `lpr`). +/// (This is not checked by the `state_dir` implementation, +/// but violation of this rule will result in code that doesn't work at all on Windows.) +/// +/// It is important that slugs are distinct within an instance. +/// Specifically, +/// each slug provided to a method on the same [`InstanceStateHandle`] +/// (or a clone of it) +/// must be different. +/// Violating this rule does not result in memory-unsafety, +/// but might result in incorrect operation due to concurrent filesystem access, +/// including possible data loss and corruption. +/// (Typically, the slug is fixed, and the [`StorageHandle`]s are usually +/// obtained during instance construction, so ensuring this is straightforward.) +// We could implement a runtime check for this by retaining a table of in-use slugs, +// possibly only with `cfg(debug_assertions)`. However I think this isn't worth the code: +// it would involve an Arc<Mutex<SlugsInUseTable>> in InstanceStateHnndle and StorageHandle, +// and Drop impls to remove unused entries (and `raw_subdir` would have imprecise checking +// unless it returned a Drop newtype around CheckedDir). +// +// TODO #1192 for now we are using the name Slug here. +// When we implement this we may wish to unify parts of the implementation +// with any general facility that arises from #1192. +// +// This is a trait implemented by `str` for convenience of call sites. +// The implementing Functions here that take slugs will do a runtime syntax check. +// Doing it this way avoids error handling and newtype boilerplate at call sites, +// which I think is overkill for an error case that's not at all likely to happen. +pub trait Slug: ToString {} + +impl<T: ToString + ?Sized> Slug for T {} + +/// Is an instance still relevant? +/// +/// Returned by [`InstancePurgeHandler::name_filter`]. +/// +/// See [`StateDirectory::purge_instances`] for details of the semantics. +#[derive(Debug, Clone, Copy, Eq, PartialEq, Ord, PartialOrd, Hash)] +#[allow(clippy::exhaustive_enums)] // this is a boolean +pub enum Liveness { + /// This instance is not known to be interesting + /// + /// It could be perhaps expired, if it's been long enough + PossiblyUnused, + /// This instance is still wanted + Live, +} + +impl StateDirectory { + /// Create a new `StateDirectory` from a directory and mistrust configuration + #[allow(clippy::needless_pass_by_value)] // TODO HSS remove + pub fn new(state_dir: impl AsRef<Path>, mistrust: Mistrust) -> Result<Self> { todo!() } + + /// Acquires (creates and locks) a storage for an instance + /// + /// Ensures the existence and suitability of a subdirectory named `kind_identity`, + /// and locks it for exclusive access. + /// + /// `kind` and `identity` have syntactic restrictions - + /// see [`InstanceIdString`]. + pub fn acquire_instance<I: InstanceIdentity>( + &self, + identity: &I, + ) -> Result<InstanceStateHandle> { + todo!() + } + + /// List the instances of a particular kind + /// + /// Returns the instance identities. + /// + /// (The implementation lists subdirectories named `kind_*`.) + /// + /// Concurrency: + /// An instance which is not being removed or created will be + /// listed (or not) according to whether it's present. + /// But, in the presence of concurrent calls to `acquire_instance` and `delete` + /// on different instances, + /// is not guaranteed to provide a snapshot: + /// serialisation is not guaranteed across different instances. + #[allow(clippy::extra_unused_type_parameters)] // TODO HSS remove if possible + pub fn list_instances<I: InstanceIdentity>( + &self + ) -> impl Iterator<Item = Result<InstanceIdString>> { + let _: &Void = &self.path; + iter::empty() + } + + /// Delete instances according to selections made by the caller + /// + /// Each instance is considered in three stages. + /// + /// Firstly, it is passed to [`name_filter`](InstancePurgeHandler::name_filter). + /// If `name_filter` returns `Live`, + /// further consideration is skipped and the instance is retained. + /// + /// Secondly, the last time the instance was written to is calculated, + // This must be done with the lock held, for correctness + // but the lock must be acquired in a way that doesn't itself update the modification time. + // On Unix this is straightforward because opening for write doesn't update the mtime. + // If this is hard on another platform, we'll need a separate stamp file updated + // by an explicit Acquire operation. + // We should have a test to check that this all works as expected. + /// and compared to the return value from + /// [`retain_unused_for`](InstancePurgeHandler::retain_unused_for). + /// Again, this might mean ensure the instance is retained. + /// + /// Thirdly, the resulting `InstanceStateHandle` is passed to + /// [`dispose`](InstancePurgeHandler::dispose). + /// `dispose` may choose to call `handle.delete()`, + /// or simply drop the handle. + /// + /// Concurrency: + /// In the presence of multiple concurrent calls to `acquire_instance` and `delete`: + /// `filter` may be called for an instance which is being created or deleted + /// by another task. + /// `dispose` will be properly serialised with other activities on the same instance, + /// as implied by it receiving an `InstanceStateHandle`. + /// + /// Instances which have been acquired + /// or modified more recently than `retain_unused_for` + /// will not be offered to `dispose`. + /// + /// The expiry time is reset by calls to `acquire_instance`, + /// `StorageHandle::store` and `InstanceStateHandle::raw_subdir`; + /// it *may* be reset by calls to `StorageHandle::delete`. + pub fn purge_instances<I: InstancePurgeHandler>( + &self, + filter: &mut I, + ) -> Result<()> { + todo!() + } + + /// Tries to peek at something written by `StorageHandle::store` + /// + /// It is guaranteed that this will return either the `T` that was stored, + /// or `None` if `store` was never called, + /// or `StorageHandle::delete` was called + /// + /// So the operation is atomic, but there is no further synchronisation. + // + // Not sure if we need this, but it's logically permissible + pub fn instance_peek_storage<I: InstanceIdentity, T>( + &self, + identity: &I, + slug: &(impl Slug + ?Sized), + ) -> Result<Option<T>> { + todo!() + } +} + +/// State or cache directory for an instance of a facility +/// +/// Implies exclusive access: +/// there is only one `InstanceStateHandle` at a time, +/// across any number of processes, tasks, and threads, +/// for the same instance. +/// +/// But this type is `Clone` and the exclusive access is shared across all clones. +/// Users of the `InstanceStateHandle` must ensure that functions like +/// `storage_handle` and `raw_directory` are only called once with each `slug`. +/// (Typically, the slug is fixed, so this is straightforward.) +/// See [`Slug`] for more details. +#[allow(clippy::missing_docs_in_private_items)] // TODO HSS remove +pub struct InstanceStateHandle { + flock_guard: Arc<Todo>, +} + +impl InstanceStateHandle { + /// Obtain a [`StorageHandle`], usable for storing/retrieving a `T` + /// + /// `slug` has syntactic restrictions - see [`InstanceIdString`]. + pub fn storage_handle<T>(&self, slug: &(impl Slug + ?Sized)) -> Result<StorageHandle<T>> { todo!() } + + /// Obtain a raw filesystem subdirectory, within the directory for this instance + /// + /// This API is unsuitable platforms without a filesystem accessible via `std::fs`. + /// May therefore only be used within Arti for features + /// where we're happy to not to support such platforms (eg WASM without WASI) + /// without substantial further work. + /// + /// `slug` has syntactic restrictions - see [`InstanceIdString`]. + pub fn raw_subdir(&self, slug: &(impl Slug + ?Sized)) -> Result<InstanceRawSubdir> { todo!() } + + /// Unconditionally delete this instance directory + /// + /// For expiry, use `StateDirectory::purge_instances`, + /// and then call this in the `dispose` method. + /// + /// Will return a `BadAPIUsage` if other clones of this `InstanceStateHandle` exist. + pub fn delete(self) -> Result<()> { + // use Arc::into_inner on the lock object, + // to make sure we're actually the only surviving InstanceStateHandle + todo!() + } +} + +/// A place in the state or cache directory, where we can load/store a serialisable type +/// +/// Implies exclusive access. +/// +/// Rust mutability-xor-sharing rules enforce proper synchronisation, +/// unless multiple `StorageHandle`s are created +/// using the same [`InstanceStateHandle`] and `slug`. +pub struct StorageHandle<T> { + /// We're not sync, and we can load and store a `T` + marker: PhantomData<Cell<T>>, + /// Clone of the InstanceStateHandle's lock + flock_guard: Arc<Todo>, +} + +// Like tor_persist, but writing needs `&mut` +#[allow(missing_docs)] // TODO HSS remove +impl<T: Serialize + DeserializeOwned> StorageHandle<T> { + pub fn delete(&mut self) -> Result<()> { + todo!() + } + pub fn store(&mut self, v: &T) -> Result<()> { + todo!() + } + pub fn load(&self) -> Result<Option<T>> { + todo!() + } +} + +/// Subdirectory within an instance's state, for raw filesystem operations +/// +/// Dereferences to `fs_mistrust::CheckedDir` and can be used mostly like one. +/// Obtained from [`InstanceStateHandle::raw_subdir`]. +/// +/// Existence of this value implies exclusive access to the instance. +#[derive(Deref, Clone)] +pub struct InstanceRawSubdir { + /// The actual directory, as a [`fs_mistrust::CheckedDir`] + #[deref] + dir: CheckedDir, + /// Clone of the InstanceStateHandle's lock + flock_guard: Arc<Todo>, +} + +/// Error accessing persistent state +#[derive(Error, Clone, Debug)] +#[non_exhaustive] +pub enum Error { + // will gain variants for: + // mistrust error + // io::error + // serde error + // bug + // + // will contain information such as the fs path or bad parameters +} diff --git a/crates/tor-netdoc/src/doc/hsdesc/build.rs b/crates/tor-netdoc/src/doc/hsdesc/build.rs index f9d06d010..5b2f2a02d 100644 --- a/crates/tor-netdoc/src/doc/hsdesc/build.rs +++ b/crates/tor-netdoc/src/doc/hsdesc/build.rs @@ -34,7 +34,7 @@ use super::desc_enc::{HsDescEncNonce, HsDescEncryption, HS_DESC_ENC_NONCE_LEN}; /// This object is constructed via [`HsDescBuilder`], and then turned into a /// signed document using [`HsDescBuilder::build_sign()`]. /// -/// TODO HSS: Add an example for using this API. +/// TODO: Add an example for using this API. #[derive(Builder)] #[builder(public, derive(Debug, Clone), pattern = "owned", build_fn(vis = ""))] struct HsDesc<'a> { diff --git a/crates/tor-netdoc/src/doc/hsdesc/build/inner.rs b/crates/tor-netdoc/src/doc/hsdesc/build/inner.rs index f4c3178cb..3a029393b 100644 --- a/crates/tor-netdoc/src/doc/hsdesc/build/inner.rs +++ b/crates/tor-netdoc/src/doc/hsdesc/build/inner.rs @@ -133,7 +133,7 @@ impl<'a> NetdocBuilder for HsDescInner<'a> { // "The key is a base64 encoded curve25519 public key used to encrypt the introduction // request to service. (`KP_hss_ntor`)" // - // TODO hss: The spec allows for multiple enc-key lines, but we currently only ever encode + // TODO: The spec allows for multiple enc-key lines, but we currently only ever encode // a single one. encoder .item(ENC_KEY) @@ -145,7 +145,8 @@ impl<'a> NetdocBuilder for HsDescInner<'a> { // The subject key is the the ed25519 equivalent of the svc_ntor_key curve25519 public // encryption key. - // TODO hss: should the sign bit be 0 or 1? + // TODO (#1221): this is wrong. The sign bit should be computed according to appendix A + // from propo 228. let signbit = 0; let ed_svc_ntor_key = convert_curve25519_to_ed25519_public(&intro_point.svc_ntor_key, signbit) |