From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id B3EA677437 for ; Thu, 21 Oct 2021 20:15:13 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id A9E0222239 for ; Thu, 21 Oct 2021 20:15:13 +0200 (CEST) Received: from mx-out.trafficplex.de (mx-out.trafficplex.de [IPv6:2a00:f48:2000:affe:1337:2:0:1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id 323492222D for ; Thu, 21 Oct 2021 20:15:11 +0200 (CEST) MIME-Version: 1.0 DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=lima-city.de; s=managedbylima-20161105; t=1634840110; bh=AW9S+Dd7YukFyXLtxJiBuayko/jklGXym1OymxVkrBE=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=UzBtxoaOo22NWyjaOQWu6dYsWR5XNuFQAWoa0UT234vD4M6sd/ATppYf+h6ctPz/T W5HaaL0gVlIATuR7ee+NJW0zUk3jgLW0sMo3QIprv4mnRnyek4SIGyyV/WCDe0txCH 6DhGAQhCWI8k4FcSbCm0U2GzwZP3qjw2u+eZJgtg= Date: Thu, 21 Oct 2021 20:15:10 +0200 From: =?UTF-8?Q?Phillipp_R=C3=B6ll?= To: Thomas Lamprecht Cc: Proxmox VE development discussion In-Reply-To: References: <20211021084650.26753-1-pr@lima-city.de> User-Agent: Roundcube Webmail/1.4.11 Message-ID: X-Sender: pr@lima-city.de Organization: TrafficPlex GmbH Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit 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 DKIM_VALID_AU -0.1 Message has a valid DKIM or DK signature from author's domain DKIM_VALID_EF -0.1 Message has a valid DKIM or DK signature from envelope-from domain SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pve-devel] [PATCH] Check only first part of script parameter for executability X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 21 Oct 2021 18:15:13 -0000 Hi Thomas, I'll skip the legal stuff for now, until we figure out if the patch has a chance to be merged, ok? That would break existing configuration that use a script with whitespace in its path name. There could be two checks, check if either the full script name is executable OR just the first part (split by whitespaces). Can you please provide your usecase where the arguments Proxmox VE sets for the script aren't enough? So that we've a better picture about possible options. We have a backup & restore management system, where the state, retention period etc. of a backup is stored. All proxmox backup states are updated by a hook script calling a REST API. We need to pass the ID of the backup from the management system to the hook script, which is no longer possible since the check was introduced. We comment/patch these checks out automatically on all systems, but that's a bit annoying. So actually the (much) earlier introduction of the check already broke our use case, but before today the pain to report it/change something simply wasn't great enough. --- Mit freundlichen Grüßen Phillipp Röll Am 2021-10-21 11:05, schrieb Thomas Lamprecht: > Hi, > > thanks for your effort in contributing to Proxmox VE! > > A few things inline > > On 21.10.21 10:46, Phillipp Röll wrote: >> When giving a script parameter including arguments to the dump >> process, >> the full script parameter including the arguments is checked for >> executablility. The following will always abort with a "not >> executable" >> error: >> >> vzdump 100 --script "/usr/local/bin/myscript.pl myarg" >> >> If we split the script argument and check only the first part, the >> check works as expected. > > your sign-off is missing here and it doesn't seems like we have a > signed > CLA from you or your company here, and to be able to take in a patch > you'd > need to send one to . For details please check: > https://pve.proxmox.com/wiki/Developer_Documentation#Software_License_and_Copyright > >> --- >> PVE/VZDump.pm | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/PVE/VZDump.pm b/PVE/VZDump.pm >> index 88ac11b3..fe1a7f23 100644 >> --- a/PVE/VZDump.pm >> +++ b/PVE/VZDump.pm >> @@ -630,7 +630,9 @@ sub run_hook_script { >> my $script = $opts->{script}; >> return if !$script; >> >> - if (!-x $script) { >> + my @script_args = split /\s+/, $script; > > That would break existing configuration that use a script with > whitespace in its > path name. > > Can you please provide your usecase where the arguments Proxmox VE sets > for the > script aren't enough? So that we've a better picture about possible > options. > > cheers, > Thomas