[pve-devel] [PATCH storage 1/1] file_size_info: implement untrusted mode

[Fabian Grünbichler]

this allows checking some extra attributes for images which come from a
potentially malicious source.

since file_size_info is not part of the plugin API, no API bump is needed. if
desired, a similar check could also be implemented in volume_size_info, which
would entail bumping both APIVER and APIAGE (since the additional parameter
would make checking untrusted volumes opt-in for external plugins).

Signed-off-by: Fabian Grünbichler <f.gruenbich...@proxmox.com>
---
 src/PVE/Storage.pm        |  4 ++--
 src/PVE/Storage/Plugin.pm | 35 ++++++++++++++++++++++++++++++-----
 2 files changed, 32 insertions(+), 7 deletions(-)

diff --git a/src/PVE/Storage.pm b/src/PVE/Storage.pm
index 57b2038..3f0b9ae 100755
--- a/src/PVE/Storage.pm
+++ b/src/PVE/Storage.pm
@@ -233,9 +233,9 @@ sub storage_ids {
 }
 
 sub file_size_info {
-    my ($filename, $timeout) = @_;
+    my ($filename, $timeout, $untrusted) = @_;
 
-    return PVE::Storage::Plugin::file_size_info($filename, $timeout);
+    return PVE::Storage::Plugin::file_size_info($filename, $timeout, 
$untrusted);
 }
 
 sub get_volume_attribute {
diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm
index 6444390..b03e183 100644
--- a/src/PVE/Storage/Plugin.pm
+++ b/src/PVE/Storage/Plugin.pm
@@ -943,15 +943,25 @@ sub free_image {
     return undef;
 }
 
+# set $untrusted if the file in question might be malicious since it isn't
+# created by our stack
+# this makes certain checks fatal, and adds extra checks for known problems 
like
+# - backing files (qcow2/vmdk)
+# - external data files (qcow2)
 sub file_size_info {
-    my ($filename, $timeout) = @_;
+    my ($filename, $timeout, $untrusted) = @_;
 
     my $st = File::stat::stat($filename);
 
     if (!defined($st)) {
        my $extramsg = -l $filename ? ' - dangling symlink?' : '';
-       warn "failed to stat '$filename'$extramsg\n";
-       return undef;
+       my $msg = "failed to stat '$filename'$extramsg\n";
+       if ($untrusted) {
+           die $msg;
+       } else {
+           warn $msg;
+           return undef;
+       }
     }
 
     if (S_ISDIR($st->mode)) {
@@ -977,12 +987,27 @@ sub file_size_info {
 
     my $info = eval { decode_json($json) };
     if (my $err = $@) {
-       warn "could not parse qemu-img info command output for '$filename' - 
$err\n";
-       return wantarray ? (undef, undef, undef, undef, $st->ctime) : undef;
+       my $msg = "could not parse qemu-img info command output for '$filename' 
- $err\n";
+       if ($untrusted) {
+           die $msg;
+       } else {
+           warn $msg;
+           return wantarray ? (undef, undef, undef, undef, $st->ctime) : undef;
+       }
+    }
+
+    if ($untrusted) {
+       if (my $format_specific = $info->{'format-specific'}) {
+           if ($format_specific->{type} eq 'qcow2' && 
$format_specific->{data}->{"data-file"}) {
+               die "$filename: 'data-file' references are not allowed!\n";
+           }
+       }
     }
 
     my ($size, $format, $used, $parent) = $info->@{qw(virtual-size format 
actual-size backing-filename)};
 
+    die "backing file not allowed for untrusted image '$filename'!\n" if 
$untrusted && $parent;
+
     ($size) = ($size =~ /^(\d+)$/); # untaint
     die "size '$size' not an integer\n" if !defined($size);
     # coerce back from string
-- 
2.39.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel