[pve-devel] [PATCH-SERIES v7 manager/common/storage] fix #1710: add downlaod from url button

[pve-devel] [PATCH-SERIES v7 manager/common/storage] fix #1710: add downlaod from url button

[Lorenz Stechauner]

changes to v6:
* Fixed regex for parsing Content-Disposition for filenames (query_url_metadata)
* Removed mime type check in frontend completely


pve-manager:
Lorenz Stechauner (5):
  api: nodes: add query_url_metadata method
  api: nodes: refactor aplinfo to use common download function
  ui: add HashAlgorithmSelector
  ui: Utils: change download task format
  fix #1710: ui: storage: add download from url button

 PVE/API2/Nodes.pm                          | 159 +++++++------
 www/manager6/Makefile                      |   1 +
 www/manager6/Utils.js                      |   2 +-
 www/manager6/form/HashAlgorithmSelector.js |  16 ++
 www/manager6/storage/Browser.js            |   8 +
 www/manager6/storage/ContentView.js        | 247 +++++++++++++++++++--
 6 files changed, 342 insertions(+), 91 deletions(-)
 create mode 100644 www/manager6/form/HashAlgorithmSelector.js


pve-common:
Lorenz Stechauner (1):
  tools: add download_file_from_url

 src/PVE/Tools.pm | 124 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 124 insertions(+)


pve-storage:
Lorenz Stechauner (1):
 status: add download_url method

 PVE/API2/Storage/Status.pm | 118 +++++++++++++++++++++++++++++++++++--
 PVE/Storage.pm             |  10 ++++
 2 files changed, 123 insertions(+), 5 deletions(-)

-- 
2.20.1

[pve-devel] [PATCH v7 common 1/1] tools: add download_file_from_url

[Lorenz Stechauner]

code is based on
manager:PVE/API2/Nodes.pm:aplinfo

Signed-off-by: Lorenz Stechauner <l.stechauner@proxmox.com>
---
 src/PVE/Tools.pm | 124 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 124 insertions(+)

diff --git a/src/PVE/Tools.pm b/src/PVE/Tools.pm
index 16ae3d2..7b82e00 100644
--- a/src/PVE/Tools.pm
+++ b/src/PVE/Tools.pm
@@ -1829,4 +1829,128 @@ sub safe_compare {
     return $cmp->($left, $right);
 }
 
+
+# opts
+#  -> hash_required
+#       if 1, at least one checksum has to be specified otherwise an error will be thrown
+#  -> http_proxy
+#  -> https_proxy
+#  -> verify_certificates
+#  -> sha(1|224|256|384|512)sum
+#  -> md5sum
+sub download_file_from_url {
+    my ($dest, $url, $opts) = @_;
+
+    my $tmpdest = "$dest.tmp.$$";
+
+    my $algorithm;
+    my $expected;
+
+    for ('sha512', 'sha384', 'sha256', 'sha224', 'sha1', 'md5') {
+	if (defined($opts->{"${_}sum"})) {
+	    $algorithm = $_;
+	    $expected = $opts->{"${_}sum"};
+	    last;
+	}
+    }
+
+    die "checksum required but not specified\n" if ($opts->{hash_required} && !$algorithm);
+
+    my $worker = sub  {
+	my $upid = shift;
+
+	print "downloading $url to $dest\n";
+
+	eval {
+	    if (-f $dest && $algorithm) {
+		print "calculating checksum of existing file...\n";
+		my $correct = check_file_hash($algorithm, $expected, $dest);
+
+		if ($correct) {
+		    print "file already exists, no need to download\n";
+		    return;
+		} else {
+		    print "mismatch, downloading\n";
+		}
+	    }
+
+	    my @cmd = ('/usr/bin/wget', '--progress=dot:mega', '-O', $tmpdest, $url);
+
+	    local %ENV;
+	    if ($opts->{http_proxy}) {
+		$ENV{http_proxy} = $opts->{http_proxy};
+	    }
+	    if ($opts->{https_proxy}) {
+		$ENV{https_proxy} = $opts->{https_proxy};
+	    }
+
+	    my $verify = $opts->{verify_certificates} // 1;
+	    if (!$verify) {
+		push @cmd, '--no-check-certificate';
+	    }
+
+	    if (run_command([[@cmd]]) != 0) {
+		die "download failed: $!\n";
+	    }
+
+	    if ($algorithm) {
+		print "calculating checksum...\n";
+
+		my $correct = check_file_hash($algorithm, $expected, $tmpdest);
+
+		if ($correct) {
+		    print "checksum verified\n";
+		} else {
+		    die "checksum mismatch\n";
+		}
+	    } else {
+		print "no checksum for verification specified\n";
+	    }
+
+	    if (!rename($tmpdest, $dest)) {
+		die "unable to save file: $!\n";
+	    }
+	};
+	my $err = $@;
+
+	unlink $tmpdest;
+
+	if ($err) {
+	    print "\n";
+	    die $err;
+	}
+
+	print "download finished\n";
+    };
+
+    my $rpcenv = PVE::RPCEnvironment::get();
+    my $user = $rpcenv->get_user();
+
+    (my $filename = $dest) =~ s!.*/([^/]*)$!$1!;
+
+    return $rpcenv->fork_worker('download', $filename, $user, $worker);
+}
+
+sub check_file_hash {
+    my ($algorithm, $expected, $filename) = @_;
+
+    my $algorithm_map = {
+	'md5' => sub { Digest::MD5->new },
+	'sha1' => sub { Digest::SHA->new(1) },
+	'sha224' => sub { Digest::SHA->new(224) },
+	'sha256' => sub { Digest::SHA->new(256) },
+	'sha384' => sub { Digest::SHA->new(384) },
+	'sha512' => sub { Digest::SHA->new(512) },
+    };
+
+    my $digester = $algorithm_map->{$algorithm}->() or die "unknown algorithm '$algorithm'\n";
+
+    open(my $fh, '<', $filename) or die "unable to open '$filename': $!\n";
+    binmode($fh);
+
+    my $digest = $digester->addfile($fh)->hexdigest;
+
+    return lc($digest) eq lc($expected);
+}
+
 1;
-- 
2.20.1

[pve-devel] [PATCH v7 storage 1/1] status: add download_url method

[Lorenz Stechauner]

Signed-off-by: Lorenz Stechauner <l.stechauner@proxmox.com>
---
 PVE/API2/Storage/Status.pm | 118 +++++++++++++++++++++++++++++++++++--
 PVE/Storage.pm             |  10 ++++
 2 files changed, 123 insertions(+), 5 deletions(-)

diff --git a/PVE/API2/Storage/Status.pm b/PVE/API2/Storage/Status.pm
index 897b4a7..4c1d1fc 100644
--- a/PVE/API2/Storage/Status.pm
+++ b/PVE/API2/Storage/Status.pm
@@ -412,11 +412,7 @@ __PACKAGE__->register_method ({
 	my $size = -s $tmpfilename;
 	die "temporary file '$tmpfilename' does not exist\n" if !defined($size);
 
-	my $filename = $param->{filename};
-
-	chomp $filename;
-	$filename =~ s/^.*[\/\\]//;
-	$filename =~ s/[^-a-zA-Z0-9_.]/_/g;
+	my $filename = PVE::Storage::normalize_content_filename($param->{filename});
 
 	my $path;
 
@@ -497,4 +493,116 @@ __PACKAGE__->register_method ({
 	return $upid;
    }});
 
+__PACKAGE__->register_method({
+    name => 'download_url',
+    path => '{storage}/download-url',
+    method => 'POST',
+    description => "Download templates and ISO images by using an URL.",
+    proxyto => 'node',
+    permissions => {
+	check => [ 'and',
+	    ['perm', '/storage/{storage}', [ 'Datastore.AllocateTemplate' ]],
+	    ['perm', '/', [ 'Sys.Audit', 'Sys.Modify' ]],
+	],
+    },
+    protected => 1,
+    parameters => {
+	additionalProperties => 0,
+	properties => {
+	    node => get_standard_option('pve-node'),
+	    storage => get_standard_option('pve-storage-id'),
+	    url => {
+		description => "The URL to download the file from.",
+		type => 'string',
+		pattern => 'https?://.*',
+	    },
+	    content => {
+		description => "Content type.",
+		type => 'string', format => 'pve-storage-content',
+	    },
+	    filename => {
+		description => "The name of the file to create.",
+		type => 'string',
+	    },
+	    checksum => {
+		description => "The expected checksum of the file.",
+		type => 'string',
+		requires => 'checksum-algorithm',
+		optional => 1,
+	    },
+	    'checksum-algorithm' => {
+		description => "The algorithm to calculate the checksum of the file.",
+		type => 'string',
+		enum => ['md5', 'sha1', 'sha224', 'sha256', 'sha384', 'sha512'],
+		requires => 'checksum',
+		optional => 1,
+	    },
+	    'verify-certificates' => {
+		description => "If false, no SSL/TLS certificates will be verified.",
+		type => 'boolean',
+		optional => 1,
+		default => 1,
+	    }
+	},
+    },
+    returns => {
+	type => "string"
+    },
+    code => sub {
+	my ($param) = @_;
+
+	my $cfg = PVE::Storage::config();
+
+	my ($node, $storage) = $param->@{'node', 'storage'};
+	my $scfg = PVE::Storage::storage_check_enabled($cfg, $storage, $node);
+
+	die "can't upload to storage type '$scfg->{type}, not a file based storage!'\n" if !defined($scfg->{path});
+
+	my ($content, $url) = $param->@{'content', 'url'};
+
+	my $filename = PVE::Storage::normalize_content_filename($param->{filename});
+	my $path;
+
+	# MIME type is checked in front end only
+	# this check is omitted here intentionally and replaced by file extension check
+	if ($content eq 'iso') {
+	    if ($filename !~ m![^/]+$PVE::Storage::iso_extension_re$!) {
+		raise_param_exc({ filename => "missing '.iso' or '.img' extension" });
+	    }
+	    $path = PVE::Storage::get_iso_dir($cfg, $storage);
+	} elsif ($content eq 'vztmpl') {
+	    if ($filename !~ m![^/]+\.tar\.[gx]z$!) {
+		raise_param_exc({ filename => "missing '.tar.gz' or '.tar.xz' extension" });
+	    }
+	    $path = PVE::Storage::get_vztmpl_dir($cfg, $storage);
+	} else {
+	    raise_param_exc({ content => "upload content type '$content' not allowed" });
+	}
+
+	die "storage '$storage' does not support '$content' content\n" if !$scfg->{content}->{$content};
+
+	PVE::Storage::activate_storage($cfg, $storage);
+	File::Path::make_path($path);
+
+	my $dest = "$path/$filename";
+
+	my $opts = {
+	    hash_required => 0,
+	    verify_certificates => $param->{'verify-certificates'} // 1,
+	};
+
+	my ($checksum, $checksum_algorithm) = $param->@{'checksum', 'checksum-algorithm'};
+	if ($checksum) {
+	    $opts->{"${checksum_algorithm}sum"} = $checksum;
+	    $opts->{hash_required} = 1;
+	}
+
+	my $dccfg = PVE::Cluster::cfs_read_file('datacenter.cfg');
+	if ($dccfg->{http_proxy}) {
+	    $opts->{http_proxy} = $dccfg->{http_proxy};
+	}
+
+	return PVE::Tools::download_file_from_url($dest, $url, $opts);
+    }});
+
 1;
diff --git a/PVE/Storage.pm b/PVE/Storage.pm
index aa36bad..f9a3d49 100755
--- a/PVE/Storage.pm
+++ b/PVE/Storage.pm
@@ -1929,4 +1929,14 @@ sub assert_sid_unused {
     return undef;
 }
 
+sub normalize_content_filename {
+    my ($filename) = @_;
+
+    chomp $filename;
+    $filename =~ s/^.*[\/\\]//;
+    $filename =~ s/[^-a-zA-Z0-9_.]/_/g;
+
+    return $filename;
+}
+
 1;
-- 
2.20.1

[pve-devel] [PATCH v7 manager 1/5] api: nodes: add query_url_metadata method

[Lorenz Stechauner]

Signed-off-by: Lorenz Stechauner <l.stechauner@proxmox.com>
---
 PVE/API2/Nodes.pm | 96 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 96 insertions(+)

diff --git a/PVE/API2/Nodes.pm b/PVE/API2/Nodes.pm
index e58d9c10..77fa710a 100644
--- a/PVE/API2/Nodes.pm
+++ b/PVE/API2/Nodes.pm
@@ -11,6 +11,7 @@ use JSON;
 use POSIX qw(LONG_MAX);
 use Time::Local qw(timegm_nocheck);
 use Socket;
+use IO::Socket::SSL;
 
 use PVE::API2Tools;
 use PVE::APLInfo;
@@ -238,6 +239,7 @@ __PACKAGE__->register_method ({
 	    { name => 'netstat' },
 	    { name => 'network' },
 	    { name => 'qemu' },
+	    { name => 'query-url-metadata' },
 	    { name => 'replication' },
 	    { name => 'report' },
 	    { name => 'rrd' }, # fixme: remove?
@@ -1595,6 +1597,100 @@ __PACKAGE__->register_method({
 	return $rpcenv->fork_worker('download', undef, $user, $worker);
     }});
 
+__PACKAGE__->register_method({
+    name => 'query_url_metadata',
+    path => 'query-url-metadata',
+    method => 'GET',
+    description => "Query metadata of an URL: file size, file name and mime type.",
+    proxyto => 'node',
+    permissions => {
+	check => ['perm', '/', [ 'Sys.Audit', 'Sys.Modify' ]],
+    },
+    parameters => {
+	additionalProperties => 0,
+	properties => {
+	    node => get_standard_option('pve-node'),
+	    url => {
+		description => "The URL to query the metadata from.",
+		type => 'string',
+		pattern => 'https?://.*',
+	    },
+	    'verify-certificates' => {
+		description => "If false, no SSL/TLS certificates will be verified.",
+		type => 'boolean',
+		optional => 1,
+		default => 1,
+	    }
+	},
+    },
+    returns => {
+	type => "object",
+	properties => {
+	    filename => {
+		type => 'string',
+		optional => 1,
+	    },
+	    size => {
+		type => 'integer',
+		renderer => 'bytes',
+		optional => 1,
+	    },
+	    mimetype => {
+		type => 'string',
+		optional => 1,
+	    },
+	},
+    },
+    code => sub {
+	my ($param) = @_;
+
+	my $url = $param->{url};
+
+	my $ua = LWP::UserAgent->new();
+
+	my $dccfg = PVE::Cluster::cfs_read_file('datacenter.cfg');
+	if ($dccfg->{http_proxy}) {
+	    $ua->proxy('http', $dccfg->{http_proxy});
+	}
+
+	my $verify = $param->{'verify-certificates'} // 1;
+	if (!$verify) {
+	    $ua->ssl_opts(
+		verify_hostname => 0,
+		SSL_verify_mode => IO::Socket::SSL::SSL_VERIFY_NONE,
+	    );
+	}
+
+	my $req = HTTP::Request->new(HEAD => $url);
+	my $res = $ua->request($req);
+
+	die "invalid server response: '" . $res->status_line() . "'\n" if ($res->code() != 200);
+
+	my $size = $res->header("Content-Length");
+	my $disposition = $res->header("Content-Disposition");
+	my $type = $res->header("Content-Type");
+
+	my $filename;
+
+	if ($disposition && ($disposition =~ m/filename="([^"]*)"/ || $disposition =~ m/filename=([^;]*)/)) {
+	    $filename = $1;
+	} elsif ($url =~ m!^[^?]+/([^?/]*)(?:\?.*)?$!) {
+	    $filename = $1;
+	}
+
+	# Content-Type: text/html; charset=utf-8
+	if ($type && $type =~ m/^([^;]+);/) {
+	    $type = $1;
+	}
+
+	my $ret = {};
+	$ret->{filename} = $filename if $filename;
+	$ret->{size} = $size + 0 if $size;
+	$ret->{mimetype} = $type if $type;
+
+	return $ret;
+    }});
+
 __PACKAGE__->register_method({
     name => 'report',
     path => 'report',
-- 
2.20.1

[pve-devel] [PATCH v7 manager 2/5] api: nodes: refactor aplinfo to use common download function

[Lorenz Stechauner]

Signed-off-by: Lorenz Stechauner <l.stechauner@proxmox.com>
---
 PVE/API2/Nodes.pm | 89 ++++++-----------------------------------------
 1 file changed, 10 insertions(+), 79 deletions(-)

diff --git a/PVE/API2/Nodes.pm b/PVE/API2/Nodes.pm
index 77fa710a..49abe93a 100644
--- a/PVE/API2/Nodes.pm
+++ b/PVE/API2/Nodes.pm
@@ -1513,88 +1513,19 @@ __PACKAGE__->register_method({
 	my $src = $pd->{location};
 	my $tmpldir = PVE::Storage::get_vztmpl_dir($cfg, $param->{storage});
 	my $dest = "$tmpldir/$template";
-	my $tmpdest = "$tmpldir/${template}.tmp.$$";
 
-	my $worker = sub  {
-	    my $upid = shift;
-
-	    print "starting template download from: $src\n";
-	    print "target file: $dest\n";
-
-	    my $check_hash = sub {
-		my ($template_info, $filename, $noerr) = @_;
-
-		my $digest;
-		my $expected;
-
-		eval {
-		    open(my $fh, '<', $filename) or die "Can't open '$filename': $!";
-		    binmode($fh);
-		    if (defined($template_info->{sha512sum})) {
-			$expected = $template_info->{sha512sum};
-			$digest = Digest::SHA->new(512)->addfile($fh)->hexdigest;
-		    } elsif (defined($template_info->{md5sum})) {
-			#fallback to MD5
-			$expected = $template_info->{md5sum};
-			$digest = Digest::MD5->new->addfile($fh)->hexdigest;
-		    } else {
-			die "no expected checksum defined";
-		    }
-		    close($fh);
-		};
-
-		die "checking hash failed - $@\n" if $@ && !$noerr;
-
-		return ($digest, $digest ? lc($digest) eq lc($expected) : 0);
-	    };
-
-	    eval {
-		if (-f $dest) {
-		    my ($hash, $correct) = &$check_hash($pd, $dest, 1);
-
-		    if ($hash && $correct) {
-			print "file already exists $hash - no need to download\n";
-			return;
-		    }
-		}
-
-		local %ENV;
-		my $dccfg = PVE::Cluster::cfs_read_file('datacenter.cfg');
-		if ($dccfg->{http_proxy}) {
-		    $ENV{http_proxy} = $dccfg->{http_proxy};
-		}
-
-		my @cmd = ('/usr/bin/wget', '--progress=dot:mega', '-O', $tmpdest, $src);
-		if (system (@cmd) != 0) {
-		    die "download failed - $!\n";
-		}
-
-		my ($hash, $correct) = &$check_hash($pd, $tmpdest);
-
-		die "could not calculate checksum\n" if !$hash;
-
-		if (!$correct) {
-		    my $expected = $pd->{sha512sum} // $pd->{md5sum};
-		    die "wrong checksum: $hash != $expected\n";
-		}
-
-		if (!rename($tmpdest, $dest)) {
-		    die "unable to save file - $!\n";
-		}
-	    };
-	    my $err = $@;
-
-	    unlink $tmpdest;
-
-	    if ($err) {
-		print "\n";
-		die $err if $err;
-	    }
-
-	    print "download finished\n";
+	my $opts = {
+	    hash_required => 1,
+	    sha512sum => $pd->{sha512sum},
+	    md5sum => $pd->{md5sum},
 	};
 
-	return $rpcenv->fork_worker('download', undef, $user, $worker);
+	my $dccfg = PVE::Cluster::cfs_read_file('datacenter.cfg');
+	if ($dccfg->{http_proxy}) {
+	    $opts->{http_proxy} = $dccfg->{http_proxy};
+	}
+
+	return PVE::Tools::download_file_from_url($dest, $src, $opts);
     }});
 
 __PACKAGE__->register_method({
-- 
2.20.1

[pve-devel] [PATCH v7 manager 3/5] ui: add HashAlgorithmSelector

[Lorenz Stechauner]

Signed-off-by: Lorenz Stechauner <l.stechauner@proxmox.com>
---
 www/manager6/Makefile                      |  1 +
 www/manager6/form/HashAlgorithmSelector.js | 16 ++++++++++++++++
 2 files changed, 17 insertions(+)
 create mode 100644 www/manager6/form/HashAlgorithmSelector.js

diff --git a/www/manager6/Makefile b/www/manager6/Makefile
index 506b5a4e..1aa62c9b 100644
--- a/www/manager6/Makefile
+++ b/www/manager6/Makefile
@@ -38,6 +38,7 @@ JSSRC= 							\
 	form/GlobalSearchField.js			\
 	form/GroupSelector.js				\
 	form/GuestIDSelector.js				\
+	form/HashAlgorithmSelector.js			\
 	form/HotplugFeatureSelector.js			\
 	form/IPProtocolSelector.js			\
 	form/IPRefSelector.js				\
diff --git a/www/manager6/form/HashAlgorithmSelector.js b/www/manager6/form/HashAlgorithmSelector.js
new file mode 100644
index 00000000..5ae7a08b
--- /dev/null
+++ b/www/manager6/form/HashAlgorithmSelector.js
@@ -0,0 +1,16 @@
+Ext.define('PVE.form.hashAlgorithmSelector', {
+    extend: 'Proxmox.form.KVComboBox',
+    alias: ['widget.pveHashAlgorithmSelector'],
+    config: {
+	deleteEmpty: false,
+    },
+    comboItems: [
+	['__default__', 'None'],
+	['md5', 'MD5'],
+	['sha1', 'SHA-1'],
+	['sha224', 'SHA-224'],
+	['sha256', 'SHA-256'],
+	['sha384', 'SHA-384'],
+	['sha512', 'SHA-512'],
+    ],
+});
-- 
2.20.1

[pve-devel] [PATCH v7 manager 4/5] ui: Utils: change download task format

[Lorenz Stechauner]

Signed-off-by: Lorenz Stechauner <l.stechauner@proxmox.com>
---
 www/manager6/Utils.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
index d9567979..9fef29bf 100644
--- a/www/manager6/Utils.js
+++ b/www/manager6/Utils.js
@@ -1776,7 +1776,7 @@ Ext.define('PVE.Utils', {
 	    clusterjoin: ['', gettext('Join Cluster')],
 	    dircreate: [gettext('Directory Storage'), gettext('Create')],
 	    dirremove: [gettext('Directory'), gettext('Remove')],
-	    download: ['', gettext('Download')],
+	    download: [gettext('File'), gettext('Download')],
 	    hamigrate: ['HA', gettext('Migrate')],
 	    hashutdown: ['HA', gettext('Shutdown')],
 	    hastart: ['HA', gettext('Start')],
-- 
2.20.1

[pve-devel] [PATCH v7 manager 5/5] fix #1710: ui: storage: add download from url button

[Lorenz Stechauner]

Signed-off-by: Lorenz Stechauner <l.stechauner@proxmox.com>
---
 www/manager6/storage/Browser.js     |   8 +
 www/manager6/storage/ContentView.js | 247 +++++++++++++++++++++++++---
 2 files changed, 231 insertions(+), 24 deletions(-)

diff --git a/www/manager6/storage/Browser.js b/www/manager6/storage/Browser.js
index 5fee94c7..fe5df3e2 100644
--- a/www/manager6/storage/Browser.js
+++ b/www/manager6/storage/Browser.js
@@ -53,6 +53,9 @@ Ext.define('PVE.storage.Browser', {
 	    let plugin = res.plugintype;
 	    let contents = res.content.split(',');
 
+	    let enableUpload = !!caps.storage['Datastore.AllocateTemplate'];
+	    let enableDownloadUrl = enableUpload && !!(caps.nodes['Sys.Audit'] && caps.nodes['Sys.Modify']);
+
 	    if (contents.includes('backup')) {
 		me.items.push({
 		    xtype: 'pveStorageBackupView',
@@ -91,6 +94,8 @@ Ext.define('PVE.storage.Browser', {
 		    itemId: 'contentIso',
 		    content: 'iso',
 		    pluginType: plugin,
+		    enableUploadButton: enableUpload,
+		    enableDownloadUrlButton: enableDownloadUrl,
 		    useUploadButton: true,
 		});
 	    }
@@ -101,6 +106,9 @@ Ext.define('PVE.storage.Browser', {
 		    iconCls: 'fa fa-file-o lxc',
 		    itemId: 'contentVztmpl',
 		    pluginType: plugin,
+		    enableUploadButton: enableUpload,
+		    enableDownloadUrlButton: enableDownloadUrl,
+		    useUploadButton: true,
 		});
 	    }
 	    if (contents.includes('snippets')) {
diff --git a/www/manager6/storage/ContentView.js b/www/manager6/storage/ContentView.js
index dd6df4b1..6171d30c 100644
--- a/www/manager6/storage/ContentView.js
+++ b/www/manager6/storage/ContentView.js
@@ -191,6 +191,191 @@ Ext.define('PVE.storage.Upload', {
     },
 });
 
+Ext.define('PVE.storage.DownloadUrl', {
+    extend: 'Proxmox.window.Edit',
+    alias: 'widget.pveStorageDownloadUrl',
+    mixins: ['Proxmox.Mixin.CBind'],
+
+    isCreate: true,
+
+    method: 'POST',
+
+    showTaskViewer: true,
+
+    title: gettext('Download from URL'),
+    submitText: gettext('Download'),
+
+    cbindData: function(initialConfig) {
+	var me = this;
+	return {
+	    nodename: me.nodename,
+	    storage: me.storage,
+	    contents: me.contents,
+	    content: me.contents[0],
+	};
+    },
+
+    cbind: {
+	url: '/nodes/{nodename}/storage/{storage}/download-url',
+    },
+
+    controller: {
+	xclass: 'Ext.app.ViewController',
+
+	urlChange: function(field) {
+	    let me = this;
+	    let view = me.getView();
+	    field = view.down('[name=url]');
+	    field.setValidation("Waiting for response...");
+	    field.validate();
+	    view.setValues({ size: "" });
+	    Proxmox.Utils.API2Request({
+		url: `/nodes/${view.nodename}/query-url-metadata`,
+		method: 'GET',
+		params: {
+		    url: field.getValue(),
+		    'verify-certificates': view.getValues()['verify-certificates'],
+		},
+		failure: function(res, opt) {
+		    field.setValidation(res.result.message);
+		    field.validate();
+		    view.setValues({
+			size: "",
+			mimetype: "",
+		    });
+		},
+		success: function(res, opt) {
+		    field.setValidation();
+		    field.validate();
+
+		    let data = res.result.data;
+		    view.setValues({
+			filename: data.filename || "",
+			size: (data.size && Proxmox.Utils.format_size(data.size)) || "",
+			mimetype: data.mimetype || "",
+		    });
+		},
+	    });
+	},
+
+	hashChange: function(field) {
+	    let checksum = Ext.getCmp('downloadUrlChecksum');
+	    if (field.getValue() === '__default__') {
+		checksum.setDisabled(true);
+		checksum.setValue("");
+		checksum.allowBlank = true;
+	    } else {
+		checksum.setDisabled(false);
+		checksum.allowBlank = false;
+	    }
+	},
+    },
+
+    items: [
+	{
+	    xtype: 'inputpanel',
+	    waitMsgTarget: true,
+	    border: false,
+	    columnT: [
+		{
+		    xtype: 'textfield',
+		    name: 'url',
+		    allowBlank: false,
+		    fieldLabel: gettext('URL'),
+		    listeners: {
+			change: {
+			    buffer: 500,
+			    fn: 'urlChange',
+			},
+		    },
+		},
+		{
+		    xtype: 'textfield',
+		    name: 'filename',
+		    allowBlank: false,
+		    fieldLabel: gettext('File name'),
+		},
+	    ],
+	    column1: [
+		{
+		    xtype: 'pveContentTypeSelector',
+		    fieldLabel: gettext('Content'),
+		    name: 'content',
+		    allowBlank: false,
+		    cbind: {
+			cts: '{contents}',
+			value: '{content}',
+		    },
+		},
+	    ],
+	    column2: [
+		{
+		    xtype: 'textfield',
+		    name: 'size',
+		    disabled: true,
+		    fieldLabel: gettext('File size'),
+		    emptyText: gettext('unknown'),
+		},
+	    ],
+	    advancedColumn1: [
+		{
+		    xtype: 'pveHashAlgorithmSelector',
+		    name: 'checksum-algorithm',
+		    fieldLabel: gettext('Hash algorithm'),
+		    allowBlank: true,
+		    hasNoneOption: true,
+		    value: '__default__',
+		    listeners: {
+			change: 'hashChange',
+		    },
+		},
+		{
+		    xtype: 'textfield',
+		    name: 'checksum',
+		    fieldLabel: gettext('Checksum'),
+		    allowBlank: true,
+		    disabled: true,
+		    emptyText: gettext('none'),
+		    id: 'downloadUrlChecksum',
+		},
+	    ],
+	    advancedColumn2: [
+		{
+		    xtype: 'textfield',
+		    fieldLabel: gettext('MIME type'),
+		    name: 'mimetype',
+		    disabled: true,
+		    editable: false,
+		    emptyText: gettext('unknown'),
+		},
+		{
+		    xtype: 'proxmoxcheckbox',
+		    name: 'verify-certificates',
+		    fieldLabel: gettext('Verify certificates'),
+		    uncheckedValue: 0,
+		    checked: true,
+		    listeners: {
+			change: 'urlChange',
+		    },
+		},
+	    ],
+	},
+    ],
+
+    initComponent: function() {
+        var me = this;
+
+	if (!me.nodename) {
+	    throw "no node name specified";
+	}
+	if (!me.storage) {
+	    throw "no storage ID specified";
+	}
+
+        me.callParent();
+    },
+});
+
 Ext.define('PVE.storage.ContentView', {
     extend: 'Ext.grid.GridPanel',
 
@@ -249,36 +434,50 @@ Ext.define('PVE.storage.ContentView', {
 
 	Proxmox.Utils.monStoreErrors(me, store);
 
-	let uploadButton = Ext.create('Proxmox.button.Button', {
-	    text: gettext('Upload'),
-	    handler: function() {
-		let win = Ext.create('PVE.storage.Upload', {
-		    nodename: nodename,
-		    storage: storage,
-		    contents: [content],
-		});
-		win.show();
-		win.on('destroy', reload);
-	    },
-	});
-
-	let removeButton = Ext.create('Proxmox.button.StdRemoveButton', {
-	    selModel: sm,
-	    delay: 5,
-	    callback: function() {
-		reload();
-	    },
-	    baseurl: baseurl + '/',
-	});
-
 	if (!me.tbar) {
 	    me.tbar = [];
 	}
 	if (me.useUploadButton) {
-	    me.tbar.push(uploadButton);
+	    me.tbar.unshift(
+		{
+		    xtype: 'button',
+		    text: gettext('Upload'),
+		    disabled: !me.enableUploadButton,
+		    handler: function() {
+			Ext.create('PVE.storage.Upload', {
+			    nodename: nodename,
+			    storage: storage,
+			    contents: [content],
+			    autoShow: true,
+			    taskDone: () => reload(),
+			});
+		    },
+		},
+		{
+		    xtype: 'button',
+		    text: gettext('Download from URL'),
+		    disabled: !me.enableDownloadUrlButton,
+		    handler: function() {
+			Ext.create('PVE.storage.DownloadUrl', {
+			    nodename: nodename,
+			    storage: storage,
+			    contents: [content],
+			    autoShow: true,
+			    taskDone: () => reload(),
+			});
+		    },
+		},
+		'-',
+	    );
 	}
 	if (!me.useCustomRemoveButton) {
-	    me.tbar.push(removeButton);
+	    me.tbar.push({
+		xtype: 'proxmoxStdRemoveButton',
+		selModel: sm,
+		delay: 5,
+		callback: () => reload(),
+		baseurl: baseurl + '/',
+	    });
 	}
 	me.tbar.push(
 	    '->',
-- 
2.20.1

Re: [pve-devel] [PATCH v7 manager 2/5] api: nodes: refactor aplinfo to use common download function

[Lorenz Stechauner]

a common function to download arbitrary files from urls has been defined 
as PVE::Tools::download_file_from_url and is now used.

On 14.06.21 11:05, Lorenz Stechauner wrote:
> Signed-off-by: Lorenz Stechauner <l.stechauner@proxmox.com>
> ---
>   PVE/API2/Nodes.pm | 89 ++++++-----------------------------------------
>   1 file changed, 10 insertions(+), 79 deletions(-)
>
> diff --git a/PVE/API2/Nodes.pm b/PVE/API2/Nodes.pm
> index 77fa710a..49abe93a 100644
> --- a/PVE/API2/Nodes.pm
> +++ b/PVE/API2/Nodes.pm
> @@ -1513,88 +1513,19 @@ __PACKAGE__->register_method({
>   	my $src = $pd->{location};
>   	my $tmpldir = PVE::Storage::get_vztmpl_dir($cfg, $param->{storage});
>   	my $dest = "$tmpldir/$template";
> -	my $tmpdest = "$tmpldir/${template}.tmp.$$";
>   
> -	my $worker = sub  {
> -	    my $upid = shift;
> -
> -	    print "starting template download from: $src\n";
> -	    print "target file: $dest\n";
> -
> -	    my $check_hash = sub {
> -		my ($template_info, $filename, $noerr) = @_;
> -
> -		my $digest;
> -		my $expected;
> -
> -		eval {
> -		    open(my $fh, '<', $filename) or die "Can't open '$filename': $!";
> -		    binmode($fh);
> -		    if (defined($template_info->{sha512sum})) {
> -			$expected = $template_info->{sha512sum};
> -			$digest = Digest::SHA->new(512)->addfile($fh)->hexdigest;
> -		    } elsif (defined($template_info->{md5sum})) {
> -			#fallback to MD5
> -			$expected = $template_info->{md5sum};
> -			$digest = Digest::MD5->new->addfile($fh)->hexdigest;
> -		    } else {
> -			die "no expected checksum defined";
> -		    }
> -		    close($fh);
> -		};
> -
> -		die "checking hash failed - $@\n" if $@ && !$noerr;
> -
> -		return ($digest, $digest ? lc($digest) eq lc($expected) : 0);
> -	    };
> -
> -	    eval {
> -		if (-f $dest) {
> -		    my ($hash, $correct) = &$check_hash($pd, $dest, 1);
> -
> -		    if ($hash && $correct) {
> -			print "file already exists $hash - no need to download\n";
> -			return;
> -		    }
> -		}
> -
> -		local %ENV;
> -		my $dccfg = PVE::Cluster::cfs_read_file('datacenter.cfg');
> -		if ($dccfg->{http_proxy}) {
> -		    $ENV{http_proxy} = $dccfg->{http_proxy};
> -		}
> -
> -		my @cmd = ('/usr/bin/wget', '--progress=dot:mega', '-O', $tmpdest, $src);
> -		if (system (@cmd) != 0) {
> -		    die "download failed - $!\n";
> -		}
> -
> -		my ($hash, $correct) = &$check_hash($pd, $tmpdest);
> -
> -		die "could not calculate checksum\n" if !$hash;
> -
> -		if (!$correct) {
> -		    my $expected = $pd->{sha512sum} // $pd->{md5sum};
> -		    die "wrong checksum: $hash != $expected\n";
> -		}
> -
> -		if (!rename($tmpdest, $dest)) {
> -		    die "unable to save file - $!\n";
> -		}
> -	    };
> -	    my $err = $@;
> -
> -	    unlink $tmpdest;
> -
> -	    if ($err) {
> -		print "\n";
> -		die $err if $err;
> -	    }
> -
> -	    print "download finished\n";
> +	my $opts = {
> +	    hash_required => 1,
> +	    sha512sum => $pd->{sha512sum},
> +	    md5sum => $pd->{md5sum},
>   	};
>   
> -	return $rpcenv->fork_worker('download', undef, $user, $worker);
> +	my $dccfg = PVE::Cluster::cfs_read_file('datacenter.cfg');
> +	if ($dccfg->{http_proxy}) {
> +	    $opts->{http_proxy} = $dccfg->{http_proxy};
> +	}
> +
> +	return PVE::Tools::download_file_from_url($dest, $src, $opts);
>       }});
>   
>   __PACKAGE__->register_method({

Re: [pve-devel] [PATCH v7 common 1/1] tools: add download_file_from_url

[Lorenz Stechauner]

adds a common function to download arbitrary files from urls.

security notice: this function does not perform any permission checking.
the calling function and/or api endpoint has to make sure, that only 
authorized users may use this function.

caution: This function is able to download files from internal networks 
(which would not be visible/accessible from outside).

On 14.06.21 11:05, Lorenz Stechauner wrote:
> code is based on
> manager:PVE/API2/Nodes.pm:aplinfo
>
> Signed-off-by: Lorenz Stechauner <l.stechauner@proxmox.com>
> ---
>   src/PVE/Tools.pm | 124 +++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 124 insertions(+)
>
> diff --git a/src/PVE/Tools.pm b/src/PVE/Tools.pm
> index 16ae3d2..7b82e00 100644
> --- a/src/PVE/Tools.pm
> +++ b/src/PVE/Tools.pm
> @@ -1829,4 +1829,128 @@ sub safe_compare {
>       return $cmp->($left, $right);
>   }
>   
> +
> +# opts
> +#  -> hash_required
> +#       if 1, at least one checksum has to be specified otherwise an error will be thrown
> +#  -> http_proxy
> +#  -> https_proxy
> +#  -> verify_certificates
> +#  -> sha(1|224|256|384|512)sum
> +#  -> md5sum
> +sub download_file_from_url {
> +    my ($dest, $url, $opts) = @_;
> +
> +    my $tmpdest = "$dest.tmp.$$";
> +
> +    my $algorithm;
> +    my $expected;
> +
> +    for ('sha512', 'sha384', 'sha256', 'sha224', 'sha1', 'md5') {
> +	if (defined($opts->{"${_}sum"})) {
> +	    $algorithm = $_;
> +	    $expected = $opts->{"${_}sum"};
> +	    last;
> +	}
> +    }
> +
> +    die "checksum required but not specified\n" if ($opts->{hash_required} && !$algorithm);
> +
> +    my $worker = sub  {
> +	my $upid = shift;
> +
> +	print "downloading $url to $dest\n";
> +
> +	eval {
> +	    if (-f $dest && $algorithm) {
> +		print "calculating checksum of existing file...\n";
> +		my $correct = check_file_hash($algorithm, $expected, $dest);
> +
> +		if ($correct) {
> +		    print "file already exists, no need to download\n";
> +		    return;
> +		} else {
> +		    print "mismatch, downloading\n";
> +		}
> +	    }
> +
> +	    my @cmd = ('/usr/bin/wget', '--progress=dot:mega', '-O', $tmpdest, $url);
> +
> +	    local %ENV;
> +	    if ($opts->{http_proxy}) {
> +		$ENV{http_proxy} = $opts->{http_proxy};
> +	    }
> +	    if ($opts->{https_proxy}) {
> +		$ENV{https_proxy} = $opts->{https_proxy};
> +	    }
> +
> +	    my $verify = $opts->{verify_certificates} // 1;
> +	    if (!$verify) {
> +		push @cmd, '--no-check-certificate';
> +	    }
> +
> +	    if (run_command([[@cmd]]) != 0) {
> +		die "download failed: $!\n";
> +	    }
> +
> +	    if ($algorithm) {
> +		print "calculating checksum...\n";
> +
> +		my $correct = check_file_hash($algorithm, $expected, $tmpdest);
> +
> +		if ($correct) {
> +		    print "checksum verified\n";
> +		} else {
> +		    die "checksum mismatch\n";
> +		}
> +	    } else {
> +		print "no checksum for verification specified\n";
> +	    }
> +
> +	    if (!rename($tmpdest, $dest)) {
> +		die "unable to save file: $!\n";
> +	    }
> +	};
> +	my $err = $@;
> +
> +	unlink $tmpdest;
> +
> +	if ($err) {
> +	    print "\n";
> +	    die $err;
> +	}
> +
> +	print "download finished\n";
> +    };
> +
> +    my $rpcenv = PVE::RPCEnvironment::get();
> +    my $user = $rpcenv->get_user();
> +
> +    (my $filename = $dest) =~ s!.*/([^/]*)$!$1!;
> +
> +    return $rpcenv->fork_worker('download', $filename, $user, $worker);
> +}
> +
> +sub check_file_hash {
> +    my ($algorithm, $expected, $filename) = @_;
> +
> +    my $algorithm_map = {
> +	'md5' => sub { Digest::MD5->new },
> +	'sha1' => sub { Digest::SHA->new(1) },
> +	'sha224' => sub { Digest::SHA->new(224) },
> +	'sha256' => sub { Digest::SHA->new(256) },
> +	'sha384' => sub { Digest::SHA->new(384) },
> +	'sha512' => sub { Digest::SHA->new(512) },
> +    };
> +
> +    my $digester = $algorithm_map->{$algorithm}->() or die "unknown algorithm '$algorithm'\n";
> +
> +    open(my $fh, '<', $filename) or die "unable to open '$filename': $!\n";
> +    binmode($fh);
> +
> +    my $digest = $digester->addfile($fh)->hexdigest;
> +
> +    return lc($digest) eq lc($expected);
> +}
> +
>   1;

Re: [pve-devel] [PATCH v7 manager 5/5] fix #1710: ui: storage: add download from url button

[Lorenz Stechauner]

uses the common function PVE::Tools::download_file_from_url to download 
a iso image or container template.

note: Only users with permissions `Sys.Audit` and `Sys.Modify` on `/` 
are permitted to use the api endpoints due to security reasons. (it is 
possible to download files from internal networks which would be not 
visible/accessible from outside)

On 14.06.21 11:05, Lorenz Stechauner wrote:
> Signed-off-by: Lorenz Stechauner <l.stechauner@proxmox.com>
> ---
>   www/manager6/storage/Browser.js     |   8 +
>   www/manager6/storage/ContentView.js | 247 +++++++++++++++++++++++++---
>   2 files changed, 231 insertions(+), 24 deletions(-)
>
> diff --git a/www/manager6/storage/Browser.js b/www/manager6/storage/Browser.js
> index 5fee94c7..fe5df3e2 100644
> --- a/www/manager6/storage/Browser.js
> +++ b/www/manager6/storage/Browser.js
> @@ -53,6 +53,9 @@ Ext.define('PVE.storage.Browser', {
>   	    let plugin = res.plugintype;
>   	    let contents = res.content.split(',');
>   
> +	    let enableUpload = !!caps.storage['Datastore.AllocateTemplate'];
> +	    let enableDownloadUrl = enableUpload && !!(caps.nodes['Sys.Audit'] && caps.nodes['Sys.Modify']);
> +
>   	    if (contents.includes('backup')) {
>   		me.items.push({
>   		    xtype: 'pveStorageBackupView',
> @@ -91,6 +94,8 @@ Ext.define('PVE.storage.Browser', {
>   		    itemId: 'contentIso',
>   		    content: 'iso',
>   		    pluginType: plugin,
> +		    enableUploadButton: enableUpload,
> +		    enableDownloadUrlButton: enableDownloadUrl,
>   		    useUploadButton: true,
>   		});
>   	    }
> @@ -101,6 +106,9 @@ Ext.define('PVE.storage.Browser', {
>   		    iconCls: 'fa fa-file-o lxc',
>   		    itemId: 'contentVztmpl',
>   		    pluginType: plugin,
> +		    enableUploadButton: enableUpload,
> +		    enableDownloadUrlButton: enableDownloadUrl,
> +		    useUploadButton: true,
>   		});
>   	    }
>   	    if (contents.includes('snippets')) {
> diff --git a/www/manager6/storage/ContentView.js b/www/manager6/storage/ContentView.js
> index dd6df4b1..6171d30c 100644
> --- a/www/manager6/storage/ContentView.js
> +++ b/www/manager6/storage/ContentView.js
> @@ -191,6 +191,191 @@ Ext.define('PVE.storage.Upload', {
>       },
>   });
>   
> +Ext.define('PVE.storage.DownloadUrl', {
> +    extend: 'Proxmox.window.Edit',
> +    alias: 'widget.pveStorageDownloadUrl',
> +    mixins: ['Proxmox.Mixin.CBind'],
> +
> +    isCreate: true,
> +
> +    method: 'POST',
> +
> +    showTaskViewer: true,
> +
> +    title: gettext('Download from URL'),
> +    submitText: gettext('Download'),
> +
> +    cbindData: function(initialConfig) {
> +	var me = this;
> +	return {
> +	    nodename: me.nodename,
> +	    storage: me.storage,
> +	    contents: me.contents,
> +	    content: me.contents[0],
> +	};
> +    },
> +
> +    cbind: {
> +	url: '/nodes/{nodename}/storage/{storage}/download-url',
> +    },
> +
> +    controller: {
> +	xclass: 'Ext.app.ViewController',
> +
> +	urlChange: function(field) {
> +	    let me = this;
> +	    let view = me.getView();
> +	    field = view.down('[name=url]');
> +	    field.setValidation("Waiting for response...");
> +	    field.validate();
> +	    view.setValues({ size: "" });
> +	    Proxmox.Utils.API2Request({
> +		url: `/nodes/${view.nodename}/query-url-metadata`,
> +		method: 'GET',
> +		params: {
> +		    url: field.getValue(),
> +		    'verify-certificates': view.getValues()['verify-certificates'],
> +		},
> +		failure: function(res, opt) {
> +		    field.setValidation(res.result.message);
> +		    field.validate();
> +		    view.setValues({
> +			size: "",
> +			mimetype: "",
> +		    });
> +		},
> +		success: function(res, opt) {
> +		    field.setValidation();
> +		    field.validate();
> +
> +		    let data = res.result.data;
> +		    view.setValues({
> +			filename: data.filename || "",
> +			size: (data.size && Proxmox.Utils.format_size(data.size)) || "",
> +			mimetype: data.mimetype || "",
> +		    });
> +		},
> +	    });
> +	},
> +
> +	hashChange: function(field) {
> +	    let checksum = Ext.getCmp('downloadUrlChecksum');
> +	    if (field.getValue() === '__default__') {
> +		checksum.setDisabled(true);
> +		checksum.setValue("");
> +		checksum.allowBlank = true;
> +	    } else {
> +		checksum.setDisabled(false);
> +		checksum.allowBlank = false;
> +	    }
> +	},
> +    },
> +
> +    items: [
> +	{
> +	    xtype: 'inputpanel',
> +	    waitMsgTarget: true,
> +	    border: false,
> +	    columnT: [
> +		{
> +		    xtype: 'textfield',
> +		    name: 'url',
> +		    allowBlank: false,
> +		    fieldLabel: gettext('URL'),
> +		    listeners: {
> +			change: {
> +			    buffer: 500,
> +			    fn: 'urlChange',
> +			},
> +		    },
> +		},
> +		{
> +		    xtype: 'textfield',
> +		    name: 'filename',
> +		    allowBlank: false,
> +		    fieldLabel: gettext('File name'),
> +		},
> +	    ],
> +	    column1: [
> +		{
> +		    xtype: 'pveContentTypeSelector',
> +		    fieldLabel: gettext('Content'),
> +		    name: 'content',
> +		    allowBlank: false,
> +		    cbind: {
> +			cts: '{contents}',
> +			value: '{content}',
> +		    },
> +		},
> +	    ],
> +	    column2: [
> +		{
> +		    xtype: 'textfield',
> +		    name: 'size',
> +		    disabled: true,
> +		    fieldLabel: gettext('File size'),
> +		    emptyText: gettext('unknown'),
> +		},
> +	    ],
> +	    advancedColumn1: [
> +		{
> +		    xtype: 'pveHashAlgorithmSelector',
> +		    name: 'checksum-algorithm',
> +		    fieldLabel: gettext('Hash algorithm'),
> +		    allowBlank: true,
> +		    hasNoneOption: true,
> +		    value: '__default__',
> +		    listeners: {
> +			change: 'hashChange',
> +		    },
> +		},
> +		{
> +		    xtype: 'textfield',
> +		    name: 'checksum',
> +		    fieldLabel: gettext('Checksum'),
> +		    allowBlank: true,
> +		    disabled: true,
> +		    emptyText: gettext('none'),
> +		    id: 'downloadUrlChecksum',
> +		},
> +	    ],
> +	    advancedColumn2: [
> +		{
> +		    xtype: 'textfield',
> +		    fieldLabel: gettext('MIME type'),
> +		    name: 'mimetype',
> +		    disabled: true,
> +		    editable: false,
> +		    emptyText: gettext('unknown'),
> +		},
> +		{
> +		    xtype: 'proxmoxcheckbox',
> +		    name: 'verify-certificates',
> +		    fieldLabel: gettext('Verify certificates'),
> +		    uncheckedValue: 0,
> +		    checked: true,
> +		    listeners: {
> +			change: 'urlChange',
> +		    },
> +		},
> +	    ],
> +	},
> +    ],
> +
> +    initComponent: function() {
> +        var me = this;
> +
> +	if (!me.nodename) {
> +	    throw "no node name specified";
> +	}
> +	if (!me.storage) {
> +	    throw "no storage ID specified";
> +	}
> +
> +        me.callParent();
> +    },
> +});
> +
>   Ext.define('PVE.storage.ContentView', {
>       extend: 'Ext.grid.GridPanel',
>   
> @@ -249,36 +434,50 @@ Ext.define('PVE.storage.ContentView', {
>   
>   	Proxmox.Utils.monStoreErrors(me, store);
>   
> -	let uploadButton = Ext.create('Proxmox.button.Button', {
> -	    text: gettext('Upload'),
> -	    handler: function() {
> -		let win = Ext.create('PVE.storage.Upload', {
> -		    nodename: nodename,
> -		    storage: storage,
> -		    contents: [content],
> -		});
> -		win.show();
> -		win.on('destroy', reload);
> -	    },
> -	});
> -
> -	let removeButton = Ext.create('Proxmox.button.StdRemoveButton', {
> -	    selModel: sm,
> -	    delay: 5,
> -	    callback: function() {
> -		reload();
> -	    },
> -	    baseurl: baseurl + '/',
> -	});
> -
>   	if (!me.tbar) {
>   	    me.tbar = [];
>   	}
>   	if (me.useUploadButton) {
> -	    me.tbar.push(uploadButton);
> +	    me.tbar.unshift(
> +		{
> +		    xtype: 'button',
> +		    text: gettext('Upload'),
> +		    disabled: !me.enableUploadButton,
> +		    handler: function() {
> +			Ext.create('PVE.storage.Upload', {
> +			    nodename: nodename,
> +			    storage: storage,
> +			    contents: [content],
> +			    autoShow: true,
> +			    taskDone: () => reload(),
> +			});
> +		    },
> +		},
> +		{
> +		    xtype: 'button',
> +		    text: gettext('Download from URL'),
> +		    disabled: !me.enableDownloadUrlButton,
> +		    handler: function() {
> +			Ext.create('PVE.storage.DownloadUrl', {
> +			    nodename: nodename,
> +			    storage: storage,
> +			    contents: [content],
> +			    autoShow: true,
> +			    taskDone: () => reload(),
> +			});
> +		    },
> +		},
> +		'-',
> +	    );
>   	}
>   	if (!me.useCustomRemoveButton) {
> -	    me.tbar.push(removeButton);
> +	    me.tbar.push({
> +		xtype: 'proxmoxStdRemoveButton',
> +		selModel: sm,
> +		delay: 5,
> +		callback: () => reload(),
> +		baseurl: baseurl + '/',
> +	    });
>   	}
>   	me.tbar.push(
>   	    '->',

Re: [pve-devel] [PATCH v7 storage 1/1] status: add download_url method

[Lorenz Stechauner]

uses common function PVE::Tools::download_file_from_url to download iso 
files.

Only users with permissions `Sys.Audit` and `Sys.Modify` on `/` are 
permitted to perform this action. This restriction is due to the fact, 
that the download function is able to download files from internal 
networks (which are not visible/accessible from outside). Users with 
these permissions anyway have the means to alter node (network) config, 
so this does not create any further security risk.

On 14.06.21 11:05, Lorenz Stechauner wrote:
> Signed-off-by: Lorenz Stechauner <l.stechauner@proxmox.com>
> ---
>   PVE/API2/Storage/Status.pm | 118 +++++++++++++++++++++++++++++++++++--
>   PVE/Storage.pm             |  10 ++++
>   2 files changed, 123 insertions(+), 5 deletions(-)
>
> diff --git a/PVE/API2/Storage/Status.pm b/PVE/API2/Storage/Status.pm
> index 897b4a7..4c1d1fc 100644
> --- a/PVE/API2/Storage/Status.pm
> +++ b/PVE/API2/Storage/Status.pm
> @@ -412,11 +412,7 @@ __PACKAGE__->register_method ({
>   	my $size = -s $tmpfilename;
>   	die "temporary file '$tmpfilename' does not exist\n" if !defined($size);
>   
> -	my $filename = $param->{filename};
> -
> -	chomp $filename;
> -	$filename =~ s/^.*[\/\\]//;
> -	$filename =~ s/[^-a-zA-Z0-9_.]/_/g;
> +	my $filename = PVE::Storage::normalize_content_filename($param->{filename});
>   
>   	my $path;
>   
> @@ -497,4 +493,116 @@ __PACKAGE__->register_method ({
>   	return $upid;
>      }});
>   
> +__PACKAGE__->register_method({
> +    name => 'download_url',
> +    path => '{storage}/download-url',
> +    method => 'POST',
> +    description => "Download templates and ISO images by using an URL.",
> +    proxyto => 'node',
> +    permissions => {
> +	check => [ 'and',
> +	    ['perm', '/storage/{storage}', [ 'Datastore.AllocateTemplate' ]],
> +	    ['perm', '/', [ 'Sys.Audit', 'Sys.Modify' ]],
> +	],
> +    },
> +    protected => 1,
> +    parameters => {
> +	additionalProperties => 0,
> +	properties => {
> +	    node => get_standard_option('pve-node'),
> +	    storage => get_standard_option('pve-storage-id'),
> +	    url => {
> +		description => "The URL to download the file from.",
> +		type => 'string',
> +		pattern => 'https?://.*',
> +	    },
> +	    content => {
> +		description => "Content type.",
> +		type => 'string', format => 'pve-storage-content',
> +	    },
> +	    filename => {
> +		description => "The name of the file to create.",
> +		type => 'string',
> +	    },
> +	    checksum => {
> +		description => "The expected checksum of the file.",
> +		type => 'string',
> +		requires => 'checksum-algorithm',
> +		optional => 1,
> +	    },
> +	    'checksum-algorithm' => {
> +		description => "The algorithm to calculate the checksum of the file.",
> +		type => 'string',
> +		enum => ['md5', 'sha1', 'sha224', 'sha256', 'sha384', 'sha512'],
> +		requires => 'checksum',
> +		optional => 1,
> +	    },
> +	    'verify-certificates' => {
> +		description => "If false, no SSL/TLS certificates will be verified.",
> +		type => 'boolean',
> +		optional => 1,
> +		default => 1,
> +	    }
> +	},
> +    },
> +    returns => {
> +	type => "string"
> +    },
> +    code => sub {
> +	my ($param) = @_;
> +
> +	my $cfg = PVE::Storage::config();
> +
> +	my ($node, $storage) = $param->@{'node', 'storage'};
> +	my $scfg = PVE::Storage::storage_check_enabled($cfg, $storage, $node);
> +
> +	die "can't upload to storage type '$scfg->{type}, not a file based storage!'\n" if !defined($scfg->{path});
> +
> +	my ($content, $url) = $param->@{'content', 'url'};
> +
> +	my $filename = PVE::Storage::normalize_content_filename($param->{filename});
> +	my $path;
> +
> +	# MIME type is checked in front end only
> +	# this check is omitted here intentionally and replaced by file extension check
> +	if ($content eq 'iso') {
> +	    if ($filename !~ m![^/]+$PVE::Storage::iso_extension_re$!) {
> +		raise_param_exc({ filename => "missing '.iso' or '.img' extension" });
> +	    }
> +	    $path = PVE::Storage::get_iso_dir($cfg, $storage);
> +	} elsif ($content eq 'vztmpl') {
> +	    if ($filename !~ m![^/]+\.tar\.[gx]z$!) {
> +		raise_param_exc({ filename => "missing '.tar.gz' or '.tar.xz' extension" });
> +	    }
> +	    $path = PVE::Storage::get_vztmpl_dir($cfg, $storage);
> +	} else {
> +	    raise_param_exc({ content => "upload content type '$content' not allowed" });
> +	}
> +
> +	die "storage '$storage' does not support '$content' content\n" if !$scfg->{content}->{$content};
> +
> +	PVE::Storage::activate_storage($cfg, $storage);
> +	File::Path::make_path($path);
> +
> +	my $dest = "$path/$filename";
> +
> +	my $opts = {
> +	    hash_required => 0,
> +	    verify_certificates => $param->{'verify-certificates'} // 1,
> +	};
> +
> +	my ($checksum, $checksum_algorithm) = $param->@{'checksum', 'checksum-algorithm'};
> +	if ($checksum) {
> +	    $opts->{"${checksum_algorithm}sum"} = $checksum;
> +	    $opts->{hash_required} = 1;
> +	}
> +
> +	my $dccfg = PVE::Cluster::cfs_read_file('datacenter.cfg');
> +	if ($dccfg->{http_proxy}) {
> +	    $opts->{http_proxy} = $dccfg->{http_proxy};
> +	}
> +
> +	return PVE::Tools::download_file_from_url($dest, $url, $opts);
> +    }});
> +
>   1;
> diff --git a/PVE/Storage.pm b/PVE/Storage.pm
> index aa36bad..f9a3d49 100755
> --- a/PVE/Storage.pm
> +++ b/PVE/Storage.pm
> @@ -1929,4 +1929,14 @@ sub assert_sid_unused {
>       return undef;
>   }
>   
> +sub normalize_content_filename {
> +    my ($filename) = @_;
> +
> +    chomp $filename;
> +    $filename =~ s/^.*[\/\\]//;
> +    $filename =~ s/[^-a-zA-Z0-9_.]/_/g;
> +
> +    return $filename;
> +}
> +
>   1;

Re: [pve-devel] [PATCH v7 manager 1/5] api: nodes: add query_url_metadata method

[Lorenz Stechauner]

metadata is gained using a HEAD request.

Due to the ability of this api endpoint to request files on internal 
networks (which would not be visible/accessible from outside) it is 
restricted to users with permissions `Sys.Audit` and `Sys.Modify` on 
`/`. Users with these permissions are able to alter node (network) 
config anyway, so this should not create any further security risk.

On 14.06.21 11:05, Lorenz Stechauner wrote:
> Signed-off-by: Lorenz Stechauner <l.stechauner@proxmox.com>
> ---
>   PVE/API2/Nodes.pm | 96 +++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 96 insertions(+)
>
> diff --git a/PVE/API2/Nodes.pm b/PVE/API2/Nodes.pm
> index e58d9c10..77fa710a 100644
> --- a/PVE/API2/Nodes.pm
> +++ b/PVE/API2/Nodes.pm
> @@ -11,6 +11,7 @@ use JSON;
>   use POSIX qw(LONG_MAX);
>   use Time::Local qw(timegm_nocheck);
>   use Socket;
> +use IO::Socket::SSL;
>   
>   use PVE::API2Tools;
>   use PVE::APLInfo;
> @@ -238,6 +239,7 @@ __PACKAGE__->register_method ({
>   	    { name => 'netstat' },
>   	    { name => 'network' },
>   	    { name => 'qemu' },
> +	    { name => 'query-url-metadata' },
>   	    { name => 'replication' },
>   	    { name => 'report' },
>   	    { name => 'rrd' }, # fixme: remove?
> @@ -1595,6 +1597,100 @@ __PACKAGE__->register_method({
>   	return $rpcenv->fork_worker('download', undef, $user, $worker);
>       }});
>   
> +__PACKAGE__->register_method({
> +    name => 'query_url_metadata',
> +    path => 'query-url-metadata',
> +    method => 'GET',
> +    description => "Query metadata of an URL: file size, file name and mime type.",
> +    proxyto => 'node',
> +    permissions => {
> +	check => ['perm', '/', [ 'Sys.Audit', 'Sys.Modify' ]],
> +    },
> +    parameters => {
> +	additionalProperties => 0,
> +	properties => {
> +	    node => get_standard_option('pve-node'),
> +	    url => {
> +		description => "The URL to query the metadata from.",
> +		type => 'string',
> +		pattern => 'https?://.*',
> +	    },
> +	    'verify-certificates' => {
> +		description => "If false, no SSL/TLS certificates will be verified.",
> +		type => 'boolean',
> +		optional => 1,
> +		default => 1,
> +	    }
> +	},
> +    },
> +    returns => {
> +	type => "object",
> +	properties => {
> +	    filename => {
> +		type => 'string',
> +		optional => 1,
> +	    },
> +	    size => {
> +		type => 'integer',
> +		renderer => 'bytes',
> +		optional => 1,
> +	    },
> +	    mimetype => {
> +		type => 'string',
> +		optional => 1,
> +	    },
> +	},
> +    },
> +    code => sub {
> +	my ($param) = @_;
> +
> +	my $url = $param->{url};
> +
> +	my $ua = LWP::UserAgent->new();
> +
> +	my $dccfg = PVE::Cluster::cfs_read_file('datacenter.cfg');
> +	if ($dccfg->{http_proxy}) {
> +	    $ua->proxy('http', $dccfg->{http_proxy});
> +	}
> +
> +	my $verify = $param->{'verify-certificates'} // 1;
> +	if (!$verify) {
> +	    $ua->ssl_opts(
> +		verify_hostname => 0,
> +		SSL_verify_mode => IO::Socket::SSL::SSL_VERIFY_NONE,
> +	    );
> +	}
> +
> +	my $req = HTTP::Request->new(HEAD => $url);
> +	my $res = $ua->request($req);
> +
> +	die "invalid server response: '" . $res->status_line() . "'\n" if ($res->code() != 200);
> +
> +	my $size = $res->header("Content-Length");
> +	my $disposition = $res->header("Content-Disposition");
> +	my $type = $res->header("Content-Type");
> +
> +	my $filename;
> +
> +	if ($disposition && ($disposition =~ m/filename="([^"]*)"/ || $disposition =~ m/filename=([^;]*)/)) {
> +	    $filename = $1;
> +	} elsif ($url =~ m!^[^?]+/([^?/]*)(?:\?.*)?$!) {
> +	    $filename = $1;
> +	}
> +
> +	# Content-Type: text/html; charset=utf-8
> +	if ($type && $type =~ m/^([^;]+);/) {
> +	    $type = $1;
> +	}
> +
> +	my $ret = {};
> +	$ret->{filename} = $filename if $filename;
> +	$ret->{size} = $size + 0 if $size;
> +	$ret->{mimetype} = $type if $type;
> +
> +	return $ret;
> +    }});
> +
>   __PACKAGE__->register_method({
>       name => 'report',
>       path => 'report',

[pve-devel] applied: [PATCH v7 common 1/1] tools: add download_file_from_url

[Thomas Lamprecht]

On 14.06.21 11:05, Lorenz Stechauner wrote:
> code is based on
> manager:PVE/API2/Nodes.pm:aplinfo
> 

applied, with a slightly adapted commit message you send afterwards and some followups.

I'm a bit sorry to not check on this more closely again earlier as I found quite some
issues when finally wanting to apply this, I fixed up most of them in a followup but the
remaining part of the series needs to adapt.

> Signed-off-by: Lorenz Stechauner <l.stechauner@proxmox.com>
> ---
>  src/PVE/Tools.pm | 124 +++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 124 insertions(+)
> 
> diff --git a/src/PVE/Tools.pm b/src/PVE/Tools.pm
> index 16ae3d2..7b82e00 100644
> --- a/src/PVE/Tools.pm
> +++ b/src/PVE/Tools.pm
> @@ -1829,4 +1829,128 @@ sub safe_compare {
>      return $cmp->($left, $right);
>  }
>  
> +
> +# opts
> +#  -> hash_required
> +#       if 1, at least one checksum has to be specified otherwise an error will be thrown
> +#  -> http_proxy
> +#  -> https_proxy
> +#  -> verify_certificates
> +#  -> sha(1|224|256|384|512)sum
> +#  -> md5sum
> +sub download_file_from_url {
> +    my ($dest, $url, $opts) = @_;
> +
> +    my $tmpdest = "$dest.tmp.$$";

If we'd kept the fork_worker here (see below) this wouldn't be ideal, as now it's using
the pveproxy worker pid, which can be shared by multiple in-flight requests, I'd have moved
it down into the worker sub which is actually it's own process and has a more unique pid..

> +
> +    my $algorithm;
> +    my $expected;
> +

prefixed above two variables with "checksum_" to avoid over general names in longer methods.

> +    for ('sha512', 'sha384', 'sha256', 'sha224', 'sha1', 'md5') {
> +	if (defined($opts->{"${_}sum"})) {
> +	    $algorithm = $_;
> +	    $expected = $opts->{"${_}sum"};
> +	    last;
> +	}
> +    }
> +
> +    die "checksum required but not specified\n" if ($opts->{hash_required} && !$algorithm);
> +
> +    my $worker = sub  {
> +	my $upid = shift;
> +
> +	print "downloading $url to $dest\n";
> +
> +	eval {
> +	    if (-f $dest && $algorithm) {
> +		print "calculating checksum of existing file...\n";
> +		my $correct = check_file_hash($algorithm, $expected, $dest);
> +
> +		if ($correct) {
> +		    print "file already exists, no need to download\n";
> +		    return;
> +		} else {
> +		    print "mismatch, downloading\n";

made a die "abort" out of above, IMO this is not really safe, admin should delete
the existing file first explicitly (we may relax this in the future if actually
requested by users, but as default I'd like to start out with safer behavior).

> +		}
> +	    }
> +
> +	    my @cmd = ('/usr/bin/wget', '--progress=dot:mega', '-O', $tmpdest, $url);

Changed to a array ref and dropped the /usr/bin/, which may be wrong (in theory) on some
systems.

> +
> +	    local %ENV;
> +	    if ($opts->{http_proxy}) {
> +		$ENV{http_proxy} = $opts->{http_proxy};
> +	    }
> +	    if ($opts->{https_proxy}) {
> +		$ENV{https_proxy} = $opts->{https_proxy};
> +	    }
> +
> +	    my $verify = $opts->{verify_certificates} // 1;
> +	    if (!$verify) {
> +		push @cmd, '--no-check-certificate';
> +	    }
> +
> +	    if (run_command([[@cmd]]) != 0) {
> +		die "download failed: $!\n";
> +	    }

why double array refs? I changed above three lines to:

run_command($cmd, errmsg => "download failed");

> +
> +	    if ($algorithm) {
> +		print "calculating checksum...\n";
> +
> +		my $correct = check_file_hash($algorithm, $expected, $tmpdest);
> +
> +		if ($correct) {
> +		    print "checksum verified\n";

not telling what was wrong is never good, i.e., the calculated checksum should be printed here.
I made 'check_file_hash' to 'get_file_hash' and let it just return the computed digest, that
way we do not need to pass expected either and just check ourself.

> +		} else {
> +		    die "checksum mismatch\n";
> +		}
> +	    } else {
> +		print "no checksum for verification specified\n";

nit: just noise, makes the task log unnecessarily longer 

> +	    }
> +
> +	    if (!rename($tmpdest, $dest)) {
> +		die "unable to save file: $!\n";

nit: it's already saved, so this error is misleading, I changed it to "unable to rename temporary file: $!"
to better reflect where the actual issues happened.

> +	    }
> +	};
> +	my $err = $@;
> +
> +	unlink $tmpdest;

this can fail too, and we should warn the admin about that 

> +
> +	if ($err) {
> +	    print "\n";
> +	    die $err;
> +	}
> +
> +	print "download finished\n";
> +    };
> +
> +    my $rpcenv = PVE::RPCEnvironment::get();

you use PVE::RPCEnvironment here but are missing an use statement, it works only as other
modules in pveproxy/pvedaemon include it and perl has no usage-hygiene.

But it cannot be "fixed" by just using this here, there's a reason that we have no other
fork_worker/RPCEnv use in PVE::Tools, PVE::RPCEnvironment is in pve-access-control, which
is a consumer of pve-common, so adding this here would create a circular dependency which
we want to avoid at all costs (makes bootstrapping a huge PITA without any benefit).

So I dropped the whole worker stuff, that needs to happen at the caller side.

> +    my $user = $rpcenv->get_user();
> +
> +    (my $filename = $dest) =~ s!.*/([^/]*)$!$1!;

not really safe, would break quite a bit if the filename contains a colon : or white-space,
this needs to be encoded

> +
> +    return $rpcenv->fork_worker('download', $filename, $user, $worker);
> +}
> +
> +sub check_file_hash {
> +    my ($algorithm, $expected, $filename) = @_;
> +
> +    my $algorithm_map = {
> +	'md5' => sub { Digest::MD5->new },
> +	'sha1' => sub { Digest::SHA->new(1) },
> +	'sha224' => sub { Digest::SHA->new(224) },
> +	'sha256' => sub { Digest::SHA->new(256) },
> +	'sha384' => sub { Digest::SHA->new(384) },
> +	'sha512' => sub { Digest::SHA->new(512) },

You use the Digest:: modules but never add an import use statement for that, meaning again
that it's up to luck (i.e., some other module imports it) if this works..

E.g., the following minimal test does not work even if the worker stuff is dropped:

perl -we 'use PVE::Tools; PVE::Tools::download_file_from_url("/tmp/foo",
  "http://download.proxmox.com/iso/proxmox-ve_3.4-102d4547-6.iso", {md5sum => "37efacd45b70d5d720b11b468f26136b"});'

> +    };
> +
> +    my $digester = $algorithm_map->{$algorithm}->() or die "unknown algorithm '$algorithm'\n";
> +
> +    open(my $fh, '<', $filename) or die "unable to open '$filename': $!\n";
> +    binmode($fh);
> +
> +    my $digest = $digester->addfile($fh)->hexdigest;
> +
> +    return lc($digest) eq lc($expected);
> +}
> +
>  1;
>