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 [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id A50C01FF165 for <inbox@lore.proxmox.com>; Thu, 27 Mar 2025 12:57:31 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id D7284B150; Thu, 27 Mar 2025 12:57:25 +0100 (CET) Message-ID: <8e3faf53-fd97-47e4-a72b-bec209906ef6@proxmox.com> Date: Thu, 27 Mar 2025 12:57:21 +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: <20250327104730.199623-1-m.sandoval@proxmox.com> <20250327104730.199623-2-m.sandoval@proxmox.com> Content-Language: en-US, de-DE From: Christian Ebner <c.ebner@proxmox.com> In-Reply-To: <20250327104730.199623-2-m.sandoval@proxmox.com> X-SPAM-LEVEL: Spam detection results: 0 AWL 0.031 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 SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [mod.rs, systemd.io] Subject: Re: [pbs-devel] [PATCH backup v2 2/7] pbs-client: add 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> 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