Re: [pbs-devel] [PATCH backup] pbs-client: read credentials from $CREDENTIALS_DIRECTORY

["Jörg Behrmann" <proxmox@behrmj87m.dialup.fu-berlin.de>]

Hi!

Sorry to chime in from the sidelines, I just saw this patch set, which makes me
very happy.

It would be great if everything that can be set via envvar could be set, via
credential as well, most importantly I am thinking about the repository, since
systemd can accept credentials passed in via smbios type 11 strings.

There is a bug report against the PVE web UI open to be able to set arbitrary
key pars for that [1]. This would allow to configure the pbs client for a VM
directly from the PVE web UI.

Another comment further down inline.

Thanks for this works!

best regards,
Jörg Behrmann

[1] https://bugzilla.proxmox.com/show_bug.cgi?id=5601

On Mon, Mar 24, 2025 at 01:35:42PM +0100, Maximiliano Sandoval wrote:
> Allows to load credentials passed down by systemd. A possible use-case
> is safely storing the server's password in a file encrypted by the
> systems TPM, e.g. via
> 
> ```
> systemd-ask-password -n | systemd-creds encrypt --name=pbs-password - my-api-token.cred
> ```
> 
> which then can be used via
> 
> ```
> systemd-run --pipe --wait --property=LoadCredentialEncrypted=pbs-password:my-api-token.cred \
> proxmox-backup-client ...
> ```
> 
> or from inside a service.
> 
> Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
> ---
>  pbs-client/src/tools/key_source.rs |  4 +-
>  pbs-client/src/tools/mod.rs        | 81 ++++++++++++++++++++++++++++--
>  2 files changed, 80 insertions(+), 5 deletions(-)
> 
> diff --git a/pbs-client/src/tools/key_source.rs b/pbs-client/src/tools/key_source.rs
> index 7968e0c2a..94b86e8b6 100644
> --- a/pbs-client/src/tools/key_source.rs
> +++ b/pbs-client/src/tools/key_source.rs
> @@ -345,8 +345,8 @@ pub(crate) unsafe fn set_test_default_master_pubkey(value: Result<Option<Vec<u8>
>  pub fn get_encryption_key_password() -> Result<Vec<u8>, Error> {
>      // fixme: implement other input methods
>  
> -    if let Some(password) = super::get_secret_from_env("PBS_ENCRYPTION_PASSWORD")? {
> -        return Ok(password.as_bytes().to_vec());
> +    if let Some(password) = super::get_encryption_password()? {
> +        return Ok(password);
>      }
>  
>      // If we're on a TTY, query the user for a password
> diff --git a/pbs-client/src/tools/mod.rs b/pbs-client/src/tools/mod.rs
> index 8068dc004..8b0392744 100644
> --- a/pbs-client/src/tools/mod.rs
> +++ b/pbs-client/src/tools/mod.rs
> @@ -2,7 +2,7 @@
>  use std::collections::HashMap;
>  use std::env::VarError::{NotPresent, NotUnicode};
>  use std::fs::File;
> -use std::io::{BufRead, BufReader};
> +use std::io::{BufRead, BufReader, Read};
>  use std::os::unix::fs::OpenOptionsExt;
>  use std::os::unix::io::FromRawFd;
>  use std::process::Command;
> @@ -28,6 +28,16 @@ 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";
> +
> +/// Directory with system [credential]s. See systemd-creds(1).
> +///
> +/// [credential]: https://systemd.io/CREDENTIALS/
> +const CREDENTIALS_DIRECTORY: &str = "CREDENTIALS_DIRECTORY";
> +/// Credential name of the encryption password.
> +const CRED_PBS_ENCRYPTION_PASSWORD: &str = "pbs-encryption-password";
> +/// Credential name of the the password.
> +const CRED_PBS_PASSWORD: &str = "pbs-password";
>  
>  pub const REPO_URL_SCHEMA: Schema = StringSchema::new("Repository URL.")
>      .format(&BACKUP_REPO_URL)
> @@ -40,6 +50,43 @@ pub const CHUNK_SIZE_SCHEMA: Schema = IntegerSchema::new("Chunk size in KB. Must
>      .default(4096)
>      .schema();
>  
> +/// Retrieves a secret stored in a [credential] provided by systemd.
> +///
> +/// Returns `None` if the credential does not exist or on errors.
> +///
> +/// [credential]: https://systemd.io/CREDENTIALS/
> +pub fn get_credential(cred_name: &str) -> Option<Vec<u8>> {
> +    // This is fundamentally a fn that returns a Result, but in practice we
> +    // would ignore errors so we hide the result in the public API.
> +    fn get_credential_inner(cred_name: &str) -> Result<Option<Vec<u8>>, Error> {
> +        let Some(creds_dir) = std::env::var_os(CREDENTIALS_DIRECTORY) else {
> +            return Ok(None);
> +        };
> +        let path = std::path::Path::new(&creds_dir).join(cred_name);
> +
> +        let mut file = File::open(&path)?;
> +        let mut buffer = vec![];
> +
> +        // We read the whole contents without a BufRead. As per
> +        // systemd-creds(1): Credentials are limited-size binary or textual
> +        // objects.
> +        file.read_to_end(&mut buffer)?;
> +
> +        Ok(Some(buffer))
> +    }
> +    get_credential_inner(cred_name)
> +        .inspect_err(|err| {
> +            if err
> +                .downcast_ref::<std::io::Error>()
> +                .is_some_and(|e| e.kind() != std::io::ErrorKind::NotFound)
> +            {
> +                proxmox_log::error!("{err:#}")
> +            }
> +        })
> +        .ok()
> +        .flatten()
> +}
> +
>  /// Helper to read a secret through a environment variable (ENV).
>  ///
>  /// Tries the following variable names in order and returns the value
> @@ -118,6 +165,34 @@ pub fn get_secret_from_env(base_name: &str) -> Result<Option<String>, Error> {
>      Ok(None)
>  }
>  
> +/// Gets the backup server's password.
> +///
> +/// We first try reading from the `PBS_PASSWORD` environment variable, then we
> +/// try reading from the `pbs-password` [credential]. We ignore errors on the
> +/// latter.
> +///
> +/// [credential]: https://systemd.io/CREDENTIALS/
> +pub fn get_password() -> Result<Option<String>, Error> {
> +    let password = get_secret_from_env(ENV_VAR_PBS_PASSWORD)?.or_else(|| {
> +        get_credential(CRED_PBS_PASSWORD).and_then(|bytes| String::from_utf8(bytes).ok())
> +    });
> +    Ok(password)
> +}
> +
> +/// Gets an encryption password.
> +///
> +/// We first try reading from the `PBS_ENCRYPTION_PASSWORD` environment
> +/// variable, then we try reading from the `pbs-encryption-password`

The name for credentials is pretty free form and dashes are the correct
namespacing for systemd units, but when grepping for SetCredential= and
LoadCredential= in the systemd codebase, you'll see that dots are the more
idiomatic way of namespacing credentials, this idiom has also spread to other
projects, e.g. util-linux (see the credential support in agetty).

The better name would therefore be pbs.encryption.password or
pbs.encryption-password or pbs.encryption_password, depending on what exactly
the namespacing you want to communicate is.

> +/// [credential]. We ignore errors on the latter.
> +///
> +/// [credential]: https://systemd.io/CREDENTIALS/
> +pub fn get_encryption_password() -> Result<Option<Vec<u8>>, Error> {
> +    let password = get_secret_from_env(ENV_VAR_PBS_ENCRYPTION_PASSWORD)?
> +        .map(String::into_bytes)
> +        .or_else(|| get_credential(CRED_PBS_ENCRYPTION_PASSWORD));
> +    Ok(password)
> +}
> +
>  pub fn get_default_repository() -> Option<String> {
>      std::env::var("PBS_REPOSITORY").ok()
>  }
> @@ -181,7 +256,7 @@ fn connect_do(
>  ) -> Result<HttpClient, Error> {
>      let fingerprint = std::env::var(ENV_VAR_PBS_FINGERPRINT).ok();
>  
> -    let password = get_secret_from_env(ENV_VAR_PBS_PASSWORD)?;
> +    let password = get_password()?;
>      let options = HttpClientOptions::new_interactive(password, fingerprint).rate_limit(rate_limit);
>  
>      HttpClient::new(server, port, auth_id, options)
> @@ -190,7 +265,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 password = get_secret_from_env(ENV_VAR_PBS_PASSWORD).unwrap_or(None);
> +    let password = get_password().unwrap_or(None);
>  
>      // ticket cache, but no questions asked
>      let options = HttpClientOptions::new_interactive(password, fingerprint).interactive(false);
> -- 
> 2.39.5
> 
> 
> 
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

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