Re: [pve-devel] [PATCH v6 qemu-server 2/6] mtunnel: add API endpoints

["Fabian Grünbichler" <f.gruenbichler@proxmox.com>]

On September 30, 2022 1:52 pm, Stefan Hanreich wrote:
> 
> 
> On 9/28/22 14:50, Fabian Grünbichler wrote:
>> the following two endpoints are used for migration on the remote side
>> 
>> POST /nodes/NODE/qemu/VMID/mtunnel
>> 
>> which creates and locks an empty VM config, and spawns the main qmtunnel
>> worker which binds to a VM-specific UNIX socket.
>> 
>> this worker handles JSON-encoded migration commands coming in via this
>> UNIX socket:
>> - config (set target VM config)
>> -- checks permissions for updating config
>> -- strips pending changes and snapshots
>> -- sets (optional) firewall config
>> - disk (allocate disk for NBD migration)
>> -- checks permission for target storage
>> -- returns drive string for allocated volume
>> - disk-import, query-disk-import, bwlimit
>> -- handled by PVE::StorageTunnel
>> - start (returning migration info)
>> - fstrim (via agent)
>> - ticket (creates a ticket for a WS connection to a specific socket)
>> - resume
>> - stop
>> - nbdstop
>> - unlock
>> - quit (+ cleanup)
>> 
>> this worker serves as a replacement for both 'qm mtunnel' and various
>> manual calls via SSH. the API call will return a ticket valid for
>> connecting to the worker's UNIX socket via a websocket connection.
>> 
>> GET+WebSocket upgrade /nodes/NODE/qemu/VMID/mtunnelwebsocket
>> 
>> gets called for connecting to a UNIX socket via websocket forwarding,
>> i.e. once for the main command mtunnel, and once each for the memory
>> migration and each NBD drive-mirror/storage migration.
>> 
>> access is guarded by a short-lived ticket binding the authenticated user
>> to the socket path. such tickets can be requested over the main mtunnel,
>> which keeps track of socket paths currently used by that
>> mtunnel/migration instance.
>> 
>> each command handler should check privileges for the requested action if
>> necessary.
>> 
>> both mtunnel and mtunnelwebsocket endpoints are not proxied, the
>> client/caller is responsible for ensuring the passed 'node' parameter
>> and the endpoint handling the call are matching.
>> 
>> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
>> ---
>> 
>> Notes:
>>      v6:
>>      - check for Sys.Incoming in mtunnel
>>      - add definedness checks in 'config' command
>>      - switch to vm_running_locally in 'resume' command
>>      - moved $socket_addr closer to usage
>>      v5:
>>      - us vm_running_locally
>>      - move '$socket_addr' declaration closer to usage
>>      v4:
>>      - add timeout to accept()
>>      - move 'bwlimit' to PVE::StorageTunnel and extend it
>>      - mark mtunnel(websocket) as non-proxied, and check $node accordingly
>>      v3:
>>      - handle meta and vmgenid better
>>      - handle failure of 'config' updating
>>      - move 'disk-import' and 'query-disk-import' handlers to pve-guest-common
>>      - improve tunnel exit by letting client close the connection
>>      - use strict VM config parser
>>      v2: incorporated Fabian Ebner's feedback, mainly:
>>      - use modified nbd alloc helper instead of duplicating
>>      - fix disk cleanup, also cleanup imported disks
>>      - fix firewall-conf vs firewall-config mismatch
>>      
>>      requires
>>      - pve-access-control with tunnel ticket support (already marked in d/control)
>>      - pve-access-control with Sys.Incoming privilege (not yet applied/bumped!)
>>      - pve-http-server with websocket fixes (could be done via breaks? or bumped in
>>        pve-manager..)
>> 
>>   PVE/API2/Qemu.pm | 527 ++++++++++++++++++++++++++++++++++++++++++++++-
>>   debian/control   |   2 +-
>>   2 files changed, 527 insertions(+), 2 deletions(-)
>> 
>> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
>> index 3ec31c26..9270ca74 100644
>> --- a/PVE/API2/Qemu.pm
>> +++ b/PVE/API2/Qemu.pm
>> @@ -4,10 +4,13 @@ use strict;
>>   use warnings;
>>   use Cwd 'abs_path';
>>   use Net::SSLeay;
>> -use POSIX;
>>   use IO::Socket::IP;
>> +use IO::Socket::UNIX;
>> +use IPC::Open3;
>> +use JSON;
>>   use URI::Escape;
>>   use Crypt::OpenSSL::Random;
>> +use Socket qw(SOCK_STREAM);
>>   
>>   use PVE::Cluster qw (cfs_read_file cfs_write_file);;
>>   use PVE::RRD;
>> @@ -38,6 +41,7 @@ use PVE::VZDump::Plugin;
>>   use PVE::DataCenterConfig;
>>   use PVE::SSHInfo;
>>   use PVE::Replication;
>> +use PVE::StorageTunnel;
>>   
>>   BEGIN {
>>       if (!$ENV{PVE_GENERATING_DOCS}) {
>> @@ -1087,6 +1091,7 @@ __PACKAGE__->register_method({
>>   	    { subdir => 'spiceproxy' },
>>   	    { subdir => 'sendkey' },
>>   	    { subdir => 'firewall' },
>> +	    { subdir => 'mtunnel' },
>>   	    ];
>>   
>>   	return $res;
>> @@ -4965,4 +4970,524 @@ __PACKAGE__->register_method({
>>   	return PVE::QemuServer::Cloudinit::dump_cloudinit_config($conf, $param->{vmid}, $param->{type});
>>       }});
>>   
>> +__PACKAGE__->register_method({
>> +    name => 'mtunnel',
>> +    path => '{vmid}/mtunnel',
>> +    method => 'POST',
>> +    protected => 1,
>> +    description => 'Migration tunnel endpoint - only for internal use by VM migration.',
>> +    permissions => {
>> +	check =>
>> +	[ 'and',
>> +	  ['perm', '/vms/{vmid}', [ 'VM.Allocate' ]],
>> +	  ['perm', '/', [ 'Sys.Incoming' ]],
>> +	],
>> +	description => "You need 'VM.Allocate' permissions on '/vms/{vmid}' and Sys.Incoming" .
>> +	               " on '/'. Further permission checks happen during the actual migration.",
>> +    },
>> +    parameters => {
>> +	additionalProperties => 0,
>> +	properties => {
>> +	    node => get_standard_option('pve-node'),
>> +	    vmid => get_standard_option('pve-vmid'),
>> +	    storages => {
>> +		type => 'string',
>> +		format => 'pve-storage-id-list',
>> +		optional => 1,
>> +		description => 'List of storages to check permission and availability. Will be checked again for all actually used storages during migration.',
>> +	    },
>> +	},
>> +    },
>> +    returns => {
>> +	additionalProperties => 0,
>> +	properties => {
>> +	    upid => { type => 'string' },
>> +	    ticket => { type => 'string' },
>> +	    socket => { type => 'string' },
>> +	},
>> +    },
>> +    code => sub {

[...]

>> +		my $cmd = delete $parsed->{cmd};
>> +		if (!defined($cmd)) {
>> +		    $reply_err->("'cmd' missing");
>> +		} elsif ($state->{exit}) {
>> +		    $reply_err->("tunnel is in exit-mode, processing '$cmd' cmd not possible");
>> +		    next;
>> +		} elsif (my $handler = $cmd_handlers->{$cmd}) {
>> +		    print "received command '$cmd'\n";
>> +		    eval {
>> +			if ($cmd_desc->{$cmd}) {
>> +			    PVE::JSONSchema::validate($cmd_desc->{$cmd}, $parsed);
> 
> might the params be flipped here?
> 

yes! thanks for catching (and wow - our schema handling is flexible that 
it never choked on that!).

I'll do some tests and see whether flipping breaks anything (the same is 
also true for pve-container, since this part is duplicated there).

>> +			} else {
>> +			    $parsed = {};
>> +			}
>> +			my $res = $run_locked->($handler, $parsed);
>> +			$reply_ok->($res);
>> +		    };
>> +		    $reply_err->("failed to handle '$cmd' command - $@")
>> +			if $@;
>> +		} else {
>> +		    $reply_err->("unknown command '$cmd' given");
>> +		}
>> +	    }
>> +
>> +	    if ($state->{exit}) {
>> +		print "mtunnel exited\n";
>> +	    } else {
>> +		die "mtunnel exited unexpectedly\n";
>> +	    }
>> +	};
>> +
>> +	my $socket_addr = "/run/qemu-server/$vmid.mtunnel";
>> +	my $ticket = PVE::AccessControl::assemble_tunnel_ticket($authuser, "/socket/$socket_addr");
>> +	my $upid = $rpcenv->fork_worker('qmtunnel', $vmid, $authuser, $realcmd);
>> +
>> +	return {
>> +	    ticket => $ticket,
>> +	    upid => $upid,
>> +	    socket => $socket_addr,
>> +	};
>> +    }});
>> +
>> +__PACKAGE__->register_method({
>> +    name => 'mtunnelwebsocket',
>> +    path => '{vmid}/mtunnelwebsocket',
>> +    method => 'GET',
>> +    permissions => {
>> +	description => "You need to pass a ticket valid for the selected socket. Tickets can be created via the mtunnel API call, which will check permissions accordingly.",
>> +        user => 'all', # check inside
>> +    },
>> +    description => 'Migration tunnel endpoint for websocket upgrade - only for internal use by VM migration.',
>> +    parameters => {
>> +	additionalProperties => 0,
>> +	properties => {
>> +	    node => get_standard_option('pve-node'),
>> +	    vmid => get_standard_option('pve-vmid'),
>> +	    socket => {
>> +		type => "string",
>> +		description => "unix socket to forward to",
>> +	    },
>> +	    ticket => {
>> +		type => "string",
>> +		description => "ticket return by initial 'mtunnel' API call, or retrieved via 'ticket' tunnel command",
>> +	    },
>> +	},
>> +    },
>> +    returns => {
>> +	type => "object",
>> +	properties => {
>> +	    port => { type => 'string', optional => 1 },
>> +	    socket => { type => 'string', optional => 1 },
>> +	},
>> +    },
>> +    code => sub {
>> +	my ($param) = @_;
>> +
>> +	my $rpcenv = PVE::RPCEnvironment::get();
>> +	my $authuser = $rpcenv->get_user();
>> +
>> +	my $nodename = PVE::INotify::nodename();
>> +	my $node = extract_param($param, 'node');
>> +
>> +	raise_param_exc({ node => "node needs to be 'localhost' or local hostname '$nodename'" })
>> +	    if $node ne 'localhost' && $node ne $nodename;
>> +
>> +	my $vmid = $param->{vmid};
>> +	# check VM exists
>> +	PVE::QemuConfig->load_config($vmid);
>> +
>> +	my $socket = $param->{socket};
>> +	PVE::AccessControl::verify_tunnel_ticket($param->{ticket}, $authuser, "/socket/$socket");
>> +
>> +	return { socket => $socket };
>> +    }});
>> +
>>   1;
>> diff --git a/debian/control b/debian/control
>> index a90ecd6f..ce469cbd 100644
>> --- a/debian/control
>> +++ b/debian/control
>> @@ -33,7 +33,7 @@ Depends: dbus,
>>            libjson-perl,
>>            libjson-xs-perl,
>>            libnet-ssleay-perl,
>> -         libpve-access-control (>= 5.0-7),
>> +         libpve-access-control (>= 7.0-7),
>>            libpve-cluster-perl,
>>            libpve-common-perl (>= 7.1-4),
>>            libpve-guest-common-perl (>= 4.1-1),
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>