Re: [pbs-devel] [PATCH backup v2 2/7] pbs-client: add helper for getting UTF-8 secrets

[Christian Ebner <c.ebner@proxmox.com>]

On 3/27/25 11:47, Maximiliano Sandoval wrote:
> We are going to add more credentials so it makes sense to have a common
> helper to get the secrets.
> 
> Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
> ---
>   pbs-client/src/tools/mod.rs | 19 +++++++++++++++++++
>   1 file changed, 19 insertions(+)
> 
> diff --git a/pbs-client/src/tools/mod.rs b/pbs-client/src/tools/mod.rs
> index a42fa114..efd2e139 100644
> --- a/pbs-client/src/tools/mod.rs
> +++ b/pbs-client/src/tools/mod.rs
> @@ -153,6 +153,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"))

This check for valid UTF-8 could rather happen directly in 
get_credential since you will enforce get_encryption_password to return 
a String anyways in patch 4? And the others already return a String anyways.

Only allowing UTF-8 content from get_credential would bring us in line 
with secrets obtained from the other sources, and since this has been 
introduced only recently (and never packaged until now), this could 
still be changed without break changes.

As a side effect this helper would than simplify further.

> +    } else {
> +        Ok(None)
> +    }
> +}
> +
>   /// Gets the backup server's password.
>   ///
>   /// Looks for a password in the `PBS_PASSWORD` environment variable, if there



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