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 6CACB1FF16F
for <inbox@lore.proxmox.com>; Thu, 27 Mar 2025 10:25:35 +0100 (CET)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
by firstgate.proxmox.com (Proxmox) with ESMTP id B37758516;
Thu, 27 Mar 2025 10:25:29 +0100 (CET)
Message-ID: <87465dcb-d972-45d3-9ee5-a2f2d43a3637@proxmox.com>
Date: Thu, 27 Mar 2025 10:24:55 +0100
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
To: Proxmox Backup Server development discussion
<pbs-devel@lists.proxmox.com>, Maximiliano Sandoval <m.sandoval@proxmox.com>
References: <20250326142609.399793-1-m.sandoval@proxmox.com>
<20250326142609.399793-5-m.sandoval@proxmox.com>
Content-Language: en-US, de-DE
From: Christian Ebner <c.ebner@proxmox.com>
In-Reply-To: <20250326142609.399793-5-m.sandoval@proxmox.com>
X-SPAM-LEVEL: Spam detection results: 0
AWL 0.029 Adjusted score from AWL reputation of From: address
BAYES_00 -1.9 Bayes spam probability is 0 to 1%
DMARC_MISSING 0.1 Missing DMARC policy
KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
Validity was blocked. See
https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
information.
RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
Validity was blocked. See
https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
information.
RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
Validity was blocked. See
https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
information.
SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record
SPF_PASS -0.001 SPF: sender matches SPF record
Subject: Re: [pbs-devel] [PATCH backup 4/5] pbs-client: make common helper
for getting UTF-8 secrets
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: Proxmox Backup Server development discussion
<pbs-devel@lists.proxmox.com>
Content-Transfer-Encoding: 7bit
Content-Type: text/plain; charset="us-ascii"; Format="flowed"
Errors-To: pbs-devel-bounces@lists.proxmox.com
Sender: "pbs-devel" <pbs-devel-bounces@lists.proxmox.com>
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