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) server-digest SHA256) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id CC7C6978CB for ; Wed, 17 Apr 2024 11:49:34 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id B04EE1E85 for ; Wed, 17 Apr 2024 11:49:04 +0200 (CEST) Received: from nena.proxmox.com (unknown [94.136.29.99]) by firstgate.proxmox.com (Proxmox) with ESMTP for ; Wed, 17 Apr 2024 11:49:03 +0200 (CEST) Received: by nena.proxmox.com (Postfix, from userid 1000) id 905721071B3; Wed, 17 Apr 2024 11:49:03 +0200 (CEST) From: Mira Limbeck To: pve-devel@lists.proxmox.com Date: Wed, 17 Apr 2024 11:48:57 +0200 Message-Id: <20240417094857.58520-1-m.limbeck@proxmox.com> X-Mailer: git-send-email 2.39.2 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.602 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 KAM_LAZY_DOMAIN_SECURITY 1 Sending domain does not have any anti-forgery methods RDNS_NONE 0.793 Delivered to internal network by a host with no rDNS SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_NONE 0.001 SPF: sender does not publish an 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. [storage.pm] Subject: [pve-devel] [PATCH v3 storage] fix insecure migration failing if waiting on lock 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: Wed, 17 Apr 2024 09:49:34 -0000 both STDOUT and STDERR are written into `$info` which is then parsed for IP and port of the target socket listening. when the ports file can't be locked immediately `trying to acquire lock...` is printed on STDERR and in turn written into `$info`. trying to parse the IP then fails, resulting in a migration or replication failing. the bare open3 call is replaced by the run_command wrapper from pve-common to use a safe wrapper around open3 with the same functionality. STDERR is read separatey from STDOUT and the last line of STDERR is kept in case of errors. Fixes: 57acd6a ("fix #1452: also log stderr of remote command with insecure storage migration") Signed-off-by: Mira Limbeck --- v3: - added log prefix for remote error logs - fixed style issues v2: - incorporated Fiona's suggestions - added `Fixes: ...` to commit message - kept old ip/port matching including # untaint comments - added logging for all messages in STDERR - simplified branches src/PVE/Storage.pm | 91 ++++++++++++++++++++++++++++------------------ 1 file changed, 55 insertions(+), 36 deletions(-) diff --git a/src/PVE/Storage.pm b/src/PVE/Storage.pm index 40314a8..ade4523 100755 --- a/src/PVE/Storage.pm +++ b/src/PVE/Storage.pm @@ -851,43 +851,62 @@ sub storage_migrate { eval { if ($insecure) { - my $input = IO::File->new(); - my $info = IO::File->new(); - open3($input, $info, $info, @$recv) - or die "receive command failed: $!\n"; - close($input); - - my $try_ip = <$info> // ''; - my ($ip) = $try_ip =~ /^($PVE::Tools::IPRE)$/ # untaint - or die "no tunnel IP received, got '$try_ip'\n"; - - my $try_port = <$info> // ''; - my ($port) = $try_port =~ /^(\d+)$/ # untaint - or die "no tunnel port received, got '$try_port'\n"; - - my $socket = IO::Socket::IP->new(PeerHost => $ip, PeerPort => $port, Type => SOCK_STREAM) - or die "failed to connect to tunnel at $ip:$port\n"; - # we won't be reading from the socket - shutdown($socket, 0); - - eval { run_command($cmds, output => '>&'.fileno($socket), errfunc => $match_volid_and_log); }; - my $send_error = $@; - - # don't close the connection entirely otherwise the receiving end - # might not get all buffered data (and fails with 'connection reset by peer') - shutdown($socket, 1); - - # wait for the remote process to finish - while (my $line = <$info>) { - $match_volid_and_log->("[$target_sshinfo->{name}] $line"); - } + my $ip; + my $port; + my $socket; + my $send_error; + + my $handle_insecure_migration = sub { + my $line = shift; + + if (!$ip) { + ($ip) = $line =~ /^($PVE::Tools::IPRE)$/ # untaint + or die "no tunnel IP received, got '$line'\n"; + } elsif (!$port) { + ($port) = $line =~ /^(\d+)$/ # untaint + or die "no tunnel port received, got '$line'\n"; + + # create socket, run command + $socket = IO::Socket::IP->new( + PeerHost => $ip, + PeerPort => $port, + Type => SOCK_STREAM, + ); + die "failed to connect to tunnel at $ip:$port\n" if !$socket; + # we won't be reading from the socket + shutdown($socket, 0); + + eval { + run_command( + $cmds, + output => '>&'.fileno($socket), + errfunc => $match_volid_and_log, + ); + }; + $send_error = $@; + + # don't close the connection entirely otherwise the receiving end + # might not get all buffered data (and fails with 'connection reset by peer') + shutdown($socket, 1); + } else { + $match_volid_and_log->("[$target_sshinfo->{name}] $line"); + } + }; - # now close the socket - close($socket); - if (!close($info)) { # does waitpid() - die "import failed: $!\n" if $!; - die "import failed: exit code ".($?>>8)."\n"; - } + eval { + run_command( + $recv, + outfunc => $handle_insecure_migration, + errfunc => sub { + my $line = shift; + + $match_volid_and_log->("[$target_sshinfo->{name}] $line"); + }, + ); + }; + my $recv_err = $@; + close($socket) if $socket; + die "failed to run insecure migration: $recv_err\n" if $recv_err; die $send_error if $send_error; } else { -- 2.39.2