From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id 623AF1FF161 for ; Tue, 27 Aug 2024 11:37:17 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 12A33306DD; Tue, 27 Aug 2024 11:37:44 +0200 (CEST) Date: Tue, 27 Aug 2024 11:37:09 +0200 From: Wolfgang Bumiller To: Gabriel Goller Message-ID: References: <20240823091215.124453-1-g.goller@proxmox.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20240823091215.124453-1-g.goller@proxmox.com> X-SPAM-LEVEL: Spam detection results: 0 AWL 0.089 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 T_SCC_BODY_TEXT_LINE -0.01 - URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [proxmox.com, proxmox-backup-proxy.rs] Subject: Re: [pbs-devel] [PATCH proxmox-backup] proxy: check permissions on proxy.key and proxy.pem files X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox Backup Server development discussion Cc: pbs-devel@lists.proxmox.com Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pbs-devel-bounces@lists.proxmox.com Sender: "pbs-devel" NAK On Fri, Aug 23, 2024 at 11:12:15AM GMT, Gabriel Goller wrote: > Check the owner and permission of the proxy.key and proxy.pem files. > This avoids openssl's unhelpful error message and prints a nicer one. > > Motivation: https://forum.proxmox.com/threads/proxmox-backup-tailscale-proxmox-backup-proxy-service-wont-boot.153204 > > Signed-off-by: Gabriel Goller > --- > > Note: not sure about the correct permissions, we currently default to > 640, but maybe a minimum of 400 is enough? > > src/bin/proxmox-backup-proxy.rs | 27 +++++++++++++++++++++++++++ > 1 file changed, 27 insertions(+) > > diff --git a/src/bin/proxmox-backup-proxy.rs b/src/bin/proxmox-backup-proxy.rs > index 041f3aff999c..544196b8bc5d 100644 > --- a/src/bin/proxmox-backup-proxy.rs > +++ b/src/bin/proxmox-backup-proxy.rs > @@ -367,6 +367,30 @@ async fn run() -> Result<(), Error> { > Ok(()) > } > > +/// Check permissions and owner of passed path. > +fn check_permissions>(path: T, file_mode: u32) -> Result<(), Error> { > + match nix::sys::stat::stat(path.as_ref()) { > + Ok(stat) => { > + if stat.st_uid != u32::from(pbs_config::backup_user()?.uid) > + || stat.st_gid != u32::from(pbs_config::backup_group()?.gid) > + || stat.st_mode & 0o770 < file_mode If you want to test whether you can open a file, you should either just `open(2)` it, or, if you really want to avoid it, use `access(2)`. You do not ever want to attempt to try to perform the kernel's permission checks yourself. There could be ACLs, AppArmor profiles, ... and while we can say that, for now, this is not supposed to be the case, it's bad practice in general. Also note that this only covers the case at a point in time where the certificate isn't actually loaded, and won't help with changes to the permissions while a daemon is already running. A better approach to handle this specific case would be to adapt `proxmox-rest-server`'s handling of `Tls::PemFiles` so that instead of using `openssl`'s ".set_private_key_file()` convenience methods, it loads the files, and handles `EPERM`/`ENOENT`/... with useful error messags, and then uses `acceptor.set_private_key(PKey::private_key_from_pem(data)?)` _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel