From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <s.sterz@proxmox.com>
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 0C734F564
for <pve-devel@lists.proxmox.com>; Fri, 16 Dec 2022 10:01:53 +0100 (CET)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
by firstgate.proxmox.com (Proxmox) with ESMTP id E211A1FC2A
for <pve-devel@lists.proxmox.com>; Fri, 16 Dec 2022 10:01:52 +0100 (CET)
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 <pve-devel@lists.proxmox.com>; Fri, 16 Dec 2022 10:01:51 +0100 (CET)
Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1])
by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 2EE48451CA
for <pve-devel@lists.proxmox.com>; Fri, 16 Dec 2022 10:01:51 +0100 (CET)
Message-ID: <8177369c-e2cb-d2ee-fb08-6290795e1ccf@proxmox.com>
Date: Fri, 16 Dec 2022 10:01:49 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
Thunderbird/102.5.0
Content-Language: en-US
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
Mira Limbeck <m.limbeck@proxmox.com>
References: <20221215165700.2061397-1-m.limbeck@proxmox.com>
From: Stefan Sterz <s.sterz@proxmox.com>
In-Reply-To: <20221215165700.2061397-1-m.limbeck@proxmox.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
X-SPAM-LEVEL: Spam detection results: 0
AWL -0.081 Adjusted score from AWL reputation of From: address
BAYES_00 -1.9 Bayes spam probability is 0 to 1%
KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
NICE_REPLY_A -0.001 Looks like a legit reply (A)
SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record
SPF_PASS -0.001 SPF: sender matches SPF record
URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See
http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more
information. [report.pm]
Subject: Re: [pve-devel] [PATCH manager] report: filter comments in VM/CT
configs
X-BeenThere: pve-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox VE development discussion <pve-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>,
<mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/>
List-Post: <mailto:pve-devel@lists.proxmox.com>
List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>,
<mailto:pve-devel-request@lists.proxmox.com?subject=subscribe>
X-List-Received-Date: Fri, 16 Dec 2022 09:01:53 -0000
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.limbeck@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',
> ;
> }