From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <pbs-devel-bounces@lists.proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9])
by lore.proxmox.com (Postfix) with ESMTPS id 430801FF15C
for <inbox@lore.proxmox.com>; Wed, 26 Mar 2025 11:10:48 +0100 (CET)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
by firstgate.proxmox.com (Proxmox) with ESMTP id BB72A333EA;
Wed, 26 Mar 2025 11:10:43 +0100 (CET)
DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed;
d=fu-berlin.de; s=fub01; h=In-Reply-To:Content-Transfer-Encoding:Content-Type
:MIME-Version:References:Reply-To:Message-ID:Subject:Cc:To:From:Date:Sender:
Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender:
Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:
List-Subscribe:List-Post:List-Owner:List-Archive;
bh=US67lrPOIj68Vm43IDA7i0bLf+lJETh8bEm7FpENzn0=; t=1742983841; x=1743588641;
b=JxVTD2UQLgbFIYzJ3VoHM+XfgocZb8H7DG2ajxvBReJ4JLrd8OZyx2CJ6LR24SA7sqHb2IX/FeY
8d4QPV4+ihGIEoQxCzuWoiyWMtFye42GSaV75q5AMkwKvIf8SYVbfQqat9vS5Xp0gA6dvLIV5rFVg
/l+gX8Qt9IHpMS+YZsS7PhaKYHYUA+hDDOisadbWdMZL2CujmmKx3raUDAV0MtKP24OuOoSHpEJex
HZb4j4cGwDFnYX3PLVzn6/PrN3zPWpGKE14/fab0VI+Lez8x6yIrqLPcR3REkrSqfziFXC61OCOVK
lZjto6pvUu/4SRj6c6QHbmh8InAKb02poPEA==;
Date: Wed, 26 Mar 2025 11:06:30 +0100
From: =?iso-8859-1?Q?J=F6rg?= Behrmann <proxmox@behrmj87m.dialup.fu-berlin.de>
To: Maximiliano Sandoval <m.sandoval@proxmox.com>
Message-ID: <Z-PRpqR2dS9Q_Edv@physik.fu-berlin.de>
References: <20250324123542.3248210-1-m.sandoval@proxmox.com>
MIME-Version: 1.0
Content-Disposition: inline
In-Reply-To: <20250324123542.3248210-1-m.sandoval@proxmox.com>
X-Original-Sender: proxmox@behrmj87m.dialup.fu-berlin.de
X-Originating-IP: 160.45.66.12
X-ZEDAT-Hint: PO
X-SPAM-LEVEL: Spam detection results: 0
BAYES_00 -1.9 Bayes spam probability is 0 to 1%
DKIM_SIGNED 0.1 Message has a DKIM or DK signature,
not necessarily valid
DKIM_VALID -0.1 Message has at least one valid DKIM or DK signature
DMARC_PASS -0.1 DMARC pass policy
RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/,
medium trust RCVD_IN_MSPIKE_H3 0.001 Good reputation (+3)
RCVD_IN_MSPIKE_WL 0.001 Mailspike good senders
SPF_HELO_PASS -0.001 SPF: HELO matches SPF record
SPF_PASS -0.001 SPF: sender matches SPF record
Subject: Re: [pbs-devel] [PATCH backup] pbs-client: read credentials from
$CREDENTIALS_DIRECTORY
X-BeenThere: pbs-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox Backup Server development discussion
<pbs-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pbs-devel>,
<mailto:pbs-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pbs-devel/>
List-Post: <mailto:pbs-devel@lists.proxmox.com>
List-Help: <mailto:pbs-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel>,
<mailto:pbs-devel-request@lists.proxmox.com?subject=subscribe>
Reply-To: zedv@physik.fu-berlin.de,
Proxmox Backup Server development discussion
<pbs-devel@lists.proxmox.com>
Cc: pbs-devel@lists.proxmox.com
Content-Type: text/plain; charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable
Errors-To: pbs-devel-bounces@lists.proxmox.com
Sender: "pbs-devel" <pbs-devel-bounces@lists.proxmox.com>
Hi!
Sorry to chime in from the sidelines, I just saw this patch set, which make=
s 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, si=
nce
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 arbitra=
ry
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=F6rg Behrmann
[1] https://bugzilla.proxmox.com/show_bug.cgi?id=3D5601
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=3Dpbs-password - m=
y-api-token.cred
> ```
> =
> which then can be used via
> =
> ```
> systemd-run --pipe --wait --property=3DLoadCredentialEncrypted=3Dpbs-pass=
word: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/ke=
y_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(v=
alue: Result<Option<Vec<u8>
> pub fn get_encryption_key_password() -> Result<Vec<u8>, Error> {
> // fixme: implement other input methods
> =
> - if let Some(password) =3D super::get_secret_from_env("PBS_ENCRYPTION=
_PASSWORD")? {
> - return Ok(password.as_bytes().to_vec());
> + if let Some(password) =3D 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 =3D "PBS_FINGERPRINT";
> const ENV_VAR_PBS_PASSWORD: &str =3D "PBS_PASSWORD";
> +const ENV_VAR_PBS_ENCRYPTION_PASSWORD: &str =3D "PBS_ENCRYPTION_PASSWORD=
";
> +
> +/// Directory with system [credential]s. See systemd-creds(1).
> +///
> +/// [credential]: https://systemd.io/CREDENTIALS/
> +const CREDENTIALS_DIRECTORY: &str =3D "CREDENTIALS_DIRECTORY";
> +/// Credential name of the encryption password.
> +const CRED_PBS_ENCRYPTION_PASSWORD: &str =3D "pbs-encryption-password";
> +/// Credential name of the the password.
> +const CRED_PBS_PASSWORD: &str =3D "pbs-password";
> =
> pub const REPO_URL_SCHEMA: Schema =3D StringSchema::new("Repository URL.=
")
> .format(&BACKUP_REPO_URL)
> @@ -40,6 +50,43 @@ pub const CHUNK_SIZE_SCHEMA: Schema =3D 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) =3D std::env::var_os(CREDENTIALS_DIRECTORY) =
else {
> + return Ok(None);
> + };
> + let path =3D std::path::Path::new(&creds_dir).join(cred_name);
> +
> + let mut file =3D File::open(&path)?;
> + let mut buffer =3D vec![];
> +
> + // We read the whole contents without a BufRead. As per
> + // systemd-creds(1): Credentials are limited-size binary or text=
ual
> + // 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() !=3D std::io::ErrorKind::NotFo=
und)
> + {
> + 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) -> Resul=
t<Option<String>, Error> {
> Ok(None)
> }
> =
> +/// Gets the backup server's password.
> +///
> +/// We first try reading from the `PBS_PASSWORD` environment variable, t=
hen we
> +/// try reading from the `pbs-password` [credential]. We ignore errors o=
n the
> +/// latter.
> +///
> +/// [credential]: https://systemd.io/CREDENTIALS/
> +pub fn get_password() -> Result<Option<String>, Error> {
> + let password =3D 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=3D and
LoadCredential=3D in the systemd codebase, you'll see that dots are the more
idiomatic way of namespacing credentials, this idiom has also spread to oth=
er
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 exact=
ly
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 =3D 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 =3D std::env::var(ENV_VAR_PBS_FINGERPRINT).ok();
> =
> - let password =3D get_secret_from_env(ENV_VAR_PBS_PASSWORD)?;
> + let password =3D get_password()?;
> let options =3D HttpClientOptions::new_interactive(password, fingerp=
rint).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 =3D std::env::var(ENV_VAR_PBS_FINGERPRINT).ok();
> - let password =3D get_secret_from_env(ENV_VAR_PBS_PASSWORD).unwrap_or=
(None);
> + let password =3D get_password().unwrap_or(None);
> =
> // ticket cache, but no questions asked
> let options =3D HttpClientOptions::new_interactive(password, fingerp=
rint).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