Repository navigation
Let a later embedded runtime take over TinySecurity's secret scope once the first is released #8540
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
f313cf2
d813718
e51c352
0af6ab4
de134f9
46ad028
a7a8882
621de5e
9b99316
98be9be
429bf4d
4ff7a0e
f0c5217
4e4a6c5
82ddefb
221c0e8
d3160b5
68031ec
256dc03
0798b80
af24aaa
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -25,6 +25,116 @@ type SecurityCallbackLoads = | |
| static SECURITY_CALLBACK_LOADS: std::sync::LazyLock<SecurityCallbackLoads> = | ||
| std::sync::LazyLock::new(|| tokio::sync::Mutex::new(std::collections::HashMap::new())); | ||
|
|
||
| /// Which secret scope (its `.secret_key` path) is using each admitted module. | ||
| /// | ||
| /// TinySecurity keeps the secret scope it was admitted with for the life of | ||
| /// the module: its secret registration and audit stream cannot be replaced by | ||
| /// reinitialization, and a loaded module is never unloaded. So a module is | ||
| /// handed to one scope at a time. Another scope is refused while the current | ||
| /// one is live. Once the host releases the current one ([`release_scopes`]), | ||
| /// the module may be taken over, but only by the admitting scope itself or by | ||
| /// a scope the host declared transient ([`declare_transient_scope`]): a | ||
| /// durable scope must never encrypt state it keeps under a master key that | ||
| /// belongs to a different scope. | ||
| /// | ||
| /// A plain mutex, not the async one guarding [`SECURITY_CALLBACK_LOADS`], so a | ||
| /// host can release its scope from a synchronous `Drop`. | ||
| #[derive(Default)] | ||
| struct ScopeOwnership { | ||
| /// The live owner of each admitted module; no entry once released. | ||
| owners: std::collections::HashMap<String, PathBuf>, | ||
| /// Scopes whose secret state does not outlive their runtime. | ||
| transient: std::collections::HashSet<PathBuf>, | ||
| /// Bumped by every release. A load that started before a release must | ||
| /// not publish its scope as the owner when it finishes after it. | ||
| epoch: u64, | ||
| } | ||
|
|
||
| static SECURITY_SCOPE_OWNERSHIP: std::sync::LazyLock<std::sync::Mutex<ScopeOwnership>> = | ||
| std::sync::LazyLock::new(Default::default); | ||
|
|
||
| fn ownership() -> std::sync::MutexGuard<'static, ScopeOwnership> { | ||
| SECURITY_SCOPE_OWNERSHIP | ||
| .lock() | ||
| .unwrap_or_else(std::sync::PoisonError::into_inner) | ||
| } | ||
|
|
||
| /// How a later caller came to use an admitted module's secret scope. | ||
| #[derive(Debug, PartialEq, Eq)] | ||
| enum ScopeClaim { | ||
| /// The caller's scope is the one already using the module. | ||
| Held, | ||
| /// The previous owner was released; the caller's scope now uses it. | ||
| TakenOver, | ||
| } | ||
|
|
||
| /// Whether the secret scope at `requested` may use a module admitted with | ||
| /// `admitted`, whose live owner is `owner` (`None`: released). | ||
| fn claim_scope( | ||
| owner: Option<&Path>, | ||
| admitted: &Path, | ||
| requested: &Path, | ||
| transient: bool, | ||
| ) -> Result<ScopeClaim, String> { | ||
| match owner { | ||
| Some(owner) if owner == requested => Ok(ScopeClaim::Held), | ||
| Some(_) => Err("security secret scope changed after module admission".into()), | ||
| None if requested == admitted || transient => Ok(ScopeClaim::TakenOver), | ||
| None => Err("security secret scope changed after module admission; \ | ||
| a durable scope cannot take over a released one" | ||
| .into()), | ||
| } | ||
| } | ||
|
|
||
| /// The current release epoch, captured before a load is scheduled. | ||
| pub(in crate::modules) fn scope_epoch() -> u64 { | ||
| ownership().epoch | ||
| } | ||
|
|
||
| /// Claim module `id` for `requested`, given the scope it was admitted with. | ||
| fn claim(id: &str, admitted: &Path, requested: &Path) -> Result<ScopeClaim, String> { | ||
| let ownership = ownership(); | ||
| claim_scope( | ||
| ownership.owners.get(id).map(PathBuf::as_path), | ||
| admitted, | ||
| requested, | ||
| ownership.transient.contains(requested), | ||
| ) | ||
| } | ||
|
|
||
| /// Record `key_path` as the live owner of module `id`, unless a release | ||
| /// happened since `epoch` was captured: the load that started before it | ||
| /// belongs to a released runtime. Returns whether the owner was recorded. | ||
| fn publish_owner(id: &str, key_path: PathBuf, epoch: u64) -> bool { | ||
| let mut ownership = ownership(); | ||
| if ownership.epoch != epoch { | ||
| log::debug!("[security] scope released during load; owner not recorded module={id}"); | ||
| return false; | ||
| } | ||
| ownership.owners.insert(id.to_owned(), key_path); | ||
| true | ||
| } | ||
|
|
||
| fn secret_key_path(config_dir: &Path) -> PathBuf { | ||
| config_dir.join(".secret_key") | ||
| } | ||
|
|
||
| /// Declare the config directory `config_dir` transient. See | ||
| /// [`crate::security::declare_transient_secret_scope`]. | ||
| pub(crate) fn declare_transient_scope(config_dir: &Path) { | ||
| ownership().transient.insert(secret_key_path(config_dir)); | ||
| } | ||
|
|
||
| /// Release every module's live owner and forget the declared transient | ||
| /// scopes. See [`crate::security::release_secret_scope`]. | ||
| pub(crate) fn release_scopes() { | ||
| let mut ownership = ownership(); | ||
| ownership.epoch = ownership.epoch.wrapping_add(1); | ||
| ownership.transient.clear(); | ||
| let released = ownership.owners.drain().count(); | ||
| log::debug!("[security] secret scope owners released count={released}"); | ||
|
Comment on lines
+130
to
+135
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| } | ||
|
|
||
| impl SecurityCallbackState { | ||
| fn module_config(&self, config: &Config) -> Result<serde_json::Value, String> { | ||
| let trusted_host = self.binding.unique_name().to_string(); | ||
|
|
@@ -178,27 +288,36 @@ fn opaque_id(label: &str, path: &Path, suffix: &str) -> String { | |
| format!("{:x}", hasher.finalize())[..32].to_owned() | ||
| } | ||
|
|
||
| /// Admit module `id` for `config`'s secret scope, or confirm the admitted one | ||
| /// may serve it. `epoch` is [`scope_epoch`] captured when the caller asked, | ||
| /// before the load was scheduled. | ||
| pub(in crate::modules) async fn ensure_loaded( | ||
| resolver: &Resolver, | ||
| config: &Config, | ||
| id: &str, | ||
| epoch: u64, | ||
| ) -> Result<(), String> { | ||
| let mut loads = SECURITY_CALLBACK_LOADS.lock().await; | ||
| if let Some(state) = loads.get(id) { | ||
| let key_path = config | ||
| .config_path | ||
| .parent() | ||
| .unwrap_or_else(|| Path::new(".")) | ||
| .join(".secret_key"); | ||
| if state.key_path != key_path { | ||
| return Err("security secret scope changed after module admission".into()); | ||
| } | ||
| let key_path = secret_key_path( | ||
| config | ||
| .config_path | ||
| .parent() | ||
| .unwrap_or_else(|| Path::new(".")), | ||
| ); | ||
| let claim = claim(id, &state.key_path, &key_path)?; | ||
| // Confirm the admitted module still retains this binding. Re-presenting | ||
| // it would be refused (a binding is spent once a module takes it), and | ||
| // the single-binding path accepts only a local-file source, so it | ||
| // failed every later call on a host served from the release. | ||
| super::super::ops::callback_load::confirm(resolver, id, &state.binding).await?; | ||
| register_scope(config, state).await?; | ||
| // The loads lock is held, so no other caller claims it meanwhile. | ||
| if claim == ScopeClaim::TakenOver && publish_owner(id, key_path, epoch) { | ||
| log::info!( | ||
| "[security] secret scope taken over after its owner was released module={id}" | ||
| ); | ||
| } | ||
| return Ok(()); | ||
| } | ||
|
|
||
|
|
@@ -207,7 +326,7 @@ pub(in crate::modules) async fn ensure_loaded( | |
| .parent() | ||
| .unwrap_or_else(|| Path::new(".")) | ||
| .to_path_buf(); | ||
| let key_path = key_dir.join(".secret_key"); | ||
| let key_path = secret_key_path(&key_dir); | ||
| let user_id = | ||
| crate::security::secret_storage::encrypted_store::keyring_user_id_from_dir(&key_dir); | ||
| let subject = tinysecurity_bus::SubjectScope { | ||
|
|
@@ -323,6 +442,7 @@ pub(in crate::modules) async fn ensure_loaded( | |
| .ok_or_else(|| "admitted security callback state unavailable".to_owned())?; | ||
| candidates.clear(); | ||
| loads.insert(id.to_owned(), std::sync::Arc::clone(&state)); | ||
| publish_owner(id, state.key_path.clone(), epoch); | ||
|
|
||
| register_scope(config, &state).await?; | ||
| Ok(()) | ||
|
|
@@ -362,3 +482,7 @@ async fn register_scope(config: &Config, state: &SecurityCallbackState) -> Resul | |
| } | ||
| Ok(()) | ||
| } | ||
|
|
||
| #[cfg(test)] | ||
| #[path = "host_callbacks_tests.rs"] | ||
| mod tests; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,87 @@ | ||
| //! Which secret scope may use the admitted TinySecurity module. | ||
|
|
||
| use super::*; | ||
|
|
||
| const CHANGED: &str = "security secret scope changed after module admission"; | ||
|
|
||
| fn admitted() -> &'static Path { | ||
| Path::new("/tmp/openhuman-scratch-first/.secret_key") | ||
| } | ||
|
|
||
| #[test] | ||
| fn the_owning_scope_keeps_its_module() { | ||
| assert_eq!( | ||
| claim_scope(Some(admitted()), admitted(), admitted(), false), | ||
| Ok(ScopeClaim::Held) | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn a_different_scope_is_refused_while_the_owner_is_live() { | ||
| let other = Path::new("/tmp/openhuman-scratch-second/.secret_key"); | ||
| assert_eq!( | ||
| claim_scope(Some(admitted()), admitted(), other, true), | ||
| Err(CHANGED.to_owned()), | ||
| "even a transient scope waits for the owner's release" | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn a_transient_scope_takes_over_a_module_whose_owner_was_released() { | ||
| let other = Path::new("/tmp/openhuman-scratch-second/.secret_key"); | ||
| assert_eq!( | ||
| claim_scope(None, admitted(), other, true), | ||
| Ok(ScopeClaim::TakenOver) | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn the_admitting_scope_takes_its_module_back_after_a_release() { | ||
| assert_eq!( | ||
| claim_scope(None, admitted(), admitted(), false), | ||
| Ok(ScopeClaim::TakenOver) | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn a_durable_scope_is_refused_a_module_whose_owner_was_released() { | ||
| let durable = Path::new("/home/someone/.openhuman/.secret_key"); | ||
| let refused = claim_scope(None, admitted(), durable, false).expect_err("durable refused"); | ||
| assert!(refused.starts_with(CHANGED), "{refused}"); | ||
| assert!(refused.contains("durable"), "{refused}"); | ||
| } | ||
|
|
||
| /// The ownership table is process-wide, so its lifecycle is one test. | ||
| #[test] | ||
| fn release_declare_and_claim_follow_the_hosts_lifecycle() { | ||
| let id = "tinysecurity-ownership-test"; | ||
| let first_dir = Path::new("/tmp/openhuman-scratch-first"); | ||
| let second_dir = Path::new("/tmp/openhuman-scratch-second"); | ||
| let first = secret_key_path(first_dir); | ||
| let second = secret_key_path(second_dir); | ||
|
|
||
| // The first runtime admits the module. | ||
| assert!(publish_owner(id, first.clone(), scope_epoch())); | ||
| assert_eq!(claim(id, &first, &first), Ok(ScopeClaim::Held)); | ||
|
|
||
| // A second runtime is refused while the first is live, declared or not. | ||
| declare_transient_scope(second_dir); | ||
| assert_eq!(claim(id, &first, &second), Err(CHANGED.to_owned())); | ||
|
|
||
| // A load the first runtime started before its release finishes after it: | ||
| // it must not record the released scope as the owner. | ||
| let stale_epoch = scope_epoch(); | ||
| release_scopes(); | ||
| assert!(!publish_owner(id, first.clone(), stale_epoch)); | ||
|
|
||
| // The release also forgot the second's declaration: it must declare again. | ||
| assert!(claim(id, &first, &second).is_err()); | ||
| declare_transient_scope(second_dir); | ||
| assert_eq!(claim(id, &first, &second), Ok(ScopeClaim::TakenOver)); | ||
| assert!(publish_owner(id, second.clone(), scope_epoch())); | ||
|
|
||
| // Now the second owns it, and the released first is refused. | ||
| assert_eq!(claim(id, &first, &second), Ok(ScopeClaim::Held)); | ||
| assert_eq!(claim(id, &first, &first), Err(CHANGED.to_owned())); | ||
| release_scopes(); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For sequential stateless runtimes sharing a still-live Tokio executor, a diagnostic scrub between dropping the first runtime and admitting the second can undo this release.
core::log_redaction::scrub_secrets()(also used by Sentry) still invokes the adapter with the first runtime's cachedtext::CONFIG; itsredact → client → ensure_loadedpath captures the new epoch, andclaim_scope(None, admitted, admitted, ...)allowspublish_ownerto record the old config as owner again. The second runtime is then refused every security call even though the first has been dropped. Clear/disarm the adapter when releasing ownership, or prevent diagnostic calls from acquiring a released scope, and cover a diagnostic scrub between runtime instances.AGENTS.md reference: AGENTS.md:L418-L420
Useful? React with 👍 / 👎.