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 separately from STDOUT and logged. Fixes: 57acd6a ("fix #1452: also log stderr of remote command with insecure storage migration") Signed-off-by: Mira Limbeck <m.limb...@proxmox.com> --- 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 | 74 ++++++++++++++++++++++++---------------------- 1 file changed, 39 insertions(+), 35 deletions(-) diff --git a/src/PVE/Storage.pm b/src/PVE/Storage.pm index 40314a8..b1d4b7a 100755 --- a/src/PVE/Storage.pm +++ b/src/PVE/Storage.pm @@ -851,44 +851,48 @@ 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) + 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); }; + $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 => $match_volid_and_log); }; + if (my $err = $@) { + close($socket) if $socket; + + chomp($err); + die "failed to run insecure migration: $err\n"; } + # now close the socket + close($socket) if $socket; die $send_error if $send_error; } else { push @$cmds, $recv; -- 2.39.2 _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel