[pbs-devel] [PATCH backup 0/5] Allow reading more system credentials & add docs

[pbs-devel] [PATCH backup 0/5] Allow reading more system credentials & add docs

[Maximiliano Sandoval]

These patches are a follow-up from [1]. We allow reading the fingerprint and
repository from system credentials and add documentation.

[1] https://lore.proxmox.com/pbs-devel/8728c4b8-b2ee-46d3-8cfb-014e24042251@proxmox.com/T/#t

Maximiliano Sandoval (5):
  pbs-client: use a const for the PBS_REPOSITORY env variable
  pbs-client: allow reading default repository from system credential
  pbs-client: allow reading fingerprint from system credential
  pbs-client: make common helper for getting UTF-8 secrets
  docs: client: add section about system credentials

 docs/backup-client.rst      | 36 +++++++++++++++++++++
 pbs-client/src/tools/mod.rs | 62 ++++++++++++++++++++++++++++++-------
 2 files changed, 86 insertions(+), 12 deletions(-)

-- 
2.39.5



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] [PATCH backup 1/5] pbs-client: use a const for the PBS_REPOSITORY env variable

[Maximiliano Sandoval]

Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
---
 pbs-client/src/tools/mod.rs | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/pbs-client/src/tools/mod.rs b/pbs-client/src/tools/mod.rs
index 3b19df390..a42fa1149 100644
--- a/pbs-client/src/tools/mod.rs
+++ b/pbs-client/src/tools/mod.rs
@@ -29,6 +29,7 @@ pub mod key_source;
 const ENV_VAR_PBS_FINGERPRINT: &str = "PBS_FINGERPRINT";
 const ENV_VAR_PBS_PASSWORD: &str = "PBS_PASSWORD";
 const ENV_VAR_PBS_ENCRYPTION_PASSWORD: &str = "PBS_ENCRYPTION_PASSWORD";
+const ENV_VAR_PBS_REPOSITORY: &str = "PBS_REPOSITORY";
 
 /// Directory with system [credential]s. See systemd-creds(1).
 ///
@@ -195,7 +196,7 @@ pub fn get_encryption_password() -> Result<Option<Vec<u8>>, Error> {
 }
 
 pub fn get_default_repository() -> Option<String> {
-    std::env::var("PBS_REPOSITORY").ok()
+    std::env::var(ENV_VAR_PBS_REPOSITORY).ok()
 }
 
 pub fn remove_repository_from_value(param: &mut Value) -> Result<BackupRepository, Error> {
-- 
2.39.5



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] [PATCH backup 2/5] pbs-client: allow reading default repository from system credential

[Maximiliano Sandoval]

Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
---
 pbs-client/src/tools/mod.rs | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/pbs-client/src/tools/mod.rs b/pbs-client/src/tools/mod.rs
index a42fa1149..acb932c64 100644
--- a/pbs-client/src/tools/mod.rs
+++ b/pbs-client/src/tools/mod.rs
@@ -39,6 +39,8 @@ const ENV_VAR_CREDENTIALS_DIRECTORY: &str = "CREDENTIALS_DIRECTORY";
 const CRED_PBS_ENCRYPTION_PASSWORD: &str = "proxmox-backup-client.encryption-password";
 /// Credential name of the the password.
 const CRED_PBS_PASSWORD: &str = "proxmox-backup-client.password";
+/// Credential name of the the repository.
+const CRED_PBS_REPOSITORY: &str = "proxmox-backup-client.repository";
 
 pub const REPO_URL_SCHEMA: Schema = StringSchema::new("Repository URL.")
     .format(&BACKUP_REPO_URL)
@@ -196,7 +198,17 @@ pub fn get_encryption_password() -> Result<Option<Vec<u8>>, Error> {
 }
 
 pub fn get_default_repository() -> Option<String> {
-    std::env::var(ENV_VAR_PBS_REPOSITORY).ok()
+    if let Ok(repository) = std::env::var(ENV_VAR_PBS_REPOSITORY) {
+        Some(repository)
+    } else if let Ok(Some(repository)) = get_credential(CRED_PBS_REPOSITORY).inspect_err(|err| {
+        proxmox_log::error!("Could not read credential {CRED_PBS_REPOSITORY}: {err}")
+    }) {
+        String::from_utf8(repository)
+            .inspect_err(|_err| proxmox_log::error!("non-utf8 repository credential"))
+            .ok()
+    } else {
+        None
+    }
 }
 
 pub fn remove_repository_from_value(param: &mut Value) -> Result<BackupRepository, Error> {
-- 
2.39.5



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] [PATCH backup 3/5] pbs-client: allow reading fingerprint from system credential

[Maximiliano Sandoval]

Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
---
 pbs-client/src/tools/mod.rs | 30 ++++++++++++++++++++++++++++--
 1 file changed, 28 insertions(+), 2 deletions(-)

diff --git a/pbs-client/src/tools/mod.rs b/pbs-client/src/tools/mod.rs
index acb932c64..5dd3b6b10 100644
--- a/pbs-client/src/tools/mod.rs
+++ b/pbs-client/src/tools/mod.rs
@@ -41,6 +41,8 @@ const CRED_PBS_ENCRYPTION_PASSWORD: &str = "proxmox-backup-client.encryption-pas
 const CRED_PBS_PASSWORD: &str = "proxmox-backup-client.password";
 /// Credential name of the the repository.
 const CRED_PBS_REPOSITORY: &str = "proxmox-backup-client.repository";
+/// Credential name of the the fingerprint.
+const CRED_PBS_FINGERPRINT: &str = "proxmox-backup-client.fingerprint";
 
 pub const REPO_URL_SCHEMA: Schema = StringSchema::new("Repository URL.")
     .format(&BACKUP_REPO_URL)
@@ -211,6 +213,30 @@ pub fn get_default_repository() -> Option<String> {
     }
 }
 
+/// Gets the repository fingerprint.
+///
+/// Looks for the fingerprint in the `PBS_FINGERPRINT` environment variable, if
+/// there isn't one it reads the `proxmox-backup-client.fingerprint`
+/// [credential].
+///
+/// Returns `None` if neither the environment variable or credentials are
+/// present.
+///
+/// [credential]: https://systemd.io/CREDENTIALS/
+pub fn get_fingerprint() -> Option<String> {
+    if let Ok(fingerprint) = std::env::var(ENV_VAR_PBS_FINGERPRINT) {
+        Some(fingerprint)
+    } else if let Ok(Some(fingerprint)) = get_credential(CRED_PBS_FINGERPRINT).inspect_err(|err| {
+        proxmox_log::error!("Could not read credential {CRED_PBS_FINGERPRINT}: {err}")
+    }) {
+        String::from_utf8(fingerprint)
+            .inspect_err(|_err| proxmox_log::error!("non-utf8 fingerprint credential"))
+            .ok()
+    } else {
+        None
+    }
+}
+
 pub fn remove_repository_from_value(param: &mut Value) -> Result<BackupRepository, Error> {
     if let Some(url) = param
         .as_object_mut()
@@ -268,7 +294,7 @@ fn connect_do(
     auth_id: &Authid,
     rate_limit: RateLimitConfig,
 ) -> Result<HttpClient, Error> {
-    let fingerprint = std::env::var(ENV_VAR_PBS_FINGERPRINT).ok();
+    let fingerprint = get_fingerprint();
 
     let password = get_password()?;
     let options = HttpClientOptions::new_interactive(password, fingerprint).rate_limit(rate_limit);
@@ -278,7 +304,7 @@ fn connect_do(
 
 /// like get, but simply ignore errors and return Null instead
 pub async fn try_get(repo: &BackupRepository, url: &str) -> Value {
-    let fingerprint = std::env::var(ENV_VAR_PBS_FINGERPRINT).ok();
+    let fingerprint = get_fingerprint();
     let password = get_password().unwrap_or(None);
 
     // ticket cache, but no questions asked
-- 
2.39.5



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] [PATCH backup 4/5] pbs-client: make common helper for getting UTF-8 secrets

[Maximiliano Sandoval]

Now that there are three credentials it makes sense to have a common
helper.

Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
---
 pbs-client/src/tools/mod.rs | 61 ++++++++++++++++++-------------------
 1 file changed, 30 insertions(+), 31 deletions(-)

diff --git a/pbs-client/src/tools/mod.rs b/pbs-client/src/tools/mod.rs
index 5dd3b6b10..bd553d88b 100644
--- a/pbs-client/src/tools/mod.rs
+++ b/pbs-client/src/tools/mod.rs
@@ -157,6 +157,25 @@ fn get_secret_from_env(base_name: &str) -> Result<Option<String>, Error> {
     Ok(None)
 }
 
+/// Gets a secret or value from the environment.
+///
+/// Checks for an environment variable named `env_variable`, and if missing, it
+/// checks for a system [credential] named `credential_name`. Assumes the secret
+/// is UTF-8 encoded.
+///
+/// [credential]: https://systemd.io/CREDENTIALS/
+fn get_secret_impl(env_variable: &str, credential_name: &str) -> Result<Option<String>, Error> {
+    if let Some(password) = get_secret_from_env(env_variable)? {
+        Ok(Some(password))
+    } else if let Some(password) = get_credential(credential_name)? {
+        String::from_utf8(password)
+            .map(Option::Some)
+            .map_err(|_err| format_err!("credential {credential_name} is not utf8 encoded"))
+    } else {
+        Ok(None)
+    }
+}
+
 /// Gets the backup server's password.
 ///
 /// Looks for a password in the `PBS_PASSWORD` environment variable, if there
@@ -167,15 +186,7 @@ fn get_secret_from_env(base_name: &str) -> Result<Option<String>, Error> {
 ///
 /// [credential]: https://systemd.io/CREDENTIALS/
 pub fn get_password() -> Result<Option<String>, Error> {
-    if let Some(password) = get_secret_from_env(ENV_VAR_PBS_PASSWORD)? {
-        Ok(Some(password))
-    } else if let Some(password) = get_credential(CRED_PBS_PASSWORD)? {
-        String::from_utf8(password)
-            .map(Option::Some)
-            .map_err(|_err| format_err!("non-utf8 password credential"))
-    } else {
-        Ok(None)
-    }
+    get_secret_impl(ENV_VAR_PBS_PASSWORD, CRED_PBS_PASSWORD)
 }
 
 /// Gets an encryption password.
@@ -200,17 +211,11 @@ pub fn get_encryption_password() -> Result<Option<Vec<u8>>, Error> {
 }
 
 pub fn get_default_repository() -> Option<String> {
-    if let Ok(repository) = std::env::var(ENV_VAR_PBS_REPOSITORY) {
-        Some(repository)
-    } else if let Ok(Some(repository)) = get_credential(CRED_PBS_REPOSITORY).inspect_err(|err| {
-        proxmox_log::error!("Could not read credential {CRED_PBS_REPOSITORY}: {err}")
-    }) {
-        String::from_utf8(repository)
-            .inspect_err(|_err| proxmox_log::error!("non-utf8 repository credential"))
-            .ok()
-    } else {
-        None
-    }
+    get_secret_impl(ENV_VAR_PBS_REPOSITORY, CRED_PBS_REPOSITORY)
+        .inspect_err(|err| {
+            proxmox_log::error!("could not read default repository: {err:#}");
+        })
+        .unwrap_or_default()
 }
 
 /// Gets the repository fingerprint.
@@ -224,17 +229,11 @@ pub fn get_default_repository() -> Option<String> {
 ///
 /// [credential]: https://systemd.io/CREDENTIALS/
 pub fn get_fingerprint() -> Option<String> {
-    if let Ok(fingerprint) = std::env::var(ENV_VAR_PBS_FINGERPRINT) {
-        Some(fingerprint)
-    } else if let Ok(Some(fingerprint)) = get_credential(CRED_PBS_FINGERPRINT).inspect_err(|err| {
-        proxmox_log::error!("Could not read credential {CRED_PBS_FINGERPRINT}: {err}")
-    }) {
-        String::from_utf8(fingerprint)
-            .inspect_err(|_err| proxmox_log::error!("non-utf8 fingerprint credential"))
-            .ok()
-    } else {
-        None
-    }
+    get_secret_impl(ENV_VAR_PBS_FINGERPRINT, CRED_PBS_FINGERPRINT)
+        .inspect_err(|err| {
+            proxmox_log::error!("could not read fingerprint: {err:#}");
+        })
+        .unwrap_or_default()
 }
 
 pub fn remove_repository_from_value(param: &mut Value) -> Result<BackupRepository, Error> {
-- 
2.39.5



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

Re: [pbs-devel] [PATCH backup 4/5] pbs-client: make common helper for getting UTF-8 secrets

[Christian Ebner]

This patch could be ordered before the change to the reading of the 
default repository, so you don not have to remove the hunks introduced 
by that patch again here.

Further, I see that the get_secrets_impl and get_encryption_password do 
look almost identical now, so latter could maybe be covered by the same 
implementation logic as well, doing the UTF-8 string conversion/checking 
only after?

On 3/26/25 15:26, Maximiliano Sandoval wrote:
> Now that there are three credentials it makes sense to have a common
> helper.
> 
> Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
> ---
>   pbs-client/src/tools/mod.rs | 61 ++++++++++++++++++-------------------
>   1 file changed, 30 insertions(+), 31 deletions(-)
> 
> diff --git a/pbs-client/src/tools/mod.rs b/pbs-client/src/tools/mod.rs
> index 5dd3b6b10..bd553d88b 100644
> --- a/pbs-client/src/tools/mod.rs
> +++ b/pbs-client/src/tools/mod.rs
> @@ -157,6 +157,25 @@ fn get_secret_from_env(base_name: &str) -> Result<Option<String>, Error> {
>       Ok(None)
>   }
>   
> +/// Gets a secret or value from the environment.
> +///
> +/// Checks for an environment variable named `env_variable`, and if missing, it
> +/// checks for a system [credential] named `credential_name`. Assumes the secret
> +/// is UTF-8 encoded.
> +///
> +/// [credential]: https://systemd.io/CREDENTIALS/
> +fn get_secret_impl(env_variable: &str, credential_name: &str) -> Result<Option<String>, Error> {
> +    if let Some(password) = get_secret_from_env(env_variable)? {
> +        Ok(Some(password))
> +    } else if let Some(password) = get_credential(credential_name)? {
> +        String::from_utf8(password)
> +            .map(Option::Some)
> +            .map_err(|_err| format_err!("credential {credential_name} is not utf8 encoded"))
> +    } else {
> +        Ok(None)
> +    }
> +}
> +
>   /// Gets the backup server's password.
>   ///
>   /// Looks for a password in the `PBS_PASSWORD` environment variable, if there
> @@ -167,15 +186,7 @@ fn get_secret_from_env(base_name: &str) -> Result<Option<String>, Error> {
>   ///
>   /// [credential]: https://systemd.io/CREDENTIALS/
>   pub fn get_password() -> Result<Option<String>, Error> {
> -    if let Some(password) = get_secret_from_env(ENV_VAR_PBS_PASSWORD)? {
> -        Ok(Some(password))
> -    } else if let Some(password) = get_credential(CRED_PBS_PASSWORD)? {
> -        String::from_utf8(password)
> -            .map(Option::Some)
> -            .map_err(|_err| format_err!("non-utf8 password credential"))
> -    } else {
> -        Ok(None)
> -    }
> +    get_secret_impl(ENV_VAR_PBS_PASSWORD, CRED_PBS_PASSWORD)
>   }
>   
>   /// Gets an encryption password.
> @@ -200,17 +211,11 @@ pub fn get_encryption_password() -> Result<Option<Vec<u8>>, Error> {
>   }
>   
>   pub fn get_default_repository() -> Option<String> {
> -    if let Ok(repository) = std::env::var(ENV_VAR_PBS_REPOSITORY) {
> -        Some(repository)
> -    } else if let Ok(Some(repository)) = get_credential(CRED_PBS_REPOSITORY).inspect_err(|err| {
> -        proxmox_log::error!("Could not read credential {CRED_PBS_REPOSITORY}: {err}")
> -    }) {
> -        String::from_utf8(repository)
> -            .inspect_err(|_err| proxmox_log::error!("non-utf8 repository credential"))
> -            .ok()
> -    } else {
> -        None
> -    }
> +    get_secret_impl(ENV_VAR_PBS_REPOSITORY, CRED_PBS_REPOSITORY)
> +        .inspect_err(|err| {
> +            proxmox_log::error!("could not read default repository: {err:#}");
> +        })
> +        .unwrap_or_default()
>   }
>   
>   /// Gets the repository fingerprint.
> @@ -224,17 +229,11 @@ pub fn get_default_repository() -> Option<String> {
>   ///
>   /// [credential]: https://systemd.io/CREDENTIALS/
>   pub fn get_fingerprint() -> Option<String> {
> -    if let Ok(fingerprint) = std::env::var(ENV_VAR_PBS_FINGERPRINT) {
> -        Some(fingerprint)
> -    } else if let Ok(Some(fingerprint)) = get_credential(CRED_PBS_FINGERPRINT).inspect_err(|err| {
> -        proxmox_log::error!("Could not read credential {CRED_PBS_FINGERPRINT}: {err}")
> -    }) {
> -        String::from_utf8(fingerprint)
> -            .inspect_err(|_err| proxmox_log::error!("non-utf8 fingerprint credential"))
> -            .ok()
> -    } else {
> -        None
> -    }
> +    get_secret_impl(ENV_VAR_PBS_FINGERPRINT, CRED_PBS_FINGERPRINT)
> +        .inspect_err(|err| {
> +            proxmox_log::error!("could not read fingerprint: {err:#}");
> +        })
> +        .unwrap_or_default()
>   }
>   
>   pub fn remove_repository_from_value(param: &mut Value) -> Result<BackupRepository, Error> {



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

Re: [pbs-devel] [PATCH backup 4/5] pbs-client: make common helper for getting UTF-8 secrets

[Maximiliano Sandoval]

Christian Ebner <c.ebner@proxmox.com> writes:

> This patch could be ordered before the change to the reading of the default
> repository, so you don not have to remove the hunks introduced by that patch
> again here.
>
> Further, I see that the get_secrets_impl and get_encryption_password do look
> almost identical now, so latter could maybe be covered by the same
> implementation logic as well, doing the UTF-8 string conversion/checking only
> after?

Sent v2
https://lore.proxmox.com/pbs-devel/20250327104730.199623-1-m.sandoval@proxmox.com/T/#t.


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] [PATCH backup 5/5] docs: client: add section about system credentials

[Maximiliano Sandoval]

Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
---
 docs/backup-client.rst | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/docs/backup-client.rst b/docs/backup-client.rst
index e11c0142a..aea63bd1f 100644
--- a/docs/backup-client.rst
+++ b/docs/backup-client.rst
@@ -44,6 +44,9 @@ user\@pbs!token@host:store       ``user@pbs!token`` host:8007          store
 [ff80::51]:1234:mydatastore      ``root@pam``       [ff80::51]:1234    mydatastore
 ================================ ================== ================== ===========
 
+
+.. _environment-variables:
+
 Environment Variables
 ---------------------
 
@@ -89,6 +92,39 @@ Environment Variables
    you can add arbitrary comments after the first newline.
 
 
+System Credentials
+------------------
+
+Some of the :ref:`environment variables <environment-variables>` above can be
+set using `system credentials <https://systemd.io/CREDENTIALS/>`_ instead.
+
+============================ ==============================================
+Environment Variable         Credential Name Equivalent
+============================ ==============================================
+``PBS_REPOSITORY``           ``proxmox-backup-client.repository``
+``PBS_PASSWORD``             ``proxmox-backup-client.password``
+``PBS_ENCRYPTION_PASSWORD``  ``proxmox-backup-client.encryption-password``
+``PBS_FINGERPRINT``          ``proxmox-backup-client.fingerprint``
+============================ ==============================================
+
+For example, a credential for the repository password can be stored in an
+encrypted file as follows:
+
+.. code-block:: console
+
+  # systemd-ask-password -n | systemd-creds encrypt --name=proxmox-backup-client.password - my-api-token.cred
+
+The credential can be then reused inside of unit files or in a transient scope
+unit as follows:
+
+.. code-block:: console
+
+  # systemd-run --pipe --wait \
+  --property=LoadCredentialEncrypted=proxmox-backup-client.password:my-api-token.cred \
+  --property=SetCredential=proxmox-backup-client.repository:'my_default_repository' \
+  proxmox-backup-client ...
+
+
 Output Format
 -------------
 
-- 
2.39.5



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel