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

[Thomas Lamprecht]

On 4/16/19 1:49 PM, Oguz Bektas wrote:
> looks good to me.
> 
> Tested-by: Oguz Bektas <o.bektas at proxmox.com>

applied, with your T-b tag, thanks!

> 
> 
> 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
>>