* [pdm-devel] [PATCH datacenter-manager 1/3] remote config: let save_config take ownership
2025-12-01 9:29 [pdm-devel] [PATCH datacenter-manager 0/3] token secret shadow config Fabian Grünbichler
@ 2025-12-01 9:29 ` Fabian Grünbichler
2025-12-01 9:29 ` [pdm-devel] [PATCH datacenter-manager 2/3] remote config: get token secret from shadow file if shadowed Fabian Grünbichler
` (3 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Fabian Grünbichler @ 2025-12-01 9:29 UTC (permalink / raw)
To: pdm-devel
as preparation for automatically modifying the saved config to split out token
secrets, if needed.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
lib/pdm-config/src/remotes.rs | 9 +++++----
server/src/api/remotes.rs | 6 +++---
2 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/lib/pdm-config/src/remotes.rs b/lib/pdm-config/src/remotes.rs
index 74c4671..fc78707 100644
--- a/lib/pdm-config/src/remotes.rs
+++ b/lib/pdm-config/src/remotes.rs
@@ -47,7 +47,7 @@ pub fn config() -> Result<(SectionConfigData<Remote>, ConfigDigest), Error> {
/// Replace the currently persisted remotes config
///
/// Will panic if the the remote config instance has not been set before.
-pub fn save_config(config: &SectionConfigData<Remote>) -> Result<(), Error> {
+pub fn save_config(config: SectionConfigData<Remote>) -> Result<(), Error> {
instance().save_config(config)
}
@@ -57,7 +57,7 @@ pub trait RemoteConfig {
/// Lock the remotes config
fn lock_config(&self) -> Result<ApiLockGuard, Error>;
/// Replace the currently persisted remotes config
- fn save_config(&self, remotes: &SectionConfigData<Remote>) -> Result<(), Error>;
+ fn save_config(&self, remotes: SectionConfigData<Remote>) -> Result<(), Error>;
}
/// Default, production implementation for reading/writing the `remotes.cfg`
@@ -75,11 +75,12 @@ impl RemoteConfig for DefaultRemoteConfig {
let digest = openssl::sha::sha256(content.as_bytes());
let data = Remote::parse_section_config(REMOTES_CFG_FILENAME, &content)?;
+
Ok((data, digest.into()))
}
- fn save_config(&self, config: &SectionConfigData<Remote>) -> Result<(), Error> {
- let raw = Remote::write_section_config(REMOTES_CFG_FILENAME, config)?;
+ fn save_config(&self, config: SectionConfigData<Remote>) -> Result<(), Error> {
+ let raw = Remote::write_section_config(REMOTES_CFG_FILENAME, &config)?;
replace_config(REMOTES_CFG_FILENAME, raw.as_bytes())
}
}
diff --git a/server/src/api/remotes.rs b/server/src/api/remotes.rs
index 76b005d..a7463b9 100644
--- a/server/src/api/remotes.rs
+++ b/server/src/api/remotes.rs
@@ -184,7 +184,7 @@ pub async fn add_remote(mut entry: Remote, create_token: Option<String>) -> Resu
let name = entry.id.clone();
remotes.insert(entry.id.to_owned(), entry);
- pdm_config::remotes::save_config(&remotes)?;
+ pdm_config::remotes::save_config(remotes)?;
if let Err(e) = metric_collection::trigger_metric_collection(Some(name), false).await {
log::error!("could not trigger metric collection after adding remote: {e}");
@@ -268,7 +268,7 @@ pub fn update_remote(
entry.web_url = updater.web_url;
}
- pdm_config::remotes::save_config(&remotes)?;
+ pdm_config::remotes::save_config(remotes)?;
Ok(())
}
@@ -291,7 +291,7 @@ pub fn remove_remote(id: String) -> Result<(), Error> {
http_bail!(NOT_FOUND, "no such entry {id:?}");
}
- pdm_config::remotes::save_config(&remotes)?;
+ pdm_config::remotes::save_config(remotes)?;
Ok(())
}
--
2.47.3
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 7+ messages in thread* [pdm-devel] [PATCH datacenter-manager 2/3] remote config: get token secret from shadow file if shadowed
2025-12-01 9:29 [pdm-devel] [PATCH datacenter-manager 0/3] token secret shadow config Fabian Grünbichler
2025-12-01 9:29 ` [pdm-devel] [PATCH datacenter-manager 1/3] remote config: let save_config take ownership Fabian Grünbichler
@ 2025-12-01 9:29 ` Fabian Grünbichler
2025-12-01 9:29 ` [pdm-devel] [PATCH datacenter-manager 3/3] remote config: shadow token secrets when saving Fabian Grünbichler
` (2 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Fabian Grünbichler @ 2025-12-01 9:29 UTC (permalink / raw)
To: pdm-devel
by making this an explicit call (as opposed to automatically reading and
merging the shadow config), accidental leaks are less likely.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
if we want to go down the other route, that's of course also possible..
lib/pdm-api-types/src/remotes.rs | 50 ++++++++++++++++++++++++++++++++
lib/pdm-config/src/remotes.rs | 30 +++++++++++++++++--
server/src/api/pve/lxc.rs | 2 +-
server/src/api/pve/qemu.rs | 2 +-
server/src/connection.rs | 8 +++--
5 files changed, 86 insertions(+), 6 deletions(-)
diff --git a/lib/pdm-api-types/src/remotes.rs b/lib/pdm-api-types/src/remotes.rs
index bd90ef1..f69609c 100644
--- a/lib/pdm-api-types/src/remotes.rs
+++ b/lib/pdm-api-types/src/remotes.rs
@@ -149,6 +149,56 @@ impl ApiSectionDataEntry for Remote {
}
}
+#[api(
+ properties: {
+ "id": { schema: REMOTE_ID_SCHEMA },
+ "type": { type: RemoteType },
+ },
+)]
+/// The information required to connect to a remote instance.
+#[derive(Clone, Debug, Deserialize, Serialize, PartialEq)]
+#[serde(rename_all = "kebab-case")]
+pub struct RemoteShadow {
+ #[serde(rename = "type")]
+ pub ty: RemoteType,
+
+ /// An id for this entry.
+ pub id: String,
+
+ /// The access token's secret.
+ pub token: String,
+}
+
+impl ApiSectionDataEntry for RemoteShadow {
+ const INTERNALLY_TAGGED: Option<&'static str> = Some("type");
+ const SECION_CONFIG_USES_TYPE_KEY: bool = true;
+
+ /// Get the `SectionConfig` configuration for this enum.
+ fn section_config() -> &'static SectionConfig {
+ static CONFIG: OnceLock<SectionConfig> = OnceLock::new();
+
+ CONFIG.get_or_init(|| {
+ let mut this = SectionConfig::new(&REMOTE_ID_SCHEMA).with_type_key("type");
+ for ty in ["pve", "pbs"] {
+ this.register_plugin(SectionConfigPlugin::new(
+ ty.to_string(),
+ Some("id".to_string()),
+ RemoteShadow::API_SCHEMA.unwrap_object_schema(),
+ ));
+ }
+ this
+ })
+ }
+
+ /// Maps an enum value to its type name.
+ fn section_type(&self) -> &'static str {
+ match self.ty {
+ RemoteType::Pve => "pve",
+ RemoteType::Pbs => "pbs",
+ }
+ }
+}
+
/// Since `Uri` does not directly support `serde`, we turn this into using FromStr/Display.
///
/// If we want to turn this into a property string, we can use a default_key, but may need to work
diff --git a/lib/pdm-config/src/remotes.rs b/lib/pdm-config/src/remotes.rs
index fc78707..93f80eb 100644
--- a/lib/pdm-config/src/remotes.rs
+++ b/lib/pdm-config/src/remotes.rs
@@ -5,17 +5,18 @@
use std::sync::OnceLock;
-use anyhow::Error;
+use anyhow::{bail, Error};
use proxmox_config_digest::ConfigDigest;
use proxmox_product_config::{open_api_lockfile, replace_config, ApiLockGuard};
use proxmox_section_config::typed::{ApiSectionDataEntry, SectionConfigData};
-use pdm_api_types::remotes::Remote;
+use pdm_api_types::remotes::{Remote, RemoteShadow};
use pdm_buildcfg::configdir;
pub const REMOTES_CFG_FILENAME: &str = configdir!("/remotes.cfg");
+const REMOTES_SHADOW_FILENAME: &str = configdir!("/remotes.shadow");
pub const REMOTES_CFG_LOCKFILE: &str = configdir!("/.remotes.lock");
static INSTANCE: OnceLock<Box<dyn RemoteConfig + Send + Sync>> = OnceLock::new();
@@ -44,6 +45,10 @@ pub fn config() -> Result<(SectionConfigData<Remote>, ConfigDigest), Error> {
instance().config()
}
+pub fn get_secret_token(remote: &Remote) -> Result<String, Error> {
+ instance().get_secret_token(remote)
+}
+
/// Replace the currently persisted remotes config
///
/// Will panic if the the remote config instance has not been set before.
@@ -54,6 +59,8 @@ pub fn save_config(config: SectionConfigData<Remote>) -> Result<(), Error> {
pub trait RemoteConfig {
/// Return contents of the remotes config
fn config(&self) -> Result<(SectionConfigData<Remote>, ConfigDigest), Error>;
+ /// Return contents of the remotes shadow config
+ fn get_secret_token(&self, remote: &Remote) -> Result<String, Error>;
/// Lock the remotes config
fn lock_config(&self) -> Result<ApiLockGuard, Error>;
/// Replace the currently persisted remotes config
@@ -83,6 +90,25 @@ impl RemoteConfig for DefaultRemoteConfig {
let raw = Remote::write_section_config(REMOTES_CFG_FILENAME, &config)?;
replace_config(REMOTES_CFG_FILENAME, raw.as_bytes())
}
+
+ fn get_secret_token(&self, remote: &Remote) -> Result<String, Error> {
+ // not yet rewritten into shadow config
+ if remote.token != "-" {
+ return Ok(remote.token.clone());
+ }
+
+ let shadow_content = proxmox_sys::fs::file_read_optional_string(REMOTES_SHADOW_FILENAME)?
+ .unwrap_or_default();
+
+ let shadow_config =
+ RemoteShadow::parse_section_config(REMOTES_SHADOW_FILENAME, &shadow_content)?;
+
+ if let Some(shadow_entry) = shadow_config.get(&remote.id) {
+ Ok(shadow_entry.token.clone())
+ } else {
+ bail!("No shadow entry found for remote {id}", id = remote.id);
+ }
+ }
}
/// Initialize the [`RemoteConfig`] instance.
diff --git a/server/src/api/pve/lxc.rs b/server/src/api/pve/lxc.rs
index dc95783..198137b 100644
--- a/server/src/api/pve/lxc.rs
+++ b/server/src/api/pve/lxc.rs
@@ -545,7 +545,7 @@ pub async fn lxc_remote_migrate(
"host={host},port={port},apitoken=PVEAPIToken={authid}={secret}",
host = target_host_port.host(),
authid = target.authid,
- secret = target.token,
+ secret = pdm_config::remotes::get_secret_token(target)?,
port = target_host_port.port_u16().unwrap_or(8006),
);
if let Some(fp) = target_node.fingerprint.as_deref() {
diff --git a/server/src/api/pve/qemu.rs b/server/src/api/pve/qemu.rs
index efab228..4a25ecb 100644
--- a/server/src/api/pve/qemu.rs
+++ b/server/src/api/pve/qemu.rs
@@ -593,7 +593,7 @@ pub async fn qemu_remote_migrate(
"host={host},port={port},apitoken=PVEAPIToken={authid}={secret}",
host = target_host_port.host(),
authid = target.authid,
- secret = target.token,
+ secret = pdm_config::remotes::get_secret_token(target)?,
port = target_host_port.port_u16().unwrap_or(8006),
);
if let Some(fp) = target_node.fingerprint.as_deref() {
diff --git a/server/src/connection.rs b/server/src/connection.rs
index 1c0069e..7e36671 100644
--- a/server/src/connection.rs
+++ b/server/src/connection.rs
@@ -110,9 +110,11 @@ fn prepare_connect_client(
/// authentication information and settings for the [`RemoteType`]
fn connect(remote: &Remote, target_endpoint: Option<&str>) -> Result<Client, anyhow::Error> {
let (client, info) = prepare_connect_client(remote, target_endpoint)?;
+ let token = pdm_config::remotes::get_secret_token(remote)?;
+
client.set_authentication(proxmox_client::Token {
userid: remote.authid.to_string(),
- value: remote.token.to_string(),
+ value: token,
prefix: info.prefix,
perl_compat: info.perl_compat,
});
@@ -148,10 +150,12 @@ fn prepare_connect_multi_client(remote: &Remote) -> Result<(MultiClient, Connect
fn multi_connect(remote: &Remote) -> Result<MultiClient, anyhow::Error> {
let (client, info) = prepare_connect_multi_client(remote)?;
+ let token = pdm_config::remotes::get_secret_token(remote)?;
+
client.for_each_client(|client| {
client.set_authentication(proxmox_client::Token {
userid: remote.authid.to_string(),
- value: remote.token.to_string(),
+ value: token.clone(),
prefix: info.prefix.clone(),
perl_compat: info.perl_compat,
});
--
2.47.3
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 7+ messages in thread* [pdm-devel] [PATCH datacenter-manager 3/3] remote config: shadow token secrets when saving
2025-12-01 9:29 [pdm-devel] [PATCH datacenter-manager 0/3] token secret shadow config Fabian Grünbichler
2025-12-01 9:29 ` [pdm-devel] [PATCH datacenter-manager 1/3] remote config: let save_config take ownership Fabian Grünbichler
2025-12-01 9:29 ` [pdm-devel] [PATCH datacenter-manager 2/3] remote config: get token secret from shadow file if shadowed Fabian Grünbichler
@ 2025-12-01 9:29 ` Fabian Grünbichler
2025-12-01 14:39 ` Lukas Wagner
2025-12-01 14:46 ` [pdm-devel] [PATCH datacenter-manager 0/3] token secret shadow config Lukas Wagner
2025-12-01 16:44 ` [pdm-devel] applied: " Thomas Lamprecht
4 siblings, 1 reply; 7+ messages in thread
From: Fabian Grünbichler @ 2025-12-01 9:29 UTC (permalink / raw)
To: pdm-devel
but only if the to-be-saved config contains at least one non-shadowed entry, to
avoid unnecessary work.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
lib/pdm-config/src/remotes.rs | 39 +++++++++++++++++++++++++++++++++--
1 file changed, 37 insertions(+), 2 deletions(-)
diff --git a/lib/pdm-config/src/remotes.rs b/lib/pdm-config/src/remotes.rs
index 93f80eb..677981f 100644
--- a/lib/pdm-config/src/remotes.rs
+++ b/lib/pdm-config/src/remotes.rs
@@ -3,7 +3,7 @@
//! Make sure to call [`init`] to inject a concrete `RemoteConfig` instance
//! before calling the [`lock_config`], [`config`] or [`save_config`] functions.
-use std::sync::OnceLock;
+use std::{collections::HashMap, sync::OnceLock};
use anyhow::{bail, Error};
@@ -86,7 +86,42 @@ impl RemoteConfig for DefaultRemoteConfig {
Ok((data, digest.into()))
}
- fn save_config(&self, config: SectionConfigData<Remote>) -> Result<(), Error> {
+ fn save_config(&self, mut config: SectionConfigData<Remote>) -> Result<(), Error> {
+ let mut new_shadow_entries = HashMap::new();
+ for (id, remote) in config.iter() {
+ if remote.token != "-" {
+ new_shadow_entries.insert(
+ id.to_string(),
+ RemoteShadow {
+ ty: remote.ty,
+ id: remote.id.clone(),
+ token: remote.token.clone(),
+ },
+ );
+ }
+ }
+
+ // only read and modify shadow config if needed
+ if !new_shadow_entries.is_empty() {
+ let shadow_content =
+ proxmox_sys::fs::file_read_optional_string(REMOTES_SHADOW_FILENAME)?
+ .unwrap_or_default();
+
+ let mut shadow_config =
+ RemoteShadow::parse_section_config(REMOTES_SHADOW_FILENAME, &shadow_content)?;
+
+ for (id, shadow_entry) in new_shadow_entries.into_iter() {
+ if let Some(remote) = config.get_mut(&id) {
+ remote.token = '-'.to_string();
+ }
+ shadow_config.insert(id, shadow_entry);
+ }
+ let raw = RemoteShadow::write_section_config(REMOTES_SHADOW_FILENAME, &shadow_config)?;
+ replace_config(REMOTES_SHADOW_FILENAME, raw.as_bytes())?;
+ }
+
+ // only write out remote.cfg with potentially new shadowed entries
+ // if shadow file was successfully written!
let raw = Remote::write_section_config(REMOTES_CFG_FILENAME, &config)?;
replace_config(REMOTES_CFG_FILENAME, raw.as_bytes())
}
--
2.47.3
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 7+ messages in thread