From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 22BA7D491 for ; Fri, 14 Jul 2023 13:30:48 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 0B43F2EEFD for ; Fri, 14 Jul 2023 13:30:48 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Fri, 14 Jul 2023 13:30:46 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 2D20A41F57 for ; Fri, 14 Jul 2023 13:30:46 +0200 (CEST) Date: Fri, 14 Jul 2023 13:30:38 +0200 From: Fabian =?iso-8859-1?q?Gr=FCnbichler?= To: Proxmox VE development discussion References: <20230615120329.28764-1-n.ullreich@proxmox.com> <20230615120329.28764-2-n.ullreich@proxmox.com> In-Reply-To: <20230615120329.28764-2-n.ullreich@proxmox.com> MIME-Version: 1.0 User-Agent: astroid/0.16.0 (https://github.com/astroidmail/astroid) Message-Id: <1689334046.wzedeut37c.astroid@yuna.none> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-SPAM-LEVEL: Spam detection results: 0 AWL 0.071 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record T_SCC_BODY_TEXT_LINE -0.01 - URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [pluggin.pm, proxmox.com, plugin.pm, storage.pm] Subject: Re: [pve-devel] [PATCH pve-storage v3 1/3] recursively go through subdirs to find files X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 14 Jul 2023 11:30:48 -0000 On June 15, 2023 2:03 pm, Noel Ullreich wrote: > This patch allows `get_subdir_files` to recursively call itself, so that > subdirectories of set depth can be searched. We allow searching for > isos, vztmpl and snippets but not backups. >=20 > As a security measure, when parsing a given path, parent > directories (`/../`) are forbidden. >=20 > The feature is opt-in, i.e. the searchdepth is 0 by default. It can be > changed via the API, the web interface and `pvesm` (see the other > patches). >=20 > Signed-off-by: Noel Ullreich > --- > changes from v2: > * fixed the path of the volid for snippets in Pluggin.pm (thanks @Markus) >=20 > src/PVE/Storage.pm | 7 +++++ > src/PVE/Storage/Plugin.pm | 56 ++++++++++++++++++++++++--------------- > 2 files changed, 41 insertions(+), 22 deletions(-) >=20 > diff --git a/src/PVE/Storage.pm b/src/PVE/Storage.pm > index b99ed35..250a892 100755 > --- a/src/PVE/Storage.pm > +++ b/src/PVE/Storage.pm > @@ -113,6 +113,13 @@ our $VZTMPL_EXT_RE_1 =3D qr/\.tar\.(gz|xz|zst)/i; > =20 > our $BACKUP_EXT_RE_2 =3D qr/\.(tgz|(?:tar|vma)(?:\.(${\PVE::Storage::Plu= gin::COMPRESSOR_RE}))?)/; > =20 > +# '..' is forbidden at the beginning, between two '/' and at the end > +my $dots =3D quotemeta('..'); > +my $beginning =3D qr!^$dots/!; > +my $between =3D qr!/$dots/!; > +my $end =3D qr!/$dots$!; > +our $forbidden_double_dots_re =3D qr!(?:$beginning|$between|$end)!; > + > # FIXME remove with PVE 8.0, add versioned breaks for pve-manager > our $vztmpl_extension_re =3D $VZTMPL_EXT_RE_1; > =20 > diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm > index ab6b675..87a08c3 100644 > --- a/src/PVE/Storage/Plugin.pm > +++ b/src/PVE/Storage/Plugin.pm > @@ -621,6 +621,8 @@ sub parse_name_dir { > sub parse_volname { > my ($class, $volname) =3D @_; > =20 > + die "volname must not contain parent directories '/../'\n" if $volna= me =3D~ $PVE::Storage::forbidden_double_dots_re; > + > if ($volname =3D~ m!^(\d+)/(\S+)/(\d+)/(\S+)$!) { > my ($basedvmid, $basename) =3D ($1, $2); > parse_name_dir($basename); > @@ -631,9 +633,9 @@ sub parse_volname { > my ($vmid, $name) =3D ($1, $2); > my (undef, $format, $isBase) =3D parse_name_dir($name); > return ('images', $name, $vmid, undef, undef, $isBase, $format); > - } elsif ($volname =3D~ m!^iso/([^/]+$PVE::Storage::ISO_EXT_RE_0)$!) = { > + } elsif ($volname =3D~ m!^iso/((?:[0-9A-z\_\-\.]+\/)*[^\/]+$PVE::Sto= rage::ISO_EXT_RE_0)$!) { nit: this regex part: (?:[0-9A-z\_\-\.]+\/)*[^\/]+ matching the intermediate sub-dirs and the non-extension part of the file name is repeated a few times in this patch, it might be a good idea to unify that (maybe with groups matching various parts?) > return ('iso', $1); > - } elsif ($volname =3D~ m!^vztmpl/([^/]+$PVE::Storage::VZTMPL_EXT_RE_= 1)$!) { > + } elsif ($volname =3D~ m!^vztmpl/((?:[0-9A-z\_\-\.]+\/)*[^\/]+$PVE::= Storage::VZTMPL_EXT_RE_1)$!) { > return ('vztmpl', $1); > } elsif ($volname =3D~ m!^rootdir/(\d+)$!) { > return ('rootdir', $1, $1); > @@ -643,7 +645,7 @@ sub parse_volname { > return ('backup', $fn, $2); > } > return ('backup', $fn); > - } elsif ($volname =3D~ m!^snippets/([^/]+)$!) { > + } elsif ($volname =3D~ m!^snippets/((?:[0-9A-z\_\-\.]+\/)*[^\/]+)$!)= { > return ('snippets', $1); > } > =20 > @@ -1212,28 +1214,33 @@ sub list_images { > } > =20 > # list templates ($tt =3D ) > -my $get_subdir_files =3D sub { > - my ($sid, $path, $tt, $vmid) =3D @_; > +sub get_subdir_files { > + my ($sid, $path, $tt, $scfg, $vmid, $remaining_depth) =3D @_; > + my $storage_path =3D $scfg->{path}; > + my $content_dir =3D $scfg->{"content-dirs"}->{$tt} // $vtype_subdirs= ->{$tt}; > =20 > my $res =3D []; > =20 > foreach my $fn (<$path/*>) { > - my $st =3D File::stat::stat($fn); > + my $st =3D File::stat::lstat($fn); > + > + next if (!$st); > =20 > - next if (!$st || S_ISDIR($st->mode)); > + if (S_ISDIR($st->mode)) { > + if ($remaining_depth) { > + push @$res, get_subdir_files($sid, $fn, $tt, $scfg, $vmid, $remaining_= depth-1); > + } > + next; > + } > =20 > my $info; > =20 > if ($tt eq 'iso') { > - next if $fn !~ m!/([^/]+$PVE::Storage::ISO_EXT_RE_0)$!i; > - > + next if $fn !~ m/(?:^$storage_path\/$content_dir\/)((?:[0-9A-z\_\-\= .]+\/)*[^\/]+$PVE::Storage::ISO_EXT_RE_0)/; > $info =3D { volid =3D> "$sid:iso/$1", format =3D> 'iso' }; > - > } elsif ($tt eq 'vztmpl') { > - next if $fn !~ m!/([^/]+$PVE::Storage::VZTMPL_EXT_RE_1)$!; > - > + next if $fn !~ m/(?:^$storage_path\/$content_dir\/)((?:[0-9A-z\_\-\= .]+\/)*[^\/]+$PVE::Storage::VZTMPL_EXT_RE_1)/; > $info =3D { volid =3D> "$sid:vztmpl/$1", format =3D> "t$2" }; > - > } elsif ($tt eq 'backup') { > next if $fn !~ m!/([^/]+$PVE::Storage::BACKUP_EXT_RE_2)$!; > my $original =3D $fn; > @@ -1262,9 +1269,9 @@ my $get_subdir_files =3D sub { > =20 > $info->{protected} =3D 1 if -e PVE::Storage::protection_file_path($= original); > } elsif ($tt eq 'snippets') { > - > + next if $fn !~ m/(?:^$storage_path\/$content_dir\/)((?:[0-9A-z\_\-\= .]+\/)*.+)/; > $info =3D { > - volid =3D> "$sid:snippets/". basename($fn), > + volid =3D> "$sid:snippets/$1", #basename($fn), > format =3D> 'snippet', > }; > } > @@ -1274,14 +1281,18 @@ my $get_subdir_files =3D sub { > =20 > push @$res, $info; > } > - > return $res; > }; > =20 > +sub flatten { > + map { ref eq 'ARRAY' ? flatten(@{$_}) : $_ } @_; > +} > + > # If attributes are set on a volume, they should be included in the resu= lt. > # See get_volume_attribute for a list of possible attributes. > sub list_volumes { > my ($class, $storeid, $scfg, $vmid, $content_types) =3D @_; > + my $max_depth =3D $scfg->{'subdir-depth'} // 0; > =20 > my $res =3D []; > my $vmlist =3D PVE::Cluster::get_vmlist(); > @@ -1294,17 +1305,19 @@ sub list_volumes { > my $path =3D $class->get_subdir($scfg, $type); > =20 > if ($type eq 'iso' && !defined($vmid)) { > - $data =3D $get_subdir_files->($storeid, $path, 'iso'); > + $data =3D get_subdir_files($storeid, $path, 'iso', $scfg, undef, $max_= depth); > } elsif ($type eq 'vztmpl'&& !defined($vmid)) { > - $data =3D $get_subdir_files->($storeid, $path, 'vztmpl'); > + $data =3D get_subdir_files($storeid, $path , 'vztmpl', $scfg, undef, $= max_depth); > } elsif ($type eq 'backup') { > - $data =3D $get_subdir_files->($storeid, $path, 'backup', $vmid); > + $data =3D get_subdir_files($storeid, $path, 'backup', $scfg, $vmid, $m= ax_depth); > } elsif ($type eq 'snippets') { > - $data =3D $get_subdir_files->($storeid, $path, 'snippets'); > + $data =3D get_subdir_files($storeid, $path, 'snippets', $scfg, undef, = $max_depth); > } > } > =20 > - next if !$data; > + $data =3D [flatten($data)]; > + > + next if !@$data[0]; > =20 > foreach my $item (@$data) { > if ($type eq 'images' || $type eq 'rootdir') { > @@ -1322,7 +1335,6 @@ sub list_volumes { > } else { > $item->{content} =3D $type; > } > - > push @$res, $item; > } > } > --=20 > 2.30.2 >=20 >=20 >=20 > _______________________________________________ > pve-devel mailing list > pve-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel >=20 >=20 >=20