On 12/15/22 17:57, Mira Limbeck wrote:
> Since some users keep their passwords in the VM/CT configs as comments
> and those are most of the time unnecessary when looking through the
> report, filter those.
> 
> In addition to the comments, also filter the `cipassword` option
> since it contains the hash of the password.
> 
> To facilitate the filtering, a new sub 'file2text' is introduced that
> can filter the file contents if required.
> This sub replaces the 'cat ...' commands.
> 
> Signed-off-by: Mira Limbeck <m.limb...@proxmox.com>
> ---
> I did not add print to STDERR in file2text for now since it got quite
> chatty.
> If this is wanted, I'll send a v2 adding it. But since file2text is also
> called by dir2text the 'OK' at the end won't always align, especially
> when dir2text is used.
> 
>  PVE/Report.pm | 48 +++++++++++++++++++++++++++++++++---------------
>  1 file changed, 33 insertions(+), 15 deletions(-)
> 
> diff --git a/PVE/Report.pm b/PVE/Report.pm
> index 90b7cb1c..7ebe98f7 100644
> --- a/PVE/Report.pm
> +++ b/PVE/Report.pm
> @@ -5,16 +5,34 @@ use warnings;
>  
>  use PVE::Tools;
>  
> +my sub file2text {
> +    my ($file, $filter) = @_;
> +    my $text = "\n# cat $file\n";
> +

maybe this should be something like `cat $file | grep -v
"\(\(^\s*#\)\|\(^cipassword\)\)"` or `filtered contents of $file`? just
to make it clearer to someone that just looks at the report that we
don't *just* do a `cat` anymore, but do filter sensitive information.

although, i do think that the amount of users that know what is in the
config files and don't check what is actually in the report is probably
rather small.

> +    my $contents = PVE::Tools::file_get_contents($file);
> +    if ($filter) {
> +     foreach my $line (split('\n', $contents)) {
> +         next if $line =~ m/^\s*#/;
> +         next if $line =~ m/^cipassword/;
> +
> +         $text .= "$line\n";
> +     }
> +    } else {
> +     $text .= $contents;
> +    }
> +
> +    return $text;
> +}
> +
>  # output the content of all the files of a directory
>  my sub dir2text {
> -    my ($target_dir, $regexp) = @_;
> +    my ($target_dir, $regexp, $filter) = @_;
>  
>      print STDERR "dir2text '${target_dir}${regexp}'...";
>      my $text = '';
>      PVE::Tools::dir_glob_foreach($target_dir, $regexp, sub {
>       my ($file) = @_;
> -     $text .=  "\n# cat $target_dir$file\n";
> -     $text .= PVE::Tools::file_get_contents($target_dir.$file)."\n";
> +     $text .= file2text($target_dir.$file, $filter)."\n";
>      });
>      return $text;
>  }
> @@ -30,9 +48,9 @@ my $init_report_cmds = sub {
>           cmds => [
>               'hostname',
>               'pveversion --verbose',
> -             'cat /etc/hosts',
> +             sub { file2text('/etc/hosts') },
>               'pvesubscription get',
> -             'cat /etc/apt/sources.list',
> +             sub { file2text('/etc/apt/sources.list') },
>               sub { dir2text('/etc/apt/sources.list.d/', '.*list') },
>               sub { dir2text('/etc/apt/sources.list.d/', '.*sources') },
>               'lscpu',
> @@ -50,9 +68,9 @@ my $init_report_cmds = sub {
>       storage => {
>           order => 30,
>           cmds => [
> -             'cat /etc/pve/storage.cfg',
> +             sub { file2text('/etc/pve/storage.cfg') },
>               'pvesm status',
> -             'cat /etc/fstab',
> +             sub { file2text('/etc/fstab') },
>               'findmnt --ascii',
>               'df --human -T',
>               'proxmox-boot-tool status',
> @@ -62,9 +80,9 @@ my $init_report_cmds = sub {
>           order => 40,
>           cmds => [
>               'qm list',
> -             sub { dir2text('/etc/pve/qemu-server/', '\d.*conf') },
> +             sub { dir2text('/etc/pve/qemu-server/', '\d.*conf', 1) },
>               'pct list',
> -             sub { dir2text('/etc/pve/lxc/', '\d.*conf') },
> +             sub { dir2text('/etc/pve/lxc/', '\d.*conf', 1) },
>           ],
>       },
>       network => {
> @@ -73,14 +91,14 @@ my $init_report_cmds = sub {
>               'ip -details -statistics address',
>               'ip -details -4 route show',
>               'ip -details -6 route show',
> -             'cat /etc/network/interfaces',
> +             sub { file2text('/etc/network/interfaces') },
>           ],
>       },
>       firewall => {
>           order => 50,
>           cmds => [
>               sub { dir2text('/etc/pve/firewall/', '.*fw') },
> -             'cat /etc/pve/local/host.fw',
> +             sub { file2text('/etc/pve/local/host.fw') },
>               'iptables-save',
>           ],
>       },
> @@ -89,7 +107,7 @@ my $init_report_cmds = sub {
>           cmds => [
>               'pvecm nodes',
>               'pvecm status',
> -             'cat /etc/pve/corosync.conf 2>/dev/null',
> +             sub { file2text('/etc/pve/corosync.conf') },
>               'ha-manager status',
>           ],
>       },
> @@ -135,7 +153,7 @@ my $init_report_cmds = sub {
>           'ceph df',
>           'ceph osd df tree',
>           'ceph device ls',
> -         'cat /etc/ceph/ceph.conf',
> +         sub { file2text('/etc/ceph/ceph.conf') },
>           'ceph config dump',
>           'pveceph pool ls',
>           'ceph versions',
> @@ -144,8 +162,8 @@ my $init_report_cmds = sub {
>  
>      if (cmd_exists('multipath')) {
>       push @{$report_def->{disks}->{cmds}},
> -         'cat /etc/multipath.conf',
> -         'cat /etc/multipath/wwids',
> +         sub { file2text('/etc/multipath.conf') },
> +         sub { file2text('/etc/multipath/wwids') },
>           'multipath -ll',
>           ;
>      }



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

Reply via email to