tor-keymgr: Add ArtiNativeKeystoreConfig.

Previously, the keystore config consisted of a single field in
`StorageConfig`, which encoded 2 bits of information: whether the
keystore is enabled, and its root directory:
```
[storage]
# use this path, fail if compiled out
# keystore = "/path/to/arti/keystore"
#
# use default path, fail if compiled out
# keystore = true
#
# disable
# keystore = false
```

This commit adds `ArtiNativeKeystoreConfig`, which will replace the
multi-purpose `keystore` field. The new config will look like this:
```
#[storage.keystore]
# Whether the keystore is enabled.
#
# If the `keymgr` feature is enabled and this option is:
#     * set to false, we will ignore the configured keystore path.
#     * set to "auto", the configured keystore, or the default keystore, if the
#     keystore path is not specified, will be used
#     * set to true, the configured keystore, or the default keystore, if the
#     keystore path is not specified, will be used
#
# If the `keymgr` feature is disabled and this option is:
#     * set to false, we will ignore the configured keystore path.
#     * set to "auto", we will ignore the configured keystore path.
#
# Setting this option to true when the `keymgr` feature is disabled is a
# configuration error.
#enabled = "auto"
# The root directory of the arti keystore
#path = "${ARTI_LOCAL_DATA}/keystore"
```

While `ArtiNativeKeystoreConfig` currently only has 2 fields, `enabled`
and `path`, future versions of the keystore might require additional
config options.

Cargo.lock

@@ -4550,14 +4550,17 @@ dependencies = [
 name = "tor-keymgr"
 version = "0.1.0"
 dependencies = [
+ "derive_builder_fork_arti",
  "derive_more",
  "downcast-rs",
  "dyn-clone",
  "fs-mistrust",
  "itertools 0.10.5",
+ "serde",
  "ssh-key",
  "tempfile",
  "thiserror",
+ "tor-config",
  "tor-error",
  "tor-hscrypto",
  "tor-llcrypto",

crates/tor-keymgr/Cargo.toml

@@ -18,19 +18,22 @@ default = []
 # These APIs are not covered by semantic versioning.  Using this
 # feature voids your "semver warrantee".
 
-keymgr = ["__is_experimental", "tor-error/experimental"]
+keymgr = ["__is_experimental", "tor-error/experimental", "tor-config/experimental"]
 __is_experimental = []
 full = ["fs-mistrust/full", "tor-error/full", "tor-hscrypto/full", "tor-llcrypto/full"]
 experimental = ["keymgr"]
 
 [dependencies]
+derive_builder = { version = "0.11.2", package = "derive_builder_fork_arti" }
 derive_more = "0.99.3"
 downcast-rs = "1.2.0"
 dyn-clone = "1.0.11"
 fs-mistrust = { path = "../fs-mistrust", version = "0.7.1", features = ["serde", "walkdir"] }
 itertools = "0.10.1"
+serde = { version = "1.0.103", features = ["derive"] }
 ssh-key = { version = "0.5.1", features = ["std"] }
 thiserror = "1"
+tor-config = { path = "../tor-config", version = "0.9.2" }
 tor-error = { path = "../tor-error", version = "0.5.2" }
 tor-hscrypto = { path = "../tor-hscrypto", version = "0.3.0" }
 tor-llcrypto = { path = "../tor-llcrypto", version = "0.5.2", features = ["keymgr"] }

crates/tor-keymgr/semver.md

@@ -1,2 +1,3 @@
 BREAKING: `ErasedKey` (returned by `Keystore::get`) is now a type alias to
 `Box<dyn EncodableKey>` instead of `Box<dyn Any>`
+ADDED: `config` module exposing `ArtiNativeKeystoreConfig`

crates/tor-keymgr/src/config.rs

@@ -0,0 +1,3 @@
+//! Keystore configuration.
+
+pub mod arti;

crates/tor-keymgr/src/config/arti.rs

@@ -0,0 +1,69 @@
+//! Configuration options for [`ArtiNativeKeystore`](crate::ArtiNativeKeystore)
+
+use std::path::PathBuf;
+
+pub use tor_config::{CfgPath, CfgPathError, ConfigBuildError, ConfigurationSource, Reconfigure};
+
+use derive_builder::Builder;
+use serde::{Deserialize, Serialize};
+use tor_config::{impl_standard_builder, BoolOrAuto};
+
+/// [`ArtiNativeKeystore`](crate::ArtiNativeKeystore) configuration
+#[derive(Debug, Clone, Builder, Eq, PartialEq, Serialize, Deserialize)]
+#[builder(derive(Serialize, Deserialize, Debug))]
+#[builder(build_fn(validate = "Self::validate", error = "ConfigBuildError"))]
+#[non_exhaustive]
+#[builder_struct_attr(non_exhaustive)]
+pub struct ArtiNativeKeystoreConfig {
+    /// Whether keystore use is enabled.
+    #[builder(default)]
+    pub enabled: BoolOrAuto,
+
+    /// Location on disk for the Arti keystore.
+    #[builder(setter(into), default = "default_keystore_dir()")]
+    pub path: CfgPath,
+}
+
+impl_standard_builder! { ArtiNativeKeystoreConfig }
+
+impl ArtiNativeKeystoreConfigBuilder {
+    /// Check that the keystore configuration is valid
+    #[cfg(not(feature = "keymgr"))]
+    #[allow(clippy::unnecessary_wraps)]
+    fn validate(&self) -> Result<(), ConfigBuildError> {
+        use BoolOrAuto as BoA;
+
+        // Keystore support is disabled unless the `keymgr` feature is enabled.
+        if self.enabled == Some(BoA::Explicit(true)) {
+            return Err(ConfigBuildError::Inconsistent {
+                fields: ["enabled"].map(Into::into).into_iter().collect(),
+                problem: "keystore enabled=true, but keymgr feature not enabled".into(),
+            });
+        }
+
+        Ok(())
+    }
+
+    /// Check that the keystore configuration is valid
+    #[cfg(feature = "keymgr")]
+    #[allow(clippy::unnecessary_wraps)]
+    fn validate(&self) -> Result<(), ConfigBuildError> {
+        Ok(())
+    }
+}
+
+impl ArtiNativeKeystoreConfig {
+    /// Try to expand `path` to be a path buffer.
+    #[allow(clippy::unnecessary_wraps)] // needed because of the experimental-api branch
+    pub fn expand_keystore_dir(&self) -> Result<PathBuf, ConfigBuildError> {
+        self.path.path().map_err(|e| ConfigBuildError::Invalid {
+            field: "path".to_owned(),
+            problem: e.to_string(),
+        })
+    }
+}
+
+/// Return the default keystore directory.
+fn default_keystore_dir() -> CfgPath {
+    CfgPath::new("${ARTI_LOCAL_DATA}/keystore".to_owned())
+}

crates/tor-keymgr/src/lib.rs

@@ -43,6 +43,7 @@
 // TODO HSS: write more comprehensive documentation when the API is a bit more
 // stable
 
+pub mod config;
 mod err;
 mod key_specifier;