[pve-devel] [RFC manager] fix #263: only include node rrd stats if user has Sys.Audit

[Oguz Bektas]

hi,

looks good to me.

Tested-by: Oguz Bektas <o.bektas at proxmox.com>


On Mon, Apr 15, 2019 at 02:10:27PM +0000, Thomas Lamprecht wrote:
> It makes sense to not give users without Sys.Audit permissions to
> much information over a node and this is relatively easy and cheap to
> check and enforce at those two points.
> 
> Signed-off-by: Thomas Lamprecht <t.lamprecht at proxmox.com>
> ---
> 
> saw the old bug, saw that at least at those points it's easy to enforce (there
> are naturally other ways of getting a estimate af some values of this, but one
> can still fix the low hanging fruits nonetheless).
> 
>  PVE/API2/Cluster.pm |  3 ++-
>  PVE/API2/Nodes.pm   |  6 +++++-
>  PVE/API2Tools.pm    | 22 +++++++++++++---------
>  3 files changed, 20 insertions(+), 11 deletions(-)
> 
> diff --git a/PVE/API2/Cluster.pm b/PVE/API2/Cluster.pm
> index c72a075b..5b6149a7 100644
> --- a/PVE/API2/Cluster.pm
> +++ b/PVE/API2/Cluster.pm
> @@ -341,7 +341,8 @@ __PACKAGE__->register_method({
>  
>  	if (!$param->{type} || $param->{type} eq 'node') {
>  	    foreach my $node (@$nodelist) {
> -		my $entry = PVE::API2Tools::extract_node_stats($node, $members, $rrd);
> +		my $can_audit = $rpcenv->check($authuser, "/nodes/$node", [ 'Sys.Audit' ], 1);
> +		my $entry = PVE::API2Tools::extract_node_stats($node, $members, $rrd, !$can_audit);
>  		push @$res, $entry;
>  	    }
>  	}
> diff --git a/PVE/API2/Nodes.pm b/PVE/API2/Nodes.pm
> index 8a2c2384..ad3f6e42 100644
> --- a/PVE/API2/Nodes.pm
> +++ b/PVE/API2/Nodes.pm
> @@ -2077,6 +2077,9 @@ __PACKAGE__->register_method ({
>      code => sub {
>  	my ($param) = @_;
>  
> +	my $rpcenv = PVE::RPCEnvironment::get();
> +	my $authuser = $rpcenv->get_user();
> +
>  	my $clinfo = PVE::Cluster::get_clinfo();
>  	my $res = [];
>  
> @@ -2085,7 +2088,8 @@ __PACKAGE__->register_method ({
>  	my $rrd = PVE::Cluster::rrd_dump();
>  
>  	foreach my $node (@$nodelist) {
> -	    my $entry = PVE::API2Tools::extract_node_stats($node, $members, $rrd);
> +	    my $can_audit = $rpcenv->check($authuser, "/nodes/$node", [ 'Sys.Audit' ], 1);
> +	    my $entry = PVE::API2Tools::extract_node_stats($node, $members, $rrd, !$can_audit);
>  	    $entry->{ssl_fingerprint} = PVE::Cluster::get_node_fingerprint($node);
>  	    push @$res, $entry;
>  	}
> diff --git a/PVE/API2Tools.pm b/PVE/API2Tools.pm
> index 9f782c92..4d730901 100644
> --- a/PVE/API2Tools.pm
> +++ b/PVE/API2Tools.pm
> @@ -27,7 +27,7 @@ sub get_hwaddress {
>  }
>  
>  sub extract_node_stats {
> -    my ($node, $members, $rrd) = @_;
> +    my ($node, $members, $rrd, $exclude_stats) = @_;
>  
>      my $entry = {
>  	id => "node/$node",
> @@ -37,19 +37,23 @@ sub extract_node_stats {
>      };
>  
>      if (my $d = $rrd->{"pve2-node/$node"}) {
> -		    
> +
>  	if (!$members || # no cluster
>  	    ($members->{$node} && $members->{$node}->{online})) {
> -	    $entry->{uptime} = ($d->[0] || 0) + 0;
> -	    $entry->{cpu} = ($d->[5] || 0) + 0;
> -	    $entry->{mem} = ($d->[8] || 0) + 0;
> -	    $entry->{disk} = ($d->[12] || 0) + 0;
> +	    if (!$exclude_stats) {
> +		$entry->{uptime} = ($d->[0] || 0) + 0;
> +		$entry->{cpu} = ($d->[5] || 0) + 0;
> +		$entry->{mem} = ($d->[8] || 0) + 0;
> +		$entry->{disk} = ($d->[12] || 0) + 0;
> +	    }
>  	    $entry->{status} = 'online';
>  	}
>  	$entry->{level} = $d->[1];
> -	$entry->{maxcpu} = ($d->[4] || 0) + 0;
> -	$entry->{maxmem} = ($d->[7] || 0) + 0;
> -	$entry->{maxdisk} = ($d->[11] || 0) + 0;
> +	if (!$exclude_stats) {
> +	    $entry->{maxcpu} = ($d->[4] || 0) + 0;
> +	    $entry->{maxmem} = ($d->[7] || 0) + 0;
> +	    $entry->{maxdisk} = ($d->[11] || 0) + 0;
> +	}
>      }
>  
>      if ($members && $members->{$node} &&
> -- 
> 2.20.1
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel at pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel