[pve-devel] [PATCH container] create: fix passing of template file descriptor

[Thomas Lamprecht]

On 3/20/18 9:32 AM, Wolfgang Bumiller wrote:
> This finishes the work started with 07084526aa4a ("create:
> open templates as real root"), which opened templates as
> real root, but passed it to tar via /proc/*/fd, which does
> not actually bypass the check. (Curiously tar did manage to
> figure out the file extension from it).
> 
> In order to actually extract templates the unprivileged user
> cannot access by themselves, we need to pass it to tar via
> stdin, however, this means tar cannot auto-detect the
> compression (or more accurately, it can and does, but tells
> you which option to pass it rather than just extracting
> it...)

cheeky GNU tar ...

> Signed-off-by: Wolfgang Bumiller <w.bumiller at proxmox.com>
> ---
>  src/PVE/LXC/Create.pm | 21 ++++++++++++++++-----
>  1 file changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/src/PVE/LXC/Create.pm b/src/PVE/LXC/Create.pm
> index 2a37d93..128ede9 100644
> --- a/src/PVE/LXC/Create.pm
> +++ b/src/PVE/LXC/Create.pm
> @@ -65,16 +65,28 @@ sub restore_archive {
>      my $userns_cmd = PVE::LXC::userns_command($id_map);
>  
>      my $archive_fh;
> -    my $tar_input_file = '-';
> +    my $tar_input = '<&STDIN';
> +    my @compression_opt;
>      if ($archive ne '-') {
> +	# GNU tar refuses to autodetect this... *sigh*
> +	if ($archive =~ /\.tar(\.[^.]+)?$/) {
> +	    if (defined($1)) {
> +		@compression_opt = $1 eq '.gz'  ? ('-z') :
> +		                   $1 eq '.bz2' ? ('-j') :
> +		                   $1 eq '.xz'  ? ('-J') :
> +		                   die "unrecognized compression format: $1\n";

A mapping (defined a bit above) would be maybe nicer to read? 

my $compressions = {
    '.gz' => '-z',
    ...
};

But at least you made it look symmetric :)

Looks OK:

Reviewed-by: Thomas Lamprecht <t.lamprecht at proxmox.com>

> +	    }
> +	} else {
> +	    die "file does not look like a template archive: $archive\n";
> +	}
>  	sysopen($archive_fh, $archive, O_RDONLY)
>  	    or die "failed to open '$archive': $!\n";
> -	$tar_input_file = '/proc/self/fd/'.fileno($archive_fh);
>  	my $flags = $archive_fh->fcntl(Fcntl::F_GETFD(), 0);
>  	$archive_fh->fcntl(Fcntl::F_SETFD(), $flags & ~(Fcntl::FD_CLOEXEC()));
> +	$tar_input = '<&'.fileno($archive_fh);
>      }
>  
> -    my $cmd = [@$userns_cmd, 'tar', 'xpf', $tar_input_file, '--totals',
> +    my $cmd = [@$userns_cmd, 'tar', 'xpf', '-', @compression_opt, '--totals',
>                 @PVE::Storage::Plugin::COMMON_TAR_FLAGS,
>                 '-C', $rootdir];
>  
> @@ -88,11 +100,10 @@ sub restore_archive {
>  
>      if ($archive eq '-') {
>  	print "extracting archive from STDIN\n";
> -	eval { PVE::Tools::run_command($cmd, input => "<&STDIN"); };
>      } else {
>  	print "extracting archive '$archive'\n";
> -	eval { PVE::Tools::run_command($cmd); };
>      }
> +    eval { PVE::Tools::run_command($cmd, input => $tar_input); };
>      my $err = $@;
>      close($archive_fh) if defined $archive_fh;
>      die $err if $err && !$no_unpack_error;
>