Merge branch 'keymgr-config' into 'main' (09cb5eed) · Commits · alina / Arti

crates/arti-client/Cargo.toml

+1 −0

Original line number	Diff line number	Diff line
		@@ -85,6 +85,7 @@ experimental = [
		"tor-netdoc/experimental",
		"tor-dirmgr/experimental",
		"tor-circmgr/experimental",
		"tor-config/experimental",
		]

		# Enable experimental APIs that are not yet officially supported.

crates/arti-client/src/client.rs

+5 −9

Original line number	Diff line number	Diff line
		@@ -602,21 +602,17 @@ impl<R: Runtime> TorClient<R> {
		};

		let keymgr = {
		// TODO hs: load the key store dir and permissions from the config.
		// Note: The Mistrust should be taken from StorageConfig (the keystore Mistrust needs
		// to be the same as elsewhere). Perhaps this means the keystore should be part of
		// `storage`.
		let key_store_dir = std::path::PathBuf::new();
		let permissions = Default::default();
		let key_store_dir = config.storage.expand_keystore_dir()?;
		let permissions = config.storage.permissions();

		let mut stores: Vec<Box<dyn KeyStore>> = vec![];

		// TODO hs: For now, let's ignore any errors coming from the ArtiNativeKeyStore
		// constructor. We should remove this when we implement the key store config and bail
		// if the keystore dir fails the validation checks.
		if let Ok(arti_store) =
		ArtiNativeKeyStore::from_path_and_mistrust(key_store_dir, &permissions)
		{
		if let Some(key_store_dir) = key_store_dir {
		let arti_store =
		ArtiNativeKeyStore::from_path_and_mistrust(key_store_dir, permissions)?;
		stores.push(Box::new(arti_store));
		}

crates/arti-client/src/config.rs

+97 −0

Original line number	Diff line number	Diff line
		@@ -24,6 +24,9 @@ pub use tor_guardmgr::bridge::BridgeConfigBuilder;
		#[cfg_attr(docsrs, doc(cfg(feature = "bridge-client")))]
		pub use tor_guardmgr::bridge::BridgeParseError;

		#[cfg(feature = "experimental-api")]
		pub use tor_config::ItemOrBool;

		use tor_guardmgr::bridge::BridgeConfig;

		/// Types for configuring how Tor circuits are built.
		@@ -173,6 +176,17 @@ pub struct StorageConfig {
		/// Location on disk for less-sensitive persistent state information.
		#[builder(setter(into), default = "default_state_dir()")]
		state_dir: CfgPath,
		/// Location on disk for the Arti keystore.
		//
		// TODO HSS: try to use #[serde(into / try_from)] instead (and also move the deserialization
		// code to tor-keymgr).
		#[cfg(feature = "experimental-api")] // TODO HSS: make this unconditional
		#[builder(setter(into), default = "default_keystore_dir()")]
		#[builder_field_attr(serde(
		deserialize_with = "deserialize_keystore_dir",
		default = "serde_default_keystore_dir"
		))]
		keystore_dir: Option<CfgPath>,
		/// Filesystem state to
		#[builder(sub_builder(fn_name = "build_for_arti"))]
		#[builder_field_attr(serde(default))]
		@@ -180,6 +194,47 @@ pub struct StorageConfig {
		}
		impl_standard_builder! { StorageConfig }

		/// Deserialize the `keystore_dir` storage field.
		///
		/// NOTE: The first option is needed because of the builder, the second is the actual type of the field.
		#[cfg(all(feature = "keymgr", feature = "experimental-api"))]
		#[allow(clippy::option_option)]
		fn deserialize_keystore_dir<'de, D>(deserializer: D) -> Result<Option<Option<CfgPath>>, D::Error>
		where
		D: serde::de::Deserializer<'de>,
		{
		let cfg: ItemOrBool<CfgPath> = serde::de::Deserialize::deserialize(deserializer)?;

		let cfg = match cfg {
		ItemOrBool::Item(cfg) => Some(cfg),
		ItemOrBool::Bool(true) => default_keystore_dir(),
		ItemOrBool::Bool(false) => None,
		};

		Ok(Some(cfg))
		}

		/// Deserialize the `keystore_dir` storage field.
		///
		/// NOTE: The first option is needed because of the builder, the second is the actual type of the field.
		#[cfg(all(not(feature = "keymgr"), feature = "experimental-api"))]
		#[allow(clippy::option_option)]
		fn deserialize_keystore_dir<'de, D>(deserializer: D) -> Result<Option<Option<CfgPath>>, D::Error>
		where
		D: serde::de::Deserializer<'de>,
		{
		use serde::de::Error as _;

		let cfg: ItemOrBool<CfgPath> = serde::de::Deserialize::deserialize(deserializer)?;

		match cfg {
		ItemOrBool::Item(_) \| ItemOrBool::Bool(true) => Err(D::Error::custom(
		"keystore not available unless the `keymgr` feature is enabled".to_string(),
		)),
		ItemOrBool::Bool(false) => Ok(None),
		}
		}

		/// Return the default cache directory.
		fn default_cache_dir() -> CfgPath {
		CfgPath::new("${ARTI_CACHE}".to_owned())
		@@ -190,6 +245,30 @@ fn default_state_dir() -> CfgPath {
		CfgPath::new("${ARTI_LOCAL_DATA}".to_owned())
		}

		/// Return the default keystore directory.
		#[cfg(feature = "experimental-api")]
		#[allow(clippy::unnecessary_wraps)] // needed because of the type expected by the builder
		fn default_keystore_dir() -> Option<CfgPath> {
		#[cfg(feature = "keymgr")]
		{
		Some(CfgPath::new("${ARTI_LOCAL_DATA}/keystore".to_owned()))
		}

		#[cfg(not(feature = "keymgr"))]
		{
		None
		}
		}

		/// Return the default keystore directory.
		#[cfg(feature = "experimental-api")]
		#[allow(clippy::unnecessary_wraps, clippy::option_option)] // needed because of the type expected by the builder
		fn serde_default_keystore_dir() -> Option<Option<CfgPath>> {
		Some(default_keystore_dir())
		}

		// TODO: the expand_* functions are very repetitive. Maybe we can generate them using a macro
		// instead?
		impl StorageConfig {
		/// Try to expand `state_dir` to be a path buffer.
		pub(crate) fn expand_state_dir(&self) -> Result<PathBuf, ConfigBuildError> {
		@@ -209,6 +288,24 @@ impl StorageConfig {
		problem: e.to_string(),
		})
		}
		/// Try to expand `keystore_dir` to be a path buffer.
		#[allow(clippy::unnecessary_wraps)] // needed because of the experimental-api branch
		pub(crate) fn expand_keystore_dir(&self) -> Result<Option<PathBuf>, ConfigBuildError> {
		#[cfg(feature = "experimental-api")]
		{
		self.keystore_dir
		.as_ref()
		.map(\|dir\| dir.path())
		.transpose()
		.map_err(\|e\| ConfigBuildError::Invalid {
		field: "keystore_dir".to_owned(),
		problem: e.to_string(),
		})
		}

		#[cfg(not(feature = "experimental-api"))]
		Ok(None)
		}
		/// Return the FS permissions to use for state and cache directories.
		pub(crate) fn permissions(&self) -> &Mistrust {
		&self.permissions

crates/arti/src/cfg.rs

+22 −0

Original line number	Diff line number	Diff line
		@@ -483,6 +483,28 @@ mod test {
		],
		);

		declare_exceptions(
		None,
		None, // TODO HSS TESTS (#939): Make examples for keymgr settings!
		FeatureDependent,
		&[
		// The keystore settings are feature-dependent.
		//
		// The keystore_dir option has a different default value depending on whether the
		// keystore feature is enabled (if disabled, it defaults to false, if enabled, it
		// defaults to true). Moreover, if the feature is disabled, keystore_dir cannot
		// be set to true.
		//
		// As such, this test will fail no matter which default we write in the example
		// config (if the example sets this value to `true` and the test will fail if the
		// keymgr feature disabled. If it's set to `false`, the test will fail if the
		// keymgr feature is enabled).
		//
		// See #939
		"storage.keystore_dir", // we recognise this so we can reject it if
		],
		);

		declare_exceptions(
		None,
		Some(InNew),

crates/tor-config/Cargo.toml

+8 −0

Original line number	Diff line number	Diff line
		@@ -16,8 +16,16 @@ default = ["expand-paths"]

		full = ["expand-paths", "fs-mistrust/full", "tor-basic-utils/full", "tor-error/full"]

		experimental = ["experimental-api"]
		# Enable experimental APIs that are not yet officially supported.
		#
		# These APIs are not covered by semantic versioning. Using this
		# feature voids your "semver warrantee".
		experimental-api = ["__is_experimental"]
		expand-paths = ["shellexpand", "directories"]

		__is_experimental = []

		[dependencies]
		config = { version = "0.13", default-features = false, features = ["toml"] }
		derive_builder = { version = "0.11.2", package = "derive_builder_fork_arti" }