From 820657b6bc042d2d93f50946969120cead04e829 Mon Sep 17 00:00:00 2001
From: Nick Mathewson <nickm@torproject.org>
Date: Mon, 28 Nov 2022 15:24:40 -0500
Subject: [PATCH] PtMgr: Use a persistent state directory for PT state.

This makes a `pt_state` directory inside .local/share/arti (or the
local equivalent), right next to our existing `state` dir.

Ideally we would use a separate directory for each PT, but we have a
very fuzzy "what is a specific PT" notion.

Closes #667
---
 Cargo.lock                       |  1 -
 crates/arti-client/src/client.rs |  5 +++++
 crates/arti-client/src/err.rs    |  5 +++++
 crates/tor-ptmgr/Cargo.toml      |  1 -
 crates/tor-ptmgr/src/lib.rs      | 17 ++++++++---------
 5 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/Cargo.lock b/Cargo.lock
index bcd4e3b23..d3384e192 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -4074,7 +4074,6 @@ dependencies = [
  "derive_builder_fork_arti",
  "futures",
  "serde",
- "tempfile",
  "thiserror",
  "tokio",
  "tor-chanmgr",
diff --git a/crates/arti-client/src/client.rs b/crates/arti-client/src/client.rs
index 09dee3b10..92c9b0086 100644
--- a/crates/arti-client/src/client.rs
+++ b/crates/arti-client/src/client.rs
@@ -450,8 +450,13 @@ impl<R: Runtime> TorClient<R> {
 
         #[cfg(feature = "pt-client")]
         let pt_mgr = {
+            let mut pt_state_dir = config.storage.expand_state_dir()?;
+            pt_state_dir.push("pt_state");
+            config.storage.permissions().make_directory(&pt_state_dir)?;
+
             let mgr = Arc::new(tor_ptmgr::PtMgr::new(
                 config.bridges.transports.clone(),
+                pt_state_dir,
                 runtime.clone(),
             )?);
 
diff --git a/crates/arti-client/src/err.rs b/crates/arti-client/src/err.rs
index 388b8d3e4..0c2cdcfe7 100644
--- a/crates/arti-client/src/err.rs
+++ b/crates/arti-client/src/err.rs
@@ -203,6 +203,10 @@ enum ErrorDetail {
     #[error("Problem with a pluggable transport")]
     PluggableTransport(#[from] tor_ptmgr::err::PtError),
 
+    /// We encountered a problem while inspecting or creating a directory.
+    #[error("Filesystem permissions problem")]
+    FsMistrust(#[from] fs_mistrust::Error),
+
     /// Unable to spawn task
     #[error("Unable to spawn {spawning}")]
     Spawn {
@@ -317,6 +321,7 @@ impl tor_error::HasKind for ErrorDetail {
             E::LocalAddress => EK::ForbiddenStreamTarget,
             E::ChanMgrSetup(e) => e.kind(),
             E::NoDir { error, .. } => error.kind(),
+            E::FsMistrust(_) => EK::FsPermissions,
             E::Bug(e) => e.kind(),
         }
     }
diff --git a/crates/tor-ptmgr/Cargo.toml b/crates/tor-ptmgr/Cargo.toml
index 2e89a08e6..56503d768 100644
--- a/crates/tor-ptmgr/Cargo.toml
+++ b/crates/tor-ptmgr/Cargo.toml
@@ -24,7 +24,6 @@ async-trait = "0.1.2"
 derive_builder = { version = "0.11.2", package = "derive_builder_fork_arti" }
 futures = "0.3.14"
 serde = { version = "1.0.103", features = ["derive"] }
-tempfile = "3.3"
 thiserror = "1"
 tor-chanmgr = { version = "0.7.0", path = "../tor-chanmgr", features = ["pt-client"] }
 tor-config = { version = "0.6.0", path = "../tor-config" }
diff --git a/crates/tor-ptmgr/src/lib.rs b/crates/tor-ptmgr/src/lib.rs
index a181d9bd7..8e384a6c6 100644
--- a/crates/tor-ptmgr/src/lib.rs
+++ b/crates/tor-ptmgr/src/lib.rs
@@ -52,7 +52,6 @@ use futures::channel::oneshot;
 use std::collections::HashMap;
 use std::path::PathBuf;
 use std::sync::{Arc, RwLock};
-use tempfile::TempDir;
 use tor_linkspec::PtTransportName;
 use tor_rtcompat::Runtime;
 use tracing::warn;
@@ -122,12 +121,8 @@ pub struct PtMgr<R> {
     state: Arc<RwLock<PtSharedState>>,
     /// PtReactor channel.
     tx: UnboundedSender<PtReactorMessage>,
-    /// Temporary directory to store PT state in.
-    //
-    // FIXME(eta): This should be configurable.
-    //
-    // TODO pt-client: There should be one of these per PT, if possible.
-    state_dir: TempDir,
+    /// Directory to store PT state in.
+    state_dir: PathBuf,
 }
 
 impl<R: Runtime> PtMgr<R> {
@@ -150,7 +145,11 @@ impl<R: Runtime> PtMgr<R> {
 
     /// Create a new PtMgr.
     // TODO pt-client: maybe don't have the Vec directly exposed?
-    pub fn new(transports: Vec<ManagedTransportConfig>, rt: R) -> Result<Self, PtError> {
+    pub fn new(
+        transports: Vec<ManagedTransportConfig>,
+        state_dir: PathBuf,
+        rt: R,
+    ) -> Result<Self, PtError> {
         let state = PtSharedState {
             cmethods: Default::default(),
             configured: Self::transform_config(transports),
@@ -162,7 +161,7 @@ impl<R: Runtime> PtMgr<R> {
             runtime: rt,
             state,
             tx,
-            state_dir: TempDir::new().map_err(|e| PtError::TempdirCreateFailed(Arc::new(e)))?,
+            state_dir,
         })
     }