From 1921b71ecb2ef00b742db45b83fac58f2dc4653e Mon Sep 17 00:00:00 2001 From: Junyi Ou Date: Thu, 8 Oct 2026 15:26:07 -0400 Subject: [PATCH 1/4] feat(dgw): keep provisioner task secrets in the provisioning store The provisioning store gains a third kind of provisioned material, next to the injected credentials and the connection options: the secrets of provisioner tasks, such as the API key of an AI analysis. A provisioner task needs them while it runs, but they must never be written to disk. Task secrets are keyed by Task ID and follow their own policy, as the credential intent asks for each kind of provisioning: a new secret replaces the old one, and a secret goes away when its Task ends or at its expiry, through the existing cleanup task. Like the injected credentials, they are encrypted on the way in with the in-memory master key, and decrypted into a zeroizing value only when read. The store treats a secret as an opaque string, so each task decides what it holds. The session halves are unchanged. --- devolutions-gateway/src/credential/mod.rs | 10 ++ devolutions-gateway/src/provisioning.rs | 166 ++++++++++++++++++++-- 2 files changed, 167 insertions(+), 9 deletions(-) diff --git a/devolutions-gateway/src/credential/mod.rs b/devolutions-gateway/src/credential/mod.rs index 8c8673089..9fd017d90 100644 --- a/devolutions-gateway/src/credential/mod.rs +++ b/devolutions-gateway/src/credential/mod.rs @@ -7,6 +7,16 @@ use secrecy::ExposeSecret as _; use self::crypto::MASTER_KEY; +/// Encrypts any secret with the in-memory key that protects injected credentials. +pub(crate) fn encrypt_secret(secret: &secrecy::SecretString) -> anyhow::Result { + MASTER_KEY.lock().encrypt(secret.expose_secret()) +} + +/// Decrypts a secret from [`encrypt_secret`] into a short-lived value that zeroizes on drop. +pub(crate) fn decrypt_secret(encrypted: &EncryptedPassword) -> anyhow::Result { + MASTER_KEY.lock().decrypt(encrypted) +} + /// Credential at the application protocol level #[derive(Debug, Clone)] pub enum AppCredential { diff --git a/devolutions-gateway/src/provisioning.rs b/devolutions-gateway/src/provisioning.rs index e581b6719..402c83fc7 100644 --- a/devolutions-gateway/src/provisioning.rs +++ b/devolutions-gateway/src/provisioning.rs @@ -6,11 +6,12 @@ use anyhow::Context as _; use async_trait::async_trait; use devolutions_gateway_task::{ShutdownSignal, Task}; use parking_lot::Mutex; +use secrecy::SecretString; use tokio::sync::Notify; use tracing::{debug, instrument, warn}; use uuid::Uuid; -use crate::credential::{AppCredentialMapping, CleartextAppCredentialMapping}; +use crate::credential::{AppCredentialMapping, CleartextAppCredentialMapping, EncryptedPassword}; use crate::target_connection_options::TargetConnectionOptions; /// Error returned when inserting into the credentials half of the provisioning store. @@ -68,19 +69,29 @@ struct ConnectionOptionsEntry { expires_at: time::OffsetDateTime, } -/// Two independent token-keyed stores that together provision a session. +#[derive(Debug, Clone)] +struct TaskSecretEntry { + secret: EncryptedPassword, + expires_at: time::OffsetDateTime, +} + +/// What the provisioner hands Gateway to use later, kept in memory only, each kind with its own policy. /// -/// The credentials store is the encryption boundary: cleartext mappings are encrypted on the way -/// in, so entries only ever hold encrypted material. Token-only rows (`mapping = None`) match the -/// existing `provision-token` behavior on master. The connection-options store holds plaintext -/// routing metadata only and has no crypto dependency. +/// Two token-keyed stores together provision a session. The credentials store is the encryption +/// boundary: cleartext mappings are encrypted on the way in, so entries only ever hold encrypted +/// material. Token-only rows (`mapping = None`) match the existing `provision-token` behavior on +/// master. The connection-options store holds plaintext routing metadata only and has no crypto +/// dependency. Both are keyed by the association-token JTI. The halves are provisioned by separate +/// preflight operations and may arrive, expire, or be replaced independently. /// -/// Both are keyed by the association-token JTI. The halves are provisioned by separate preflight -/// operations and may arrive, expire, or be replaced independently. +/// The task-secrets store holds what a provisioner task needs but must never write to disk, such as +/// the API key of an AI analysis. It is keyed by Task ID, encrypted like the credentials, and its +/// entries go away when their Task ends or at their expiry. #[derive(Debug, Clone)] pub struct ProvisioningStore { credentials: Arc>>, connection_options: Arc>>, + task_secrets: Arc>>, cleanup_notify: Arc, } @@ -95,6 +106,7 @@ impl ProvisioningStore { Self { credentials: Arc::new(Mutex::new(HashMap::new())), connection_options: Arc::new(Mutex::new(HashMap::new())), + task_secrets: Arc::new(Mutex::new(HashMap::new())), cleanup_notify: Arc::new(Notify::new()), } } @@ -302,9 +314,57 @@ impl ProvisioningStore { self.connection_options.lock().get(&jti).map(|entry| entry.expires_at) } + /// Stores the secret of a provisioner task, encrypted, until `expires_at`; it replaces any earlier secret of the Task. + /// + /// The store does not look inside: the task decides what the secret holds, such as a serialized API key and URL. + pub fn insert_task_secret( + &self, + task_id: Uuid, + secret: &SecretString, + expires_at: time::OffsetDateTime, + ) -> anyhow::Result<()> { + let secret = crate::credential::encrypt_secret(secret).context("encrypt the task secret")?; + + self.task_secrets + .lock() + .insert(task_id, TaskSecretEntry { secret, expires_at }); + self.cleanup_notify.notify_one(); + + Ok(()) + } + + /// Returns the decrypted secret of `task_id`, or `None` once it is removed or expired. + /// + /// The returned value zeroizes on drop; keep it only as long as it is needed. + pub fn task_secret(&self, task_id: Uuid, now: time::OffsetDateTime) -> anyhow::Result> { + let encrypted = { + let mut task_secrets = self.task_secrets.lock(); + + let Some(entry) = task_secrets.get(&task_id) else { + return Ok(None); + }; + if now >= entry.expires_at { + task_secrets.remove(&task_id); + return Ok(None); + } + + entry.secret.clone() + }; + + crate::credential::decrypt_secret(&encrypted) + .context("decrypt the task secret") + .map(Some) + } + + /// Removes the secret of `task_id`, as when its Task ends. + pub fn remove_task_secret(&self, task_id: Uuid) { + self.task_secrets.lock().remove(&task_id); + } + fn remove_expired(&self, now: time::OffsetDateTime) { self.credentials.lock().retain(|_, entry| now < entry.expires_at); self.connection_options.lock().retain(|_, entry| now < entry.expires_at); + self.task_secrets.lock().retain(|_, entry| now < entry.expires_at); } fn next_expiry(&self) -> Option { @@ -315,7 +375,12 @@ impl ProvisioningStore { .values() .map(|entry| entry.expires_at) .min(); - credentials_expiry.into_iter().chain(options_expiry).min() + let task_secrets_expiry = self.task_secrets.lock().values().map(|entry| entry.expires_at).min(); + credentials_expiry + .into_iter() + .chain(options_expiry) + .chain(task_secrets_expiry) + .min() } } @@ -409,6 +474,89 @@ mod tests { serde_json::from_value(serde_json::json!({ "krb_kdc": "tcp://dc.example:88" })).expect("options") } + #[test] + fn task_secret_is_kept_encrypted_and_read_back() { + use secrecy::ExposeSecret as _; + + let store = ProvisioningStore::new(); + let task_id = Uuid::new_v4(); + let now = time::OffsetDateTime::now_utc(); + + store + .insert_task_secret( + task_id, + &SecretString::from("sk-task-secret"), + now + time::Duration::hours(1), + ) + .expect("insert"); + + assert!( + !format!("{store:?}").contains("sk-task-secret"), + "only the encrypted secret is held" + ); + let secret = store.task_secret(task_id, now).expect("decrypt").expect("present"); + assert_eq!(secret.expose_secret(), "sk-task-secret"); + + store + .insert_task_secret( + task_id, + &SecretString::from("sk-replaced"), + now + time::Duration::hours(1), + ) + .expect("replace"); + let secret = store.task_secret(task_id, now).expect("decrypt").expect("present"); + assert_eq!(secret.expose_secret(), "sk-replaced"); + } + + #[test] + fn task_secret_goes_away_when_removed_or_expired() { + let store = ProvisioningStore::new(); + let now = time::OffsetDateTime::now_utc(); + let removed = Uuid::new_v4(); + let expired = Uuid::new_v4(); + let expiring = Uuid::new_v4(); + + store + .insert_task_secret(removed, &SecretString::from("a"), now + time::Duration::hours(1)) + .expect("insert"); + store + .insert_task_secret(expired, &SecretString::from("b"), now) + .expect("insert"); + store + .insert_task_secret(expiring, &SecretString::from("c"), now + time::Duration::minutes(1)) + .expect("insert"); + + store.remove_task_secret(removed); + assert!(store.task_secret(removed, now).expect("read").is_none()); + assert!( + store.task_secret(expired, now).expect("read").is_none(), + "due at its expiry" + ); + + assert_eq!(store.next_expiry(), Some(now + time::Duration::minutes(1))); + store.remove_expired(now + time::Duration::minutes(1)); + assert!(store.task_secrets.lock().is_empty(), "the cleanup task removes it"); + } + + #[test] + fn task_secrets_are_independent_from_session_provisioning() { + let store = ProvisioningStore::new(); + let id = Uuid::new_v4(); + let token = association_token(id); + let now = time::OffsetDateTime::now_utc(); + store + .insert_credentials(token.clone(), Some(mapping()), time::Duration::minutes(5)) + .expect("insert credentials"); + store + .insert_task_secret(id, &SecretString::from("sk"), now + time::Duration::hours(1)) + .expect("insert secret"); + + store.remove_task_secret(id); + + assert_eq!(store.mapping_status(id), MappingStatus::Available); + store.get_mapping(id, &token).expect("credentials stay"); + } + #[test] fn take_returns_token_only_entry() { let store = ProvisioningStore::new(); From 4e05fced0e0fff23488375ddc4fbf4231a53f87d Mon Sep 17 00:00:00 2001 From: Junyi Ou Date: Thu, 8 Oct 2026 16:33:23 -0400 Subject: [PATCH 2/4] fix(dgw): read the clock when a task secret is read task_secret took the current time from its caller, so a caller passing an earlier time could still read an expired secret. It now reads the clock itself. --- devolutions-gateway/src/provisioning.rs | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/devolutions-gateway/src/provisioning.rs b/devolutions-gateway/src/provisioning.rs index 402c83fc7..90b265659 100644 --- a/devolutions-gateway/src/provisioning.rs +++ b/devolutions-gateway/src/provisioning.rs @@ -336,7 +336,11 @@ impl ProvisioningStore { /// Returns the decrypted secret of `task_id`, or `None` once it is removed or expired. /// /// The returned value zeroizes on drop; keep it only as long as it is needed. - pub fn task_secret(&self, task_id: Uuid, now: time::OffsetDateTime) -> anyhow::Result> { + pub fn task_secret(&self, task_id: Uuid) -> anyhow::Result> { + self.task_secret_at(task_id, time::OffsetDateTime::now_utc()) + } + + fn task_secret_at(&self, task_id: Uuid, now: time::OffsetDateTime) -> anyhow::Result> { let encrypted = { let mut task_secrets = self.task_secrets.lock(); @@ -494,7 +498,7 @@ mod tests { !format!("{store:?}").contains("sk-task-secret"), "only the encrypted secret is held" ); - let secret = store.task_secret(task_id, now).expect("decrypt").expect("present"); + let secret = store.task_secret_at(task_id, now).expect("decrypt").expect("present"); assert_eq!(secret.expose_secret(), "sk-task-secret"); store @@ -504,7 +508,7 @@ mod tests { now + time::Duration::hours(1), ) .expect("replace"); - let secret = store.task_secret(task_id, now).expect("decrypt").expect("present"); + let secret = store.task_secret_at(task_id, now).expect("decrypt").expect("present"); assert_eq!(secret.expose_secret(), "sk-replaced"); } @@ -527,9 +531,9 @@ mod tests { .expect("insert"); store.remove_task_secret(removed); - assert!(store.task_secret(removed, now).expect("read").is_none()); + assert!(store.task_secret_at(removed, now).expect("read").is_none()); assert!( - store.task_secret(expired, now).expect("read").is_none(), + store.task_secret_at(expired, now).expect("read").is_none(), "due at its expiry" ); From a85d7f36915beab0e3f72ec5eb2a7dcea18e3e0f Mon Sep 17 00:00:00 2001 From: Junyi Ou Date: Mon, 12 Oct 2026 20:51:04 -0400 Subject: [PATCH 3/4] docs(dgw): describe the task secret store by its contract Keep the existing `ProvisioningStore` paragraphs as they were and only add the task-secrets paragraph, and describe the task secret methods by what they guarantee rather than by what the AI analysis caller stores. Co-Authored-By: Claude Opus 5.5 (1M context) --- devolutions-gateway/src/provisioning.rs | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/devolutions-gateway/src/provisioning.rs b/devolutions-gateway/src/provisioning.rs index 90b265659..ba67f7c05 100644 --- a/devolutions-gateway/src/provisioning.rs +++ b/devolutions-gateway/src/provisioning.rs @@ -77,12 +77,15 @@ struct TaskSecretEntry { /// What the provisioner hands Gateway to use later, kept in memory only, each kind with its own policy. /// -/// Two token-keyed stores together provision a session. The credentials store is the encryption -/// boundary: cleartext mappings are encrypted on the way in, so entries only ever hold encrypted -/// material. Token-only rows (`mapping = None`) match the existing `provision-token` behavior on -/// master. The connection-options store holds plaintext routing metadata only and has no crypto -/// dependency. Both are keyed by the association-token JTI. The halves are provisioned by separate -/// preflight operations and may arrive, expire, or be replaced independently. +/// Two independent token-keyed stores together provision a session. +/// +/// The credentials store is the encryption boundary: cleartext mappings are encrypted on the way +/// in, so entries only ever hold encrypted material. Token-only rows (`mapping = None`) match the +/// existing `provision-token` behavior on master. The connection-options store holds plaintext +/// routing metadata only and has no crypto dependency. +/// +/// Both are keyed by the association-token JTI. The halves are provisioned by separate preflight +/// operations and may arrive, expire, or be replaced independently. /// /// The task-secrets store holds what a provisioner task needs but must never write to disk, such as /// the API key of an AI analysis. It is keyed by Task ID, encrypted like the credentials, and its @@ -314,9 +317,9 @@ impl ProvisioningStore { self.connection_options.lock().get(&jti).map(|entry| entry.expires_at) } - /// Stores the secret of a provisioner task, encrypted, until `expires_at`; it replaces any earlier secret of the Task. + /// Stores the encrypted secret of a provisioner task until `expires_at`, replacing any earlier secret of the Task. /// - /// The store does not look inside: the task decides what the secret holds, such as a serialized API key and URL. + /// The secret is opaque to the store. pub fn insert_task_secret( &self, task_id: Uuid, @@ -360,7 +363,7 @@ impl ProvisioningStore { .map(Some) } - /// Removes the secret of `task_id`, as when its Task ends. + /// Removes the secret of `task_id`. pub fn remove_task_secret(&self, task_id: Uuid) { self.task_secrets.lock().remove(&task_id); } From 25c06b47e9903d721a43ac78b67aae4c795d9e06 Mon Sep 17 00:00:00 2001 From: Junyi Ou Date: Tue, 13 Oct 2026 11:08:42 -0400 Subject: [PATCH 4/4] docs(dgw): say what the task secret store and its tests actually check - The store keeps an entry until it is removed or expires; removing it when the Task ends is the caller's job. - `insert_task_secret` takes the secret in clear and encrypts it. - The tests are named after what they check: the Debug output never shows the secret, and the cleanup pass, not the cleanup task, removes it. Co-Authored-By: Claude Opus 5.5 (1M context) --- devolutions-gateway/src/provisioning.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/devolutions-gateway/src/provisioning.rs b/devolutions-gateway/src/provisioning.rs index ba67f7c05..dcf61f52b 100644 --- a/devolutions-gateway/src/provisioning.rs +++ b/devolutions-gateway/src/provisioning.rs @@ -88,8 +88,8 @@ struct TaskSecretEntry { /// operations and may arrive, expire, or be replaced independently. /// /// The task-secrets store holds what a provisioner task needs but must never write to disk, such as -/// the API key of an AI analysis. It is keyed by Task ID, encrypted like the credentials, and its -/// entries go away when their Task ends or at their expiry. +/// the API key of an AI analysis. It is keyed by Task ID, encrypted like the credentials, and an +/// entry stays until it is removed or reaches its expiry. #[derive(Debug, Clone)] pub struct ProvisioningStore { credentials: Arc>>, @@ -317,7 +317,7 @@ impl ProvisioningStore { self.connection_options.lock().get(&jti).map(|entry| entry.expires_at) } - /// Stores the encrypted secret of a provisioner task until `expires_at`, replacing any earlier secret of the Task. + /// Encrypts and stores the secret of a provisioner task until `expires_at`, replacing any earlier secret of the Task. /// /// The secret is opaque to the store. pub fn insert_task_secret( @@ -482,7 +482,7 @@ mod tests { } #[test] - fn task_secret_is_kept_encrypted_and_read_back() { + fn task_secret_is_read_back_and_replaced() { use secrecy::ExposeSecret as _; let store = ProvisioningStore::new(); @@ -499,7 +499,7 @@ mod tests { assert!( !format!("{store:?}").contains("sk-task-secret"), - "only the encrypted secret is held" + "the store's Debug output never shows the secret" ); let secret = store.task_secret_at(task_id, now).expect("decrypt").expect("present"); assert_eq!(secret.expose_secret(), "sk-task-secret"); @@ -542,7 +542,7 @@ mod tests { assert_eq!(store.next_expiry(), Some(now + time::Duration::minutes(1))); store.remove_expired(now + time::Duration::minutes(1)); - assert!(store.task_secrets.lock().is_empty(), "the cleanup task removes it"); + assert!(store.task_secrets.lock().is_empty(), "the cleanup pass removes it"); } #[test]